diff --git a/tests/test_manage_show_screenshots.py b/tests/test_manage_show_screenshots.py new file mode 100644 index 0000000000..2fa3a3ad9f --- /dev/null +++ b/tests/test_manage_show_screenshots.py @@ -0,0 +1,160 @@ +"""`manage.py show` names every picture a switch's row carries, not only the first. + +docs/switches.md lets one row show more than one picture - "A row may show more than one picture - `vpn_pill` +carries the pill up and the pill at rest - and every picture in a row is held to those rules" - and +tests/test_switches.py reads them all, which is why the page can make that promise. tools/manage.py's +switches() kept one path per key (`re.search` stops at the first ``), so `show vpn_pill` printed +`docs/switches/vpn_pill.jpg` and never `vpn_pill_rest.jpg`, the picture the page shows beside it: a reader +who opened the one path the tool gave them saw half of what the page documents for the key they were +looking up. Nothing read what show printed, so the suite stayed green. + +The reader here is the tool's own `img_srcs()`, held to the page's rule in both directions: an empty src +keeps its place as an empty string (a picture the page draws but names nothing for), and a picture whose src +is there is never dropped. The tree test then holds every picture the shipped page gives a key to the item +`show` prints, so a key that gains a second picture - or a reader that stops reading one - fails here. + +No TV: this is the offline half of tools/manage.py, over the checkout's own docs/switches.md. + +Run from the repository root: python3 -m unittest discover -s tests -v +""" +import contextlib +import io +import re +import sys +import tempfile +import unittest +from pathlib import Path + +ROOT = Path(__file__).resolve().parent.parent +sys.path.insert(0, str(ROOT / "tools")) +import manage # noqa: E402 + + +def pictures(item): + """The files `show` prints for one switch, in the order it prints them.""" + return [Path(p).name for p in item.get("screenshots") or []] + + +class ImgSrcsTest(unittest.TestCase): + def test_it_reads_every_picture_of_a_row_in_the_order_the_page_draws_them(self): + cell = (' ' + '') + self.assertEqual(["switches/vpn_pill.jpg", "switches/vpn_pill_rest.jpg"], manage.img_srcs(cell)) + + def test_one_picture_reads_as_one(self): + self.assertEqual(["switches/dock_band.jpg"], manage.img_srcs('')) + + def test_an_img_without_a_src_keeps_its_place(self): + cell = ' no source' + self.assertEqual(["switches/dock_band.jpg", ""], manage.img_srcs(cell)) + + def test_a_cell_with_no_picture_reads_as_none(self): + self.assertEqual([], manage.img_srcs("no picture here")) + self.assertEqual([], manage.img_srcs("")) + + +class StandInTreeTest(unittest.TestCase): + """A tree of its own, so a page rewritten in another shape cannot pass by reading nothing.""" + + KEYS = ( + "# Themes\n\n## Keys\n\n| Key | Type | Meaning |\n|---|---|---|\n" + "| `two_pictures` | bool | A switch the page shows on both sides (false by default) |\n" + "| `one_picture` | bool | A switch with a single picture (false by default) |\n" + "| `no_picture` | bool | A key from before the screenshot rule (false by default) |\n" + ) + PAGE = ( + "# Switches on a TV\n\n## Screenshots\n\n| Key | Screenshot |\n|---|---|\n" + '| `two_pictures` | |\n' + '| `one_picture` | |\n' + ) + + def setUp(self): + self.tmp = tempfile.TemporaryDirectory() + self.root = Path(self.tmp.name) + (self.root / "themes").mkdir() + (self.root / "themes" / "README.md").write_text(self.KEYS, encoding="utf-8") + (self.root / "docs").mkdir() + (self.root / "docs" / "switches.md").write_text(self.PAGE, encoding="utf-8") + self.items = {s["id"]: s for s in manage.switches(self.root)} + + def tearDown(self): + self.tmp.cleanup() + + def test_the_reader_finds_the_tree(self): + self.assertEqual({"two_pictures", "one_picture", "no_picture"}, set(self.items), + "the stand-in tree no longer reads as three switches") + + def test_a_row_with_two_pictures_keeps_both(self): + self.assertEqual(["two.jpg", "two_rest.jpg"], pictures(self.items["two_pictures"])) + + def test_the_first_picture_is_still_the_single_screenshot(self): + # Everything else that reads an item still expects one path there. + self.assertEqual("two.jpg", Path(self.items["two_pictures"]["screenshot"]).name) + self.assertEqual(["one.jpg"], pictures(self.items["one_picture"])) + + def test_a_key_with_no_row_carries_no_picture(self): + self.assertEqual([], pictures(self.items["no_picture"])) + self.assertIsNone(self.items["no_picture"]["screenshot"]) + + def test_show_prints_both_pictures_of_a_switch(self): + from unittest import mock + out = io.StringIO() + with mock.patch.object(manage, "find", return_value=self.items["two_pictures"]), \ + contextlib.redirect_stdout(out): + self.assertEqual(0, manage.cmd_show("two_pictures")) + printed = [line.split(":", 1)[1].strip() for line in out.getvalue().splitlines() + if line.strip().startswith("screenshot")] + self.assertEqual(["two.jpg", "two_rest.jpg"], [Path(p).name for p in printed], + "show printed one picture of a row that carries two") + + +class ShippedPageTest(unittest.TestCase): + """The page the checkout ships: every picture it gives a key reaches that key's item, and show prints it.""" + + def setUp(self): + self.items = {s["id"]: s for s in manage.switches()} + self.page = (ROOT / "docs" / "switches.md").read_text(encoding="utf-8") + + def test_every_picture_in_the_page_reaches_a_key_and_is_a_file(self): + pairs = [] + for line in self.page.splitlines(): + if not line.startswith("|"): + continue + cells = [c.strip() for c in line.strip().strip("|").split("|")] + if len(cells) < 2: + continue + keys = re.findall(r"`([a-z_]+)`", cells[0]) + srcs = [s for s in manage.img_srcs(cells[1]) if s] + for key in keys: + for src in srcs: + pairs.append((key, src)) + self.assertGreaterEqual(len(pairs), 45, "the page's screenshot table moved: is the reader still right?") + for key, src in pairs: + with self.subTest(key=key, picture=src): + self.assertIn(key, self.items, f"docs/switches.md shows a picture for `{key}`, which the Keys table does not name") + self.assertIn((ROOT / "docs" / src).resolve(), [Path(p) for p in self.items[key]["screenshots"]], + f"`{key}` carries a picture in docs/switches.md that show does not report") + + def test_the_keys_with_two_pictures_are_read_with_both(self): + for key in ("vpn_pill", "icon_only_vpn"): + with self.subTest(key=key): + self.assertEqual(2, len(self.items[key]["screenshots"]), + f"`{key}` has two pictures in docs/switches.md; the reader lost one") + + def test_show_prints_the_picture_the_page_shows_at_rest(self): + out = io.StringIO() + with contextlib.redirect_stdout(out): + self.assertEqual(0, manage.cmd_show("vpn_pill")) + self.assertIn("vpn_pill_rest.jpg", out.getvalue(), + "show printed one of the two pictures docs/switches.md gives vpn_pill") + + def test_a_one_picture_key_prints_one_line(self): + out = io.StringIO() + with contextlib.redirect_stdout(out): + self.assertEqual(0, manage.cmd_show("columns")) + lines = [line for line in out.getvalue().splitlines() if line.strip().startswith("screenshot")] + self.assertEqual(1, len(lines), "a key with one picture printed more than one screenshot line") + + +if __name__ == "__main__": + unittest.main() diff --git a/tools/manage.py b/tools/manage.py index 1299507b6f..dc1dfea26b 100644 --- a/tools/manage.py +++ b/tools/manage.py @@ -11,7 +11,9 @@ TV_HOST= tools/venv/bin/python -I tools/manage.py remove TV_HOST= tools/venv/bin/python -I tools/manage.py set --theme -install on an installed add-on updates it (pm install -r). set writes the key into a theme folder of your own and +install on an installed add-on updates it (pm install -r). show prints every picture a switch's row carries in +docs/switches.md, since a few keys have two (the pill up and at rest for `vpn_pill`), and a reader who copies +only the first path would see half of what the page documents. set writes the key into a theme folder of your own and pushes it with the platform's theme script, as themes/README.md describes; it never edits a folder in this checkout, so pointing --theme at the project's themes/ or examples/ is refused before anything is written. set reads the value the way the key's own type in the Keys table needs - true or false for a bool, a JSON list, object or number where the @@ -84,6 +86,21 @@ def img_src(cell): return m.group(1) if m else None +def img_srcs(cell): + """Every in a cell, in the order the page draws them. + + docs/switches.md lets one row show more than one picture - `vpn_pill` carries the pill up and the pill at + rest - and tests/test_switches.py already reads every one of them. A reader that stops at the first `` reports half of what the page gives the key, which is what `show` did while the page grew a second + picture per row. A picture with no src keeps its place as an empty string, as that page's own reader has it. + """ + out = [] + for tag in re.findall(r"]*>", cell or ""): + m = re.search(r'\bsrc="([^"]*)"', tag) + out.append(m.group(1) if m else "") + return out + + def plain(cell): """A Markdown cell as text: links to their words, code marks dropped.""" s = re.sub(r"\[([^\]]+)\]\([^)]*\)", r"\1", cell or "") @@ -149,15 +166,17 @@ def switches(root=ROOT): sw = root / "docs" / "switches.md" if sw.is_file(): for c in table_rows(sw.read_text(encoding="utf-8"), "| Key |"): - src = img_src(c[1]) if len(c) > 1 else None - if src: + srcs = [str((sw.parent / s).resolve()) for s in img_srcs(c[1]) if s] if len(c) > 1 else [] + if srcs: for k in re.findall(r"`([a-z_]+)`", c[0]): - shots[k] = str((sw.parent / src).resolve()) + shots[k] = srcs out = [] for c in table_rows(keys_part, "| Key |"): for k in re.findall(r"`([a-z_]+)`", c[0]): + pictures = shots.get(k, []) out.append({"kind": "switch", "name": k, "id": k, "type": plain(c[1]) if len(c) > 1 else "", - "what": plain(c[2]) if len(c) > 2 else "", "screenshot": shots.get(k)}) + "what": plain(c[2]) if len(c) > 2 else "", + "screenshot": pictures[0] if pictures else None, "screenshots": pictures}) return out @@ -541,9 +560,14 @@ def cmd_show(name): item = find(name) if not item: sys.exit(f"no add-on, theme or switch called {name}") - for k in ("kind", "name", "what", "type", "author", "tested", "permissions", "package", "screenshot"): + for k in ("kind", "name", "what", "type", "author", "tested", "permissions", "package"): if item.get(k): print(f"{k:>12}: {item[k]}") + # A switch's row in docs/switches.md may carry more than one picture (the state on each side of the + # switch); print every one, or a reader copying a path gets half of what the page shows. + pictures = item.get("screenshots") or ([item["screenshot"]] if item.get("screenshot") else []) + for picture in pictures: + print(f"{'screenshot':>12}: {picture}") if item["kind"] == "add-on": how = item["script"] or "the steps in addons/README.md" if sys.platform == "win32" and item["script"]: