CI: mock Scryfall's deck-export endpoint, the last live-network call in test_valid_url - #663
Merged
Conversation
…in test_valid_url master went RED at f947346 (PR #660, the monolith) on test_valid_url[scryfall]/[scryfall_with_www] - 2 failed, 3542 passed. The failure is NOT in the monolith and touches no pipeline code: it is the third instance of a failure mode this test already documents twice. WHAT HAPPENED. `Scryfall.retrieve_card_list` fetches `https://api.scryfall.com/decks/<id>/export/text` for real. CI received an HTML document instead of the text export - `<!DOCTYPE html>` appears verbatim in the `base.py:64` "Invalid response" log line - so `query_import_site` raised `InvalidURLException`. WHY THIS IS NOT A SCRYFALL OUTAGE, AND WHY A RETRY IS NOT THE FIX. The endpoint is healthy: a direct curl from outside CI returns HTTP 200, `text/plain`, and the expected body. The immediately preceding master commit (1d433dd) went green on the identical test 111 minutes earlier, and nothing between the two touches integrations. What CI got was an edge/bot-challenge interstitial served to the GitHub Actions runner's address space - a property of WHERE the request originates, not of the code or of Scryfall's contract. The next runner sits in the same address space, so a re-run is a coin flip, not a remedy. THE FIX IS THE ONE THIS FILE ALREADY ESTABLISHED. tappedout.net (2026-07-20) and manastack.com (2026-07-22) each hit this same class of live-network false-red and each was mocked in place rather than skipped, so the site's own `retrieve_card_list` parsing stays covered and only the transport is faked. Scryfall now gets the same treatment, and it was the last un-mocked flaky caller left in this parametrize. TWO THINGS THAT ARE EASY TO GET WRONG HERE, both handled and both commented in place: - THE HOST IS `api.scryfall.com`, NOT `Scryfall.get_host_names()`. The two existing mocks build their patterns from `get_host_names()` because those sites fetch from the host they are matched on. Scryfall does not: it matches the `scryfall.com` deck page a user pastes but requests `netloc="api.scryfall.com"`. A pattern derived from `get_host_names()` would match nothing and leave the test exactly as red. - THE MOCK IS SCOPED TO `/decks/`. `api.scryfall.com` also serves this suite's bulk-data, DFC and meld calls; a host-wide mock would gut `test_get_double_faced_card_pairs`/`test_get_meld_pairs`. `real_http=True` still carries every other Scryfall request to the real API unchanged. The mocked body is the real response captured byte for byte (od -c verified: CRLF line endings, no trailing newline), so `retrieve_card_list`'s own `"// Sideboard"` strip runs on genuine input and the recorded snapshot is unchanged. VERIFICATION (mutation red -> restore -> green, local, python 3.10): - baseline, before this change: test_valid_url[scryfall] passes locally, because this box's IP is not the one being challenged - which is itself the evidence that the failure is origin-dependent rather than code-dependent. - mutate the mocked body ("3 Past in Flames" -> "3 MUTANT"): 2 failed (scryfall, scryfall_with_www) on the snapshot comparison. This proves the mock is genuinely serving the request - had it fallen through to the real network, altering the mock body could not have changed the outcome - and that the snapshot assertion is live rather than vacuous. - restore: 13 passed, 2 skipped, 13 snapshots passed. - black --check: unchanged. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013NhYmT1PxCcyemA16dFDxN
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
1 participant
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.
Why master is red, and why it is not the monolith
masteratf947346b(PR #660, the monolith) fails Backend tests:2 failed, 3542 passed, 11 skipped. Both failures areTestMTGIntegration::test_valid_url[scryfall]and[scryfall_with_www]. Neither touches any pipeline code. The monolith is not implicated, and the first monolith run is not compromised by this.Scryfall.retrieve_card_listfetcheshttps://api.scryfall.com/decks/<id>/export/textfor real. CI received an HTML document instead of the text export —<!DOCTYPE html>appears verbatim in thebase.py:64"Invalid response" log line — soquery_import_siteraisedInvalidURLException.Why a retry is not the fix
The endpoint is healthy. A direct
curlfrom outside CI returns HTTP 200,text/plain, and the expected body. The preceding master commit (1d433dd6) went green on the identical test 111 minutes earlier, and nothing between the two touches integrations.What CI got was an edge/bot-challenge interstitial served to the GitHub Actions runner's address space — a property of where the request originates, not of the code or of Scryfall's contract. The next runner sits in the same address space, so a re-run is a coin flip rather than a remedy. (A re-run was kicked off in parallel with this PR purely to unblock
deploy.shfaster if it happens to land green; it does not remove the need for this change.)The fix is the one this file already established, twice
tappedout.net(2026-07-20) andmanastack.com(2026-07-22) each hit this same class of live-network false-red, and each was mocked in place rather than skipped, so the site's ownretrieve_card_listparsing stays covered and only the transport is faked. Scryfall now gets the same treatment. It was the last un-mocked flaky caller left in this parametrize.Two things that are easy to get wrong here, both handled and both commented in place:
api.scryfall.com, notScryfall.get_host_names(). The two existing mocks build their patterns fromget_host_names()because those sites fetch from the host they are matched on. Scryfall does not: it matches thescryfall.comdeck page a user pastes, but requestsnetloc="api.scryfall.com". A pattern derived fromget_host_names()would match nothing and leave the test exactly as red./decks/.api.scryfall.comalso serves this suite's bulk-data, DFC and meld calls; a host-wide mock would guttest_get_double_faced_card_pairs/test_get_meld_pairs.real_http=Truestill carries every other Scryfall request to the real API unchanged.The mocked body is the real response captured byte for byte (
od -cverified: CRLF line endings, no trailing newline), soretrieve_card_list's own"// Sideboard"strip runs on genuine input and the recorded snapshot is unchanged.Verification — mutation red, restore, green
test_valid_url[scryfall]passes — this box's IP is not the one being challenged, which is itself the evidence the failure is origin-dependent, not code-dependent"3 Past in Flames"→"3 MUTANT"scryfall,scryfall_with_www) on the snapshot comparisonThe mutation is not in the class that survives by asserting a value both paths reach: had the mock fallen through to the real network, altering the mock body could not have changed the outcome. Red-on-mutation therefore proves the mock is genuinely serving the request and that the snapshot assertion is live.
black --check,ruff,isort,mypyall pass (pre-commit).Scope
One test file, 36 added lines, no production code. Deliberately shipped alone: a red-master fix should be reviewable and mergeable without accepting any behaviour change alongside it.
🤖 Generated with Claude Code
https://claude.ai/code/session_013NhYmT1PxCcyemA16dFDxN