Skip to content

refactor(api-proxy tests): extract shared upstream mock cycle helpers#5028

Merged
lpcox merged 3 commits into
mainfrom
copilot/duplicate-code-token-limit-proxy-tests
Jun 15, 2026
Merged

refactor(api-proxy tests): extract shared upstream mock cycle helpers#5028
lpcox merged 3 commits into
mainfrom
copilot/duplicate-code-token-limit-proxy-tests

Conversation

Copilot AI commented Jun 15, 2026

Copy link
Copy Markdown
Contributor

~70 lines of identical upstream HTTP mock scaffolding were duplicated across 8 test cases in server.token-guards.test.js and server.token-steering.test.js. Each test hand-built the same EventEmitter upstream request, https.request spy, proxy response, and event emission sequence.

New helpers in test-helpers/server-mock-factories.js

  • createMockUpstreamCycle(https) — spies on https.request, captures the upstream response handler, and returns { spy, upstreamRequest, responseHandler } (handler exposed via getter, populated after flushPromises())
  • completeUpstreamResponse(responseHandler, { statusCode, body }) — calls the handler with a makeProxyRes(), optionally emits a JSON data chunk, then emits 'end'

Refactoring

Before:

let responseHandler;
const upstreamRequest = new EventEmitter();
upstreamRequest.end = jest.fn();
upstreamRequest.write = jest.fn();
upstreamRequest.destroy = jest.fn();
const httpsRequestSpy = jest.spyOn(https, 'request').mockImplementation((options, cb) => {
  responseHandler = cb;
  return upstreamRequest;
});
// ...send request, flushPromises...
const proxyRes = new EventEmitter();
proxyRes.statusCode = 200;
proxyRes.headers = { 'content-type': 'application/json' };
proxyRes.pipe = jest.fn();
responseHandler(proxyRes);
proxyRes.emit('data', Buffer.from(JSON.stringify({ model: 'gpt-4o', usage: { ... } })));
proxyRes.emit('end');

After:

const cycle = createMockUpstreamCycle(https);
// ...send request, flushPromises...
completeUpstreamResponse(cycle.responseHandler, {
  statusCode: 200,
  body: { model: 'gpt-4o', usage: { ... } },
});
  • All remaining inline new EventEmitter() + jest.fn() upstream request constructions in both files replaced with the existing makeProxyReq() factory
  • Removed now-unused const { EventEmitter } = require('events') import from server.token-guards.test.js

…teUpstreamResponse helpers

Reduces ~70 lines of duplicated upstream request/response scaffolding
across server.token-guards.test.js and server.token-steering.test.js.

- Add createMockUpstreamCycle(https) to server-mock-factories.js: sets up
  the https.request spy, captures the upstream response handler, and
  returns a pre-built mock upstream request via makeProxyReq().
- Add completeUpstreamResponse(responseHandler, { statusCode, body }) to
  server-mock-factories.js: calls the captured handler with a mocked
  proxy response, optionally emits a JSON data chunk, then emits 'end'.
- Refactor 6 tests in server.token-guards.test.js to use these helpers.
- Refactor 2 tests in server.token-steering.test.js to use these helpers.
- Replace all remaining inline `new EventEmitter() + jest.fn()` upstream
  request patterns with the existing makeProxyReq() factory.
- Remove the now-unused `const { EventEmitter } = require('events')`
  import from server.token-guards.test.js.

Closes #5003
Copilot AI changed the title [WIP] Refactor duplicate token-limit proxy test setup refactor(api-proxy tests): extract shared upstream mock cycle helpers Jun 15, 2026
Copilot finished work on behalf of lpcox June 15, 2026 14:12
Copilot AI requested a review from lpcox June 15, 2026 14:12
@lpcox lpcox marked this pull request as ready for review June 15, 2026 14:37
Copilot AI review requested due to automatic review settings June 15, 2026 14:37
@github-actions

github-actions Bot commented Jun 15, 2026

Copy link
Copy Markdown
Contributor

✅ Coverage Check Passed

Overall Coverage

Metric Base PR Delta
Lines 96.86% 96.90% 📈 +0.04%
Statements 96.73% 96.77% 📈 +0.04%
Functions 98.81% 98.81% ➡️ +0.00%
Branches 91.24% 91.27% 📈 +0.03%
📁 Per-file Coverage Changes (1 files)
File Lines (Before → After) Statements (Before → After)
src/workdir-setup.ts 92.6% → 94.4% (+1.85%) 92.6% → 94.4% (+1.85%)

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

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Refactors the api-proxy Jest tests to reduce duplicated upstream HTTPS request/response mock scaffolding by introducing reusable helper functions, improving readability and consistency across token guard/steering test suites.

Changes:

  • Added createMockUpstreamCycle() and completeUpstreamResponse() helpers to standardize upstream mock setup and response completion.
  • Replaced repeated inline EventEmitter upstream request stubs with the existing makeProxyReq() factory across token steering/guard tests.
  • Simplified token guard tests by using the new helpers and removing an unused EventEmitter import.
