Stop a fake team inheriting a real same-named team's plugin data - #36
Open
corsonknowles wants to merge 1 commit into
Open
Stop a fake team inheriting a real same-named team's plugin data#36corsonknowles wants to merge 1 commit into
corsonknowles wants to merge 1 commit into
Conversation
Plugin's registry is keyed by team name, and the around hook installed by Testing.enable! only busts it after an example that built a fake team. An example that reads only real teams therefore leaves real-team plugin instances in the registry, and a later fake team sharing a real team's name silently receives the real team's cached plugin data instead of its own. This makes suites order-dependent in a way that is hard to trace: the spec passes in isolation and fails only when some earlier example happened to touch a real team of the same name. Bust the plugin registry as each fake team is registered so the fake always registers first. Only Plugin.bust_caches! is called, not CodeTeams.bust_caches!, so CodeTeams.all stays memoized and no team YAML is re-read. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
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
CodeTeams::Plugin's registry is keyed by team name (register_teamusesteam.raw_hash['name']), and thearoundhook installed byTesting.enable!only busts it after an example that built a fake team:An example that reads only real teams satisfies neither branch, so it leaves real-team plugin instances in the registry. A later
code_team_with_config(name: 'Some Real Team', ...)then hits that cached entry and silently receives the real team's plugin data —files,slack,github, whatever the host app's plugins expose — instead of the config it just declared.The failure mode is nasty to trace: the spec passes in isolation and fails only when some earlier example happened to touch a real team of the same name, so it presents as a seed-dependent flake with no visible connection to team config. We hit this in a downstream app where a fake
Developer Productivity Railsteam inherited the real team's protected-files list, silently protecting a file the example never declared.Fix
Bust the plugin registry as each fake team is registered, so the fake always registers ahead of a same-named real team.
Only
Plugin.bust_caches!is called, notCodeTeams.bust_caches!—CodeTeams.allstays memoized and no team YAML is re-read, so this doesn't reintroduce the cost the.any?guard exists to avoid. The cost is paid only by suites that actually build fake teams.Test
The added spec reproduces the leak deterministically inside a single example (no ordering dependency): write a real
My Teamwith plugin data, read it through a plugin so the registry caches it, then build a fakeMy Teamand assert it sees its own data.Without the fix:
bundle exec rspec(20 examples, 0 failures),bundle exec rubocop, andbundle exec srb tcare all green with the fix.