Skip to content

refactor(tests): replace mailer mocks with email delivery assertions#2617

Open
mroderick wants to merge 10 commits into
codebar:masterfrom
mroderick:refactor/mailer-tests
Open

refactor(tests): replace mailer mocks with email delivery assertions#2617
mroderick wants to merge 10 commits into
codebar:masterfrom
mroderick:refactor/mailer-tests

Conversation

@mroderick
Copy link
Copy Markdown
Collaborator

@mroderick mroderick commented May 14, 2026

Summary

Replaces mock-based mailer tests with assertions on ActionMailer::Base.deliveries. This improves test isolation and eliminates flaky tests in parallel test execution.

Problem

The existing tests used expect(Mailer).to receive(:method) and expect_any_instance_of(Mailer) which:

  • Cause race conditions in parallel test runs (same mock catches calls from other processes)
  • Test implementation details rather than behavior
  • Are globally scoped, causing interference between tests

Solution

Replace mock expectations with assertions on actual email delivery:

  • Verify email count: expect { action }.to change { ActionMailer::Base.deliveries.count }
  • Verify email recipients: expect(email.to).to include(member.email)
  • Verify email content: expect(email.body.encoded).to match(/pattern/)

Files Changed

File Changes
spec/support/shared_examples/behaves_like_sending_workshop_emails.rb Replace mailer mocks with delivery count assertions
spec/models/invitation_manager_spec.rb Convert all mailer mocks (waiting list, reminders, meetings, async)
spec/models/feedback_request_spec.rb Simplify to single email delivery test
spec/models/eligibility_inquiry_spec.rb Replace spy with delivery assertion
spec/models/attendance_warning_spec.rb Replace spy with delivery assertion
spec/features/subscribing_to_emails_spec.rb Replace all expect_any_instance_of mocks
spec/mailers/member_mailer_spec.rb Replace expect_any_instance_of with content verification

Test Results

All 59 affected tests pass:

  • 37 invitation_manager tests
  • 10 invitation_manager_logging tests
  • 7 feedback_request tests
  • 2 eligibility_inquiry tests
  • 3 attendance_warning tests
  • 8 feature tests (subscriptions)
  • 15 mailer tests

Benefits

  1. Better parallel test isolation - No shared global state from mocks
  2. Tests behavior, not implementation - Verifies emails are actually sent
  3. More resilient - Won't break if internal method names change
  4. Clearer intent - Tests describe what should happen, not how

mroderick added 10 commits May 14, 2026 11:04
…d of mocks

Replaces mock expectations on mailer classes with assertions about
ActionMailer::Base.deliveries. This improves test isolation for parallel
execution.

Changes:
- 'creates an invitation for each student' now verifies emails sent via delivery count
- 'creates an invitation for each coach' now verifies emails sent via delivery count
- Tests verify recipient email addresses match expected members
- Removes expect(mailer).to receive(:invite_student/coach) mocks
…ertions

Replaces mock expectations with ActionMailer::Base.deliveries count
assertions for #send_waiting_list_emails tests.

Changes:
- 'emails coaches when there are free coach spots' - verifies email delivery
- 'does not email coaches when no coach spots' - verifies no emails sent
- 'emails students when there are free student spots' - verifies email delivery
- 'does not email students when no student spots' - verifies no emails sent

Removes expect(WorkshopInvitationMailer).to receive(:notify_waiting_list) mocks
Replaces mock expectations with ActionMailer::Base.deliveries count
assertions for reminder email tests.

Changes:
- 'emails all attending members' (monthly) - uses _without_delay method
- 'emails all attending members' (workshop) - verifies email delivery count
- 'emails waiting list' - uses _without_delay method

Notes:
- Monthly and waiting list tests remain marked :wip as they were before
- These methods are async and require _without_delay for sync testing

Removes expect(Mailer).to receive(:attendance_reminder) mocks
Replaces mock expectations with ActionMailer::Base.deliveries count
assertions for #send_meeting_emails tests.

Changes:
- 'emails all invitees that are not banned' - verifies email count excludes banned
- 'emails valid invitees only once' - verifies email count excludes already invited

Uses _without_delay method for synchronous testing.

Removes expect(MeetingInvitationMailer).to receive(:invite) mocks
Replaces mock expectations with ActionMailer::Base.deliveries count
assertions for async behavior tests.

Changes:
- 'sends invitation emails' (async context) - verifies email delivery
- 'sends attendance reminder emails' (async context) - verifies email delivery

Uses _without_delay methods for synchronous testing.

Removes remaining expect(WorkshopInvitationMailer).to receive mocks
…ertions

Simplifies after create hook tests by removing mock-based tests
and replacing with direct email delivery verification.

Changes:
- Consolidates two mock-based tests into one email delivery test
- Verifies email is sent and has expected subject

Removes allow().to receive() and have_received() mocks
…ssertions

Replaces spy-based mock test with email delivery verification.

Changes:
- 'sends an eligibility check email' - verifies email delivery count and recipient

Removes allow().to receive() and have_received() mocks
…sertions

Replaces spy-based mock test with email delivery verification.

Changes:
- 'sends an attendance warning email' - finds email by recipient and subject
- Uses flexible matching to account for other emails in deliveries

Removes allow().to receive() and have_received() mocks
… assertions

Replaces expect_any_instance_of mocks with ActionMailer::Base.deliveries
verification for all welcome email scenarios.

Changes:
- First-time coach subscription - verifies email sent and contains 'coach'
- First-time student subscription - verifies email sent and contains 'student'
- Second subscription tests - clears deliveries and verifies no new emails
- Unsubscribe/resubscribe tests - verifies no duplicate emails sent

Adds before hook to clear deliveries for consistent test isolation.

Removes all expect_any_instance_of(MemberMailer) mocks which were
problematic for parallel test execution.
…rtions

Replaces expect_any_instance_of mocks with direct mail content
verification for welcome email tests.

Changes:
- 'sends the coach welcome email to coaches' - verifies email body contains coach text
- 'sends the student welcome email to students' - verifies email body contains student text
- 'sends a ban email to a member' - verifies recipient and email body

Removes all expect_any_instance_of(MemberMailer) mocks which were
problematic for parallel test execution.
@mroderick mroderick requested a review from olleolleolle May 14, 2026 13:13
@mroderick
Copy link
Copy Markdown
Collaborator Author

@olleolleolle is this the most Rails idiomatic way of testing email sending?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant