[test-parallel] test: add t.Parallel() to safe pkg/cli test cases - #53568
Conversation
Added t.Parallel() to top-level tests and table-driven subtests in: - pkg/cli/audit_expanded_test.go - pkg/cli/bootstrap_profile_git_test.go - pkg/cli/cli_consistency_help_test.go All modified tests use per-test isolated resources (t.TempDir/testutil.TempDir, fresh cobra command instances) with no process-wide state mutation (no os.Setenv/t.Setenv/os.Chdir), no shared mutable globals, and no fixed ports/paths. Verified with 'go test -race' on affected tests and full 'go test ./pkg/cli/...' (2 unrelated pre-existing failures confirmed present on main, caused by sandbox network/TTY restrictions). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
✅ PR Code Quality Reviewer completed the code quality review.
|
|
✅ Ponytail Reviewer completed successfully! Lean already. Ship.
|
|
🧠 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
|
|
✅ Design Decision Gate 🏗️ completed the design decision gate check. No ADR enforcement needed: PR #53568 does not have the 'implementation' label and has only 36 new lines of code in business logic directories (threshold: 100).
|
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
No blocking issues found in the changed lines.
Notes
I checked the newly added t.Parallel() calls against the touched tests and the helper/command construction paths they exercise. The changed cases appear to stay within per-test temp directories or freshly constructed command instances, and I did not find a concrete shared-state hazard in the modified lines that would justify a blocking review comment.
🔎 Code quality review by PR Code Quality Reviewer · gpt54 · 10.6 AIC · ⌖ 6.74 AIC · ⊞ 4.5K
Comment /review to run again
There was a problem hiding this comment.
LGTM. The changes are confined to adding t.Parallel() to top-level test functions and their table-driven subtests in the pkg/cli package. All tests use t.TempDir() / testutil.TempDir for isolation, and the module targets Go 1.26.5 (≥ 1.22), so loop-variable capture in range loops is safe. No shared mutable state or other parallelism hazards are introduced.
🧵 Reviewed using Impeccable skills by Impeccable Skills Reviewer · sonnet46 · 21.4 AIC · ⌖ 8.78 AIC · ⊞ 5.7K
There was a problem hiding this comment.
Skills-Based Review 🧠
Applied /tdd — changes look correct and safe to approve.
📋 Review Summary
Assessment
All t.Parallel() additions are correct:
audit_expanded_test.go— top-level tests and subtests all usetestutil.TempDir/t.TempDir()for isolation; no shared mutable state.bootstrap_profile_git_test.go— each test operates on its owninitBootstrapGitRepodirectory; no shared globals or env mutation.cli_consistency_help_test.go— tests construct freshcobra.Commandtrees viaNewXCommand();runtime.Caller(0)for doc-path resolution is read-only and race-safe.
The PR description correctly identifies and excludes the unsafe files (t.Setenv users, stdout/stderr mutators). The validation section (race-detector pass + pre-existing failure acknowledgement) is thorough.
Positive Highlights
- ✅ Conservative approach: only safe files modified
- ✅ Clear exclusion rationale for every omitted file
- ✅ Race-detector validation cited in the PR body
🧠 Reviewed using Matt Pocock's skills by Matt Pocock Skills Reviewer · sonnet46 · 19.5 AIC · ⌖ 9.74 AIC · ⊞ 7.8K
Comment /matt to run again
There was a problem hiding this comment.
Pull request overview
Adds safe Go test parallelization across isolated pkg/cli tests.
Changes:
- Parallelizes independent audit tests and table-driven subtests.
- Parallelizes isolated Git repository tests.
- Parallelizes CLI help consistency tests.
Show a summary per file
| File | Description |
|---|---|
pkg/cli/audit_expanded_test.go |
Parallelizes isolated audit tests. |
pkg/cli/bootstrap_profile_git_test.go |
Parallelizes per-test repository scenarios. |
pkg/cli/cli_consistency_help_test.go |
Parallelizes independent help checks. |
Review details
💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.
- Files reviewed: 3/3 changed files
- Comments generated: 0
- Review effort level: Balanced
|
🎉 This pull request is included in a new release. Release: |
Caution
agentic threat detected
Threat detection flagged this output in warn mode. Manual review is REQUIRED before any follow-up automation.
Details
Potential security threats were detected in the agent output.
Review the workflow run logs for details.
-> Generated by PR Description Updater for #53568 · auto · 45.4 AIC · ⌖ 5.09 AIC · ⊞ 7.6K · ◷