Skip to content

Conversation

@fbacall
Copy link
Contributor

@fbacall fbacall commented Dec 4, 2025

Prevents intermittent failures when tests that selectively load fixtures are run before others.

Potentially f_ixes #1182 , #2436 and maybe some others

@fbacall fbacall requested review from Copilot and kdp-cloud December 4, 2025 11:37
Copilot finished reviewing on behalf of fbacall December 4, 2025 11:44
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR consolidates fixture loading across the test suite to prevent intermittent test failures caused by execution order dependencies. Instead of selectively declaring fixtures in individual test files, all fixtures are now loaded globally through the ActiveSupport::TestCase base class.

  • Removed individual fixtures declarations from approximately 100 test files (unit, functional, and integration tests)
  • Added fixtures :all to ActiveSupport::TestCase in test/test_helper.rb to ensure consistent fixture availability
  • Cleaned up minor whitespace inconsistencies in test/unit/jobs/auth_lookup_job_test.rb

Reviewed changes

Copilot reviewed 102 out of 103 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
test/test_helper.rb Added fixtures :all to ActiveSupport::TestCase base class and removed commented-out fixture declaration
test/unit/*.rb (23 files) Removed individual fixtures declarations from unit tests
test/unit/permissions/*.rb (3 files) Removed fixtures declarations from permission-related unit tests
test/unit/jobs/*.rb (3 files) Removed fixtures declarations from job tests; whitespace cleanup in auth_lookup_job_test.rb
test/unit/extended_metadatas/emt_extractor_test.rb Removed fixtures declaration
test/unit/bio_schema/data_dump_test.rb Removed fixtures declaration
test/functional/*.rb (47 files) Removed individual fixtures declarations from controller tests
test/functional/publishing/*.rb (3 files) Removed fixtures declarations from publishing-related tests
test/functional/sharing/batch_sharing_change_test.rb Removed fixtures declaration
test/functional/ga4gh/trs/v2/*.rb (3 files) Removed fixtures declarations from GA4GH TRS API tests
test/integration/*.rb (5 files) Removed individual fixtures declarations from integration tests

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines 187 to 188
# Note: You'll currently still have to declare fixtures explicitly in integration tests
# -- they do not yet inherit this setting
Copy link

Copilot AI Dec 4, 2025

Choose a reason for hiding this comment

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

The comment stating "Note: You'll currently still have to declare fixtures explicitly in integration tests -- they do not yet inherit this setting" is now outdated and misleading. Since ActionDispatch::IntegrationTest inherits from ActiveSupport::TestCase in Rails, integration tests will inherit the fixtures :all setting declared above. This comment should be removed to avoid confusion.

Copilot uses AI. Check for mistakes.
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.

2 participants