ci: actually run the Zeitwerk check - #423
Merged
Merged
Conversation
The step was guarded by `if grep -q zeitwerk config/application.rb`, which never matches: Redmine configures Zeitwerk in config/initializers/zeitwerk.rb, not application.rb. So the step has been reporting success without ever running the task. Verified on a recent run: the step passes with zero log output, no "eager loading" and no "All is good!". The guard is unnecessary anyway, since the rake task ships with Rails 6+ and every row in this matrix has it.
There was a problem hiding this comment.
Pull request overview
This PR fixes a false-positive CI signal by ensuring the Rails Zeitwerk autoloading check actually runs in the PostGIS test workflow, so constant/file mismatches are caught during CI rather than surfacing only in production eager-load boots.
Changes:
- Removes the ineffective grep-based guard that prevented
zeitwerk:checkfrom running. - Runs
bundle exec rake zeitwerk:checkunconditionally in the Redmine working directory. - Adds inline workflow documentation explaining why the check matters and why the guard was incorrect.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
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
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.
The Zeitwerk check step in
test-postgis.ymlhas never run.It is guarded by:
config/application.rbdoes not mention Zeitwerk in any supported Redmine version. Redmine configures it inconfig/initializers/zeitwerk.rb, so the grep always fails and the step exits 0 without doing anything.Evidence: on the most recent run, the "Zeitwerk check" step is green with zero log output — no "Hold on, I am eager loading the application", no "All is good!". A step that reports success without running is worse than no step, because the repository looks covered.
Why it matters
Test and development environments have
eager_load = false; production has it true. So a filename/constant mismatch that nothing references at boot passes every test and then kills a production instance at startup with aNameError.Not hypothetical:
redmine_gtt_syncshipped exactly this bug. Its CI was green on Redmine 6.1 and 7.0 while every production image built over almost four weeks failed to boot (uninitialized constant RedmineGttSync::Oauth). Running this task reproduces that failure, so an active gate would have caught it the day it landed.The change
Drop the guard and run the task unconditionally. It ships with Rails 6+, so every row in this matrix has it; the guard protected against nothing.
Verified against Redmine 7.0.0 / Ruby 4.0.6 with this plugin installed alongside nine others:
rake zeitwerk:checkreports "All is good!", so this should not turn the matrix red. If a row does fail, that is a real latent bug worth surfacing.Companion changes apply the same fix to redmine_gtt_fiware (identical dead guard) and add the step to redmine_custom (missing entirely).