Skip to content

tests: attempt to persist seed data and truncate test data#5410

Merged
BenTopping merged 22 commits intoy25-181-upgrade-to-rails-72from
cucumber-fix
Jan 16, 2026
Merged

tests: attempt to persist seed data and truncate test data#5410
BenTopping merged 22 commits intoy25-181-upgrade-to-rails-72from
cucumber-fix

Conversation

@BenTopping
Copy link
Copy Markdown
Contributor

@BenTopping BenTopping commented Dec 12, 2025

Attempts at handling cucumber test data consistently. The general approach is to seed the database once on test run start and then rely on tests not mutating it. There have been several cases we have fixed where tests were mutating it and there may be more hidden but we have got to the point it is passing.

We have changed the db cleaner strategy to only clean new data created from the current test (ignore seed data). We have had to go with this approach because transactions were not working correctly after a couple of different attempts. This approach also means we can't parallelise CI using knapsack as the shared db instance causes issues when using truncation as one test may truncate another tests data.

A lot of these tests can be removed when we deprecate V1 API which will reduce the run time. We should also focus on moving off cucumber.

Proposed changes

  • Adds seeded_deletion database cleaner to maintain seed data whilst removing other tests data.
  • Updates cucumber database cleaner strategies to use new seeded_deletion.

Comment thread app/models/sample.rb Outdated
@yoldas yoldas self-assigned this Jan 9, 2026
@yoldas yoldas removed their assignment Jan 12, 2026
}
"""

@create @error
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

The submission template is generated here https://github.com/sanger/sequencescape/blob/develop/features/support/step_definitions/submission_steps.rb#L21.
It creates a pulldown_isc request type which has no validators for requiring read_length validation as expected in the test. So I am unsure why it would expect to fail.

}
"""

@create @error @asset
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Again this tests is v1 api submissions so should be safe to remove. I suspect it is something to do with transactions here https://github.com/sanger/sequencescape/blob/develop/app/api/endpoints/submissions.rb#L17. I don't think this is getting hit though so unsure.

@create @batch @authorised @error
Scenario Outline: Attempting to create a batch with invalid request details
Given the maximum batch size for the pipeline "Cluster formation PE" is 2
Given the UUID for the pipeline "HiSeq 2500 PE (spiked in controls)" is "00000000-1111-2222-3333-444444444445"
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Instead of mutating the seed data (Clust formation PE) to have a max_size of 2, we are using an different template with an existing max size of 2 so we dont need to edit seed data.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Changes here prevent editing seed data

@BenTopping BenTopping marked this pull request as ready for review January 13, 2026 14:16
@codecov
Copy link
Copy Markdown

codecov Bot commented Jan 13, 2026

Codecov Report

❌ Patch coverage is 0% with 5 lines in your changes missing coverage. Please review.
✅ Project coverage is 87.33%. Comparing base (999fce7) to head (cd455a4).
⚠️ Report is 23 commits behind head on y25-181-upgrade-to-rails-72.

Files with missing lines Patch % Lines
lib/views_schema.rb 0.00% 5 Missing ⚠️
Additional details and impacted files
@@                       Coverage Diff                       @@
##           y25-181-upgrade-to-rails-72    #5410      +/-   ##
===============================================================
+ Coverage                        83.94%   87.33%   +3.39%     
===============================================================
  Files                             1456     1456              
  Lines                            32654    32651       -3     
  Branches                          3405     3405              
===============================================================
+ Hits                             27411    28517    +1106     
+ Misses                            5229     4120    -1109     
  Partials                            14       14              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Copy link
Copy Markdown
Contributor

@StephenHulme StephenHulme left a comment

Choose a reason for hiding this comment

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

Great work - thanks so much for tackling this!

Copy link
Copy Markdown
Contributor Author

@BenTopping BenTopping Jan 16, 2026

Choose a reason for hiding this comment

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

This is a restricted / deprecated action and has been for almost 10 years. With removal of API V1 imminent I just removed this to save time.

@BenTopping BenTopping merged commit d3596f1 into y25-181-upgrade-to-rails-72 Jan 16, 2026
27 of 28 checks passed
@BenTopping BenTopping deleted the cucumber-fix branch January 16, 2026 10:27
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.

3 participants