Skip to content

Refactor retry-logic tests to centralize shared HTTPS/stdout mocks#4046

Merged
lpcox merged 3 commits into
mainfrom
copilot/fix-duplicate-code-in-tests
May 30, 2026
Merged

Refactor retry-logic tests to centralize shared HTTPS/stdout mocks#4046
lpcox merged 3 commits into
mainfrom
copilot/fix-duplicate-code-in-tests

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented May 30, 2026

The retry-logic test suites for Anthropics deprecated beta handling and Copilot model not supported handling repeated the same spy/mock setup in each test body, creating high-maintenance duplication. This change consolidates that setup into shared describe-level state initialized in beforeEach.

  • containers/api-proxy/server.anthropic-beta.test.js

    • Hoisted shared test setup into describe scope:
      • stdoutWriteSpy
      • responseHandlers
      • capturedOptions
      • https.request mock implementation
    • Removed repeated setup blocks from individual it(...) cases while preserving existing assertions and flow.
  • containers/api-proxy/server.model-not-supported.test.js

    • Added the same describe-level shared setup pattern for retry tests.
    • Removed duplicated local setup in retry-focused test bodies.
    • Kept the specialized per-test override for request-body capture, but reused shared state initialization.
  • Resulting test structure

    • Single initialization point for request capture + response handler plumbing.
    • Per-test bodies now focus on scenario behavior and assertions, not fixture wiring.
describe('...', () => {
  let stdoutWriteSpy;
  let responseHandlers;
  let capturedOptions;

  beforeEach(() => {
    stdoutWriteSpy = jest.spyOn(process.stdout, 'write').mockImplementation(() => true);
    responseHandlers = [];
    capturedOptions = [];
    jest.spyOn(https, 'request').mockImplementation((options, cb) => {
      capturedOptions.push(options);
      responseHandlers.push(cb);
      return makeProxyReq();
    });
  });
});

Copilot AI changed the title [WIP] Refactor spy/mock setup block in retry-logic tests Refactor retry-logic tests to centralize shared HTTPS/stdout mocks May 30, 2026
Copilot finished work on behalf of lpcox May 30, 2026 13:35
Copilot AI requested a review from lpcox May 30, 2026 13:35
@lpcox lpcox marked this pull request as ready for review May 30, 2026 13:36
Copilot AI review requested due to automatic review settings May 30, 2026 13:36
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 two retry-logic Jest test files in the API proxy to lift duplicated per-test mock setup (stdoutWriteSpy, responseHandlers, capturedOptions, and the https.request spy) into shared describe-scope state initialized in beforeEach. Pure test cleanup with no production-code changes.

Changes:

  • Hoist shared stdoutWriteSpy/responseHandlers/capturedOptions and the https.request mock into a beforeEach for the Anthropic deprecated-beta test suite.
  • Apply the same shared-setup pattern to the Copilot model-not-supported retry suite, removing per-test duplication.
  • Preserve a per-test https.request override in the "sends an identical request body on retry" case for body capture.
Show a summary per file
File Description
containers/api-proxy/server.anthropic-beta.test.js Centralizes mock setup in beforeEach; removes duplicated spy/array initialization from each it.
containers/api-proxy/server.model-not-supported.test.js Same beforeEach consolidation; one test still overrides https.request to capture bodies.

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

Comment thread containers/api-proxy/server.model-not-supported.test.js Outdated
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 30, 2026

✅ 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%)

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

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
@github-actions

This comment has been minimized.

@github-actions
Copy link
Copy Markdown
Contributor

Smoke Test: Claude Engine

  • ✅ GitHub API: 2 PR entries found
  • ✅ GitHub check: playwright PASS
  • ✅ File verify: smoke-test-claude-26685237584.txt exists

Total: PASS

💥 [THE END] — Illustrated by Smoke Claude

@github-actions
Copy link
Copy Markdown
Contributor

🔬 Smoke Test Results

Test Status
GitHub MCP connectivity
GitHub.com HTTP connectivity ⚠️ (pre-step data unavailable)
File write/read ⚠️ (pre-step data unavailable)

Overall: PARTIAL — MCP confirmed working; pre-step template vars were not substituted.

PR by @Copilot · Assignees: @lpcox @Copilot

📰 BREAKING: Report filed by Smoke Copilot

@github-actions
Copy link
Copy Markdown
Contributor

🔍 Smoke Test: API Proxy OpenTelemetry Tracing

Scenario Status Notes
Module Loading otel.js loads successfully; exports: startRequestSpan, setTokenAttributes, endSpan, endSpanError, shutdown, isEnabled + internals
Test Suite 33/33 tests passed (otel.test.js) — spans, attributes, exporters, graceful degradation all covered
Env Var Forwarding api-proxy-service.ts forwards OTEL_EXPORTER_OTLP_ENDPOINT, OTEL_EXPORTER_OTLP_HEADERS, GITHUB_AW_OTEL_TRACE_ID, GITHUB_AW_OTEL_PARENT_SPAN_ID
Token Tracker Integration onUsage callback exists in token-tracker-http.js (line 237) as the OTEL hook point
OTEL Diagnostics FileSpanExporter provides local fallback to /var/log/api-proxy/otel.jsonl when no OTLP endpoint configured

All scenarios pass. ✅

📡 OTel tracing validated by Smoke OTel Tracing

@github-actions
Copy link
Copy Markdown
Contributor

Smoke Test: Copilot BYOK (Offline) Mode

Test Result
GitHub MCP connectivity ✅ PR list returned successfully
GitHub.com connectivity ✅ (BYOK mode active)
File write/read ⚠️ Template vars not expanded in workflow
BYOK inference ✅ Responding via api-proxy → api.githubcopilot.com

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

Overall: PASS (3/3 functional tests passed; file test skipped due to unexpanded template vars)

PR author: @Copilot · Assignees: @lpcox @Copilot

🔑 BYOK report filed by Smoke Copilot BYOK

@github-actions
Copy link
Copy Markdown
Contributor

Smoke test results: Gemini. GitHub MCP: FAIL, Connectivity: FAIL, File Write: PASS, Bash: PASS. Overall: 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 Runtime Version Comparison

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

Result: ❌ Not all tests passed — 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 #4046 · sonnet46 1M ·

@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 ❌ no response

host.docker.internal is not reachable from this runner environment — service containers appear unavailable.

🔌 Service connectivity validated by Smoke Services

@github-actions
Copy link
Copy Markdown
Contributor

fix: synthesize identity files for ARC-DinD environments — ✅
feat(api-proxy): pre-startup model validation via requestedModel config — ✅
GitHub title check — ✅
File write/read — ✅
Discussion lookup/comment — ✅
Build — ❌
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

@lpcox
Copy link
Copy Markdown
Collaborator

lpcox commented May 30, 2026

@copilot address review feedback

@lpcox lpcox merged commit 88bcc82 into main May 30, 2026
66 of 68 checks passed
@lpcox lpcox deleted the copilot/fix-duplicate-code-in-tests branch May 30, 2026 13:57
Copilot stopped work on behalf of lpcox due to an error May 30, 2026 14:04
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] Spy/mock setup block duplicated in every test body across server retry-logic test files

3 participants