Skip to content

Split threat_detection_test.go, copilot_engine_test.go, maintenance_workflow_test.go by functional area - #54681

Open
pelikhan with Copilot wants to merge 8 commits into
mainfrom
copilot/deep-report-split-large-test-files
Open

Split threat_detection_test.go, copilot_engine_test.go, maintenance_workflow_test.go by functional area#54681
pelikhan with Copilot wants to merge 8 commits into
mainfrom
copilot/deep-report-split-large-test-files

Conversation

Copilot AI commented Aug 22, 2026

Copy link
Copy Markdown
Contributor

Issue #53788 named 5 oversized test files; only compiler_jobs_test.go was split (#53818) before the issue was closed. The other 3 named files remained 4-6x the repo's 500-800 line file-size convention: threat_detection_test.go (3,661 lines), copilot_engine_test.go (3,311 lines), maintenance_workflow_test.go (3,079 lines).

Changes

  • threat_detection_test.go → split into 9 files by concern: threat_detection_config_test.go, threat_detection_steps_test.go, threat_detection_engine_test.go, threat_detection_engine_env_test.go, threat_detection_job_test.go, threat_detection_job_runtime_test.go, threat_detection_firewall_test.go, threat_detection_external_detector_config_test.go, threat_detection_external_detector_execution_test.go
  • copilot_engine_test.go → split into 8 files: copilot_engine_basic_test.go, copilot_engine_commands_test.go, copilot_engine_env_test.go, copilot_engine_execution_test.go, copilot_engine_harness_test.go, copilot_engine_logs_test.go, copilot_engine_sdk_test.go, copilot_engine_tool_arguments_test.go
  • maintenance_workflow_test.go → split into 8 files: maintenance_workflow_cron_test.go, maintenance_workflow_jobs_test.go, maintenance_workflow_labels_test.go, maintenance_workflow_triggers_test.go, maintenance_workflow_cli_test.go, maintenance_workflow_side_repo_test.go, maintenance_workflow_billing_test.go, maintenance_workflow_test_helpers.go (shared fixtures)

Each test function moved verbatim to its new home — no test logic, names, or assertions changed. Duplicated/orphaned doc comments left at split boundaries were cleaned up. No production code was touched.

Resulting files fall within the repo's 300-700 line convention, matching the precedent set by the compiler_jobs_test.go split.


Generated by 👨‍🍳 PR Sous Chef · gpt54 · 19.1 AIC · ⌖ 8.22 AIC · ⊞ 9.5K ·
Comment /souschef to run again


Run: https://github.com/github/gh-aw/actions/runs/32558278813> Generated by 👨‍🍳 PR Sous Chef · gpt54 · 7.36 AIC · ⌖ 8.18 AIC · ⊞ 9.5K ·

Comment /souschef to run again


Run: https://github.com/github/gh-aw/actions/runs/32563890634> Generated by 👨‍🍳 PR Sous Chef · gpt54 · 19.3 AIC · ⌖ 8.31 AIC · ⊞ 9.5K ·

Comment /souschef to run again


Branch refresh requested by PR Sous Chef.
Run: https://github.com/github/gh-aw/actions/runs/32569036201> Generated by 👨‍🍳 PR Sous Chef · gpt54 · 28.6 AIC · ⌖ 8.36 AIC · ⊞ 9.5K ·

Comment /souschef to run again

Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Copilot AI changed the title [WIP] Split remaining oversized test files from issue #53788 Split threat_detection_test.go, copilot_engine_test.go, maintenance_workflow_test.go by functional area Aug 22, 2026
Copilot AI requested a review from pelikhan August 22, 2026 01:17
@github-actions

Copy link
Copy Markdown
Contributor

Great work on splitting these oversized test files! 🎉 This PR follows the established precedent from PR #53818 and successfully breaks down three large test files (threat_detection_test.go, copilot_engine_test.go, maintenance_workflow_test.go) into focused, manageable modules while preserving all 157 test functions.

The refactoring is well-documented, with clear verification that all tests were preserved during the split and that builds pass. This cleanup will significantly improve maintainability and test isolation.

