fix(lint): serialize Configure() and isolate test registry mutations#755
Merged
Conversation
Two related global-state issues in pkg/lint: 1. RunLinters() held the registry as a reader while calling Configure() on globally-registered linter singletons, which mutates fields like raiseError, allowedTypes, and charsets. Concurrent callers (e.g. parallel migration tests) raced on those fields. Promote to a writer lock so Configure+Lint is serialized — RunLinters has no concurrent callers in production. Fixes block#747. 2. Tests called Reset() to wipe the registry for isolation but never restored the init() snapshot, polluting later tests in the same binary. With -count=2, TestPlanChanges_MultiStatementSameTable failed because HasFKLinter was no longer registered. Add a resetForTest(t) helper that captures the init() snapshot once and restores it via t.Cleanup, and migrate the existing Reset() call sites. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
morgo
force-pushed
the
fix-lint-test-isolation
branch
from
a00317c to
f0038c3
Compare
aparajon
approved these changes
May 1, 2026
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
2 participants
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.
Summary
Two related global-state issues in
pkg/lint:Race in
RunLinters()(fixes data race: global linter singletons mutated concurrently by Configure() #747) — held the registry as a reader while callingConfigure()on globally-registered linter singletons, which mutates fields likeraiseError,allowedTypes,charsets. Concurrent callers (parallel migration tests) raced on those fields under-race. Promoted to a writer lock soConfigure()+Lint()are serialized; in productionRunLintershas no concurrent callers, so the extra serialization is free.Test pollution from bare
Reset()— tests calledReset()to wipe the registry for isolation but never restored the init() snapshot, so subsequent tests in the same binary saw a partial registry. With-count=2,TestPlanChanges_MultiStatementSameTablefailed becauseHasFKLinterwas no longer registered. Added aresetForTest(t)helper that captures the init() snapshot once and restores it viat.Cleanup. Migrated existing call sites;TestResetitself still uses bareReset()since it directly tests the wipe semantics.Test plan
go test -race ./pkg/lint/ -count=5— passesgo test -race -run "TestLintOnly|TestLintNoViolations" -count=20 ./pkg/migration/— passes (was the racing pair from data race: global linter singletons mutated concurrently by Configure() #747)go test -race -run "TestLint" ./pkg/migration/ -count=3— passesgo test ./pkg/lint/ -count=2— passes (was failing on main due to test pollution)golangci-lint run ./pkg/lint/...— cleanFixes #747
🤖 Generated with Claude Code