diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index fc9960a..24477e1 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -70,13 +70,56 @@ jobs: html-path: 'tests/good-links.html' fail-on-broken: 'true' - - name: Test with silent codes for broken links + # Informational only, and expected to report findings: the fixture holds + # a deliberately unreachable host and three deliberate redirects. It runs + # with fail-on-broken so that the PR-comment path is exercised, and with + # continue-on-error so that neither those findings nor an outage at one of + # the redirect targets can gate CI. Do not read its result as an + # assertion -- the assertions are the deterministic steps below. + - name: Live smoke scan (informational, does not gate) continue-on-error: true uses: ./ with: html-path: 'tests/broken-links.html' fail-on-broken: 'true' - silent-codes: '404,500' # Treat these as silent + timeout: 30 + + # Status 0 is what the checker reports when a request never reaches the + # server. .invalid is reserved by RFC 2606 and can never resolve, so these + # two links fail transport on every run, immediately and without contacting + # anything. That makes silent-codes '0' testable without a live host -- + # which is what the old httpstat.us fixture was trying and failing to do. + - name: Create unreachable-host fixture + run: | + mkdir -p ci-fixtures-unreachable + cat > ci-fixtures-unreachable/unreachable.html <<'HTML' + + One + Two + + HTML + + - name: Unreachable hosts are reported by default + id: unreachable-loud + uses: ./ + with: + html-path: 'ci-fixtures-unreachable' + fail-on-broken: 'false' + ai-suggestions: 'false' + timeout: 10 + + # fail-on-broken is deliberately true here: if status-0 silencing ever + # regresses, this step fails on its own rather than waiting for the + # assertions below. + - name: silent-codes 0 suppresses unreachable hosts + id: unreachable-quiet + uses: ./ + with: + html-path: 'ci-fixtures-unreachable' + fail-on-broken: 'true' + ai-suggestions: 'false' + timeout: 10 + silent-codes: '0,403,503' # The ignore-patterns fixture is built here rather than kept in tests/ so # that the counts below are exact and the other jobs keep scanning the @@ -117,7 +160,7 @@ jobs: ai-suggestions: 'false' timeout: 10 - - name: Verify ignore-patterns counts + - name: Verify action outputs run: | assert() { if [ "$2" != "$3" ]; then @@ -126,11 +169,15 @@ jobs: fi echo "ok: $1 = $2" } - assert "ignored-count with patterns" "${{ steps.test-ignored.outputs.ignored-count }}" "3" - assert "broken-link-count with patterns" "${{ steps.test-ignored.outputs.broken-link-count }}" "0" - assert "broken-links-found with patterns" "${{ steps.test-ignored.outputs.broken-links-found }}" "false" - assert "ignored-count without patterns" "${{ steps.test-not-ignored.outputs.ignored-count }}" "0" - assert "broken-link-count without patterns" "${{ steps.test-not-ignored.outputs.broken-link-count }}" "3" + assert "ignored-count with patterns" "${{ steps.test-ignored.outputs.ignored-count }}" "3" + assert "broken-link-count with patterns" "${{ steps.test-ignored.outputs.broken-link-count }}" "0" + assert "broken-links-found with patterns" "${{ steps.test-ignored.outputs.broken-links-found }}" "false" + assert "ignored-count without patterns" "${{ steps.test-not-ignored.outputs.ignored-count }}" "0" + assert "broken-link-count without patterns" "${{ steps.test-not-ignored.outputs.broken-link-count }}" "3" + assert "unreachable reported by default" "${{ steps.unreachable-loud.outputs.broken-link-count }}" "2" + assert "unreachable found by default" "${{ steps.unreachable-loud.outputs.broken-links-found }}" "true" + assert "unreachable silenced by code 0" "${{ steps.unreachable-quiet.outputs.broken-link-count }}" "0" + assert "no findings once silenced" "${{ steps.unreachable-quiet.outputs.broken-links-found }}" "false" test-empty-directory: runs-on: ubuntu-latest diff --git a/CHANGELOG.md b/CHANGELOG.md index 0ad5933..a3ffd3b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -27,6 +27,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Corrected the action's self-referencing links in issue bodies, PR comments, artifacts and `examples.md`, which still pointed at the pre-migration `QuantEcon/meta` path - The generated issue body claimed later runs would refresh it in place even when `update-existing-issue` was `false` - The temporary ignore-patterns file is removed via an `EXIT` trap, so it is not left behind when the step exits early +- The test fixtures depended on `httpstat.us` and `httpbin.org` for status codes and multi-hop redirects. Both stopped answering, so those links reported `Status: 0 (Connection Error)` and the CI step named for `silent-codes` exercised no silent-code logic at all — masked by `continue-on-error: true`. Status codes and redirect counting are now asserted against a mocked session, unreachable hosts use RFC 2606 `.invalid` names, and the live scan is labelled as the informational step it always was +- `tests/README.md` documented local commands that ran `./action.yml` as an executable, which was never possible + +*(No change to the action's behaviour in the last two entries — test and documentation only.)* ## [1.0.0] - 2025-10-01 diff --git a/tests/README.md b/tests/README.md index 9710ec1..21d695c 100644 --- a/tests/README.md +++ b/tests/README.md @@ -2,6 +2,24 @@ This directory contains test HTML files and scripts used to validate the link-checker action functionality. +## What may depend on the network + +Most of the HTML fixtures here feed a **live smoke scan** that is informational and does not gate CI. Every *assertion* about the action's behaviour runs offline, against a mocked session or a fixture under `.invalid`. + +One step is a deliberate exception. `good-links.html` is scanned with `fail-on-broken: 'true'` and no `continue-on-error`, so it gates — and its four targets are real sites. If any of them starts redirecting, or is down when CI runs, that step goes red without the action having changed. It is kept because a genuine 200 over genuine HTTP is the one thing a mock cannot give us, but treat a failure there as a fixture problem until you have ruled the action out. Nothing else that gates touches the public internet. + +**Do not point a fixture at a status-code service.** `httpstat.us` and `httpbin.org/redirect/3` were both used here and both stopped answering. When that happened the links reported `Status: 0 (Connection Error)` instead of the 404/500/503 they were named for, so the CI step called "test with silent codes" stopped exercising silent codes entirely — and because it carried `continue-on-error: true`, nothing went red to say so. A third-party service that returns a status on demand is a dependency that will fail this way eventually. + +Where each kind of coverage belongs: + +| Behaviour under test | Where it lives | Why | +|---|---|---| +| Status-code decisions (silent-codes, bot-blocking codes, redirect counting) | `test_modules.py`, with a mocked session | A status code is an input to a decision, not something that needs a server | +| Bot-blocking domain detection | `test_bot_blocking.py` | Pure substring matching on the URL; the request never has to succeed | +| A host that cannot be reached | Workflow-generated fixtures under `.invalid` | RFC 2606 reserves `.invalid`, so it can never resolve — unlike a made-up name under a registrable TLD, which stops testing anything the day somebody buys it | +| `ignore-patterns` and output plumbing | Workflow-generated fixtures + explicit assertions | Exact counts stay exact as fixtures are added to this directory | +| Real DNS, TLS and cross-host redirects | The fixtures here, live smoke scan | The one thing a mock genuinely cannot cover. Informational, except the `good-links.html` step noted above | + ## Test Files ### good-links.html @@ -9,13 +27,12 @@ This directory contains test HTML files and scripts used to validate the link-ch - Used to test successful link validation ### broken-links.html -- Contains intentionally broken links for testing error detection -- Tests various failure scenarios (404, 500, non-existent domains) -- Includes links that should trigger AI suggestions (HTTP→HTTPS, master→main) +- One host under `.invalid` that can never resolve, plus links that should trigger AI suggestions (HTTP→HTTPS, master→main, Python 2.7→3) +- Scanned by the live smoke step, which is expected to report findings and does not gate ### redirect-links.html - Contains links that redirect to test redirect handling -- Tests proper following of redirects and detection of issues +- Tests proper following of redirects, including a multi-hop chain, against hosts that are stable ### legitimate-slow-links.html - Contains links that are slow to respond but valid @@ -28,6 +45,7 @@ This directory contains test HTML files and scripts used to validate the link-ch ### test_modules.py - Unit tests for the Python modules (link_checker.py, format_results.py) - Tests module imports, ignore-pattern compilation and matching, the network-failure result, and the `--ignore-patterns-file` path end to end +- `TestStatusCodeHandling` covers the status-code decision matrix against a mocked session — silent-codes replacing rather than extending the default, the bot-blocking codes, and redirect counting ## Usage in CI @@ -43,22 +61,24 @@ The `ignore-patterns` assertions in CI use a fixture generated by the workflow r ## Running Tests Locally -You can test the action locally using these files: +`action.yml` is a composite action, not an executable — run the checker directly instead. It takes one HTML file and prints a JSON report: ```bash -# Test with good links (should pass) -./action.yml --html-path tests/good-links.html --fail-on-broken true +# Run the test suites (no network required) +cd tests && python test_modules.py && python test_bot_blocking.py -# Test with broken links (should find issues) -./action.yml --html-path tests/broken-links.html --fail-on-broken false +# Scan a fixture and pretty-print the report +python link_checker.py tests/broken-links.html | python -m json.tool -# Test with silent codes for known issues -./action.yml --html-path tests --silent-codes "404,500" --fail-on-broken true +# See silent-codes suppress an unreachable host +python link_checker.py tests/broken-links.html --silent-codes '0,403,503' -# Run Python module tests -cd tests && python test_modules.py && python test_bot_blocking.py +# Try an ignore pattern +python link_checker.py tests/broken-links.html --ignore-patterns 'docs\.python\.org' ``` +To exercise the full action — the shell plumbing, the outputs and the issue and comment steps — push a branch and let the `test-action` job run it, or use [act](https://github.com/nektos/act). + ## Test Requirements The Python tests require the same dependencies as the main action: diff --git a/tests/broken-links.html b/tests/broken-links.html index dff9000..7007c03 100644 --- a/tests/broken-links.html +++ b/tests/broken-links.html @@ -4,34 +4,42 @@ Test Page with Broken Links + +

