Fix flaky acts_as_model specs caused by RubyLLM.logger spec leaving config nil#815
Open
cgmoore120 wants to merge 3 commits into
Open
Fix flaky acts_as_model specs caused by RubyLLM.logger spec leaving config nil#815cgmoore120 wants to merge 3 commits into
cgmoore120 wants to merge 3 commits into
Conversation
The `.logger` spec reset `RubyLLM.@config`/`@logger` to nil in its `after` hook instead of restoring the originals. Because the AR integration sets `config.model_registry_source` only once (in ActsAs.included at load time), a nil-ed config is rebuilt without that source, so any later spec in the same parallel test-queue worker that relies on the database-backed model registry (acts_as_model registry integration) silently falls back to the JSON registry and fails. Use an around hook that saves and restores the original config/logger so the global state is left untouched. This is order-dependent and only surfaced intermittently under the parallel queue.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #815 +/- ##
==========================================
- Coverage 81.47% 81.45% -0.03%
==========================================
Files 165 165
Lines 7402 7402
Branches 1233 1233
==========================================
- Hits 6031 6029 -2
- Misses 874 875 +1
- Partials 497 498 +1 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
5 tasks
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.
Problem
The
acts_as_model"model registry integration" specs (loads models from database when configured,finds models from database) fail intermittently under the parallel test queue (bin/rspec-queue), with errors like:The failures are order-dependent: they only appear when certain specs land in the same worker, so they surface non-deterministically depending on how
test-queuedistributes examples (more likely on slower Ruby versions).Root cause
The
RubyLLM.loggerspec resets the global config/logger in itsafterhook by setting them tonilinstead of restoring the originals:Once
@configis leftnil, the next access lazily rebuilds a freshConfiguration. Butconfig.model_registry_sourceis only ever set once — inRubyLLM::ActiveRecord::ActsAs.includedat load time — so the rebuilt config no longer has it. Any later spec in the same worker that relies on the database-backed model registry (Models#load_modelsreadsconfig.model_registry_source) then silently falls back to the JSON registry and fails to find the test records.This reproduces deterministically by running the two specs in order:
Fix
Use an
aroundhook that saves and restores the original@config/@logger, so the logger spec leaves global state untouched. After the fix the same ordered run passes (22 examples, 0 failures), andruby_llm_specitself stays green.