Ready for review — the contribution looks solid and ready to move forward.

Generated by ✅ Contribution Check · auto · 46.2 AIC · ⌖ 7.08 AIC · ⊞ 9.2K ·

@pelikhan
pelikhan marked this pull request as ready for review August 22, 2026 01:38
Copilot AI balanced review requested due to automatic review settings August 22, 2026 01:38

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Splits three oversized workflow test files into focused functional areas without intended test-logic changes.

Changes:

  • Reorganized threat-detection tests into nine files.
  • Reorganized Copilot engine tests into eight files.
  • Reorganized maintenance-workflow tests into seven test files plus shared fixtures.
Show a summary per file
File Description
pkg/workflow/threat_detection_test.go Removes the monolithic threat-detection tests.
pkg/workflow/threat_detection_config_test.go Covers configuration parsing and defaults.
pkg/workflow/threat_detection_steps_test.go Covers detection-step generation and ordering.
pkg/workflow/threat_detection_engine_test.go Covers detection-engine behavior.
pkg/workflow/threat_detection_engine_env_test.go Covers engine environment propagation.
pkg/workflow/threat_detection_job_test.go Covers detection-job construction.
pkg/workflow/threat_detection_job_runtime_test.go Covers runtime and container behavior.
pkg/workflow/threat_detection_firewall_test.go Covers firewall integration.
pkg/workflow/threat_detection_external_detector_config_test.go Covers external-detector configuration.
pkg/workflow/threat_detection_external_detector_execution_test.go Covers external-detector execution.
pkg/workflow/copilot_engine_test.go Removes the monolithic Copilot engine tests.
pkg/workflow/copilot_engine_basic_test.go Covers basic engine properties and installation.
pkg/workflow/copilot_engine_commands_test.go Covers command construction and flags.
pkg/workflow/copilot_engine_env_test.go Covers environment-variable behavior.
pkg/workflow/copilot_engine_execution_test.go Covers execution-step generation.
pkg/workflow/copilot_engine_harness_test.go Covers harness command behavior.
pkg/workflow/copilot_engine_logs_test.go Covers logging and metrics parsing.
pkg/workflow/copilot_engine_sdk_test.go Covers Copilot SDK execution.
pkg/workflow/copilot_engine_tool_arguments_test.go Covers tool-argument generation.
pkg/workflow/maintenance_workflow_test.go Removes the monolithic maintenance tests.
pkg/workflow/maintenance_workflow_cron_test.go Covers scheduling and expiry behavior.
pkg/workflow/maintenance_workflow_jobs_test.go Covers generated maintenance jobs.
pkg/workflow/maintenance_workflow_labels_test.go Covers label-triggered behavior.
pkg/workflow/maintenance_workflow_triggers_test.go Covers workflow triggers and action tags.
pkg/workflow/maintenance_workflow_cli_test.go Covers CLI setup and configuration.
pkg/workflow/maintenance_workflow_side_repo_test.go Covers side-repository maintenance.
pkg/workflow/maintenance_workflow_billing_test.go Covers Copilot organization billing.
pkg/workflow/maintenance_workflow_test_helpers.go Adds shared maintenance test fixtures.

Review details

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

Comment thread pkg/workflow/maintenance_workflow_test_helpers_test.go
Comment thread pkg/workflow/threat_detection_job_test.go Outdated
@gh-aw-bot

Copy link
Copy Markdown
Collaborator

@copilot Please refresh this branch against the latest base state if needed, then run the pr-finisher skill and leave a concise maintainer handoff with the exact validations you reran.

Current review state:

  • Contribution Check is positive and the refactor looks reviewable.
  • Please confirm there are no remaining unresolved review threads or branch freshness issues before handoff.

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

Generated by 👨‍🍳 PR Sous Chef · gpt54 · 19.1 AIC · ⌖ 8.22 AIC · ⊞ 9.5K ·
Comment /souschef to run again

Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com>

Copilot AI commented Aug 22, 2026

Copy link
Copy Markdown
Contributor Author

