Skip to content

Deduplicate writeConfigs test setup in config-writer.test.ts#4054

Merged
lpcox merged 2 commits into
mainfrom
copilot/fix-duplicate-code-writeconfigs
May 30, 2026
Merged

Deduplicate writeConfigs test setup in config-writer.test.ts#4054
lpcox merged 2 commits into
mainfrom
copilot/fix-duplicate-code-writeconfigs

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented May 30, 2026

src/config-writer.test.ts repeated the same writeConfigs base config object across many tests, making updates to config shape expensive and error-prone. This refactor centralizes the shared setup while preserving each test’s specific overrides.

  • Refactor: shared base config builder

    • Added buildWriteConfig(overrides?) in the writeConfigs test suite to define the canonical default config once.
  • Call site cleanup

    • Replaced repeated inline writeConfigs({...}) objects with writeConfigs(buildWriteConfig(...)).
    • Kept test intent unchanged by only moving per-test differences into overrides (e.g. sslBump, workDir, geminiApiKey, auditConfig, URL/API-proxy fields).
  • Maintainability impact

    • Removes high-volume duplication from the suite and makes future config-field changes single-point edits.
const buildWriteConfig = (
  overrides: Partial<Parameters<typeof writeConfigs>[0]> = {}
): Parameters<typeof writeConfigs>[0] => ({
  workDir: tempDir,
  sslBump: false,
  allowedDomains: [],
  agentCommand: 'echo test',
  logLevel: 'info',
  keepContainers: false,
  buildLocal: false,
  imageRegistry: 'ghcr.io/github/gh-aw-firewall',
  imageTag: 'latest',
  ...overrides,
});

await writeConfigs(buildWriteConfig({ geminiApiKey: 'test-key' }));

Copilot AI changed the title [WIP] Refactor duplicate writeConfigs calls in config-writer tests Deduplicate writeConfigs test setup in config-writer.test.ts May 30, 2026
Copilot finished work on behalf of lpcox May 30, 2026 14:09
Copilot AI requested a review from lpcox May 30, 2026 14:09
@lpcox lpcox marked this pull request as ready for review May 30, 2026 14:10
Copilot AI review requested due to automatic review settings May 30, 2026 14:10
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 src/config-writer.test.ts to deduplicate repeated writeConfigs(...) option objects by introducing a shared buildWriteConfig(overrides?) helper, making the test suite easier to maintain when the writeConfigs config shape evolves.

Changes:

  • Added buildWriteConfig helper to centralize the canonical default writeConfigs options for this suite.
  • Updated test call sites to use writeConfigs(buildWriteConfig({...overrides})) instead of repeating full config objects.
Show a summary per file
File Description
src/config-writer.test.ts Introduces a shared config builder and replaces repeated inline writeConfigs option objects with override-based calls.

Copilot's findings

Tip

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

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

@github-actions
Copy link
Copy Markdown
Contributor

✅ Coverage Check Passed

Overall Coverage

Metric Base PR Delta
Lines 96.46% 96.51% 📈 +0.05%
Statements 96.32% 96.36% 📈 +0.04%
Functions 98.26% 98.26% ➡️ +0.00%
Branches 90.61% 90.65% 📈 +0.04%
📁 Per-file Coverage Changes (1 files)
File Lines (Before → After) Statements (Before → After)
src/config-writer.ts 89.3% → 90.9% (+1.65%) 89.3% → 90.9% (+1.65%)
✨ New Files (1 files)
  • src/copilot-api-resolver.internal.ts: 100.0% lines

Coverage comparison generated by scripts/ci/compare-coverage.ts

@github-actions
Copy link
Copy Markdown
Contributor

Smoke Test: Claude Engine

  • ✅ GitHub API (recent PRs): PASS
  • ✅ GitHub check (playwright): PASS
  • ✅ File verify: PASS

Total: PASS

💥 [THE END] — Illustrated by Smoke Claude

@github-actions
Copy link
Copy Markdown
Contributor

🔬 Smoke Test Results

Test Result
GitHub MCP connectivity ✅ PASS
GitHub.com HTTP connectivity ⚠️ N/A (template vars unresolved)
File write/read ⚠️ N/A (template vars unresolved)

PR: Deduplicate writeConfigs test setup in config-writer.test.ts
Author: @Copilot | Assignees: @lpcox, @Copilot

Overall: PARTIAL — GitHub MCP confirmed working; pre-step data not injected.

📰 BREAKING: Report filed by Smoke Copilot

@github-actions
Copy link
Copy Markdown
Contributor

🔥 Smoke Test: Copilot BYOK (Offline) Mode

Test Result
GitHub MCP (list merged PRs) ✅ PR #4051 retrieved successfully
GitHub.com HTTP connectivity ✅ (pre-step passed)
File write/read ✅ (pre-step passed)
BYOK inference (this response) ✅ Agent responding via api-proxy → api.githubcopilot.com

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

Overall: PASS — PR by @Copilot, assignees: @lpcox, @Copilot

🔑 BYOK report filed by Smoke Copilot BYOK

@github-actions
Copy link
Copy Markdown
Contributor

Gemini Smoke Test Results\n- GitHub MCP Testing: ❌\n- GitHub.com Connectivity: ❌\n- File Writing Testing: ✅\n- Bash Tool Testing: ✅\n\nOverall status: 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

@github-actions
Copy link
Copy Markdown
Contributor

Chroot Version Comparison Results

Runtime Host Version Chroot Version Match?
Python Python 3.12.13 Python 3.12.3 ❌ NO
Node.js v24.16.0 v22.22.3 ❌ NO
Go go1.22.12 go1.22.12 ✅ YES

Overall: ❌ FAILED — Python and Node.js versions differ between host and chroot environments.

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 1/1 passed ✅ PASS
Go env 1/1 passed ✅ PASS
Go uuid 1/1 passed ✅ PASS
Java gson 1/1 passed ✅ PASS
Java caffeine 1/1 passed ✅ PASS
Node.js clsx All passed ✅ PASS
Node.js execa All passed ✅ PASS
Node.js p-limit All 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 #4054 · sonnet46 766.6K ·

@github-actions
Copy link
Copy Markdown
Contributor

Smoke Test Results: FAIL

Check Result
Redis PING ❌ timeout (no response on host.docker.internal:6379)
PostgreSQL pg_isready ❌ no response on port 5432
PostgreSQL SELECT 1 ❌ timeout

host.docker.internal resolves to 172.17.0.1 but none of the service containers are reachable. The GitHub Actions service containers may not be running or the network is not bridged correctly.

Overall: FAIL

🔌 Service connectivity validated by Smoke Services

@github-actions
Copy link
Copy Markdown
Contributor

Remove copilotApiResolverTestHelpers from the production API surface
Deduplicate server test HTTPS_PROXY/module-reset bootstrap across split api-proxy tests
GitHub MCP Testing: ✅
Safe Inputs GH CLI Testing: ✅
Playwright Testing: ✅
File Writing Testing: ✅
Bash Tool Testing: ✅
Discussion Interaction Testing: ✅
Build AWF: ❌
Overall: 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

@lpcox lpcox merged commit b708c8b into main May 30, 2026
63 of 66 checks passed
@lpcox lpcox deleted the copilot/fix-duplicate-code-writeconfigs branch May 30, 2026 14:46
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] writeConfigs() base config object repeated 18 times in config-writer.test.ts

3 participants