[test-parallel] test: add missing t.Parallel() in codemod_mcp_network_addallowed_test.go - #56714
Conversation
…llowed_test.go Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
✅ Design Decision Gate 🏗️ completed the design decision gate check. See the comment below for the result and any generated ADR draft. No ADR enforcement needed: PR does not have the 'implementation' label and has <=100 new lines of code in business logic directories.
|
|
✅ PR Code Quality Reviewer completed the code quality review.
|
|
Lean already. Ship. Warning Firewall blocked 4 domainsThe following domains were blocked by the firewall during workflow execution:
[!TIP] tools:
github:
mode: gh-proxySee GitHub Tools for more information on To allow these domains, add them to the network:
allowed:
- defaults
- "ab.chatgpt.com"
- "api.github.com"
- "chatgpt.com"
- "github.com"See Network Configuration for more information.
|
|
🧠 Matt Pocock Skills Reviewer has completed the skills-based review. ✅
|
|
✅ Test Quality Sentinel completed test quality analysis. Test Quality Sentinel skipped because pre-fetch PR data was unavailable: unable to fetch test file diff
|
Comment MemoryNote This comment is managed by comment memory.It stores persistent context for this thread in the code block at the top of this comment.
|
There was a problem hiding this comment.
Verdict
Non-blocking: this change only adds t.Parallel() to an isolated subtest, and the touched test body uses only local data with no shared mutable state.
Themes reviewed
- parallel test safety and shared-state hazards
- changed-line correctness impact
- need for additional coverage around the new parallelization point
I did not find an actionable correctness, concurrency, or maintainability issue in the changed lines.
🔎 Code quality review by PR Code Quality Reviewer · pi · gpt54 · 2 AIC · ⌖ 7.3 AIC · ⊞ 7K
Comment /review to run again
There was a problem hiding this comment.
LGTM — straightforward addition of the missing t.Parallel() to keep the subtest consistent with its siblings.
🧵 Reviewed using Impeccable skills by Impeccable Skills Reviewer · copilot · sonnet46 · 13.2 AIC · ⌖ 10.5 AIC · ⊞ 6.3K
There was a problem hiding this comment.
Pull request overview
Adds safe parallel execution to the remaining eligible addAllowedToNetwork subtest.
Changes:
- Calls
t.Parallel()in the comment-line network-block subtest.
Show a summary per file
| File | Description |
|---|---|
pkg/cli/codemod_mcp_network_addallowed_test.go |
Parallelizes an isolated subtest. |
Review details
💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.
- Files reviewed: 1/1 changed files
- Comments generated: 0
- Review effort level: Balanced
There was a problem hiding this comment.
Skills-Based Review 🧠
Applied /tdd — approved, no issues found.
This is a single-line addition of t.Parallel() to a subtest confirmed safe: the subtest operates only on local slices/strings via a pure function with no shared state. The PR body documents race-detector validation (go test -race), which is exactly the right evidence for this type of change.
🧠 Reviewed using Matt Pocock's skills by Matt Pocock Skills Reviewer · copilot · sonnet46 · 15.5 AIC · ⌖ 10.3 AIC · ⊞ 7.7K
Comment /matt to run again
Summary
Analyzed a batch of 25
pkg/cli/codemod_*_test.gofiles (round-robin, resuming afterpkg/cli/codemod_difc_proxy_test.go) for safet.Parallel()additions.Analysis
24 of 25 files already had
t.Parallel()on every top-level test and eligible subtest. One subtest incodemod_mcp_network_addallowed_test.gowas missing it:TestAddAllowedToNetwork/comment line inside network block is skipped, then top-level sibling ends the blockThis subtest only operates on local slices/strings via the pure
addAllowedToNetworkfunction — no env vars, globals, shared filesystem paths, or external services — so it is safe to parallelize alongside its sibling subtests.pkg/cli/codemod_factory_test.gowas flagged unsafe due to a shared package-level logger (testFactoryLog) reused across tests, so it was left unchanged.Validation
go test -race ./pkg/cli/ -run TestAddAllowedToNetwork -v— PASSgo test ./pkg/cli/... -run TestAddAllowedToNetwork— PASSState
Cache updated to
pkg/cli/codemod_messages_effective_tokens_suffix_to_ai_credits_suffix_pure_test.go(last file in this batch) for the next daily run.