Skip to content

CLI unit tests make a real network call to learn.microsoft.com, causing failures in offline and firewalled environments #194

Description

Summary

cli/test/unit/cli.test.ts mocks the MCP client but not probeEndpoint, so npm test makes a real network request to https://learn.microsoft.com/api/mcp. The suite passes when that request succeeds and fails when it does not, which makes the test result depend on network conditions rather than on the code under test.

Root cause

runDoctorChecks in cli/src/commands/doctor.ts calls two things:

const reachability = await probeEndpoint(endpoint);   // real fetch, not injectable via CliContext
const client = context.createClient({ endpoint });    // injectable, mocked in tests

probeEndpoint (cli/src/mcp/client.ts:48) accepts a fetchImpl parameter, but doctor.ts calls it with one argument, so it always uses globalThis.fetch against DEFAULT_ENDPOINT (https://learn.microsoft.com/api/mcp, cli/src/utils/contracts.ts:1). CliContext exposes createClient but no seam for probeEndpoint, so tests cannot substitute it.

report.ok requires reachability.ok, so a failed probe turns the doctor exit code into 1.

Reproduction

With network access, all tests pass:

Test Files  5 passed (5)
     Tests  32 passed (32)

With the network removed (unshare -rn), the same commit fails:

❯ test/unit/cli.test.ts:144:22
    142|     const exitCode = await runCli(['node', 'mslearn', 'doctor'], conte…
    144|     expect(exitCode).toBe(0);
- 0
+ 1

Test Files  1 failed | 4 passed (5)
     Tests  1 failed | 31 passed (32)

The failing case is "forces a fresh connection check in doctor instead of using cached tool mappings" (cli/test/unit/cli.test.ts:132). It asserts on getToolMapping call behaviour, which is fully mocked; the live endpoint probe is incidental to what the test is verifying.

Why this matters

Observed on the Copilot coding agent run for #174 (https://github.com/MicrosoftDocs/mcp/actions/runs/33598478336/job/100146752501), where the agent firewall blocks DNS for learn.microsoft.com:

⚠️ Warning: I tried to connect to the following addresses, but was blocked by firewall rules:
- `learn.microsoft.com.`
  Triggering Command: ... vitest/dist/workers/forks.js (dns block)

Two consequences:

  1. Unit tests are not hermetic. A Learn endpoint outage, a proxy, or an offline developer produces a red suite for reasons unrelated to the change under review.
  2. Any agent or sandboxed environment with egress restrictions gets a spurious failure on lockfile-only dependency bumps.

Suggested fix

Add a probeEndpoint seam to CliContext alongside createClient, defaulting to the real implementation and overridden in createTestContext. probeEndpoint already takes an injectable fetchImpl, so the change is limited to threading it through the context rather than reworking the probe itself.

If a test that exercises real endpoint reachability is wanted, it belongs in a separate integration suite that is not part of npm test.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Labels

No labels
No labels

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions