Skip to content

Refactor service tests to share temp workDir lifecycle setup#3204

Merged
lpcox merged 2 commits into
mainfrom
copilot/duplicate-code-resolution
May 15, 2026
Merged

Refactor service tests to share temp workDir lifecycle setup#3204
lpcox merged 2 commits into
mainfrom
copilot/duplicate-code-resolution

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented May 15, 2026

✨ Enhancement

10 service/environment tests in src/services/ duplicated the same preamble for mockConfig temp-dir setup/teardown, creating ~230 lines of repeated lifecycle code and forcing N-way edits for any fixture changes. This PR centralizes that lifecycle while preserving per-file jest.mock('execa', ...) hoisting behavior.

  • What does this improve?

    • Removes duplicated beforeEach/afterEach temp workDir setup across 10 service test files.
    • Makes fixture lifecycle changes one-touch instead of 10-touch.
    • Keeps suite-local semantics intact (mockConfig remains file-scoped in each test).
  • Why is this valuable?

    • Reduces maintenance overhead and drift risk across service tests.
    • Keeps Jest mock-hoisting constraints respected (mocks remain in each test file).
  • Implementation approach:

    • Added useTempWorkDir(...) to src/test-helpers/docker-test-fixtures.test-utils.ts.
    • Updated these files to call useTempWorkDir(...) inside describe(...):
      • agent-environment-credentials.test.ts
      • agent-environment-options.test.ts
      • agent-environment-proxy.test.ts
      • agent-environment-runtime.test.ts
      • agent-service.test.ts
      • agent-volumes.test.ts
      • api-proxy-service.test.ts
      • cli-proxy-service.test.ts
      • doh-proxy-service.test.ts
      • squid-service.test.ts
    • Removed now-redundant preamble imports only where no longer used.
// shared helper
useTempWorkDir(
  baseConfig,
  (config) => {
    mockConfig = config;
  },
  () => mockConfig
);

Copilot AI changed the title [WIP] Refactor duplicate preamble in service test files Refactor service tests to share temp workDir lifecycle setup May 15, 2026
Copilot finished work on behalf of lpcox May 15, 2026 13:36
Copilot AI requested a review from lpcox May 15, 2026 13:36
@lpcox lpcox marked this pull request as ready for review May 15, 2026 13:52
@lpcox lpcox requested a review from Mossaka as a code owner May 15, 2026 13:52
Copilot AI review requested due to automatic review settings May 15, 2026 13:52
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Refactors 10 service test files in src/services/ to share temp workDir lifecycle setup via a new useTempWorkDir helper, eliminating ~230 lines of duplicated beforeEach/afterEach code while preserving per-file jest.mock() hoisting semantics.

Changes:

  • Added useTempWorkDir(fixtureConfig, setConfig, getConfig) helper in docker-test-fixtures.test-utils.ts that sets up an awf-test- temp dir before each test and cleans it up after.
  • Replaced inline temp-dir lifecycle blocks in 10 service test files with calls to useTempWorkDir.
  • Removed now-unused fs/os/path imports from test files that no longer reference them directly.
Show a summary per file
File Description
src/test-helpers/docker-test-fixtures.test-utils.ts Adds useTempWorkDir helper encapsulating temp workDir lifecycle.
src/services/squid-service.test.ts Switches to useTempWorkDir; drops fs/os/path imports.
src/services/doh-proxy-service.test.ts Switches to useTempWorkDir; drops fs/os/path imports.
src/services/cli-proxy-service.test.ts Switches to useTempWorkDir; drops fs/os/path imports.
src/services/api-proxy-service.test.ts Switches to useTempWorkDir; drops os import (keeps fs/path for other usage).
src/services/agent-volumes.test.ts Switches to useTempWorkDir; retains other fs/path/os usage.
src/services/agent-service.test.ts Switches to useTempWorkDir; retains other fs/path/os usage.
src/services/agent-environment-runtime.test.ts Switches to useTempWorkDir; drops fs/os/path imports.
src/services/agent-environment-proxy.test.ts Switches to useTempWorkDir; drops fs/os/path imports.
src/services/agent-environment-options.test.ts Switches to useTempWorkDir; retains other fs/path/os usage.
src/services/agent-environment-credentials.test.ts Switches to useTempWorkDir; drops fs/os/path imports.

Copilot's findings

Tip

Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

  • Files reviewed: 11/11 changed files
  • Comments generated: 0

@github-actions
Copy link
Copy Markdown
Contributor

Smoke Test Results

GitHub API - Listed last 2 merged PRs
Playwright - Navigated to https://github.com, title contains "GitHub"
File verify - Smoke test file present

PASS — All 3/3 tests passed

💥 [THE END] — Illustrated by Smoke Claude

@github-actions
Copy link
Copy Markdown
Contributor

Smoke Test Results

  • File Write/Read: smoke-test-copilot-25920733542.txt exists — content verified
  • GitHub MCP: 401 Bad credentials (MCP not authenticated)
  • ⚠️ GitHub.com Connectivity: Template variables not resolved

