[test-parallel] test: add t.Parallel() to safe codemod tests in pkg/cli - #53617
Conversation
Adds t.Parallel() to TestGetPermissionsReadCodemod and related permissions codemod tests, plus TestMCPModeToTypeCodemod and its subtests. Each test constructs its own local Codemod instance (a plain struct with pure function fields) and only touches local content/frontmatter variables — no shared mutable state, env vars, working directory changes, fixed ports/paths, or external services. Validated with go test -race on the affected tests. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
Great work on improving test parallelization! 🎯 This automated contribution from the Daily Go Test Parallelizer workflow is a solid addition to the test infrastructure in What's working well:
Minor note: The PR is currently in draft status. Once ready for merge, mark it as ready for review. The changes align perfectly with the project's emphasis on maintaining clean, efficient test suites. This looks ready to move forward! ✨
|
|
✅ Ponytail Reviewer completed successfully! Lean already. Ship. Diff only adds t.Parallel() calls to existing test functions/subtests in codemod_permissions_test.go and codemod_mcp_mode_to_type_test.go - no new abstractions, dependencies, hand-rolled logic, or dead flexibility introduced. Nothing to cut.
|
|
✅ Design Decision Gate 🏗️ completed the design decision gate check. No ADR enforcement needed: PR #53617 does not have the 'implementation' label and has only 29 new lines of code in business logic directories (threshold: 100).
|
|
✅ Test Quality Sentinel completed test quality analysis. Test Quality Sentinel skipped because pre-fetch PR data was unavailable: unable to fetch test file diff
|
|
🧠 Matt Pocock Skills Reviewer has completed the skills-based review. ✅
|
|
✅ PR Code Quality Reviewer completed the code quality review.
|
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
The changes correctly add t.Parallel() to all top-level test functions and their subtests. The ordering is correct — parent t.Parallel() calls appear before subtest registrations, and each t.Run closure also calls t.Parallel() first. No shared mutable state is visible across tests, so parallelism is safe here. LGTM.
🧵 Reviewed using Impeccable skills by Impeccable Skills Reviewer · sonnet46 · 14 AIC · ⌖ 8.77 AIC · ⊞ 5.7K
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.
No blocking issues in the changed lines. The new t.Parallel() calls only touch tests that construct fresh codemod values and operate on per-test local inputs, so I do not see a shared-state or ordering hazard introduced by this diff.
### Review notes
I checked the two changed test files against the codemod implementation shape in pkg/cli/fix_codemods.go plus the relevant codemod constructors. The Codemod value is a plain struct carrying function pointers, and these tests pass independent content / frontmatter values into Apply, so the parallelization looks mechanically safe. I did not find a correctness, race, or maintainability issue in the changed lines that warrants blocking the PR.
🔎 Code quality review by PR Code Quality Reviewer · gpt54 · 6.72 AIC · ⌖ 6.88 AIC · ⊞ 7K
Comment /review to run again
There was a problem hiding this comment.
Skills-Based Review 🧠
Applied /tdd — changes look correct and safe to approve.
📋 Summary
Positive Highlights
- ✅ Correct safety analysis: both files share a
Codemodstruct with pure function fields — no mutable state, safe to parallelize - ✅ Pattern is consistent with the 13 already-parallel files in this batch
- ✅ Validation step (
go test -race) confirms no data races - ✅ PR description clearly documents which files were skipped and why (loop-variable capture risks), showing disciplined per-file review
No actionable issues found.
🧠 Reviewed using Matt Pocock's skills by Matt Pocock Skills Reviewer · sonnet46 · 14.5 AIC · ⌖ 9.79 AIC · ⊞ 7.8K
Comment /matt to run again
There was a problem hiding this comment.
Pull request overview
Adds safe parallel execution to stateless codemod tests, reducing test-suite runtime.
Changes:
- Parallelizes 22 permissions codemod tests.
- Parallelizes the MCP mode-to-type test and its six subtests.
Show a summary per file
| File | Description |
|---|---|
pkg/cli/codemod_permissions_test.go |
Enables parallel execution for permissions tests. |
pkg/cli/codemod_mcp_mode_to_type_test.go |
Enables parallel execution for the test and subtests. |
Review details
💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.
- Files reviewed: 2/2 changed files
- Comments generated: 0
- Review effort level: Balanced
|
🎉 This pull request is included in a new release. Release: |
Summary
Added
t.Parallel()calls to test functions and their subtests in two codemod test files underpkg/cli. This allows these previously sequential tests to run concurrently, reducing overall test execution time. No test logic or assertions were changed.Change Classification
Key Changes
t.Parallel()to the top-level test function and eacht.Runsubtestt.Parallel()to all test functionsImpact Assessment
No downstream impact identified. The tests are independent (each constructs its own workflow content string and codemod instance), so enabling parallel execution does not introduce shared-state races. Test behavior and coverage remain unchanged; only execution scheduling is affected.
Commits