fix(tests): mock search_servers fallback in registry-client UUID-not-found test#1264
Merged
Merged
Conversation
…found test test_find_server_by_reference_uuid_not_found mocked SimpleRegistryClient.get_server_info to raise ValueError, but find_server_by_reference catches ValueError and falls through to self.search_servers(reference) -- which was not mocked and made a real HTTPS call to api.mcp.github.com. On the Windows runner (build-release.yml -> Build & Test (windows-latest)) that runner's DNS cannot resolve api.mcp.github.com, raising socket.gaierror [Errno 11001] and failing the unit test step. Linux/macOS runners happened to resolve the host so the failure was not surfaced on the Linux-only ci.yml PR gate. Fix: also mock search_servers to return []. The test now exercises both branches of the not-found path and never touches the network, which is the existing 'no live network calls' rule documented in .github/instructions/tests.instructions.md. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes a unit test network leak in the MCP registry client test suite by ensuring the UUID-not-found path is fully mocked and cannot fall through to a real HTTPS request (which was breaking Windows CI due to DNS resolution failures).
Changes:
- Mock
SimpleRegistryClient.search_serversintest_find_server_by_reference_uuid_not_foundso the UUID lookup fallback path is hermetic. - Add a
CHANGELOG.mdentry under## [Unreleased]/### Fixeddocumenting the test fix and the Windows CI impact.
Show a summary per file
| File | Description |
|---|---|
tests/unit/test_registry_client.py |
Extends the UUID-not-found test to also mock the search_servers fallback and assert it is called, preventing real network I/O. |
CHANGELOG.md |
Records the unit test fix under Unreleased/Fixed with the PR number. |
Copilot's findings
- Files reviewed: 2/2 changed files
- Comments generated: 0
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
TL;DR
test_find_server_by_reference_uuid_not_foundmade a real HTTPS call toapi.mcp.github.combecause onlyget_server_infowas mocked. The Windows runner inbuild-release.ymlcan't resolve that hostname and raisedsocket.gaierror [Errno 11001] getaddrinfo failed, failing the unit-test step. Linux/macOS runners happened to resolve the host so the PR gate (ci.yml, Linux-only) stayed green.Failing run: https://github.com/microsoft/apm/actions/runs/25651666245
Root cause
find_server_by_reference(reference)insrc/apm_cli/registry/client.py:349:referenceis UUID-shaped, callsself.get_server_info(reference).ValueError, it catches and falls through toself.search_servers(reference)(the name-search fallback).The test mocked step (1) to raise
ValueErrorbut left step (2) unmocked.search_serversbuilds arequestscall to the live registry — on a runner that can't resolve the host, you get a DNS error instead of aValueError, propagating out of the test as a hard fail.Fix
Mock
search_serversto return[]so the not-found path completes hermetically. Test now exercises both branches of the UUID-not-found path and never touches the network — consistent with the existing.github/instructions/tests.instructions.mdrule ("Tests must never hit a real HTTP endpoint").Why this didn't surface earlier
The Linux PR gate (
ci.yml) is Linux-only by design; Windows is covered post-merge bybuild-release.yml. So this latent network leak only manifested on the Windows post-merge runner.Validation