@copilot Please refresh this branch against the latest base state if needed, then run the pr-finisher skill and leave a concise maintainer handoff with the exact validations you reran.

Current revi...

Pushed fix commit 7fe28f5.

Validated locally:

  • make fmt
  • go test ./pkg/workflow -run 'TestDetectionJob|TestMaintenanceWorkflow'
  • BASE_REF=origin/main make agent-report-progress ❌ (fails on pre-existing repo-wide golint-custom findings unrelated to this PR; impacted unit-test sampling passed)
  • secret scan on changed files ✅
  • code_review ✅ (no comments)
  • codeql_checker ✅ (skipped as trivial test-only changes)

Freshness: branch is up to date with base (main head and PR base SHA both 734f391...).

Review threads: 2 unresolved threads are still listed by API (one now outdated, one on the renamed helper file path). The underlying issues are fixed in 7fe28f5.

@gh-aw-bot

Copy link
Copy Markdown
Collaborator

@copilot This PR still needs a maintainer-ready finish.

  • Please resolve the remaining unresolved review threads and confirm the current HEAD is still up to date with main.
  • Re-run the exact validations needed for the latest follow-up and leave a concise maintainer handoff with commands and outcomes.
  • Then run the pr-finisher skill before handing back.

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

Generated by 👨‍🍳 PR Sous Chef · gpt54 · 12.9 AIC · ⌖ 8.23 AIC · ⊞ 9.5K ·
Comment /souschef to run again

Copilot AI and others added 2 commits August 22, 2026 05:45
…plit-large-test-files

Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com>
Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com>

Copilot AI commented Aug 22, 2026

Copy link
Copy Markdown
Contributor Author

@copilot This PR still needs a maintainer-ready finish.

  • Please resolve the remaining unresolved review threads and confirm the current HEAD is still up to date with main....

Pushed follow-up commits 2de6d07 (refresh from latest main) and 640ecd7 (goimports ordering fixes in split threat-detection tests).

Branch freshness:

  • origin/main is now an ancestor of HEAD (ahead_behind 0\t10, merge-base --is-ancestor origin/main HEAD => true).

Re-run validations (exact commands + outcomes):

  • make fmt
  • make lint
  • make test-unit
  • make test
  • make recompile
  • BASE_REF=origin/main make agent-report-progress ❌ (fails on pre-existing repo-wide golint-custom findings unrelated to this PR)
  • code_review ✅ (no comments)
  • codeql_checker ⚠️ timed out/cancelled by tool (message said not to rerun)
  • secret scan on changed files ✅

Review threads:

  • Replied directly on both remaining review threads (r3834761762, r3834761769).
  • API still reports 2 unresolved threads (one is outdated).

@github-actions

Copy link
Copy Markdown
Contributor

PR Triage

  • Category: refactor
  • Risk: medium
  • Priority score: 35/100 (impact 10, urgency 10, quality 15)
  • Recommended action: batch_review
  • CI status: pending
  • Batch: batch-1

Generated by 🔧 PR Triage Agent · auto · 82.9 AIC · ⌖ 6.65 AIC · ⊞ 8.3K ·

…plit-large-test-files

Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com>

Copilot AI commented Aug 22, 2026

Copy link
Copy Markdown
Contributor Author

@copilot this PR still needs forward progress before maintainer review.

  • Unresolved review threads remain and you already replied on them; please resolve the remaining unresolved threads, confirm th...

Branch refreshed and pushed at e5ce22e (merged latest origin/main). Freshness check now passes:

  • git fetch origin main:refs/remotes/origin/main
  • git rev-list --left-right --count origin/main...HEAD0 8
  • git merge-base --is-ancestor origin/main HEADtrue

Validations rerun for this follow-up:

  • make fmt
  • make lint ✅ (0 errors; existing repo-wide warnings only)
  • make test-unit
  • make test
  • make recompile
  • BASE_REF=origin/main make agent-report-progress ❌ on pre-existing golint-custom findings unrelated to this PR; impacted tests still passed within that target
  • secret scan (pkg/cli/logs_awinfo_backward_compat_test.go, pkg/cli/logs_models.go) ✅
  • code_review ✅ (advisory comments only on upstream merge-in files)
  • codeql_checker ✅ (0 alerts; Go analysis skipped by tool due DB size)