Test Page - With Broken Links

- +

This page contains broken and problematic links for testing:

- + - -

Links that should be silently reported:

- - +

Redirected links that could be improved:

- +

Good links for comparison:

- \ No newline at end of file + diff --git a/tests/redirect-links.html b/tests/redirect-links.html index f00d863..c63b8f0 100644 --- a/tests/redirect-links.html +++ b/tests/redirect-links.html @@ -4,21 +4,28 @@ Test Page with Redirects + +

Test Page - With Redirects

- +

This page contains links that redirect to test AI suggestions:

- + - +

These should generate AI suggestions:

- \ No newline at end of file + diff --git a/tests/test_modules.py b/tests/test_modules.py index 505d1b2..6a24181 100644 --- a/tests/test_modules.py +++ b/tests/test_modules.py @@ -10,6 +10,9 @@ import unittest import tempfile from pathlib import Path +from unittest import mock + +import requests # Add the parent directory to sys.path to import the modules sys.path.insert(0, str(Path(__file__).parent.parent)) @@ -130,9 +133,6 @@ def test_connection_broken_mid_response_counts_as_transport_failure(self): needs its own handler or it falls through to the catch-all and stays loud however silent-codes is set. """ - import requests - from unittest import mock - exc = requests.exceptions.ChunkedEncodingError('Connection broken') with mock.patch.object(requests.Session, 'get', side_effect=exc): quiet = link_checker.check_link( @@ -231,6 +231,92 @@ def test_link_checker_with_test_files(self): else: print("ℹ️ Test HTML files not found, skipping file-based tests") + +class TestStatusCodeHandling(unittest.TestCase): + """How check_link classifies a server that did answer. + + These used to be covered by pointing fixtures at httpstat.us. That service + stopped answering, so every one of those links reported status 0 and the + CI step named for silent-codes exercised none of this. A status code is + just an input to a decision, so it does not need a server -- mocking the + session keeps the coverage deterministic and offline. + """ + + @staticmethod + def _respond(status_code, url='https://example.com/page', history=()): + response = mock.Mock() + response.status_code = status_code + response.url = url + response.history = list(history) + response.ok = status_code < 400 + response.text = '' + return mock.patch.object(requests.Session, 'get', return_value=response) + + def _check(self, status_code, silent_codes, url='https://example.com/page', + history=()): + with self._respond(status_code, url, history): + return link_checker.check_link(url, 30, 5, silent_codes) + + def test_ok_response_is_neither_broken_nor_silent(self): + result = self._check(200, [403, 503]) + self.assertFalse(result['broken']) + self.assertFalse(result['silent']) + self.assertFalse(result['redirected']) + + def test_error_status_is_broken_unless_listed(self): + """The plain case: a 404 nobody asked to silence is a broken link""" + for code in (400, 404, 410, 500): + with self.subTest(code=code): + result = self._check(code, [403, 503]) + self.assertTrue(result['broken']) + self.assertFalse(result['silent']) + + def test_listed_status_is_silenced(self): + """silent-codes suppresses exactly the codes it names""" + for code in (404, 500): + with self.subTest(code=code): + result = self._check(code, [404, 500]) + self.assertFalse(result['broken']) + self.assertTrue(result['silent']) + + def test_silent_codes_replaces_the_default_rather_than_extending_it(self): + """403 is silent by default, and loud once silent-codes omits it + + This is why the old `silent-codes: '404,500'` CI step could not pass + even when its fixture host was up: setting silent-codes replaces + '403,503', so the 403 link became broken. + """ + self.assertTrue(self._check(403, [403, 503])['silent']) + + loud = self._check(403, [404, 500]) + self.assertTrue(loud['broken']) + self.assertFalse(loud['silent']) + + def test_bot_blocking_codes_are_silent_even_when_not_listed(self): + """429/451/503 are absorbed by is_likely_bot_blocked, not silent-codes""" + for code in (429, 451, 503): + with self.subTest(code=code): + result = self._check(code, [404, 500]) + self.assertFalse(result['broken']) + self.assertTrue(result['silent']) + self.assertTrue(result['likely_bot_blocked']) + + def test_redirected_response_is_counted_not_broken(self): + result = self._check(200, [403, 503], + url='https://example.com/final', + history=[object(), object()]) + self.assertTrue(result['redirected']) + self.assertEqual(result['redirect_count'], 2) + self.assertFalse(result['broken']) + + def test_redirect_to_an_error_is_still_broken(self): + """Following redirects to a 404 must not be excused by the redirect""" + result = self._check(404, [403, 503], + url='https://example.com/gone', + history=[object()]) + self.assertTrue(result['broken']) + + def main(): print("🧪 Running Link Checker Tests") print("=" * 40)