Enable additional Rubocop rules for Capybara tests#12413
Merged
mitchellhenke merged 3 commits intomainfrom Aug 12, 2025
Merged
Conversation
mitchellhenke
force-pushed
the
mitchellhenke/add-capybara-rubocops
branch
11 times, most recently
from
28d473b to
57a8026
Compare
solipet
approved these changes
Aug 12, 2025
Contributor
solipet
left a comment
solipet
left a comment
There was a problem hiding this comment.
This is awesome! A few minor suggestions - take or leave.
Comment on lines
30
to
31
mitchellhenke
force-pushed
the
mitchellhenke/add-capybara-rubocops
branch
6 times, most recently
from
3bdab20 to
e996593
Compare
changelog: Internal, Testing, Create Rubocop rule to enforce stricter path assertions in feature tests Co-authored-by: Doug Price <douglas.price@gsa.gov>
mitchellhenke
force-pushed
the
mitchellhenke/add-capybara-rubocops
branch
from
e996593 to
ac853ff
Compare
mitchellhenke
commented
Aug 12, 2025
Comment on lines
-24
to
-31
| before do | ||
| allow_any_instance_of(ApplicationController).to receive(:analytics).and_return(fake_analytics) | ||
| allow_any_instance_of(ApplicationController).to receive(:attempts_api_tracker).and_return( | ||
| attempts_api_tracker, | ||
| ) | ||
| sign_in_and_2fa_user(user) | ||
| complete_doc_auth_steps_before_ssn_step | ||
| end |
Contributor
Author
There was a problem hiding this comment.
There is a before do block in the context 'phone vendor outage' below which repeated these steps. This meant the test was partially running through idv before doing it again. I've pulled this outer before block into a new context to avoid this duplication.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
🛠 Summary of changes
We've seen some issues over time with race conditions in feature tests between the headless browser and server. Part of improving it has been moving towards assertions on arriving at a page, including past work in #11669, though it has the limitation of only working on
current_path, and we have a similar issue withcurrent_url. We also have conditional checks oncurrent_pathfor taking different actions or flows based on the result, which can cause similar issues.This borrows from the existing rule to implement a very similar one in
CapybaraCurrentUrlExpectLinterto address the first issue and more distinctCapybaraCurrentPathEqualityLinterto address the second. There was some discussion on test reliability in this week's Engineering Huddle related to this issue as well. I've also enabled a handful of other Capybara-related Rubocop rules that did not have any findings, but I think are worth enforcing.This did also uncover some incorrect assertions that succeeded due to the race conditions involved (example).