Repository navigation
Keep minitest below 6 so the tests run again - #327
namespaceMarcello wants to merge 1 commit into
Conversation
The propshaft bump in basecamp#287 also moved minitest from 5.26.2 to 6.0.6. Since then bin/rails test and bin/rails test:system load no tests and report 0 runs, locally and on CI, where both jobs still pass. test/test_helper.rb requires minitest/unit, which minitest 6 no longer ships, and dropping that require alone still runs nothing on the current Rails pin. With minitest 5.27 every test runs again. The pin can go once Rails moves forward, as basecamp#314 does. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0142qgjggdJ2KDdGk7RF9Xm9
There was a problem hiding this comment.
Copilot review overview
🟢 Approved
The constraint and lockfile consistently address the test-loading regression, with no blocking issues identified.
Review effort: Balanced
Findings: None
What changed in this PR
Restores test discovery on the current Rails pin by keeping Minitest below 6 until #314 lands.
Changes:
- Adds a test-group dependency constraint of
minitest < 6. - Locks Minitest to 5.27.0 and removes its Minitest 6 dependency declarations.
[!TIP]
If you aren't ready for review, convert to a draft PR.
Click "Convert to draft" or rungh pr ready --undo.
Click "Ready for review" or rungh pr readyto reengage.
| File | Description |
|---|---|
| Gemfile.lock | Locks Minitest to 5.27.0 and records the constraint. |
| Gemfile | Restricts Minitest to versions below 6 for testing. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
|
#314 (the Rails main bump) also fixes this: on minitest 6.0.6, CI runs 420 tests and 23 system tests (run (https://github.com/basecamp/once-campfire/actions/runs/37336066469)). If #314 is close to merging, this pin isn't needed. If it isn't, this is a good stopgap. |
|
Thank you @namespaceMarcello, and @rubys! I think I'm going to merge the Rails main bump, so this one is not needed, but really appreciate your fix! |
|
Closing this in favour of #314. Thanks again! |
Since #287 (the propshaft bump, which also moved minitest from 5.26.2 to 6.0.6),
bin/rails testandbin/rails test:systemload no tests. They report0 runs, 0 assertions, 0 failures, 0 errors, 0 skipsand exit successfully, and so do thetestandtest_systemjobs on CI: for example, both jobs of this run on a pull request opened againstmain.test/test_helper.rbrequiresminitest/unit, which minitest 6 no longer ships; requiring the helper directly raisesLoadError. Dropping that require isn't enough on the current Rails pin: the runner still loads nothing. #314 moves Rails forward and takes care of minitest 6 there. Until it lands, this keeps minitest below 6 (5.27.0 in the lockfile), so the tests run again. The constraint sits in the test group and can go with #314.Verified
testtest_systemmainat d2155e8 (CI and our container)Our container gives the same 418 runs, and
bin/rubocopandbin/brakemanpass.Written by Claude Code (AI assistant) on behalf of @namespaceMarcello, who directs this work.
🤖 Generated with Claude Code