Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix flaky test for user notification task #17676

Merged
merged 3 commits into from
Feb 28, 2025
Merged

fix flaky test for user notification task #17676

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
2fc7053
fix flaky test for user notification task
ewdurbin Feb 28, 2025
79dfe26
lint
ewdurbin Feb 28, 2025
44d9493
Merge branch 'main' into flaky_tos_tests_round_2
di Feb 28, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 3 additions & 14 deletions tests/unit/accounts/test_tasks.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ def test_notify_users_of_tos_update_respects_batch_size(
"terms.revision": "initial",
"terms.notification_batch_size": batch_size,
}
users_to_notify = UserFactory.create_batch(20, with_verified_primary_email=True)
UserFactory.create_batch(max(1, batch_size * 2), with_verified_primary_email=True)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It took me far longer than it should have to realize that the parametrized input was 10 not 20, and I kept trying to figure out why you were multiplying it by two.
Using max() is a nifty way to make the input of 0 work.


send_email = pretend.call_recorder(lambda request, user: None)
monkeypatch.setattr(tasks, "send_user_terms_of_service_updated", send_email)
Expand All @@ -75,19 +75,8 @@ def test_notify_users_of_tos_update_respects_batch_size(

notify_users_of_tos_update(db_request)

assert sorted(send_email.calls, key=lambda x: x.args[1]) == sorted(
[pretend.call(db_request, u) for u in users_to_notify][:batch_size],
key=lambda x: x.args[1],
)
assert sorted(
user_service.record_tos_engagement.calls, key=lambda x: x.args[0]
) == sorted(
[
pretend.call(u.id, "initial", TermsOfServiceEngagement.Notified)
for u in users_to_notify
][:batch_size],
key=lambda x: x.args[0],
)
assert len(send_email.calls) == batch_size
assert len(user_service.record_tos_engagement.calls) == batch_size


def test_notify_users_of_tos_update_does_not_renotify(
Expand Down