Overall: FAIL

📰 BREAKING: Report filed by Smoke Copilot

@github-actions
Copy link
Copy Markdown
Contributor

Smoke test Codex: FAIL
PRs: refactor: [Export Audit] Remove test-only re-exports from barrel modules; feat: auto-forward OTEL_* env vars with one-shot token protection for headers
✅ GitHub PR review, ✅ Playwright, ✅ file/bash, ✅ build
❌ safeinputs-gh, ❌ Tavily search, ❌ discussion query
Overall status: FAIL

Warning

Firewall blocked 1 domain

The following domain was blocked by the firewall during workflow execution:

  • registry.npmjs.org

To allow these domains, add them to the network.allowed list in your workflow frontmatter:

network:
  allowed:
    - defaults
    - "registry.npmjs.org"

See Network Configuration for more information.

🔮 The oracle has spoken through Smoke Codex

@github-actions
Copy link
Copy Markdown
Contributor

Chroot Smoke Test Results

Runtime Host Version Chroot Version Match?
Python Python 3.12.13 Python 3.12.3
Node.js v24.15.0 v20.20.2
Go go1.22.12 go1.22.12

Overall: ❌ Not all versions match — Python and Node.js versions differ between host and chroot environment.

Tested by Smoke Chroot

@github-actions
Copy link
Copy Markdown
Contributor

🏗️ Build Test Suite Results

Ecosystem Project Build/Install Tests Status
Bun elysia 1/1 passed ✅ PASS
Bun hono 1/1 passed ✅ PASS
C++ fmt N/A ✅ PASS
C++ json N/A ✅ PASS
Deno oak N/A 1/1 passed ✅ PASS
Deno std N/A 1/1 passed ✅ PASS
.NET hello-world N/A ✅ PASS
.NET json-parse N/A ✅ PASS
Go color passed ✅ PASS
Go env passed ✅ PASS
Go uuid passed ✅ PASS
Java gson 1/1 passed ✅ PASS
Java caffeine 1/1 passed ✅ PASS
Node.js clsx passed ✅ PASS
Node.js execa passed ✅ PASS
Node.js p-limit passed ✅ PASS
Rust fd 1/1 passed ✅ PASS
Rust zoxide 1/1 passed ✅ PASS

Overall: 8/8 ecosystems passed — ✅ PASS

Generated by Build Test Suite for issue #3204 · ● 3.8M ·

@github-actions
Copy link
Copy Markdown
Contributor

Smoke Test: Copilot BYOK — Run #859 (attempt 2)

Test Result
GitHub MCP connectivity ❌ 401 (token not available to MCP tools)
GitHub.com HTTP connectivity ⚠️ Pre-step outputs not expanded (${{ steps.smoke-data.outputs.SMOKE_HTTP_CODE }})
File write/read ⚠️ Pre-step outputs not expanded — file path unavailable
BYOK inference (api-proxy → api.githubcopilot.com) ✅ Confirmed (agent is responding)

Running in BYOK offline mode (COPILOT_OFFLINE=true) via api-proxy → api.githubcopilot.com.

Overall: ⚠️ PARTIAL — BYOK inference works; pre-step smoke-data outputs were not substituted (workflow template issue).

🔑 BYOK report filed by Smoke Copilot BYOK

@github-actions
Copy link
Copy Markdown
Contributor

Smoke Test Results — FAIL

Check Result
Redis PING ❌ timeout/no response
PostgreSQL pg_isready ❌ no response
PostgreSQL SELECT 1 ❌ connection failed

host.docker.internal is not reachable from this runner environment. Overall: FAIL

🔌 Service connectivity validated by Smoke Services

@github-actions
Copy link
Copy Markdown
Contributor

Smoke test result: FAIL. Details: Files OK, GitHub/MCP FAIL.

Warning

Firewall blocked 1 domain

The following domain was blocked by the firewall during workflow execution:

  • localhost

To allow these domains, add them to the network.allowed list in your workflow frontmatter:

network:
  allowed:
    - defaults
    - "localhost"

See Network Configuration for more information.

💎 Faceted by Smoke Gemini

@lpcox
Copy link
Copy Markdown
Collaborator

lpcox commented May 15, 2026

@copilot address review feedback

Copy link
Copy Markdown
Contributor Author

Copilot AI commented May 15, 2026

@copilot address review feedback

I checked this PR’s reviews and review threads, and there aren’t any actionable review comments to address right now (the Copilot review reports 0 comments). If you want me to handle specific feedback, please point me to the exact comment(s) or check output and I’ll apply the changes immediately.

Copilot finished work on behalf of lpcox May 15, 2026 14:39
@lpcox lpcox merged commit bf138ff into main May 15, 2026
71 of 83 checks passed
@lpcox lpcox deleted the copilot/duplicate-code-resolution branch May 15, 2026 15:02
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.

[Duplicate Code] 10 service test files share identical 23-line preamble (imports + jest mock + beforeEach/afterEach temp-dir lifecycle)

3 participants