Improve test quality for activation checkout tests with testify

[Copilot]

pkg/workflow/activation_checkout_test.go used raw if ... t.Error/t.Fatal checks, duplicated brittle job-boundary parsing logic, and left several pure helper functions in compiler_activation_job.go untested.

Testify adoption

• Replaced manual error/assertion checks with require/assert throughout the checkout test.

File co-location

• Renamed activation_checkout_test.go → compiler_activation_checkout_test.go to sit alongside compiler_activation_job.go.

Job-boundary parsing

• Removed the duplicated, indentation-heuristic job-section extraction and replaced it with the existing shared extractJobSection helper (already used across ~30 other test files but not here).
• Added a dedicated TestExtractJobSection covering job-at-end-of-file, job-followed-by-another-job, nested multi-level indentation, and missing-job cases.

New positive checkout case

• Top-level contents: write permissions are unconditionally rejected by validateDangerousPermissions, so a literal "workflow with contents: write" case isn't compilable. Instead, added a subtest using safe-outputs.create-pull-request (which requires contents: write in its own downstream job) to verify the activation job still performs only the sparse .github/.agents checkout, never a full-repo checkout, even when write-capable safe-outputs are configured.

New unit tests (no integration build tag)

• activationEventSet — string/list/map on: forms, metadata-field filtering (reaction, stop-after, etc.), invalid YAML, and unsupported types.
• buildCentralizedCommandOnSection — single event, pull_request_comment/issue_comment dedup, unknown identifiers, default (all events) behavior.
• (injectIfConditionAfterName already had coverage in compiler_activation_job_test.go.)

• Fixes [testify-expert] Improve Test Quality: pkg/workflow/activation_checkout_test.go #52336

---

Generated by 👨‍🍳 PR Sous Chef · gpt54 · 7.2 AIC · ⌖ 7.73 AIC · ⊞ 8.5K · ◷
_{Comment /souschef to run again}

---

Generated by 👨‍🍳 PR Sous Chef · gpt54 · 8.41 AIC · ⌖ 6.61 AIC · ⊞ 8.5K · ◷
_{Comment /souschef to run again}

[github-actions]

PR Triage\n\n- Category: test\n- Risk: low\n- Priority: medium\n- Score: 42/100 (impact 25 + urgency 10 + quality 7)\n- Recommended action: defer\n

Generated by 🔧 PR Triage Agent · auto · 62.8 AIC · ⌖ 2.76 AIC · ⊞ 7.8K · ◷

[Copilot]

Pull request overview

Improves activation checkout test quality and coverage.

Changes:

• Adopts Testify assertions and shared job extraction.
• Adds helper-function unit tests.
• Relocates and expands activation checkout tests.

Show a summary per file

File	Description
pkg/workflow/compiler_test_helpers_test.go	Tests shared job extraction.
pkg/workflow/compiler_activation_job_test.go	Adds activation helper tests.
pkg/workflow/compiler_activation_checkout_test.go	Relocates and expands checkout tests.
pkg/workflow/activation_checkout_test.go	Removes the previous file after relocation.

Review details

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

• Files reviewed: 4/4 changed files
• Comments generated: 2
• Review effort level: Balanced

[Copilot]

Addressed in 66172ca: the subtest now asserts the sparse checkout step (name: Checkout .github and .agents folders, sparse-checkout: |, sparse-checkout-cone-mode: true) and explicitly asserts the activation job does not contain name: Checkout repository.

[Copilot]

Addressed in 66172ca: the default case now compares the complete expected on: section (issues, issue_comment, pull_request, pull_request_review_comment, discussion, discussion_comment), so a dropped event or a missing pull_request_comment → issue_comment dedup will fail the test.

[gh-aw-bot]

@copilot Branch refresh has been requested for this PR via the update-branch action. Please finish a maintainer-ready pass by running the pr-finisher skill and summarize the outcome. If there are still review comments, address them before re-requesting attention.

Generated by 👨‍🍳 PR Sous Chef · gpt54 · 7.2 AIC · ⌖ 7.73 AIC · ⊞ 8.5K · ◷
_{Comment /souschef to run again}

[github-actions]

PR Triage

• Category: test
• Risk: low
• Priority: medium (score: 36/100)
• Score breakdown: impact 12/50, urgency 8/30, quality 16/20
• Recommended action: batch_review
• Batch: small-features-tests-cluster

Automated triage by PR Triage Agent.

Generated by 🔧 PR Triage Agent · auto · 64.3 AIC · ⌖ 2.94 AIC · ⊞ 7.8K · ◷

[gh-aw-bot]

@copilot Please run the pr-finisher skill and summarize the maintainer-ready state in one comment.

Open review feedback to resolve:

• copilot-pull-request-reviewer left review comments that still need follow-up/resolution.

Run: https://github.com/github/gh-aw/actions/runs/31833406387

Generated by 👨‍🍳 PR Sous Chef · gpt54 · 8.41 AIC · ⌖ 6.61 AIC · ⊞ 8.5K · ◷
_{Comment /souschef to run again}

[Copilot]

@copilot Please run the pr-finisher skill and summarize the maintainer-ready state in one comment.

Open review feedback to resolve:...

pr-finisher pass complete — no new code changes were required.

• ✅ Reviews — Both copilot-pull-request-reviewer threads were already addressed in code by 66172ca and each has a substantive reply:
  • Sparse-checkout invariant: the write-capable safe-outputs subtest now asserts name: Checkout .github and .agents folders, sparse-checkout: |, and sparse-checkout-cone-mode: true, and asserts the activation job contains no name: Checkout repository full-repo checkout.
  • Default comment events: buildCentralizedCommandOnSection(nil) is now compared against the complete expected on: section (issues, issue_comment, pull_request, pull_request_review_comment, discussion, discussion_comment), so a dropped event or a missing pull_request_comment → issue_comment dedup fails the test.
  • Both threads remain unresolved only because I cannot mark review threads resolved from this environment (no GitHub credentials for the GraphQL resolveReviewThread mutation). A maintainer can resolve them.
• ✅ Checks (local) — make agent-report-progress passes on the current HEAD (c2257a4, merged with main after the Firecracker removal in Remove Firecracker support #52774): formatting, lint on changed files, impacted unit tests, and all 285 workflow lock files in sync. Targeted runs also pass: TestActivationJobNoCheckoutStep (all 4 subtests, -tags integration), TestActivationEventSet, TestBuildCentralizedCommandOnSection, TestExtractJobSection.
• ❓ Checks (CI) — No CI failures exist on this branch: of the completed runs, none concluded failure; the runs on the current head are action_required, i.e. waiting on maintainer approval to execute. CI on this HEAD is therefore unverified until a maintainer approves/re-triggers the workflows.
• ✅ Mergeable — PR is open, not draft, no conflicts, and the branch already contains base main (c35faf4). mergeable_state: blocked reflects the pending required review and unapproved checks, not a conflict.

Actions taken: re-validated the merged branch locally; no file changes were needed, so nothing new was pushed.

Hand-off: a maintainer needs to approve the pending action_required workflow runs (and optionally resolve the two answered review threads); I cannot trigger workflow runs from here.

Still needed: review approval from @pelikhan and green CI. I did not merge.