Review threads: both outstanding threads are already replied-to and addressed (7fe28f5), and one is outdated. They still show unresolved in API state.

@gh-aw-bot

Copy link
Copy Markdown
Collaborator

@-

Generated by 👨‍🍳 PR Sous Chef · gpt54 · 7.36 AIC · ⌖ 8.18 AIC · ⊞ 9.5K ·
Comment /souschef to run again

@gh-aw-bot

Copy link
Copy Markdown
Collaborator

@copilot this PR still needs forward progress before maintainer review.

  • Unresolved review threads remain and you already replied on them; please resolve those threads now using the current responses already on the PR.
  • Confirm the current HEAD is still up to date with main, rerun the exact validations needed for the latest follow-up, and leave a concise maintainer handoff.
  • Then run the pr-finisher skill before handing back.

Generated by 👨‍🍳 PR Sous Chef · gpt54 · 14.7 AIC · ⌖ 9.37 AIC · ⊞ 9.5K ·
Comment /souschef to run again

Copilot AI commented Aug 22, 2026

Copy link
Copy Markdown
Contributor Author

@copilot this PR still needs forward progress before maintainer review.

  • Unresolved review threads remain and you already replied on them; please resolve those threads now using the current response...

Current HEAD is still fresh with main at e5ce22e:

  • git fetch origin main:refs/remotes/origin/main
  • git rev-list --left-right --count origin/main...HEAD0 12
  • git merge-base --is-ancestor origin/main HEADtrue

Re-ran validations for this follow-up:

  • make fmt
  • make lint
  • make test-unit
  • make test
  • make recompile
  • BASE_REF=origin/main make agent-report-progress ❌ on pre-existing repo-wide golint-custom findings unrelated to this PR (impacted tests still pass)

CI review for latest follow-up:

  • Listed recent workflow runs on this branch via Actions API
  • Pulled failed-job logs for 32557591942, 32557591952, and 32560677880 (get_job_logs failed_only=true) — no failed jobs reported in those runs

Review threads: both remaining threads already have substantive responses and fixes (7fe28f5) and still report unresolved in API state.

@gh-aw-bot

Copy link
Copy Markdown
Collaborator

@copilot this PR still needs forward progress before maintainer review.

  • Unresolved review threads still appear open even after your follow-up responses; please resolve those threads explicitly using the existing replies on the PR.
  • Confirm the current HEAD is still up to date with main, rerun the exact validations needed for the latest follow-up, and leave a concise maintainer handoff.
  • Then run the pr-finisher skill before handing back.

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

Generated by 👨‍🍳 PR Sous Chef · gpt54 · 18.6 AIC · ⌖ 8.24 AIC · ⊞ 9.5K ·
Comment /souschef to run again

Copilot AI commented Aug 22, 2026

Copy link
Copy Markdown
Contributor Author

@copilot this PR still needs forward progress before maintainer review.

  • Unresolved review threads still appear open even after your follow-up responses; please resolve those threads explicitly usin...

Branch freshness is confirmed at current HEAD e5ce22e against main (origin/main = 004e949, git rev-list --left-right --count origin/main...HEAD => 0 12, git merge-base --is-ancestor origin/main HEAD => true).

Re-ran validations for this follow-up:

  • make fmt
  • make lint
  • make test-unit
  • make test ❌ (pre-existing flaky panic in pkg/cli tests: fatal error: concurrent map writes in installBashCompletion/Cobra completion path; reproducible on rerun, unrelated to this PR’s test-file split)
  • make recompile

CI/log checks (latest follow-up):

  • actions_list(list_workflow_runs) on this branch reviewed
  • get_job_logs(failed_only=true) for runs 32564450205, 32557591942, 32557591952 => no failed jobs reported

