Move test assertions off third-party services - #4
Conversation
The fixtures outsourced server behaviour to httpstat.us and httpbin.org. Both stopped answering. httpstat.us times out from any network, not just a runner, and httpbin.org/redirect/3 now returns 503. The consequence was quiet. Every httpstat.us link reported "Status: 0 (Connection Error)" rather than the 404/500/403/503 it was named for, so the CI step called "Test with silent codes for broken links" had no status code to match and exercised none of that logic. The step carried continue-on-error: true, so nothing went red to say so. It could not have passed in any case -- silent-codes replaces the '403,503' default rather than extending it, so the 403 link was broken by construction even when the host was up. Status codes now go to test_modules.py against a mocked session, where they are deterministic and offline. A status code is an input to a decision; it never needed a server. Nine cases cover silent-codes replacing the default, the 429/451/503 bot-blocking codes that are absorbed regardless of silent-codes, and redirect counting. Verified by mutation: dropping 503 from bot_blocking_codes, hardcoding 403 silent, never marking redirected and never marking broken each turn the suite red. Unreachable hosts move to .invalid, which RFC 2606 reserves so it can never resolve. this-domain-does-not-exist-12345.com is unregistered today but sits in a registrable TLD, so it would have stopped testing anything the day somebody bought it. Two new CI steps use an .invalid fixture to assert status-0 handling end to end through the action: reported by default, silenced by silent-codes '0'. That is the coverage the httpstat.us step was reaching for, without the dependency. The live scan is kept, because real DNS, TLS and cross-host redirects are the one thing a mock cannot cover, but it is renamed to say it is informational and a comment records that its findings are expected. Also replaces the local-usage commands in tests/README.md, which told readers to run ./action.yml as an executable. It is a composite action and never was one. The replacements are four commands that run. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
🔗 Link Check Results🚨 1 broken link(s) and 3 redirect(s) were found. Build Details:
Link Check Summary
Broken Linkstests/broken-links.html - 1 broken link(s): tests/broken-links.html - 3 redirect(s): AI-Powered Suggestionstests/broken-links.html - AI Suggestions: Next Steps:
📝 This comment was automatically generated by the AI-Powered Link Checker Action. |
There was a problem hiding this comment.
Pull request overview
This PR hardens the test suite by removing reliance on third-party “status code” and redirect services, replacing them with deterministic unit tests (mocked requests.Session) and RFC 2606 .invalid fixtures so CI assertions remain stable over time.
Changes:
- Added
TestStatusCodeHandlingintests/test_modules.pyto cover status-code decision logic and redirect counting via mocked responses (offline/deterministic). - Updated HTML fixtures and test documentation to reflect the intended split between live smoke scans and offline assertions.
- Updated CI to generate
.invalidunreachable-host fixtures and assert end-to-end action outputs for status-0 reporting andsilent-codes: '0'.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
tests/test_modules.py |
Adds mocked-session unit tests for status-code handling and redirect counting. |
tests/redirect-links.html |
Removes reliance on httpbin redirect fixture; clarifies live smoke intent in comments. |
tests/broken-links.html |
Replaces registrable fake domain + status-code service links with .invalid and clarifying comments. |
tests/README.md |
Documents where offline vs live coverage belongs; updates local run commands. |
.github/workflows/ci.yml |
Adds generated .invalid fixtures and new assertions for status-0 and outputs; relabels live scan as informational. |
CHANGELOG.md |
Notes the test/fixture/documentation shift away from third-party services. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Copilot is right: tests/good-links.html is scanned with fail-on-broken and no continue-on-error, so one gating step does depend on the public internet. Its four targets return 200 with no redirects today, but a redirect appearing on any of them would turn CI red with nothing wrong in the action. Names the exception and says how to read a failure there, rather than claiming an offline guarantee the workflow does not provide. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
🔗 Link Check Results🚨 1 broken link(s) and 3 redirect(s) were found. Build Details:
Link Check Summary
Broken Linkstests/broken-links.html - 1 broken link(s): tests/broken-links.html - 3 redirect(s): AI-Powered Suggestionstests/broken-links.html - AI Suggestions: Next Steps:
📝 This comment was automatically generated by the AI-Powered Link Checker Action. |
* Serve CI fixtures from localhost so no gating step is live Closes the exception Copilot found on #4. tests/good-links.html was scanned with fail-on-broken and no continue-on-error, so one gating step depended on github.com, python.org, jupyter.org and docs.python.org all staying reachable and redirect-free. A redirect appearing on any of them turns CI red with nothing wrong in the action. A genuine 200 over genuine HTTP was the reason that step had to stay: every other deterministic fixture either skips the request (ignore-patterns) or fails transport (.invalid), so none of them exercises a successful response. The workflow now starts a small server on 127.0.0.1 and asserts four paths against it end to end -- a clean page exits 0 under fail-on-broken, a 301 is followed and counted as one redirect, a 404 is reported, and the same 404 is silenced by silent-codes. That last pair is what the httpstat.us fixture was for, before the service stopped answering and took the coverage with it. The live scans stay, because real DNS, TLS and cross-host redirects are the one thing a local server cannot cover, but all three are now informational and none can gate. Verified by running the generated serve step under the composite shell and the real checker against the real fixtures: clean 0 broken / 0 redirects, moved 0 / 1, missing 1 / 0, and missing with silent-codes 404 back to 0 / 0. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Give the fixture server an explicit route table The handler answered 200 to any path except /missing and /moved, so "A clean page passes end to end" only proved that the action requested something and got 200 -- not that it requested the URLs the fixture names. A typo in a generated fixture, or a regression that mangled the URL before the request, would have passed green. Routes are now exhaustive and anything unregistered is a 500 rather than a 404, so that silent-codes: '404' cannot suppress it. With a 404 default a typo in the missing fixture would still have been silenced and that step would still have passed. Verified by injecting a typo into each of the three fixtures in turn: clean goes from 0 broken to 1, redirect from 1 redirect to 0, and missing-with-silent-codes from 0 broken to 1. All three assertions go red where previously the first two would and the third would not. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
Follow-up to #3. The question that started this was whether the link-check failure reported on that PR was expected. It was — but for the wrong reason, and the reason was worth fixing.
What was wrong
The fixtures outsourced server behaviour to third parties, and both of them rotted.
httpstat.us/{404,500,403,503}httpbin.org/redirect/3this-domain-does-not-exist-12345.com.comThe consequence was quiet. Every
httpstat.uslink reportedStatus: 0 (Connection Error)instead of the code it was named for, so the step called Test with silent codes for broken links had no status code to match and exercised none of that logic. It carriedcontinue-on-error: true, so nothing went red to say so.It could not have passed even with the host up:
silent-codesreplaces the403,503default rather than extending it, sosilent-codes: '404,500'made the 403 link broken by construction. That step has never been able to go green.What changed
Status codes move to a mocked session in
TestStatusCodeHandling. A status code is an input to a decision — it never needed a server. Nine cases coversilent-codesreplacing rather than extending the default, the429/451/503codes absorbed byis_likely_bot_blockedregardless ofsilent-codes, and redirect counting including a redirect that lands on a 404.Verified by mutation — each of these turns the suite red:
503frombot_blocking_codes403as silentUnreachable hosts move to
.invalid, reserved by RFC 2606 so it can never resolve. Two new CI steps assert status-0 handling end to end through the action — reported by default (broken-link-count == 2), silenced bysilent-codes: '0'(0,broken-links-found == false). The second runs withfail-on-broken: 'true', so a regression fails the step on its own rather than waiting for the assertion. That is the coverage thehttpstat.usstep was reaching for, without the dependency.The live scan stays, because real DNS, TLS and cross-host redirects are the one thing a mock cannot cover. It is renamed to say it is informational, and a comment records that its findings are expected and do not gate.
tests/README.mdgains a table of where each kind of coverage belongs and an explicit "do not point a fixture at a status-code service" rule, so this does not get reintroduced. Its local-usage section told readers to run./action.ymlas an executable — it is a composite action and never was one. Replaced with four commands that actually run, each verified.Not included
No change to the action's behaviour — this is tests, fixtures and docs.
link_checker.pyandaction.ymlare untouched.