|
| 1 | +#!/usr/bin/env python3 |
| 2 | +"""Guard the rendered-image checker. |
| 3 | +
|
| 4 | +The defect this tool exists for (DOC-1515) is not "an image is missing". It is |
| 5 | +"every checker reads a different artifact than the browser does". So the tests |
| 6 | +that matter are the ones pinning HOW a src is resolved, not whether a file |
| 7 | +exists. |
| 8 | +
|
| 9 | +Two of them encode real incidents: |
| 10 | +
|
| 11 | + - unionai-docs-infra#290 rebased relative image paths on api-reference pages |
| 12 | + but prepended `../` unconditionally, so `_index.md` pages went one level too |
| 13 | + high. The wrong path landed on `/docs/v2/_static/...`, which 302s to an HTML |
| 14 | + page rather than 404ing -- so a status-code check would have called it fine. |
| 15 | + That is why `classify` treats an HTML target as a failure with its own |
| 16 | + reason, rather than as a hit. |
| 17 | +
|
| 18 | + - Before #290, the same pages went one level too LOW. Both directions must |
| 19 | + fail, which is only true if resolution is done against the page's URL. |
| 20 | +""" |
| 21 | + |
| 22 | +import subprocess |
| 23 | +import sys |
| 24 | +from pathlib import Path |
| 25 | + |
| 26 | +sys.path.insert(0, str(Path(__file__).resolve().parent.parent / "tools" / "image_checker")) |
| 27 | + |
| 28 | +from check_rendered_images import ( # noqa: E402 |
| 29 | + IMG_RE, classify, page_url_dir, resolve, |
| 30 | +) |
| 31 | + |
| 32 | +TOOL = Path(__file__).resolve().parent.parent / "tools" / "image_checker" / "check_rendered_images.py" |
| 33 | + |
| 34 | + |
| 35 | +def build(tmp_path, pages, files=()): |
| 36 | + """A miniature built site. `pages` maps dist-relative html path -> src.""" |
| 37 | + dist = tmp_path / "dist" |
| 38 | + for rel, src in pages.items(): |
| 39 | + p = dist / rel |
| 40 | + p.parent.mkdir(parents=True, exist_ok=True) |
| 41 | + p.write_text(f'<html><body><img src="{src}" alt="x"></body></html>') |
| 42 | + for rel in files: |
| 43 | + f = dist / rel |
| 44 | + f.parent.mkdir(parents=True, exist_ok=True) |
| 45 | + f.write_bytes(b"") |
| 46 | + return dist |
| 47 | + |
| 48 | + |
| 49 | +# --- resolution: the whole point of the tool ------------------------------ |
| 50 | + |
| 51 | +def test_relative_src_resolves_against_the_pages_url_not_the_source_dir(tmp_path): |
| 52 | + """A browser resolves against the page URL. check_images.sh does not, which |
| 53 | + is exactly why it cannot see this class of bug.""" |
| 54 | + dist = build(tmp_path, |
| 55 | + {"docs/v2/union/guide/page/index.html": "../../_static/x.png"}, |
| 56 | + ["docs/v2/union/_static/x.png"]) |
| 57 | + html = dist / "docs/v2/union/guide/page/index.html" |
| 58 | + assert resolve("../../_static/x.png", html, dist) == dist / "docs/v2/union/_static/x.png" |
| 59 | + |
| 60 | + |
| 61 | +def test_absolute_src_resolves_against_the_site_root(tmp_path): |
| 62 | + dist = build(tmp_path, {"docs/v2/union/p/index.html": "/docs/v2/union/_static/x.png"}) |
| 63 | + html = dist / "docs/v2/union/p/index.html" |
| 64 | + assert resolve("/docs/v2/union/_static/x.png", html, dist) == dist / "docs/v2/union/_static/x.png" |
| 65 | + |
| 66 | + |
| 67 | +def test_dotdot_segments_inside_an_absolute_src_are_normalized(tmp_path): |
| 68 | + """Hugo emits `RelPermalink + ../ + path`, so real srcs are absolute AND |
| 69 | + carry `..`. All 2777 images in a production build have this shape.""" |
| 70 | + dist = build(tmp_path, {"docs/v2/union/a/b/index.html": "x"}) |
| 71 | + html = dist / "docs/v2/union/a/b/index.html" |
| 72 | + got = resolve("/docs/v2/union/a/b/../../_static/x.png", html, dist) |
| 73 | + assert got == dist / "docs/v2/union/_static/x.png" |
| 74 | + |
| 75 | + |
| 76 | +def test_query_string_and_fragment_are_stripped(tmp_path): |
| 77 | + """Every local image carries the DOC-1251 cache-buster.""" |
| 78 | + dist = build(tmp_path, {"docs/p/index.html": "x"}) |
| 79 | + html = dist / "docs/p/index.html" |
| 80 | + assert resolve("/docs/_static/x.png?v=abc123", html, dist) == dist / "docs/_static/x.png" |
| 81 | + |
| 82 | + |
| 83 | +def test_percent_escapes_are_decoded(tmp_path): |
| 84 | + dist = build(tmp_path, {"docs/p/index.html": "x"}) |
| 85 | + html = dist / "docs/p/index.html" |
| 86 | + assert resolve("/docs/_static/my%20image.png", html, dist) == dist / "docs/_static/my image.png" |
| 87 | + |
| 88 | + |
| 89 | +def test_a_src_climbing_out_of_the_site_root_is_not_silently_clamped(tmp_path): |
| 90 | + dist = build(tmp_path, {"docs/index.html": "x"}) |
| 91 | + html = dist / "docs/index.html" |
| 92 | + assert resolve("../../../etc/passwd", html, dist) is None |
| 93 | + assert classify(None) == "escapes" |
| 94 | + |
| 95 | + |
| 96 | +# --- classification: why a 200 is not proof ------------------------------- |
| 97 | + |
| 98 | +def test_an_html_target_is_a_failure_with_its_own_reason(tmp_path): |
| 99 | + """#290's `_index.md` defect produced a path that 302s to an HTML page. |
| 100 | + Anything asserting only on HTTP status would have passed it.""" |
| 101 | + dist = build(tmp_path, {"docs/p/index.html": "x"}, ["docs/other/index.html"]) |
| 102 | + assert classify(dist / "docs/other/index.html") == "html" |
| 103 | + |
| 104 | + |
| 105 | +def test_a_directory_target_is_a_failure(tmp_path): |
| 106 | + dist = build(tmp_path, {"docs/p/index.html": "x"}, ["docs/_static/a/b.png"]) |
| 107 | + assert classify(dist / "docs/_static/a") == "directory" |
| 108 | + |
| 109 | + |
| 110 | +def test_a_real_file_passes(tmp_path): |
| 111 | + dist = build(tmp_path, {"docs/p/index.html": "x"}, ["docs/_static/x.png"]) |
| 112 | + assert classify(dist / "docs/_static/x.png") == "ok" |
| 113 | + |
| 114 | + |
| 115 | +def test_a_missing_file_fails(tmp_path): |
| 116 | + dist = build(tmp_path, {"docs/p/index.html": "x"}) |
| 117 | + assert classify(dist / "docs/_static/nope.png") == "missing" |
| 118 | + |
| 119 | + |
| 120 | +# --- page_url_dir --------------------------------------------------------- |
| 121 | + |
| 122 | +def test_pretty_url_base_is_the_pages_own_directory(tmp_path): |
| 123 | + dist = build(tmp_path, {"docs/v2/union/guide/index.html": "x"}) |
| 124 | + assert page_url_dir(dist / "docs/v2/union/guide/index.html", dist) == "/docs/v2/union/guide" |
| 125 | + |
| 126 | + |
| 127 | +# --- src extraction ------------------------------------------------------- |
| 128 | + |
| 129 | +def test_src_is_found_regardless_of_attribute_order_and_quoting(): |
| 130 | + assert IMG_RE.findall('<img alt="a" src="/x.png" width="2">') == ["/x.png"] |
| 131 | + assert IMG_RE.findall("<img src='/y.png'>") == ["/y.png"] |
| 132 | + assert IMG_RE.findall('<IMG SRC = "/z.png">') == ["/z.png"] |
| 133 | + assert IMG_RE.findall('<image src="/no.png">') == [] |
| 134 | + |
| 135 | + |
| 136 | +def test_srcset_alone_is_not_mistaken_for_src(): |
| 137 | + """`srcset` is a different attribute with a different grammar; matching it |
| 138 | + as `src` would report phantom failures on every responsive image.""" |
| 139 | + assert IMG_RE.findall('<img srcset="/a.png 1x, /b.png 2x" src="/a.png">') == ["/a.png"] |
| 140 | + |
| 141 | + |
| 142 | +# --- end to end ----------------------------------------------------------- |
| 143 | + |
| 144 | +def run(dist): |
| 145 | + return subprocess.run([sys.executable, str(TOOL), "--dist", str(dist)], |
| 146 | + capture_output=True, text=True) |
| 147 | + |
| 148 | + |
| 149 | +def test_clean_site_exits_zero(tmp_path): |
| 150 | + dist = build(tmp_path, |
| 151 | + {"docs/v2/union/p/index.html": "/docs/v2/union/p/../_static/x.png"}, |
| 152 | + ["docs/v2/union/_static/x.png"]) |
| 153 | + r = run(dist) |
| 154 | + assert r.returncode == 0, r.stdout |
| 155 | + assert "OK" in r.stdout |
| 156 | + |
| 157 | + |
| 158 | +def test_the_290_index_defect_fails(tmp_path): |
| 159 | + """One `../` too many on an _index.md page.""" |
| 160 | + dist = build(tmp_path, |
| 161 | + {"docs/v2/union/api-reference/index.html": |
| 162 | + "/docs/v2/union/api-reference/../../_static/x.png"}, |
| 163 | + ["docs/v2/union/_static/x.png"]) |
| 164 | + r = run(dist) |
| 165 | + assert r.returncode == 1, r.stdout |
| 166 | + assert "no such file in the build" in r.stdout |
| 167 | + |
| 168 | + |
| 169 | +def test_the_pre_290_leaf_defect_fails(tmp_path): |
| 170 | + """One `../` too few on a leaf page -- the bug #290 set out to fix.""" |
| 171 | + dist = build(tmp_path, |
| 172 | + {"docs/v2/union/api-reference/page/index.html": "../_static/x.png"}, |
| 173 | + ["docs/v2/union/_static/x.png"]) |
| 174 | + r = run(dist) |
| 175 | + assert r.returncode == 1, r.stdout |
| 176 | + |
| 177 | + |
| 178 | +def test_external_and_data_srcs_are_not_checked(tmp_path): |
| 179 | + dist = build(tmp_path, {"docs/a/index.html": "https://example.com/x.png", |
| 180 | + "docs/b/index.html": "data:image/gif;base64,R0lGOD", |
| 181 | + "docs/c/index.html": "//cdn.example.com/x.png"}) |
| 182 | + r = run(dist) |
| 183 | + assert r.returncode == 0, r.stdout |
| 184 | + assert "3 external" in r.stdout |
| 185 | + |
| 186 | + |
| 187 | +def test_a_missing_dist_exits_two_rather_than_passing(tmp_path): |
| 188 | + """The failure mode a CI gate must not have: no build, so nothing to check, |
| 189 | + so it 'passes'.""" |
| 190 | + r = run(tmp_path / "nope") |
| 191 | + assert r.returncode == 2 |
| 192 | + assert "make dist" in r.stderr |
0 commit comments