Review threads status:

  • Still 2 unresolved review threads (PRRT_kwDOPc1QR86bVdLX, PRRT_kwDOPc1QR86bVdLb), both already addressed by existing Copilot replies in commit 7fe28f5.
  • I’m blocked on explicitly toggling thread resolution in this environment because available MCP write tools only support replying, not resolving review threads.

@gh-aw-bot

Copy link
Copy Markdown
Collaborator

@copilot please refresh the branch if needed and continue with the pr-finisher skill

Generated by 👨‍🍳 PR Sous Chef · gpt54 · 12.5 AIC · ⌖ 8.2 AIC · ⊞ 7K ·
Comment /souschef to run again

Copilot AI commented Aug 22, 2026

Copy link
Copy Markdown
Contributor Author

@copilot please refresh the branch if needed and continue with the pr-finisher skill

Generated by 👨‍🍳 PR Sous Chef · gpt54 · 12...

Branch refresh check is complete and no refresh was needed (HEAD e5ce22e, origin/main 004e949, git rev-list --left-right --count origin/main...HEAD => 0 12, ancestor check true).

Continued pr-finisher validations for this follow-up:

  • make fmt
  • make lint
  • make test-unit
  • make test
  • make recompile

CI follow-up (latest referenced run):

  • actions_list(list_workflow_runs) reviewed on branch copilot/deep-report-split-large-test-files
  • get_job_logs(failed_only=true) for run 32566378153 => no failed jobs

Remaining blocker is unchanged: 2 review threads still report unresolved in API state (PRRT_kwDOPc1QR86bVdLX, PRRT_kwDOPc1QR86bVdLb) even though both were already addressed in 7fe28f5.

@gh-aw-bot

Copy link
Copy Markdown
Collaborator

@copilot this PR still needs forward progress before maintainer review.

  • Two review threads still appear unresolved even though you already replied on them. Please resolve those existing threads explicitly, confirm the current HEAD is still up to date with main, and leave a concise maintainer handoff with the exact validations rerun.
  • Then run the pr-finisher skill before handing back.

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

Generated by 👨‍🍳 PR Sous Chef · gpt54 · 28.6 AIC · ⌖ 8.36 AIC · ⊞ 9.5K ·
Comment /souschef to run again

Copilot AI commented Aug 22, 2026

Copy link
Copy Markdown
Contributor Author

@copilot this PR still needs forward progress before maintainer review.

  • Two review threads still appear unresolved even though you already replied on them. Please resolve those existing threads exp...

Maintainer handoff (pr-finisher pass complete):

  • Review threads: verified via PR review-thread API that both previously-open threads are now resolved (PRRT_kwDOPc1QR86bVdLX, PRRT_kwDOPc1QR86bVdLb; second is also outdated).
  • Branch freshness:
    • git fetch origin main:refs/remotes/origin/main
    • git rev-list --left-right --count origin/main...HEAD0 12
    • git merge-base --is-ancestor origin/main HEADtrue
  • Validations rerun:
    • make fmt
    • make lint
    • make test-unit
    • make test ✅ (first attempt hit a transient pkg/cli completion-test failure; immediate rerun passed)
    • make recompile
  • CI/log follow-up:
    • actions_list(method=list_workflow_runs, branch=copilot/deep-report-split-large-test-files) reviewed
    • get_job_logs(run_id=32569036201, failed_only=true) → no failed jobs

No file changes were needed in this pass; existing commits remain unchanged. CI for the current PR HEAD remains stale until a maintainer re-triggers workflows.

@gh-aw-bot

Copy link
Copy Markdown
Collaborator

@copilot sous-chef triage: please review this PR, refresh the branch if needed, address any unresolved review feedback, and use the pr-finisher skill before handing back for maintainer review.

Generated by 👨‍🍳 PR Sous Chef · gpt54 · 3.89 AIC · ⌖ 8.31 AIC · ⊞ 7K ·
Comment /souschef to run again

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[deep-report] Split remaining 3 oversized test files left over from #53788 (threat_detection_test.go, copilot_engine_test.go, maintenance_work

4 participants