refactor: extract useCleanupTestDir to eliminate repeated test boilerplate#3481
Conversation
Remove ~60 lines of repeated beforeEach/afterEach boilerplate from container-cleanup-branches.test.ts by introducing useCleanupTestDir in docker-test-fixtures.test-utils.ts.
|
| Metric | Base | PR | Delta |
|---|---|---|---|
| Lines | 95.73% | 95.79% | 📈 +0.06% |
| Statements | 95.56% | 95.62% | 📈 +0.06% |
| Functions | 96.86% | 96.67% | 📉 -0.19% |
| Branches | 89.30% | 89.31% | 📈 +0.01% |
📁 Per-file Coverage Changes (2 files)
| File | Lines (Before → After) | Statements (Before → After) |
|---|---|---|
src/test-helpers/docker-test-fixtures.test-utils.ts |
100.0% → 94.7% (-5.27%) | 100.0% → 95.0% (-5.00%) |
src/config-writer.ts |
83.0% → 85.6% (+2.54%) | 83.0% → 85.6% (+2.54%) |
Coverage comparison generated by scripts/ci/compare-coverage.ts
There was a problem hiding this comment.
Pull request overview
This PR refactors test setup in container-cleanup-branches.test.ts by extracting repeated temp-dir lifecycle boilerplate into a shared Jest helper (useCleanupTestDir) in the existing docker test fixtures utilities module.
Changes:
- Added
useCleanupTestDir(resetMocks?)helper to create/remove anawf-*temp dir per test with an optional mock-reset callback. - Refactored all
describeblocks incontainer-cleanup-branches.test.tsto useuseCleanupTestDir()andgetDir()instead of localtestDirsetup/teardown.
Show a summary per file
| File | Description |
|---|---|
src/test-helpers/docker-test-fixtures.test-utils.ts |
Adds useCleanupTestDir helper to standardize temp dir lifecycle for cleanup-focused unit tests. |
src/container-cleanup-branches.test.ts |
Replaces repeated per-describe temp dir setup/teardown with the new helper and updates call sites to use getDir(). |
Copilot's findings
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 2/2 changed files
- Comments generated: 1
| afterEach(() => { | ||
| if (fs.existsSync(testDir)) { | ||
| fs.rmSync(testDir, { recursive: true, force: true }); | ||
| } | ||
| }); |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
@copilot address review feedback |
Addressed in commit Updated |
|
| Metric | Base | PR | Delta |
|---|---|---|---|
| Lines | 95.73% | 95.79% | 📈 +0.06% |
| Statements | 95.56% | 95.62% | 📈 +0.06% |
| Functions | 96.86% | 96.70% | 📉 -0.16% |
| Branches | 89.30% | 89.24% | 📉 -0.06% |
📁 Per-file Coverage Changes (3 files)
| File | Lines (Before → After) | Statements (Before → After) |
|---|---|---|
src/test-helpers/docker-test-fixtures.test-utils.ts |
100.0% → 90.9% (-9.10%) | 100.0% → 90.9% (-9.10%) |
src/config-writer.ts |
83.0% → 85.6% (+2.54%) | 83.0% → 85.6% (+2.54%) |
src/commands/validate-options.ts |
87.3% → 100.0% (+12.70%) | 86.4% → 100.0% (+13.62%) |
✨ New Files (4 files)
src/commands/validators/agent-options.ts: 93.0% linessrc/commands/validators/config-assembly.ts: 83.0% linessrc/commands/validators/log-and-limits.ts: 95.2% linessrc/commands/validators/network-options.ts: 81.3% lines
Coverage comparison generated by scripts/ci/compare-coverage.ts
Smoke Test Results✅ GitHub API: 2 PR entries confirmed in recent-prs.json Result: PASS
|
🔥 Smoke Test: Copilot BYOK (Offline) Mode
Running in BYOK offline mode ( Author: Overall: PARTIAL PASS (core BYOK path ✅; pre-step data not injected into prompt)
|
🔬 Smoke Test Results
PR: "refactor: extract useCleanupTestDir to eliminate repeated test boilerplate" Overall: PARTIAL — MCP ✅, pre-step outputs were not interpolated so HTTP/file tests could not be verified.
|
|
Gemini Smoke Test: GitHub MCP: FAIL, Connectivity: FAIL, File Writing: PASS, Bash Tool: PASS. Overall: FAIL. Warning Firewall blocked 1 domainThe following domain was blocked by the firewall during workflow execution:
network:
allowed:
- defaults
- "localhost"See Network Configuration for more information.
|
Smoke Test SummaryMerged PRs: ✅ fix: flatten OTEL cache attribute names for Sentry compatibility; fix: align OTEL attributes with gen_ai semconv spec Warning Firewall blocked 1 domainThe following domain was blocked by the firewall during workflow execution:
network:
allowed:
- defaults
- "registry.npmjs.org"See Network Configuration for more information.
|
Chroot Smoke Test Results
Overall: ❌ Not all tests passed — Python and Node.js versions differ between host and chroot.
|
🏗️ Build Test Suite Results
Overall: 8/8 ecosystems passed — ✅ PASS
|
Smoke Test Results
Overall: FAIL —
|
Each of the 6
describeblocks incontainer-cleanup-branches.test.tsduplicated ~10 lines of identicallet testDir / beforeEach / afterEachsetup — ~60 lines of boilerplate with no meaningful variation except which mock was reset.Changes
useCleanupTestDirinsrc/test-helpers/docker-test-fixtures.test-utils.ts: managesawf-*temp dir lifecycle (beforeEachcreate,afterEachremove) with an optionalresetMockscallback (defaults tojest.clearAllMocks())src/container-cleanup-branches.test.tsto use the helper; removed the localmakeTmpDirfunctionEach block now reads:
The helper follows the same pattern as the existing
useTempWorkDirin the same file.