[test-parallel] Daily Go Test Parallelizer: add t.Parallel() to 6 safe test files

[github-actions]

test> Generated by PR Description Updater for #53967 · auto · 42.9 AIC · ⌖ 5.43 AIC · ⊞ 7.7K · ◷

[github-actions]

✅ Ponytail Reviewer completed successfully!

Lean already. Ship.

Generated by Ponytail Reviewer for #53967

[github-actions]

✅ PR Code Quality Reviewer completed the code quality review.

🔎 Code quality review by PR Code Quality Reviewer

[github-actions]

✅ Design Decision Gate 🏗️ completed the design decision gate check.

No ADR enforcement needed: PR #53967 does not have the 'implementation' label and has only 38 new lines of code in business logic directories (threshold: 100).

🏗️ ADR gate enforced by Design Decision Gate 🏗️

[github-actions]

✅ Test Quality Sentinel completed test quality analysis.

Test Quality Sentinel skipped because pre-fetch PR data was unavailable: unable to fetch test file diff

🧪 Test quality analysis by Test Quality Sentinel

[github-actions]

🧠 Matt Pocock Skills Reviewer has completed the skills-based review. ✅

🧠 Reviewed using Matt Pocock's skills by Matt Pocock Skills Reviewer

[github-actions]

Comment Memory

reviewed_at: 2026-08-19T00:00:00Z
review_event: REQUEST_CHANGES
top_themes:
  - parallelized test still reads fixed external /tmp path
  - repository-root workflow readers are not isolated fixtures
files_reviewed:
  - pkg/cli/contribution_check_workflow_contract_test.go
  - pkg/cli/copilot_agent_test.go
  - pkg/cli/copilot_metrics_fix_test.go
  - pkg/cli/copilot_token_extraction_test.go
  - pkg/cli/drain3_integration_test.go
  - pkg/cli/effective_tokens_compliance_test.go
comment_count: 2

Note

This comment is managed by comment memory.

It stores persistent context for this thread in the code block at the top of this comment.
Edit only the text inside the backtick fences; workflow metadata and the footer are regenerated automatically.

Learn more about comment memory

🔎 Code quality review by PR Code Quality Reviewer · gpt54 · 13.4 AIC · ⌖ 6.97 AIC · ⊞ 7K · ◷
_{Comment /review to run again}

[github-actions]

Requesting changes

Adding t.Parallel() here is not actually safe across this batch: at least two of these tests still touch shared external/process-wide state, so parallel execution can make them flaky in CI.

Blocking themes

• copilot_token_extraction_test.go now runs a test in parallel that depends on a fixed host path outside t.TempDir().
• contribution_check_workflow_contract_test.go now runs two top-level tests in parallel even though both synchronously walk/read the live repository tree and compiled workflow artifacts, which increases cross-test coupling with any other test mutating repo contents.

🔎 Code quality review by PR Code Quality Reviewer · gpt54 · 13.4 AIC · ⌖ 6.97 AIC · ⊞ 7K
_{Comment /review to run again}

[github-actions]

This test now runs in parallel even though it depends on a fixed host path outside the test sandbox, so another concurrent test/job touching the same /tmp/run-20696085597/... tree can make it nondeterministic.

💡 Why this is a real flake risk

TestCopilotTokenExtractionWithRealLogData is no longer self-contained once t.Parallel() is added: it reads from a hard-coded absolute path instead of test-local fixtures. That means the test now races any other parallel work in the same process or runner that creates, deletes, or rewrites that tree, and the failure mode is exactly the kind of CI-only heisenbug that is painful to reproduce.

A safer fix is to keep this specific test serial, or better, move the sample log under a testdata fixture so the test only reads repository-controlled input.

func TestCopilotTokenExtractionWithRealLogData(t *testing.T) {
    // no t.Parallel() until the hard-coded external path is removed
}

[github-actions]

These two top-level tests now run in parallel while both read live workflow source and compiled lock files from the repository root, which makes them coupled to any other test mutating those files and undermines the “safe to parallelize” claim in the PR.

💡 Why this should stay serial

The justification in the PR body says these files were only parallelized when they had no shared paths or cross-test ordering concerns, but both tests here resolve the same repo root and inspect the same workflow artifacts on disk. That is not isolated test input; it is shared process-visible state.

Even if this file’s two tests only read today, marking them parallel bakes in hidden coupling to the rest of the package test suite and makes future workflow-rewriting tests much easier to break. The conservative move is to leave repository-fixture readers serial unless the input is copied into t.TempDir() first.

func TestContributionCheckWorkflowSafeOutputContract(t *testing.T) {
    // keep serial, or copy the workflow files into a temp fixture first
}

[github-actions]

The parallelization changes look correct. All modified tests use isolated temp directories (t.TempDir() or os.MkdirTemp), and Go 1.22+ loop-variable semantics eliminate the old capture concern. No shared mutable state is introduced across the 6 test files. LGTM.

🧵 Reviewed using Impeccable skills by Impeccable Skills Reviewer · sonnet46 · 21.7 AIC · ⌖ 11.6 AIC · ⊞ 5.7K

[Copilot]

Pull request overview

Adds safe parallel execution to isolated Go tests in pkg/cli, reducing test-suite runtime without changing production behavior.

Changes:

• Parallelizes independent top-level tests across six files.
• Parallelizes table-driven subtests where fixtures are isolated.

Show a summary per file

File	Description
pkg/cli/contribution_check_workflow_contract_test.go	Parallelizes read-only workflow contract tests.
pkg/cli/copilot_agent_test.go	Parallelizes isolated agent parsing and detection tests.
pkg/cli/copilot_metrics_fix_test.go	Parallelizes temp-directory metrics tests.
pkg/cli/copilot_token_extraction_test.go	Parallelizes isolated token extraction tests.
pkg/cli/drain3_integration_test.go	Parallelizes independent Drain3 tests.
pkg/cli/effective_tokens_compliance_test.go	Parallelizes pure compliance calculations.

Review details

💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.

• Files reviewed: 6/6 changed files
• Comments generated: 0
• Review effort level: Balanced

[github-actions]

Skills-Based Review 🧠

Applied /tdd — approving with one minor style note.

📋 Key Themes & Highlights

Key Themes

• t.Parallel() placement is correct: top-level functions get it first, table-driven subtests get it inside the t.Run closure — both in the right positions.
• Safety analysis is well-documented: the PR body clearly states the criteria used and which files were left untouched and why.
• Race detector validation passed: go test -race was run on all modified functions before merge — this is the right gate for parallelism changes.

Positive Highlights

• ✅ No logic changes whatsoever — purely additive, zero-risk to test correctness.
• ✅ t.TempDir() already used in most files — correct for parallel tests.
• ✅ Pre-existing test failures are clearly attributed to the base branch, not to this PR.
• ✅ Go 1.26 loop-variable scoping note is accurate — no spurious tt := tt rebinding needed.

One Minor Note

In copilot_agent_test.go, the parallel subtests of TestCopilotCodingAgentDetector_IsGitHubCopilotCodingAgent use os.MkdirTemp("", ...) with defer os.RemoveAll. Prefer t.TempDir() — it cleans up automatically when the subtest ends and is the idiomatic choice in parallel subtests. Not blocking, but worth a follow-up.

🧠 Reviewed using Matt Pocock's skills by Matt Pocock Skills Reviewer · sonnet46 · 20.8 AIC · ⌖ 9.92 AIC · ⊞ 7.8K
_{Comment /matt to run again}

[github-actions]

🎉 This pull request is included in a new release.

Release: v0.87.2