From 3dbe4b7c16848dea3bff7fbe49a1d53828229518 Mon Sep 17 00:00:00 2001 From: wilfordgrimley <2397930+WilfordGrimley@users.noreply.github.com> Date: Thu, 30 Jul 2026 14:54:38 +0000 Subject: [PATCH] CI: mock Scryfall's deck-export endpoint, the last live-network call in test_valid_url master went RED at f947346b (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//export/text` for real. CI received an HTML document instead of the text export - `` 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 (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, 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 Claude-Session: https://claude.ai/code/session_013NhYmT1PxCcyemA16dFDxN --- .../cardpicker/tests/test_integrations.py | 36 +++++++++++++++++++ 1 file changed, 36 insertions(+) diff --git a/MPCAutofill/cardpicker/tests/test_integrations.py b/MPCAutofill/cardpicker/tests/test_integrations.py index c446d7cee..d35517238 100644 --- a/MPCAutofill/cardpicker/tests/test_integrations.py +++ b/MPCAutofill/cardpicker/tests/test_integrations.py @@ -197,6 +197,42 @@ def test_valid_url(self, client, django_settings, snapshot, url: str): } }, ) + # CI baseline cleanup, 2026-07-30: SCRYFALL/SCRYFALL_WITH_WWW made master RED at + # f947346b (PR #660) with the same shape as the two above, but a DIFFERENT root cause, + # and the difference is the reason this is a mock rather than a skip. The endpoint is + # NOT broken: `curl https://api.scryfall.com/decks/71bb2d40-.../export/text` from + # outside CI returns the expected `text/plain` body with HTTP 200, and the immediately + # preceding master commit (1d433dd6) went green on the identical test 111 minutes + # earlier. What CI received instead was an HTML document (`` in the + # `base.py:64` "Invalid response" log line), i.e. an edge/bot-challenge interstitial + # served to the GitHub Actions runner's IP - a property of WHERE the request came from, + # not of the code or of Scryfall's contract. That class of failure cannot be fixed by + # retrying (the next runner is in the same address space) and cannot be gated on a + # config flag, which is exactly the tappedout/manastack situation. + # + # THE HOST HERE IS `api.scryfall.com`, NOT `Scryfall.get_host_names()`. The two mocks + # above build their pattern from `get_host_names()` because those sites fetch from the + # same host they are matched on. `Scryfall.retrieve_card_list` does not: it matches + # `scryfall.com`/`www.scryfall.com` (the deck page URL a user pastes) but requests + # `netloc="api.scryfall.com"`. A `get_host_names()`-derived pattern would silently + # match nothing and leave the test exactly as red as before. + # + # SCOPED TO THE DECK-EXPORT PATH, deliberately. `api.scryfall.com` also serves this + # suite's bulk-data and DFC/meld calls (`scryfall_bulk_data`, + # `test_get_double_faced_card_pairs`, `test_get_meld_pairs`), and a host-wide mock + # would gut them. The `/decks/` prefix is the narrowest pattern that covers + # `retrieve_card_list`'s one request and nothing else; `real_http=True` still carries + # every other Scryfall call to the real API unchanged. + # + # BODY IS THE REAL RESPONSE, BYTE FOR BYTE (captured 2026-07-30 by direct curl, + # `od -c`-verified CRLF line endings and no trailing newline), so + # `retrieve_card_list`'s own `"// Sideboard"` strip still runs on genuine input and + # the existing recorded snapshot needs no change - the parsing stays covered, only the + # transport is faked. + mock.get( + re.compile(r"^https://api\.scryfall\.com/decks/"), + text="4 Brainstorm\r\n1 Delver of Secrets\r\n3 Past in Flames\r\n\r\n// Sideboard", + ) decklist = MTGIntegration.query_import_site(url) assert decklist assert Counter(decklist.splitlines()) == snapshot