Split threat_detection_test.go, copilot_engine_test.go, maintenance_workflow_test.go by functional area#54681
Conversation
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
|
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 ( 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.
|
There was a problem hiding this comment.
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
|
@copilot Please refresh this branch against the latest base state if needed, then run the Current review state:
Run: https://github.com/github/gh-aw/actions/runs/32544819956
|
Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com>
Pushed fix commit Validated locally:
Freshness: branch is up to date with base ( 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 |
|
@copilot This PR still needs a maintainer-ready finish.
Run: https://github.com/github/gh-aw/actions/runs/32554681386
|
…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>
Pushed follow-up commits Branch freshness:
Re-run validations (exact commands + outcomes):
Review threads:
|
PR Triage
|
…plit-large-test-files Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com>
Branch refreshed and pushed at
Validations rerun for this follow-up:
Review threads: both outstanding threads are already replied-to and addressed ( |
|
@-
|
|
@copilot this PR still needs forward progress before maintainer review.
|
Current HEAD is still fresh with
Re-ran validations for this follow-up:
CI review for latest follow-up:
Review threads: both remaining threads already have substantive responses and fixes ( |
|
@copilot this PR still needs forward progress before maintainer review.
Run: https://github.com/github/gh-aw/actions/runs/32564450205
|
Branch freshness is confirmed at current HEAD Re-ran validations for this follow-up:
CI/log checks (latest follow-up):
Review threads status:
|
|
@copilot please refresh the branch if needed and continue with the pr-finisher skill
|
Branch refresh check is complete and no refresh was needed (HEAD Continued
CI follow-up (latest referenced run):
Remaining blocker is unchanged: 2 review threads still report unresolved in API state ( |
|
@copilot this PR still needs forward progress before maintainer review.
Run: https://github.com/github/gh-aw/actions/runs/32569036201
|
Maintainer handoff (pr-finisher pass complete):
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. |
|
@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.
|
Issue #53788 named 5 oversized test files; only
compiler_jobs_test.gowas 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.gocopilot_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.gomaintenance_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.gosplit.Run: https://github.com/github/gh-aw/actions/runs/32558278813> Generated by 👨🍳 PR Sous Chef · gpt54 · 7.36 AIC · ⌖ 8.18 AIC · ⊞ 9.5K · ◷
Run: https://github.com/github/gh-aw/actions/runs/32563890634> Generated by 👨🍳 PR Sous Chef · gpt54 · 19.3 AIC · ⌖ 8.31 AIC · ⊞ 9.5K · ◷
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 · ◷