Show a summary per file
File Description
containers/api-proxy/test-helpers/server-mock-factories.js Adds shared upstream-cycle mock helpers for HTTPS request spying and response completion.
containers/api-proxy/server.token-steering.test.js Refactors upstream mock setup to use makeProxyReq() and completeUpstreamResponse() instead of duplicated scaffolding.
containers/api-proxy/server.token-guards.test.js Refactors multiple guard tests to use createMockUpstreamCycle() / completeUpstreamResponse() and makeProxyReq().

Copilot's findings

Tip

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

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

Comment on lines +126 to +129
function completeUpstreamResponse(responseHandler, { statusCode = 200, body = null } = {}) {
const proxyRes = makeProxyRes(statusCode);
responseHandler(proxyRes);
if (body !== null) {
@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

Copy link
Copy Markdown
Contributor

Smoke test results:

  • GitHub MCP connectivity: ✅
  • GitHub.com connectivity: ✅
  • File write/read test: ✅
  • BYOK inference test: ✅

Running in direct BYOK mode (AWF_AUTH_TYPE=github-oidc + AWF_AUTH_AZURE_* + COPILOT_PROVIDER_BASE_URL) via api-proxy → Azure OpenAI (Foundry, o4-mini-aw) authenticated via Microsoft Entra

Overall: PASS

🪪 BYOK (AOAI Entra) report filed by Smoke Copilot BYOK AOAI (Entra)

@github-actions

This comment has been minimized.

Throws a descriptive error if called before the https.request callback
is captured, rather than the generic 'is not a function' TypeError.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@github-actions

Copy link
Copy Markdown
Contributor

🔬 Smoke Test Results

PR: refactor(api-proxy tests): extract shared upstream mock cycle helpers
Author: @Copilot | Assignees: @lpcox @Copilot

Test Status
GitHub MCP connectivity
GitHub.com HTTP connectivity ❌ (template vars not expanded)
File write/read ❌ (template vars not expanded)

Overall: FAIL — Pre-step template variables were not substituted at runtime.

📰 BREAKING: Report filed by Smoke Copilot

@github-actions

Copy link
Copy Markdown
Contributor

PR: refactor(api-proxy tests): extract shared upstream mock cycle helpers
✅ GitHub PR review
✅ Safe Inputs GH CLI
✅ Playwright
✅ File write/read
✅ Discussion interaction
✅ Build
Overall: PASS

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 Version Comparison Results

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

Overall: ❌ 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

Smoke Test: API Proxy OpenTelemetry Tracing

Scenario Result Details
1. Module Loading otel.js loads successfully; exports: startRequestSpan, setTokenAttributes, setBudgetAttributes, endSpan, endSpanError, shutdown, isEnabled, _provider, plus exporter helpers
2. Test Suite 59 tests passed, 0 failed across otel.test.js + otel-fanout.test.js
3. Env Var Forwarding src/services/api-proxy-service-config.ts forwards GH_AW_OTLP_ENDPOINTS, OTEL_EXPORTER_OTLP_ENDPOINT, OTEL_EXPORTER_OTLP_HEADERS, GITHUB_AW_OTEL_TRACE_ID, GITHUB_AW_OTEL_PARENT_SPAN_ID, and OTEL_SERVICE_NAME to the api-proxy container
4. Token Tracker Integration onUsage callback exists in token-tracker-http.js (line 251); invoked after normalized usage extraction, with onSpanEnd callback also present
5. OTEL Diagnostics No OTLP endpoint configured in this run; spans fall back to /var/log/api-proxy/otel.jsonl per the graceful-degradation path

All scenarios pass. OTEL tracing integration is complete and working correctly.

📡 OTel tracing validated by Smoke OTel Tracing

@github-actions

Copy link
Copy Markdown
Contributor

🔥 Smoke Test: PAT Auth — PASS

Test Result
GitHub MCP connectivity
GitHub.com HTTP connectivity
File write/read

Overall: PASS · Auth mode: PAT (COPILOT_GITHUB_TOKEN)

cc @lpcox @Copilot

🔑 PAT report filed by Smoke Copilot PAT

@github-actions

Copy link
Copy Markdown
Contributor

✅ Smoke Test: Copilot BYOK (Direct) Mode — PASS

Test Results:

  • ✅ GitHub MCP connectivity: 2 merged PRs fetched
  • ✅ GitHub.com connectivity: HTTP 200
  • ✅ File write/read: /tmp/gh-aw/agent/smoke-test-copilot-byok.txt verified
  • ✅ BYOK inference: Running in direct BYOK mode (COPILOT_PROVIDER_API_KEY) via api-proxy → api.githubcopilot.com

Status: All smoke tests passed. Agent running with BYOK credentials routed through api-proxy sidecar.

CC: @lpcox @Copilot

🔑 BYOK report filed by Smoke Copilot BYOK

@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 #5028 ·

@github-actions

Copy link
Copy Markdown
Contributor

Smoke Test: GitHub Actions Services Connectivity

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

Overall: FAILhost.docker.internal is unreachable from this runner. All service connectivity checks timed out or returned no response.

🔌 Service connectivity validated by Smoke Services

@github-actions

Copy link
Copy Markdown
Contributor

Gemini Smoke Test Results

Overall 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

@lpcox lpcox merged commit 20b629a into main Jun 15, 2026
78 of 84 checks passed
@lpcox lpcox deleted the copilot/duplicate-code-token-limit-proxy-tests branch June 15, 2026 15:21
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.

3 participants