fix(test): restore the CLI output globals after every test that sets them - #1233
Merged
Conversation
…them The shuffle lane added in #1230 went red on its first unrelated PR, which is what it is for. globalOutputFormat and globalJSONOutput are package-level flag targets in cmd/mcpproxy. Fifteen tests assigned them and never restored, so whichever ran last left its choice installed for the rest of the binary. In file order that is invisible. Under -shuffle=on it is not: TestVersionCommandTableOutput and TestOutputActivityError_TableFormat both render through clioutput.ResolveFormat and assert the default table output, so an inherited "json" fails them. Seed 1788937037978355714 reproduces both on origin/main. Adds setOutputGlobals(t, format, jsonOut), which saves the pair and restores it from t.Cleanup, and routes all fifteen sites through it. The sites that already saved and restored by hand are left alone. No production code changes, and no test's intended format changes. Verified: the CI seed fails on main and passes here; five fresh -race -shuffle=on runs of the package are green; golangci-lint v2 with the CI config reports 0 issues. Note for a follow-up, not touched here: cmd/mcpproxy/activity_cmd_test.go is already unformatted on origin/main.
Deploying mcpproxy-docs with
|
| Latest commit: |
20fa67c
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://db9c2eab.mcpproxy-docs.pages.dev |
| Branch Preview URL: | https://fix-cli-output-globals-test.mcpproxy-docs.pages.dev |
Dumbris
enabled auto-merge (squash)
September 9, 2026 07:09
|
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
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.
What the shuffle lane found
The advisory
Unit Tests (shuffle)lane from #1230 went red on its first unrelated PR (#1232, docs only). That is the lane working as intended.globalOutputFormatandglobalJSONOutputare package-level flag targets incmd/mcpproxy. Fifteen tests assigned them and never restored, so whichever ran last left its choice installed for the rest of the test binary. In file order that is invisible. Under-shuffle=onit is not:TestVersionCommandTableOutputandTestOutputActivityError_TableFormatboth render throughclioutput.ResolveFormatand assert the default table output, so an inherited"json"fails them.Reproduce on
origin/main:go test -count=1 -shuffle=1788937037978355714 ./cmd/mcpproxy/Same class as #1222 (registry fixtures leaking the SSRF allow-policy) and the
BlockedValuesleak fixed in #1230, different global.The fix
Adds
setOutputGlobals(t, format, jsonOut)in a newcmd/mcpproxy/output_globals_test.go: it saves the pair and restores it fromt.Cleanup, so a failing or panicking test still restores. All fifteen bare assignment sites route through it (3 intools_cmd_test.go, 12 inupstream_cmd_test.go). The sites that already saved and restored by hand are left alone. No production code changes; no test's intended format changes.TestSetOutputGlobalsRestorespins the restore half, so removing thet.Cleanupfails a test rather than only showing up as a shuffled-order surprise later.Verification
1788937037978355714onorigin/mainokgo test -count=1 -race -shuffle=on ./cmd/mcpproxy/× 5okgolangci-lintv2,.github/.golangci.yml,./cmd/mcpproxy/...Follow-up, not touched here
cmd/mcpproxy/activity_cmd_test.gois already unformatted onorigin/main(gofmt -lflags it before and after this change). Left alone to keep this diff to the one problem.