Repository navigation
[test-parallel] test: add t.Parallel() to safe tests in pkg/cli (batch) - #56039
Conversation
Add t.Parallel() to top-level tests and table-driven subtests in: - compile_batch_operations_test.go (TestRunActionlintOnFiles_EmptyList, TestRunBatchDirectoryTool_NonStrictSwallowsErrors, TestRunBatchDirectoryTool_StrictWrapsErrors) - compile_config_test.go (TestCompileConfig_ShellcheckEnabled and subtests) - compile_dependabot_validation_test.go (TestCompileDependabotValidation and subtests) These tests operate on pure in-memory logic with no os.Chdir, env mutation, or shared global state, so they are safe to run in parallel. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
✅ PR Code Quality Reviewer completed the code quality review. Reviewed PR #56039 and found no actionable changed-line issues worth commenting on; no GitHub write beyond completion signaling was needed.
|
|
🧠 Matt Pocock Skills Reviewer has completed the skills-based review. ✅
|
|
✅ 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 (7 additions detected across 3 files).
|
|
✅ Test Quality Sentinel completed test quality analysis. Test Quality Sentinel skipped because pre-fetch PR data was unavailable: unable to fetch test file diff
|
|
✅ Ponytail Reviewer completed successfully! Lean already. Ship. No over-engineering findings in this test-parallelization diff.
|
There was a problem hiding this comment.
Pull request overview
Adds safe test parallelization in pkg/cli to reduce test runtime.
Changes:
- Parallelizes three batch-operation tests.
- Parallelizes shellcheck configuration table tests.
- Parallelizes Dependabot validation table tests.
Show a summary per file
| File | Description |
|---|---|
pkg/cli/compile_batch_operations_test.go |
Runs independent batch-tool tests concurrently. |
pkg/cli/compile_config_test.go |
Parallelizes pure configuration checks. |
pkg/cli/compile_dependabot_validation_test.go |
Parallelizes validation cases. |
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
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Skills-Based Review 🧠
Applied /tdd — approving. The additions are correct, the PR description is thorough, and the author validated with -race before submitting.
📋 Summary
What was checked
- All three changed files add
t.Parallel()only to pure-logic tests with no shared mutable state, noos.Chdir, not.Setenv, and no filesystem/git side effects — exactly the right candidates. - Table-driven subtests in
compile_config_test.goandcompile_dependabot_validation_test.gocorrectly addt.Parallel()to both the parent and subtest loops (the standard Go pattern). - The PR description clearly documents which files were skipped and why, demonstrating solid understanding of parallelism safety.
Positive Highlights
- ✅
-racerun confirmed clean before merge - ✅ Skipped files correctly identified (chdir, Setenv, external processes)
- ✅ No production code touched — zero risk surface
🧠 Reviewed using Matt Pocock's skills by Matt Pocock Skills Reviewer · copilot · sonnet46 · 20.4 AIC · ⌖ 9.84 AIC · ⊞ 7.6K
Comment /matt to run again
There was a problem hiding this comment.
All added t.Parallel() calls are in pure unit tests with no shared mutable state, global variables, filesystem writes, or environment mutations. Safe to parallelize. LGTM.
🧵 Reviewed using Impeccable skills by Impeccable Skills Reviewer · copilot · sonnet46 · 18.6 AIC · ⌖ 9.02 AIC · ⊞ 6.2K
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.
|
|
🎉 This pull request is included in a new release. Release: |
Summary
Adds
t.Parallel()to test functions and their table-driven subtests across three files inpkg/clithat were identified as safe for parallel execution. This is part of a batch effort to speed up the test suite by enabling parallelism for tests without shared mutable state or ordering dependencies.Change Classification
Key Changes
t.Parallel()to three test functionst.Parallel()to the main test function and its nested subtestst.Parallel()to the main test function and its nested subtestsImpact Assessment
No downstream impact identified. Changes are limited to test files and only add parallel execution markers; no test logic or assertions were modified.
Commits