Fix race conditions in system specs Fixes #6693#6724
Closed
AlexWheeler wants to merge 4 commits intorubyforgood:mainfrom
Closed
Fix race conditions in system specs Fixes #6693#6724AlexWheeler wants to merge 4 commits intorubyforgood:mainfrom
AlexWheeler wants to merge 4 commits intorubyforgood:mainfrom
Conversation
- Fix DatabaseCleaner pollution in `volunteer_datatable_spec.rb`: Replaced `before(:all)` and manual `DatabaseCleaner` strategy with standard `before(:each)` and RSpec transactional fixtures. This prevents data created in this spec from leaking into subsequent system tests (like `case_contacts/new_spec.rb`), which was causing `CaseContact.started` counts to be incorrect.
- Fix global time leakage: Multiple specs were using `travel_to` without ensuring `travel_back` was called, causing the frozen time to persist into other tests. This led to 'Date can't be in the future' validation errors in unrelated tests (e.g., `case_contacts/index_spec.rb`).
- Updated `health_spec.rb` to use block syntax for `travel_to`.
- Added `after { travel_back }` to `banners_spec.rb`, `volunteer_birthday_reminder_service_spec.rb`, `backfill_case_contact_started_metadata_service_spec.rb`, and various system specs in `placements/`, `court_dates/`, and `casa_cases/`.
- Stabilize `bulk_court_dates/new_spec.rb`:
- Replaced `first(:css, 'textarea').native.send_keys` with `find(...).set` to reliably interact with the court order field.
- Added an explicit expectation for the success flash message before navigating to the case page. This fixes a race condition where the test would attempt to visit the next page before the form submission and redirect completed.
- Fixed date input handling to use the local `now` variable instead of the symbol `:now`.
- Stabilize `court_dates/edit_spec.rb`:
- Fixed time leakage.
- Replaced unreliable `first('textarea').send_keys` with `all(...).last.set` to target the correct dynamically added text area, resolving `ElementNotInteractable` or missing data issues.
These changes ensure that specs run in isolation without side effects (time/data) affecting the global suite stability.
Identified and fixed more specs that were using `travel_to` without `travel_back`. These leaks were causing failures in `spec/system/casa_cases/new_spec.rb` and other time-sensitive tests when run as part of the full suite. Fixed files: - spec/callbacks/case_contact_metadata_callback_spec.rb - spec/requests/imports_spec.rb - spec/requests/notifications_spec.rb - spec/services/case_contacts_contact_dates_spec.rb - spec/services/emancipation_checklist_reminder_service_spec.rb - spec/lib/importers/case_importer_spec.rb - spec/lib/tasks/supervisor_weekly_digest_spec.rb - spec/support/shared_examples/shows_court_dates_links.rb
Patched multiple specs that were using `travel_to` without resetting the time, which was causing failures in subsequent tests (specifically `spec/system/case_contacts/edit_spec.rb`).
Fixed files:
- spec/system/casa_cases/edit_spec.rb: Added `after { travel_back }` to ensure time is reset after the test block.
- spec/models/case_court_report_context_spec.rb: Added `after { travel_back }`.
- spec/requests/case_contact_reports_spec.rb: Added `after { travel_back }`.
- spec/requests/case_court_reports_spec.rb: Added `after { travel_back }`.
This resolves issues where frozen time from one spec (e.g., set to 2020) would persist into others, causing date-dependent logic and validations to fail unpredictably.
AlexWheeler
requested review from
FireLemons,
compwron and
elasticspoon
as code owners
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
1 participant
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.
Still have 1 flaky test working on getting fixed, but figured I'd open a PR to see if I'm on the right track. Started by changing the tests to test the UI changes instead of just DB so that it is blocking, but that lead me down a rabbit hole of flaky tests. Seems to flip flop between /spec/system/supervisors/edit_spec.rb and spec/system/case_contacts/new_spec.rb.
What github issue is this PR for, if any?
Resolves #6693
What changed, and why?
travel_backwheretravel_tois used without a block withouttravel_backotherwise than can pollute other specs'instead of", hopefully that doesn't make diff hard to read.How is this tested? (please write rspec and jest tests!) 💖💪
rspec with some capybara since that is what the specs are already using.
Screenshots please :)
Just test suite so not much to show other than code