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.

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 spec "Notifications User not logged in" #4467

Merged
merged 1 commit into from
Apr 9, 2021

Conversation

javierm
Copy link
Member

@javierm javierm commented Apr 7, 2021

References

Background

This issue can be reproduced running rspec spec/system/moderation/proposal_notifications_spec.rb:71 spec/system/notifications_spec.rb:126 --order defined.

We had a test failing several times in GitHub Actions where a user was still logged in even after logout.

One possible cause is a concurrency issue because the process running the test and the process running the browser both access the same database connection. Maybe some database operations leak between tests due to that.

Objectives

  • Make the notification tests more robust

@javierm javierm self-assigned this Apr 7, 2021
@javierm javierm added this to Reviewing in Consul Democracy via automation Apr 7, 2021
@javierm javierm force-pushed the fix_flaky_notification_spec branch from 5cd39ad to 0f1fa51 Compare April 7, 2021 19:11
@javierm javierm force-pushed the fix_flaky_notification_spec branch 2 times, most recently from 05fad2b to b37872a Compare April 8, 2021 21:20
We had a test failing several times in GitHub Actions where a user was
still logged in even after logout.

This issue can be reproduced running:

```
rspec spec/system/moderation/proposal_notifications_spec.rb:71 \
spec/system/notifications_spec.rb:126 --order defined
```

One possible cause is a concurrency issue because the process running
the test and the process running the browser both access the same
database connection. Maybe some database operations leak between tests
due to that, particularly if the previous test accessed the database
after starting the browser as well.

A way to avoid this possible cause is setting up the database before
starting the browser with a call to `visit`.
@javierm javierm force-pushed the fix_flaky_notification_spec branch from b37872a to b9da024 Compare April 9, 2021 10:54
Consul Democracy automation moved this from Reviewing to Testing Apr 9, 2021
@javierm javierm merged commit 843f61e into master Apr 9, 2021
@javierm javierm deleted the fix_flaky_notification_spec branch April 9, 2021 14:29
Consul Democracy automation moved this from Testing to Release 1.3.0 Apr 9, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants