From 38986628c45d294f464b23fcf9f8dc50afff05a6 Mon Sep 17 00:00:00 2001 From: "Peter B. Johnson" Date: Sun, 20 Sep 2026 23:05:30 +0100 Subject: [PATCH 1/2] implement: Test the draft workflow end to end against convert (t49) --- tests/conftest.py | 54 +++ .../against_convert/PartsOneSol/spec.yaml | 6 + .../against_convert/PartsSepSol/commands.json | 61 ++++ tests/fixtures/against_convert/README.md | 64 ++++ .../against_convert/crlf/commands.json | 51 +++ .../against_convert/docx/commands.json | 51 +++ tests/test_against_convert.py | 333 ++++++++++++++++++ tests/test_exports.py | 38 +- 8 files changed, 627 insertions(+), 31 deletions(-) create mode 100644 tests/fixtures/against_convert/PartsOneSol/spec.yaml create mode 100644 tests/fixtures/against_convert/PartsSepSol/commands.json create mode 100644 tests/fixtures/against_convert/README.md create mode 100644 tests/fixtures/against_convert/crlf/commands.json create mode 100644 tests/fixtures/against_convert/docx/commands.json create mode 100644 tests/test_against_convert.py diff --git a/tests/conftest.py b/tests/conftest.py index a882ab0..1496b29 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -4,6 +4,7 @@ import shutil from collections.abc import Iterator from pathlib import Path +from typing import Any import pytest @@ -47,6 +48,59 @@ """Every spec folder, so that covering another kind of document is a folder and no code.""" +AGAINST_CONVERT_DIR = Path(__file__).parent / "fixtures" / "against_convert" +"""One document per folder, beside the spec or commands that take it down both routes.""" + +AGAINST_CONVERT = sorted( + path for path in AGAINST_CONVERT_DIR.iterdir() if path.is_dir() +) +"""Every folder of the above, so that covering another document is a folder and no code.""" + + +def key_paths(value: Any, path: str = "") -> set[str]: + """Every key of a JSON value, at every depth, as ``.parts[0].workedSolution``. + + Args: + value: A question, a set, or any part of one, as JSON reads it. + path: What to write in front of each key, for a value taken out of another. + + Returns: + One path per key, list items numbered, so that two files can be compared by the + shape they hold rather than by what they say. + """ + if isinstance(value, dict): + paths = set() + for key, item in value.items(): + paths |= {f"{path}.{key}"} | key_paths(item, f"{path}.{key}") + return paths + if isinstance(value, list): + return set().union( + *(key_paths(item, f"{path}[{i}]") for i, item in enumerate(value)) + ) + return set() + + +def unexported_keys(written: dict, exported: set[str]) -> list[str]: + """The keys a written question or set holds that Lambda Feedback never exports there. + + Args: + written: A question or a set as in2lambda wrote it, as JSON reads it. + exported: The key paths real exports hold, as :func:`key_paths` reads them off + one export or off all of them at once. + + Returns: + The paths of the written file that are in none of them, in order. + """ + missing = key_paths(written) - exported + # Lambda Feedback leaves a part's workedSolution out of its export when the part + # has none, but the writer always emits one, so only then may it be absent. + for i, part in enumerate(written.get("parts", [])): + if not part["workedSolution"]["content"]: + prefix = f".parts[{i}].workedSolution" + missing = {key for key in missing if not key.startswith(prefix)} + return sorted(missing) + + def frozen_sources(folder: Path) -> list[str]: """The documents a draft or spec folder freezes, in the order they are its sources. diff --git a/tests/fixtures/against_convert/PartsOneSol/spec.yaml b/tests/fixtures/against_convert/PartsOneSol/spec.yaml new file mode 100644 index 0000000..6b5394b --- /dev/null +++ b/tests/fixtures/against_convert/PartsOneSol/spec.yaml @@ -0,0 +1,6 @@ +question: Para +part: ListItem +solution: Div +strip: ['(?m)^::: \{\.solution\}\n', '(?m)^:::$'] +ignore: Header +layout: PartsOneSol diff --git a/tests/fixtures/against_convert/PartsSepSol/commands.json b/tests/fixtures/against_convert/PartsSepSol/commands.json new file mode 100644 index 0000000..d32156e --- /dev/null +++ b/tests/fixtures/against_convert/PartsSepSol/commands.json @@ -0,0 +1,61 @@ +[ + { + "args": { + "block": "b1" + }, + "by": "tests", + "command": "mark ignore" + }, + { + "args": { + "text": "s3:4" + }, + "by": "tests", + "command": "question add" + }, + { + "args": { + "question": "q1", + "text": "s5:9" + }, + "by": "tests", + "command": "part add" + }, + { + "args": { + "question": "q1", + "text": "s10:11" + }, + "by": "tests", + "command": "part add" + }, + { + "args": { + "at": 14, + "block": "b3" + }, + "by": "tests", + "command": "split block" + }, + { + "args": { + "text": "b3a" + }, + "by": "tests", + "command": "question add" + }, + { + "args": { + "block": "b3b" + }, + "by": "tests", + "command": "mark ignore" + }, + { + "args": { + "text": "b4" + }, + "by": "tests", + "command": "question add" + } +] diff --git a/tests/fixtures/against_convert/README.md b/tests/fixtures/against_convert/README.md new file mode 100644 index 0000000..f46b1f9 --- /dev/null +++ b/tests/fixtures/against_convert/README.md @@ -0,0 +1,64 @@ +# One document down both routes + +Each folder here takes one document through the draft workflow - `source add`, a spec or a +run of commands, `validate`, `build` - and compares the set in the zip with the set +`in2lambda convert` makes of the same document. The two routes are otherwise tested +against fixtures of their own, and neither test says whether they agree. + +A folder named after a filter is about the `example.tex` that filter ships, converted with +that filter. Any other folder is named after a folder of `fixtures/sources`, and is +converted with `PartsOneSol`. A folder holding a `spec.yaml` fills the draft in by running +that spec; a folder holding a `commands.json` applies each command in it in order. To +cover another document, add a folder: no test names any of them. + +The line ranges in a `commands.json` are lines of the markdown pandoc writes, not of the +document itself, as the ranges in `fixtures/sources` are. They were produced with +**pandoc 3.9.0.2**. + +## What the comparison ignores + +The comparison is of each question's main text and of each part's text. Four differences +between the routes are not differences in what a question says, and are taken off both +sides before comparing: + +- **Line breaks.** The draft quotes the lines pandoc wrapped; convert writes a paragraph + on one line. Every run of whitespace is compared as one space. +- **Image references.** Convert writes every image as `![pictureTag](path)`; the draft keeps + the alt text the document wrote, which is empty for `\includegraphics`. The set read back + from the zip names each file as it sits in the export's `media/`, where the set convert + returns holds the path the document wrote. Both sides are compared by the file's name. +- **A lone empty part.** A question the draft writes without parts exports as one part with + nothing in it, because Lambda Feedback's template fills a question holding no part with + placeholder wording. Convert writes no part at all. A single part with no text is + dropped from both. +- **Typographic quotes.** Pandoc's LaTeX reader writes `’` where its `commonmark_x` writer + writes `'`, so `aren’t` reaches convert and `aren't` reaches the draft. Both are folded + to the ASCII quotes. + +Worked solutions are not compared. The `PartsOneSol` filter matches a solution environment +only when the Div's first element stringifies to `Solution`, which pandoc never writes, so +convert drops every worked solution in that layout (t52). `PartsSepSol`'s example, and the +three source documents, write no solution at all. `test_the_draft_holds_the_solutions_convert_drops` +asserts on the draft side that the two solution environments of `PartsOneSol/example.tex` +reach `q1.solution` and `q2.solution`. + +## Which layouts are here + +`PartsOneSol` and `PartsSepSol`. The `PartPartSolSol` and `PartSolPartSol` examples nest +each question's parts and their solutions inside one top-level list item, which is one +block of the frozen source: no selector reaches inside it (t51), and no draft command +writes a `qN.pM.solution` (t53). Add a folder for each of those layouts when both land. + +## What does not run yet + +`docx` is not compared. `in2lambda.filters.markdown.image_directories` opens the document +as UTF-8 text to look for a `\graphicspath`, so `in2lambda convert` raises a +`UnicodeDecodeError` over any .docx that holds an image. The comparison reports that +folder as an expected failure naming the cause, and compares the two routes as soon as +convert reads the document. The folder's draft is still built, exported and replayed. + +`PartsOneSol/spec.yaml` quotes the solution environment holding `$$1+1 = 2$$`, which +`source add` freezes on one line and `in2lambda validate` reports as an error (t44), so +`build` refuses the draft. The test probes for that rather than naming the folder: it +freezes a source of display maths, and skips a folder whose report holds the same finding. +The folder runs unchanged once t44 writes display maths in block form. diff --git a/tests/fixtures/against_convert/crlf/commands.json b/tests/fixtures/against_convert/crlf/commands.json new file mode 100644 index 0000000..17d5cde --- /dev/null +++ b/tests/fixtures/against_convert/crlf/commands.json @@ -0,0 +1,51 @@ +[ + { + "args": { + "block": "b1" + }, + "by": "tests", + "command": "mark ignore" + }, + { + "args": { + "text": "b2" + }, + "by": "tests", + "command": "question add" + }, + { + "args": { + "block": "b3" + }, + "by": "tests", + "command": "mark ignore" + }, + { + "args": { + "block": "b4" + }, + "by": "tests", + "command": "mark ignore" + }, + { + "args": { + "block": "b5" + }, + "by": "tests", + "command": "mark ignore" + }, + { + "args": { + "block": "b6" + }, + "by": "tests", + "command": "mark ignore" + }, + { + "args": { + "block": "b7" + }, + "by": "tests", + "command": "mark ignore" + } +] diff --git a/tests/fixtures/against_convert/docx/commands.json b/tests/fixtures/against_convert/docx/commands.json new file mode 100644 index 0000000..17d5cde --- /dev/null +++ b/tests/fixtures/against_convert/docx/commands.json @@ -0,0 +1,51 @@ +[ + { + "args": { + "block": "b1" + }, + "by": "tests", + "command": "mark ignore" + }, + { + "args": { + "text": "b2" + }, + "by": "tests", + "command": "question add" + }, + { + "args": { + "block": "b3" + }, + "by": "tests", + "command": "mark ignore" + }, + { + "args": { + "block": "b4" + }, + "by": "tests", + "command": "mark ignore" + }, + { + "args": { + "block": "b5" + }, + "by": "tests", + "command": "mark ignore" + }, + { + "args": { + "block": "b6" + }, + "by": "tests", + "command": "mark ignore" + }, + { + "args": { + "block": "b7" + }, + "by": "tests", + "command": "mark ignore" + } +] diff --git a/tests/test_against_convert.py b/tests/test_against_convert.py new file mode 100644 index 0000000..31f96c4 --- /dev/null +++ b/tests/test_against_convert.py @@ -0,0 +1,333 @@ +"""Takes one document down both routes and compares what each makes of it. + +`in2lambda convert` reads a document with a filter and writes a set. The draft workflow +freezes the same document, fills a draft's fields in from it, checks the draft over and +builds the set from the fields. Each folder in ``fixtures/against_convert`` is one +document put through both, so that a difference between the two is a test failure rather +than something a first real run finds. The folder's README says which documents are here, +which are not, and what the comparison leaves out. + +The zip a draft builds is also compared with the real exports in ``fixtures/exports``, and +each draft is replayed from its log, so that one run covers what the route writes as well +as what it says. +""" + +import json +import shutil +import tempfile +import warnings +from functools import cache +from itertools import zip_longest +from pathlib import Path + +import pytest +from click.testing import CliRunner +from conftest import ( + AGAINST_CONVERT, + AGAINST_CONVERT_DIR, + EXPORTS, + SOURCES_DIR, + key_paths, + needs_compiler, + unexported_keys, +) + +import in2lambda.draft +import in2lambda.draft.report +import in2lambda.source +from in2lambda.api.question import Question +from in2lambda.api.set import Set +from in2lambda.filters import builtin_filters +from in2lambda.json_convert.json_convert import _IMAGE +from in2lambda.main import cli, runner +from in2lambda.validation import _location +from in2lambda.validation.delimiters import MathDelimiterError + +# Both routes compile the set and render its maths, which is what the two are being +# compared over, so a machine without the toolchain runs none of this. +pytestmark = needs_compiler + +each_folder = pytest.mark.parametrize( + "folder", AGAINST_CONVERT, ids=lambda path: path.name +) + +PARTS_ONE_SOL = AGAINST_CONVERT_DIR / "PartsOneSol" +"""The one folder whose document writes a worked solution, which its filter drops.""" + +_INLINE_DISPLAY = MathDelimiterError.MISSING_NEWLINE_AFTER_OPENING_DISPLAY.value +"""What the checks report over ``$$x$$`` on one line, which is what t44 is about.""" + +_QUOTES = str.maketrans({"\u2018": "'", "\u2019": "'", "\u201c": '"', "\u201d": '"'}) +"""Pandoc's LaTeX reader writes typographic quotes; its commonmark_x writer writes ASCII.""" + + +@cache +def _display_maths_frozen_inline() -> bool: + """Whether `source add` writes display maths on one line, which `build` refuses. + + Pandoc's commonmark_x writer puts ``$$x = y$$`` on one line, and + `in2lambda.validation.delimiters` reports that as an error, so a draft quoting a + block of display maths cannot be built until t44 writes it in block form. This is + asked of a document here rather than answered by naming the folders it applies to, + so that those folders run as they stand once t44 is in. + """ + with tempfile.TemporaryDirectory() as directory: + probe = Path(directory) / "probe.tex" + probe.write_text( + "\\documentclass{article}\n\\begin{document}\n\\[ x = y \\]\n" + "\\end{document}\n" + ) + _, sources = in2lambda.source.frozen(in2lambda.source.add([str(probe)])) + return "$$x = y$$" in sources[0] + + +def _document(folder: Path, tmp_path: Path, filters_dir: str) -> tuple[Path, str]: + """Copies the document a folder is about into `tmp_path`, with the files beside it. + + A folder named after a filter is about the ``example.tex`` that filter ships, read + with that filter; the figures the example refers to are copied with it. Any other + folder is named after a folder of ``fixtures/sources``, and is read with PartsOneSol. + """ + if folder.name in builtin_filters(): + shutil.copytree(Path(filters_dir) / folder.name, tmp_path, dirs_exist_ok=True) + return tmp_path / "example.tex", folder.name + shutil.copytree(SOURCES_DIR / folder.name, tmp_path, dirs_exist_ok=True) + (source,) = tmp_path.glob("source.*") + return source, "PartsOneSol" + + +def _built(folder: Path, tmp_path: Path, filters_dir: str) -> tuple[Path, Path, str]: + """A folder's document in `tmp_path`, with its draft built, checked and exported. + + Returns: + The draft, the document it was frozen from, and the filter `in2lambda convert` + reads that document with. The set is in ``out/set`` beside them, and the zip + Lambda Feedback imports is ``out/set.zip``. + """ + source, layout = _document(folder, tmp_path, filters_dir) + cli_runner = CliRunner() + assert cli_runner.invoke(cli, ["source", "add", source.name]).exit_code == 0 + draft_path = tmp_path / f"{source.stem}.draft.json" + + if (spec := folder / "spec.yaml").is_file(): + shutil.copy(spec, tmp_path) + in2lambda.draft.execute( + in2lambda.draft.spec_command(spec.name, "tests", draft_path), draft_path + ) + else: + for entry in json.loads((folder / "commands.json").read_text()): + in2lambda.draft.execute(entry, draft_path) + + with warnings.catch_warnings(record=True) as said: + warnings.simplefilter("always") + report = in2lambda.draft.report.validate(draft_path) + if _display_maths_frozen_inline() and any( + _INLINE_DISPLAY in finding["message"] for finding in report + ): + pytest.skip("t44: display maths is frozen inline") + # The checks compile the set and render its maths, which is two of the stages this + # test is about, so a run that reported neither has not been over them. + missing = [ + str(warning.message) for warning in said if "install" in str(warning.message) + ] + assert not missing, missing + + result = cli_runner.invoke(cli, ["build"]) + assert result.exit_code == 0, result.output + return draft_path, source, layout + + +def _text(markdown: str) -> str: + """A field as both routes say it, with the three differences in wording taken off. + + An image reference is compared by the file's name: convert writes the alt text + ``pictureTag`` where the draft keeps the alt text the document wrote, and the + exported set names the file as it sits in ``media/`` where the set convert returns + still holds the path the document wrote. The draft also quotes the lines pandoc + wrapped where convert writes a paragraph on one line, and the two readers write + quotes differently. The folder's README says all of them. + """ + named = _IMAGE.sub(lambda reference: f"![]({Path(reference[1]).name})", markdown) + return " ".join(named.translate(_QUOTES).split()) + + +def _parts(question: Question) -> list[str]: + """Each part's text, dropping a lone part that has none. + + A question the draft writes without parts exports as one part holding nothing, + because Lambda Feedback's template fills a question holding no part with placeholder + wording. Convert writes no part at all. The empty part says nothing either way. + """ + texts = [_text(part.text) for part in question.parts] + return [] if texts == [""] else texts + + +def _same(drafted: Set, converted: Set) -> None: + """Raises unless both sets say the same thing, naming the first place they differ. + + Args: + drafted: The set built from a draft, as `Set.from_json` reads its zip. + converted: The set `in2lambda convert` made of the same document. + + Raises: + AssertionError: the two hold a different number of questions or parts, or one + question or part says something the other does not. The message names the + question, the part and the field as `in2lambda.validation` names them. + """ + questions = zip_longest(drafted.questions, converted.questions) + for number, (draft_question, convert_question) in enumerate(questions, start=1): + where = _location(number, "") + assert ( + draft_question is not None + ), f"{where}: convert wrote this question and the draft did not" + assert ( + convert_question is not None + ), f"{where}: the draft wrote this question and convert did not" + + drafted_text = _text(draft_question.main_text) + converted_text = _text(convert_question.main_text) + assert drafted_text == converted_text, ( + f"{_location(number, '', field='main text')}: the draft says " + f"{drafted_text!r} and convert says {converted_text!r}" + ) + + texts = zip_longest(_parts(draft_question), _parts(convert_question)) + for index, (draft_part, convert_part) in enumerate(texts): + part = _location(number, "", index, "text") + assert ( + draft_part is not None + ), f"{part}: convert wrote this part and the draft did not" + assert ( + convert_part is not None + ), f"{part}: the draft wrote this part and convert did not" + assert draft_part == convert_part, ( + f"{part}: the draft says {draft_part!r} and convert says " + f"{convert_part!r}" + ) + + +@each_folder +def test_the_draft_route_agrees_with_convert( + folder: Path, tmp_path: Path, filters_dir: str, monkeypatch +) -> None: + """Both routes make the same questions and parts of the same document.""" + monkeypatch.chdir(tmp_path) + _, source, layout = _built(folder, tmp_path, filters_dir) + + try: + converted = runner(str(source), layout) + except UnicodeDecodeError: + # `image_directories` opens the document as UTF-8 text to look for a + # \graphicspath, so convert raises this over every .docx holding an image. The + # draft route reads the same document, which is how this test found it. + pytest.xfail("convert reads the document as text to find \\graphicspath") + + _same(Set.from_json(str(tmp_path / "out" / "set.zip")), converted) + + +@each_folder +def test_the_written_keys_exist_in_a_real_export( + folder: Path, tmp_path: Path, filters_dir: str, monkeypatch +) -> None: + """A draft's zip holds no key, at any depth, that Lambda Feedback never exports. + + `test_exports` asks this of a set written back from the export it was read from. + A draft's set is written from its fields, and no export stands behind it, so every + key of it is looked for in the union of every export in ``fixtures/exports``. + """ + monkeypatch.chdir(tmp_path) + _built(folder, tmp_path, filters_dir) + + exported = set().union( + *( + key_paths(json.loads(file.read_text())) + for export_dir in EXPORTS + for file in export_dir.glob("*.json") + ) + ) + missing = {} + for file in (tmp_path / "out" / "set").glob("*.json"): + keys = unexported_keys(json.loads(file.read_text()), exported) + if keys: + missing[file.name] = keys + assert not missing, missing + + +@each_folder +def test_the_draft_replays( + folder: Path, tmp_path: Path, filters_dir: str, monkeypatch +) -> None: + """The log of a draft built and checked this way rebuilds it, byte for byte.""" + monkeypatch.chdir(tmp_path) + draft_path, _, _ = _built(folder, tmp_path, filters_dir) + written = draft_path.read_bytes() + + result = CliRunner().invoke(cli, ["draft", "replay"]) + + assert result.exit_code == 0, result.output + assert draft_path.read_bytes() == written + + +def test_the_draft_holds_the_solutions_convert_drops( + tmp_path: Path, filters_dir: str, monkeypatch +) -> None: + """The draft writes both solution environments that the PartsOneSol filter drops. + + That filter matches a Div only when its first element stringifies to ``Solution``, + which pandoc writes for no solution environment, so convert exports the layout's + examples with no worked solution at all. Fixing it is t52; this says what each route + does with the same two environments in the meantime. + """ + monkeypatch.chdir(tmp_path) + draft_path, source, layout = _built(PARTS_ONE_SOL, tmp_path, filters_dir) + + fields = json.loads(draft_path.read_text())["fields"] + assert "The solution is copied across all parts." in fields["q1.solution"]["value"] + assert fields["q2.solution"]["value"] == "And here's the solution" + assert not [ + part + for question in runner(str(source), layout).questions + for part in question.parts + if part.worked_solution + ] + + +def test_a_spec_that_swaps_part_and_solution_is_caught( + tmp_path: Path, monkeypatch +) -> None: + """A draft that says something else about a question fails, naming that question. + + The four folders agree, so a comparison that could not tell them from a draft built + wrongly would pass them as well. The spec here reads each solution as the part and + each part as the solution. + """ + monkeypatch.chdir(tmp_path) + source = tmp_path / "source.md" + source.write_text( + "# Hydraulics\n\n" + "Q1. Find the load the large piston carries.\n\n" + "1. State the pressure under the small piston.\n\n" + "::: {.solution}\n$F = pA$.\n:::\n" + ) + (tmp_path / "spec.yaml").write_text( + "question: Para\n" + "part: Div\n" + "solution: ListItem\n" + "strip: ['(?m)^::: \\{\\.solution\\}\\n', '(?m)^:::$']\n" + "ignore: Header\n" + "layout: PartsOneSol\n" + ) + cli_runner = CliRunner() + assert cli_runner.invoke(cli, ["source", "add", "source.md"]).exit_code == 0 + draft_path = tmp_path / "source.draft.json" + in2lambda.draft.execute( + in2lambda.draft.spec_command("spec.yaml", "tests", draft_path), draft_path + ) + in2lambda.draft.report.validate(draft_path) + assert cli_runner.invoke(cli, ["build"]).exit_code == 0 + + with pytest.raises(AssertionError, match='Question 1 "", part \\(a\\), text'): + _same( + Set.from_json(str(tmp_path / "out" / "set.zip")), + runner(str(source), "PartsOneSol"), + ) diff --git a/tests/test_exports.py b/tests/test_exports.py index 710bcdd..4104629 100644 --- a/tests/test_exports.py +++ b/tests/test_exports.py @@ -15,7 +15,7 @@ from pathlib import Path import pytest -from conftest import EXPORTS +from conftest import EXPORTS, key_paths, unexported_keys from in2lambda.api.part import Part from in2lambda.api.question import Question @@ -58,34 +58,6 @@ def _modelled(question_set: Set) -> dict: } -def _key_paths(value, path: str = "") -> set[str]: - if isinstance(value, dict): - paths = set() - for key, item in value.items(): - paths |= {f"{path}.{key}"} | _key_paths(item, f"{path}.{key}") - return paths - if isinstance(value, list): - return set().union( - *(_key_paths(item, f"{path}[{i}]") for i, item in enumerate(value)) - ) - return set() - - -def _unexported_keys(written: dict, exported: dict) -> list[str]: - # An export may list a part's areas out of order; the writer puts them in order, - # so compare each written area with the exported one of the same number. - for part in exported.get("parts", []): - part["responseAreas"].sort(key=lambda area: area["orderNumber"]) - missing = _key_paths(written) - _key_paths(exported) - # Lambda Feedback leaves a part's workedSolution out of its export when the part - # has none, but the writer always emits one, so only then may it be absent. - for i, part in enumerate(written.get("parts", [])): - if not part["workedSolution"]["content"]: - prefix = f".parts[{i}].workedSolution" - missing = {key for key in missing if not key.startswith(prefix)} - return sorted(missing) - - @each_export def test_export_round_trips(export_dir: Path, tmp_path: Path) -> None: """Writing a loaded export reproduces its file names and reloads to the same set.""" @@ -129,7 +101,11 @@ def test_written_keys_exist_in_export(export_dir: Path, tmp_path: Path) -> None: missing = {} for file in written.glob("*.json"): exported = json.loads((export_dir / file.name).read_text()) - keys = _unexported_keys(json.loads(file.read_text()), exported) + # An export may list a part's areas out of order; the writer puts them in + # order, so compare each written area with the exported one of the same number. + for part in exported.get("parts", []): + part["responseAreas"].sort(key=lambda area: area["orderNumber"]) + keys = unexported_keys(json.loads(file.read_text()), key_paths(exported)) if keys: missing[file.name] = keys assert not missing, missing @@ -308,7 +284,7 @@ def test_one_figure_used_by_two_questions_is_copied_once(tmp_path: Path) -> None def _area_shape(area: dict) -> frozenset[str]: # Without indices, an area's shape is the keys it has, not how many tests, cases # or symbols it lists. - return frozenset(re.sub(r"\[\d+\]", "[]", path) for path in _key_paths(area)) + return frozenset(re.sub(r"\[\d+\]", "[]", path) for path in key_paths(area)) def test_response_areas_built_in_python_write_as_exported(tmp_path: Path) -> None: From 05d008b075b3a73ee14d596a452f011b69cd8547 Mon Sep 17 00:00:00 2001 From: "Peter B. Johnson" Date: Sun, 20 Sep 2026 23:37:18 +0100 Subject: [PATCH 2/2] implement: Test the draft workflow end to end against convert (t49) --- CHANGELOG.md | 1 + in2lambda/filters/markdown.py | 17 +- .../PartPartSolSol/commands.json | 100 ++++++++ .../PartPartSolSol/differs.txt | 2 + .../PartSolPartSol/commands.json | 123 ++++++++++ .../PartSolPartSol/differs.txt | 2 + .../against_convert/PartsOneSol/differs.txt | 4 + .../against_convert/PartsSepSol/commands.json | 6 +- tests/fixtures/against_convert/README.md | 70 +++--- tests/test_against_convert.py | 223 ++++++++---------- tests/test_runner.py | 13 + 11 files changed, 394 insertions(+), 167 deletions(-) create mode 100644 tests/fixtures/against_convert/PartPartSolSol/commands.json create mode 100644 tests/fixtures/against_convert/PartPartSolSol/differs.txt create mode 100644 tests/fixtures/against_convert/PartSolPartSol/commands.json create mode 100644 tests/fixtures/against_convert/PartSolPartSol/differs.txt create mode 100644 tests/fixtures/against_convert/PartsOneSol/differs.txt diff --git a/CHANGELOG.md b/CHANGELOG.md index c7f635f..cdf366c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,4 +19,5 @@ - A draft can freeze more than one document, which is how a sheet written as a question file and a separate solutions file is drafted: `in2lambda source add questions.docx solutions.docx` freezes them as source 1 and source 2 of the one `questions.draft.json`, and `in2lambda source add solutions.docx --draft questions.docx` adds a file to a draft already written as its next source. Every block id and line range of a source after the first carries its number - `2/b3`, `2/s10:14`, with `1/b3` meaning the `b3` it always did - and a field quoted from one records which source it came from, so that the same line number in two documents is two different places. `in2lambda source show` prints each source under its number and its name. `in2lambda spec run` runs the spec over every source: the first is laid out as its `layout` says, and in any source after it the `question` selector picks out the marker written above each question's solutions while everything else the spec picks out is a solution, paired onto the questions and parts of the first the way `in2lambda convert -a` pairs an answers file. A draft now holds `sources`, a list of `{source, hash, blocks}` in the order they were frozen, rather than those three at the top level, so a draft written before this is refused as one nothing here wrote; `in2lambda source add --start-over` freezes the document again. In the Python API, `in2lambda.source.add` takes a list of files and the draft to freeze them into; `in2lambda.source.frozen` and `in2lambda.draft.apply` hand back and take the markdown of every source rather than of one; `in2lambda.spec.fields` takes one `(blocks, markdown)` pair per source in place of its `elements` and `markdown` arguments; and `in2lambda.spec.Field` carries the number of the source its ranges are lines of, which anything constructing one has to say. - Every command that works on a draft - `in2lambda source show`, each of the `in2lambda draft` commands, `in2lambda spec run`, `in2lambda validate`, `in2lambda build` and `in2lambda render` - takes `--draft`, naming either the draft or the source it was frozen from. Left off, it uses the one draft in the current directory, and where there is more than one it is refused naming them rather than acting on whichever sorts first. `in2lambda spec run` names its SPEC from the draft's directory. The Python functions behind them take the draft's path rather than a directory: `in2lambda.source.frozen`, `in2lambda.source.show`, `in2lambda.draft.execute`, `in2lambda.draft.replay`, `in2lambda.draft.spec_command`, `in2lambda.draft.report.validate`, `in2lambda.draft.export.build` and `in2lambda.draft.export.render`. `in2lambda.source.draft_of` says where a document's draft goes and `in2lambda.source.find` is what the command line resolves `--draft` with. - Importing `in2lambda.katex_convert` no longer writes a file called `log` into the working directory. What it has to say about a converted expression goes to the `in2lambda.katex_convert` logger, which is silent unless the application configures logging. +- `in2lambda convert` now reads a .docx that holds an image. Looking in the document for the directories a `\graphicspath` names read it as UTF-8 text, and a .docx is a zip, so converting any Word document with a figure in it raised `UnicodeDecodeError`. A document that cannot be read as text is now taken to name no directory, which is what a .docx does. - The Python API is unchanged: `in2lambda.main.runner` and everything under `in2lambda.api` take the same arguments and return the same objects. diff --git a/in2lambda/filters/markdown.py b/in2lambda/filters/markdown.py index 14bca99..2bf0e4f 100644 --- a/in2lambda/filters/markdown.py +++ b/in2lambda/filters/markdown.py @@ -48,12 +48,17 @@ def image_directories(tex_file: str) -> list[str]: >>> image_directories(tex_file) [] """ - with open(tex_file, "r") as file: - for line in file: - if "graphicspath" in line: - # Matches anything surrounded by curly braces, but excludes the top level - # graphicspath brace. - return [match.strip() for match in re.findall(r"{([^{]*?)}", line)] + try: + with open(tex_file, "r") as file: + for line in file: + if "graphicspath" in line: + # Matches anything surrounded by curly braces, but excludes the top level + # graphicspath brace. + return [match.strip() for match in re.findall(r"{([^{]*?)}", line)] + except UnicodeDecodeError: + # `graphicspath` is a TeX command, and a .docx is a zip rather than text: reading + # one as UTF-8 raises where there is nothing in it to find. + pass return [] diff --git a/tests/fixtures/against_convert/PartPartSolSol/commands.json b/tests/fixtures/against_convert/PartPartSolSol/commands.json new file mode 100644 index 0000000..55a0ab5 --- /dev/null +++ b/tests/fixtures/against_convert/PartPartSolSol/commands.json @@ -0,0 +1,100 @@ +[ + { + "args": { + "block": "b1" + }, + "by": "tests", + "command": "mark ignore" + }, + { + "args": { + "at": 9, + "block": "b2" + }, + "by": "tests", + "command": "split block" + }, + { + "args": { + "text": "s3:4" + }, + "by": "tests", + "command": "question add" + }, + { + "args": { + "question": "q1", + "text": "s5:6" + }, + "by": "tests", + "command": "part add" + }, + { + "args": { + "question": "q1", + "text": "s7:8" + }, + "by": "tests", + "command": "part add" + }, + { + "args": { + "block": "b2b" + }, + "by": "tests", + "command": "mark ignore" + }, + { + "args": { + "at": 17, + "block": "b3" + }, + "by": "tests", + "command": "split block" + }, + { + "args": { + "text": "b3a" + }, + "by": "tests", + "command": "question add" + }, + { + "args": { + "at": 18, + "block": "b3b" + }, + "by": "tests", + "command": "split block" + }, + { + "args": { + "block": "b3ba" + }, + "by": "tests", + "command": "mark ignore" + }, + { + "args": { + "at": 19, + "block": "b3bb" + }, + "by": "tests", + "command": "split block" + }, + { + "args": { + "question": "q2", + "text": "b3bba" + }, + "by": "tests", + "command": "question solution" + }, + { + "args": { + "block": "b3bbb" + }, + "by": "tests", + "command": "mark ignore" + } +] diff --git a/tests/fixtures/against_convert/PartPartSolSol/differs.txt b/tests/fixtures/against_convert/PartPartSolSol/differs.txt new file mode 100644 index 0000000..7b0c31a --- /dev/null +++ b/tests/fixtures/against_convert/PartPartSolSol/differs.txt @@ -0,0 +1,2 @@ +Question 1 "", part (a), worked solution: the draft says '' and convert says '$1+1=2$' # t53: no command writes qN.pM.solution +Question 1 "", part (b), worked solution: the draft says '' and convert says '$2+2=4$' # t53: no command writes qN.pM.solution diff --git a/tests/fixtures/against_convert/PartSolPartSol/commands.json b/tests/fixtures/against_convert/PartSolPartSol/commands.json new file mode 100644 index 0000000..e79f14f --- /dev/null +++ b/tests/fixtures/against_convert/PartSolPartSol/commands.json @@ -0,0 +1,123 @@ +[ + { + "args": { + "block": "b1" + }, + "by": "tests", + "command": "mark ignore" + }, + { + "args": { + "at": 7, + "block": "b2" + }, + "by": "tests", + "command": "split block" + }, + { + "args": { + "at": 11, + "block": "b2b" + }, + "by": "tests", + "command": "split block" + }, + { + "args": { + "at": 13, + "block": "b2bb" + }, + "by": "tests", + "command": "split block" + }, + { + "args": { + "text": "s3:4" + }, + "by": "tests", + "command": "question add" + }, + { + "args": { + "question": "q1", + "text": "s5:6" + }, + "by": "tests", + "command": "part add" + }, + { + "args": { + "block": "b2ba" + }, + "by": "tests", + "command": "mark ignore" + }, + { + "args": { + "question": "q1", + "text": "b2bba" + }, + "by": "tests", + "command": "part add" + }, + { + "args": { + "block": "b2bbb" + }, + "by": "tests", + "command": "mark ignore" + }, + { + "args": { + "at": 19, + "block": "b3" + }, + "by": "tests", + "command": "split block" + }, + { + "args": { + "text": "b3a" + }, + "by": "tests", + "command": "question add" + }, + { + "args": { + "at": 20, + "block": "b3b" + }, + "by": "tests", + "command": "split block" + }, + { + "args": { + "block": "b3ba" + }, + "by": "tests", + "command": "mark ignore" + }, + { + "args": { + "at": 21, + "block": "b3bb" + }, + "by": "tests", + "command": "split block" + }, + { + "args": { + "question": "q2", + "text": "b3bba" + }, + "by": "tests", + "command": "question solution" + }, + { + "args": { + "block": "b3bbb" + }, + "by": "tests", + "command": "mark ignore" + } +] diff --git a/tests/fixtures/against_convert/PartSolPartSol/differs.txt b/tests/fixtures/against_convert/PartSolPartSol/differs.txt new file mode 100644 index 0000000..7788313 --- /dev/null +++ b/tests/fixtures/against_convert/PartSolPartSol/differs.txt @@ -0,0 +1,2 @@ +Question 1 "", part (a), worked solution: the draft says '' and convert says '$1+1 = 2$' # t53: no command writes qN.pM.solution +Question 1 "", part (b), worked solution: the draft says '' and convert says '$2+2=4$' # t53: no command writes qN.pM.solution diff --git a/tests/fixtures/against_convert/PartsOneSol/differs.txt b/tests/fixtures/against_convert/PartsOneSol/differs.txt new file mode 100644 index 0000000..e9d9fd5 --- /dev/null +++ b/tests/fixtures/against_convert/PartsOneSol/differs.txt @@ -0,0 +1,4 @@ +Question 1 "", part (a), worked solution: the draft says 'This is the final answer. It contains the solutions for all parts without breaking them up. $$ 1+1 = 2 $$ The solution is copied across all parts.' and convert says '' # t52: the PartsOneSol filter drops every solution +Question 1 "", part (b), text: the draft says "The filter still works even if there aren't any parts" and convert says 'The filter still works even if there aren’t any parts' # smart quotes: pandoc's LaTeX reader writes ’ where its commonmark_x writer writes ' +Question 1 "", part (b), worked solution: the draft says 'This is the final answer. It contains the solutions for all parts without breaking them up. $$ 1+1 = 2 $$ The solution is copied across all parts.' and convert says '' # t52: the PartsOneSol filter drops every solution +Question 2 "", part (a): the draft wrote this part and convert did not # t52: the PartsOneSol filter drops every solution diff --git a/tests/fixtures/against_convert/PartsSepSol/commands.json b/tests/fixtures/against_convert/PartsSepSol/commands.json index d32156e..0a2c84c 100644 --- a/tests/fixtures/against_convert/PartsSepSol/commands.json +++ b/tests/fixtures/against_convert/PartsSepSol/commands.json @@ -16,7 +16,7 @@ { "args": { "question": "q1", - "text": "s5:9" + "text": "s5:8" }, "by": "tests", "command": "part add" @@ -24,14 +24,14 @@ { "args": { "question": "q1", - "text": "s10:11" + "text": "s9:10" }, "by": "tests", "command": "part add" }, { "args": { - "at": 14, + "at": 13, "block": "b3" }, "by": "tests", diff --git a/tests/fixtures/against_convert/README.md b/tests/fixtures/against_convert/README.md index f46b1f9..fa04f16 100644 --- a/tests/fixtures/against_convert/README.md +++ b/tests/fixtures/against_convert/README.md @@ -17,9 +17,9 @@ document itself, as the ranges in `fixtures/sources` are. They were produced wit ## What the comparison ignores -The comparison is of each question's main text and of each part's text. Four differences -between the routes are not differences in what a question says, and are taken off both -sides before comparing: +Each question's main text is compared, and each part's text and worked solution. Three +differences between the routes are not differences in what a question says, and are taken +off both sides before comparing: - **Line breaks.** The draft quotes the lines pandoc wrapped; convert writes a paragraph on one line. Every run of whitespace is compared as one space. @@ -27,38 +27,32 @@ sides before comparing: the alt text the document wrote, which is empty for `\includegraphics`. The set read back from the zip names each file as it sits in the export's `media/`, where the set convert returns holds the path the document wrote. Both sides are compared by the file's name. -- **A lone empty part.** A question the draft writes without parts exports as one part with - nothing in it, because Lambda Feedback's template fills a question holding no part with - placeholder wording. Convert writes no part at all. A single part with no text is - dropped from both. -- **Typographic quotes.** Pandoc's LaTeX reader writes `’` where its `commonmark_x` writer - writes `'`, so `aren’t` reaches convert and `aren't` reaches the draft. Both are folded - to the ASCII quotes. - -Worked solutions are not compared. The `PartsOneSol` filter matches a solution environment -only when the Div's first element stringifies to `Solution`, which pandoc never writes, so -convert drops every worked solution in that layout (t52). `PartsSepSol`'s example, and the -three source documents, write no solution at all. `test_the_draft_holds_the_solutions_convert_drops` -asserts on the draft side that the two solution environments of `PartsOneSol/example.tex` -reach `q1.solution` and `q2.solution`. - -## Which layouts are here - -`PartsOneSol` and `PartsSepSol`. The `PartPartSolSol` and `PartSolPartSol` examples nest -each question's parts and their solutions inside one top-level list item, which is one -block of the frozen source: no selector reaches inside it (t51), and no draft command -writes a `qN.pM.solution` (t53). Add a folder for each of those layouts when both land. - -## What does not run yet - -`docx` is not compared. `in2lambda.filters.markdown.image_directories` opens the document -as UTF-8 text to look for a `\graphicspath`, so `in2lambda convert` raises a -`UnicodeDecodeError` over any .docx that holds an image. The comparison reports that -folder as an expected failure naming the cause, and compares the two routes as soon as -convert reads the document. The folder's draft is still built, exported and replayed. - -`PartsOneSol/spec.yaml` quotes the solution environment holding `$$1+1 = 2$$`, which -`source add` freezes on one line and `in2lambda validate` reports as an error (t44), so -`build` refuses the draft. The test probes for that rather than naming the folder: it -freezes a source of display maths, and skips a folder whose report holds the same finding. -The folder runs unchanged once t44 writes display maths in block form. +- **A lone empty part.** A question the draft writes without parts or solution exports as + one part with nothing in it, because Lambda Feedback's template fills a question holding + no part with placeholder wording. Convert writes no part at all. A single part holding + neither text nor a worked solution is dropped from both. + +## Where the two routes differ today + +A folder holding a `differs.txt` is a document the two routes make different sets of. Each +line is one difference, worded as the comparison words it - the question, the part, the +field, and what each route says there - with the ticket that would close it after ` # `. +A folder with no such file is a document the two routes say the same thing about. + +Finding a difference the file does not list fails the test, and so does agreeing where it +lists one: closing a ticket below means deleting the lines it names. + +- **t52** - the `PartsOneSol` filter matches a solution environment only when the Div's + first element stringifies to `Solution`, which pandoc writes for no solution environment, + so convert drops every worked solution in that layout. The draft writes both solutions + of `PartsOneSol/example.tex`, and the second question's solution is a part of its own + that convert has nothing to match. +- **t53** - no draft command writes a `qN.pM.solution`, and no selector reaches inside the + top-level `\item` that `PartPartSolSol/example.tex` and `PartSolPartSol/example.tex` + nest each part's solution in, so those lines are marked ignore and the draft answers + neither part. `in2lambda.draft.export` already reads such a field, and + `fixtures/specs/part_part_sol_sol` shows a spec writing one where the source is not + nested. +- **Smart quotes** - pandoc's LaTeX reader writes `’` where its `commonmark_x` writer + writes `'`, so convert uploads `aren’t` for `PartsOneSol/example.tex` and the draft + uploads `aren't`. diff --git a/tests/test_against_convert.py b/tests/test_against_convert.py index 31f96c4..95371d4 100644 --- a/tests/test_against_convert.py +++ b/tests/test_against_convert.py @@ -4,8 +4,9 @@ freezes the same document, fills a draft's fields in from it, checks the draft over and builds the set from the fields. Each folder in ``fixtures/against_convert`` is one document put through both, so that a difference between the two is a test failure rather -than something a first real run finds. The folder's README says which documents are here, -which are not, and what the comparison leaves out. +than something a first real run finds. Where the two routes do differ today, the folder +holds a ``differs.txt`` naming each place and the ticket that would close it; the folder's +README says what the comparison folds out before looking. The zip a draft builds is also compared with the real exports in ``fixtures/exports``, and each draft is replayed from its log, so that one run covers what the route writes as well @@ -14,17 +15,15 @@ import json import shutil -import tempfile import warnings -from functools import cache from itertools import zip_longest from pathlib import Path +from typing import Any, Optional import pytest from click.testing import CliRunner from conftest import ( AGAINST_CONVERT, - AGAINST_CONVERT_DIR, EXPORTS, SOURCES_DIR, key_paths, @@ -34,14 +33,12 @@ import in2lambda.draft import in2lambda.draft.report -import in2lambda.source from in2lambda.api.question import Question from in2lambda.api.set import Set from in2lambda.filters import builtin_filters from in2lambda.json_convert.json_convert import _IMAGE from in2lambda.main import cli, runner from in2lambda.validation import _location -from in2lambda.validation.delimiters import MathDelimiterError # Both routes compile the set and render its maths, which is what the two are being # compared over, so a machine without the toolchain runs none of this. @@ -51,34 +48,8 @@ "folder", AGAINST_CONVERT, ids=lambda path: path.name ) -PARTS_ONE_SOL = AGAINST_CONVERT_DIR / "PartsOneSol" -"""The one folder whose document writes a worked solution, which its filter drops.""" - -_INLINE_DISPLAY = MathDelimiterError.MISSING_NEWLINE_AFTER_OPENING_DISPLAY.value -"""What the checks report over ``$$x$$`` on one line, which is what t44 is about.""" - -_QUOTES = str.maketrans({"\u2018": "'", "\u2019": "'", "\u201c": '"', "\u201d": '"'}) -"""Pandoc's LaTeX reader writes typographic quotes; its commonmark_x writer writes ASCII.""" - - -@cache -def _display_maths_frozen_inline() -> bool: - """Whether `source add` writes display maths on one line, which `build` refuses. - - Pandoc's commonmark_x writer puts ``$$x = y$$`` on one line, and - `in2lambda.validation.delimiters` reports that as an error, so a draft quoting a - block of display maths cannot be built until t44 writes it in block form. This is - asked of a document here rather than answered by naming the folders it applies to, - so that those folders run as they stand once t44 is in. - """ - with tempfile.TemporaryDirectory() as directory: - probe = Path(directory) / "probe.tex" - probe.write_text( - "\\documentclass{article}\n\\begin{document}\n\\[ x = y \\]\n" - "\\end{document}\n" - ) - _, sources = in2lambda.source.frozen(in2lambda.source.add([str(probe)])) - return "$$x = y$$" in sources[0] +_TICKET = " # " +"""What a line of a folder's ``differs.txt`` names the ticket closing it after.""" def _document(folder: Path, tmp_path: Path, filters_dir: str) -> tuple[Path, str]: @@ -120,11 +91,7 @@ def _built(folder: Path, tmp_path: Path, filters_dir: str) -> tuple[Path, Path, with warnings.catch_warnings(record=True) as said: warnings.simplefilter("always") - report = in2lambda.draft.report.validate(draft_path) - if _display_maths_frozen_inline() and any( - _INLINE_DISPLAY in finding["message"] for finding in report - ): - pytest.skip("t44: display maths is frozen inline") + in2lambda.draft.report.validate(draft_path) # The checks compile the set and render its maths, which is two of the stages this # test is about, so a run that reported neither has not been over them. missing = [ @@ -138,91 +105,130 @@ def _built(folder: Path, tmp_path: Path, filters_dir: str) -> tuple[Path, Path, def _text(markdown: str) -> str: - """A field as both routes say it, with the three differences in wording taken off. + """A field as both routes say it, with the two differences in wording taken off. An image reference is compared by the file's name: convert writes the alt text ``pictureTag`` where the draft keeps the alt text the document wrote, and the exported set names the file as it sits in ``media/`` where the set convert returns still holds the path the document wrote. The draft also quotes the lines pandoc - wrapped where convert writes a paragraph on one line, and the two readers write - quotes differently. The folder's README says all of them. + wrapped where convert writes a paragraph on one line. The folder's README says both. """ named = _IMAGE.sub(lambda reference: f"![]({Path(reference[1]).name})", markdown) - return " ".join(named.translate(_QUOTES).split()) + return " ".join(named.split()) -def _parts(question: Question) -> list[str]: - """Each part's text, dropping a lone part that has none. +def _parts(question: Question) -> list[tuple[str, str]]: + """Each part's text and worked solution, dropping a lone part holding neither. - A question the draft writes without parts exports as one part holding nothing, - because Lambda Feedback's template fills a question holding no part with placeholder - wording. Convert writes no part at all. The empty part says nothing either way. + A question the draft writes without parts or solution exports as one part holding + nothing, because Lambda Feedback's template fills a question holding no part with + placeholder wording. Convert writes no part at all. The empty part says nothing + either way. """ - texts = [_text(part.text) for part in question.parts] - return [] if texts == [""] else texts + parts = [(_text(part.text), _text(part.worked_solution)) for part in question.parts] + return [] if parts == [("", "")] else parts + + +def _only(drafted: Optional[Any], thing: str) -> str: + """Which of the two routes wrote a question or a part the other one did not.""" + if drafted is None: + return f"convert wrote this {thing} and the draft did not" + return f"the draft wrote this {thing} and convert did not" -def _same(drafted: Set, converted: Set) -> None: - """Raises unless both sets say the same thing, naming the first place they differ. +def _differing(where: str, drafted: str, converted: str) -> list[str]: + """The line naming a field the two routes write differently, or no line at all.""" + if drafted == converted: + return [] + return [f"{where}: the draft says {drafted!r} and convert says {converted!r}"] + + +def _differences(drafted: Set, converted: Set) -> list[str]: + """Every place the two sets say something different, in question and part order. Args: drafted: The set built from a draft, as `Set.from_json` reads its zip. converted: The set `in2lambda convert` made of the same document. - Raises: - AssertionError: the two hold a different number of questions or parts, or one - question or part says something the other does not. The message names the - question, the part and the field as `in2lambda.validation` names them. + Returns: + One line per difference, naming the question, the part and the field as + `in2lambda.validation` names them and quoting what each route says there. """ + found = [] questions = zip_longest(drafted.questions, converted.questions) for number, (draft_question, convert_question) in enumerate(questions, start=1): - where = _location(number, "") - assert ( - draft_question is not None - ), f"{where}: convert wrote this question and the draft did not" - assert ( - convert_question is not None - ), f"{where}: the draft wrote this question and convert did not" - - drafted_text = _text(draft_question.main_text) - converted_text = _text(convert_question.main_text) - assert drafted_text == converted_text, ( - f"{_location(number, '', field='main text')}: the draft says " - f"{drafted_text!r} and convert says {converted_text!r}" + if draft_question is None or convert_question is None: + found.append( + f"{_location(number, '')}: {_only(draft_question, 'question')}" + ) + continue + found += _differing( + _location(number, "", field="main text"), + _text(draft_question.main_text), + _text(convert_question.main_text), ) + parts = zip_longest(_parts(draft_question), _parts(convert_question)) + for index, (draft_part, convert_part) in enumerate(parts): + if draft_part is None or convert_part is None: + found.append( + f"{_location(number, '', index)}: {_only(draft_part, 'part')}" + ) + continue + for field, drafted_value, converted_value in zip( + ("text", "worked solution"), draft_part, convert_part + ): + found += _differing( + _location(number, "", index, field), drafted_value, converted_value + ) + return found + + +def _known(folder: Path) -> list[str]: + """The differences the two routes have today, as a folder's ``differs.txt`` has them. + + Each line is one difference as :func:`_differences` words it, with the ticket that + would close it written after `` # ``. A folder with no such file is a document the + two routes say the same thing about. + """ + path = folder / "differs.txt" + if not path.is_file(): + return [] + return [ + line.split(_TICKET)[0] for line in path.read_text().splitlines() if line.strip() + ] - texts = zip_longest(_parts(draft_question), _parts(convert_question)) - for index, (draft_part, convert_part) in enumerate(texts): - part = _location(number, "", index, "text") - assert ( - draft_part is not None - ), f"{part}: convert wrote this part and the draft did not" - assert ( - convert_part is not None - ), f"{part}: the draft wrote this part and convert did not" - assert draft_part == convert_part, ( - f"{part}: the draft says {draft_part!r} and convert says " - f"{convert_part!r}" - ) + +def _same(drafted: Set, converted: Set, known: list[str]) -> None: + """Raises unless the two sets differ in exactly the places `known` names. + + Args: + drafted: The set built from a draft, as `Set.from_json` reads its zip. + converted: The set `in2lambda convert` made of the same document. + known: The differences the two routes are known to have, as :func:`_known` + reads a folder's ``differs.txt``. + + Raises: + AssertionError: the two differ somewhere `known` does not name, or agree + somewhere it does. The message names the question, the part and the field + of every difference, so that a line of ``differs.txt`` can be written from + it or found and deleted. + """ + assert _differences(drafted, converted) == known @each_folder def test_the_draft_route_agrees_with_convert( folder: Path, tmp_path: Path, filters_dir: str, monkeypatch ) -> None: - """Both routes make the same questions and parts of the same document.""" + """Both routes make the same set of the same document, bar the folder's differs.txt.""" monkeypatch.chdir(tmp_path) _, source, layout = _built(folder, tmp_path, filters_dir) - try: - converted = runner(str(source), layout) - except UnicodeDecodeError: - # `image_directories` opens the document as UTF-8 text to look for a - # \graphicspath, so convert raises this over every .docx holding an image. The - # draft route reads the same document, which is how this test found it. - pytest.xfail("convert reads the document as text to find \\graphicspath") - - _same(Set.from_json(str(tmp_path / "out" / "set.zip")), converted) + _same( + Set.from_json(str(tmp_path / "out" / "set.zip")), + runner(str(source), layout), + _known(folder), + ) @each_folder @@ -268,38 +274,14 @@ def test_the_draft_replays( assert draft_path.read_bytes() == written -def test_the_draft_holds_the_solutions_convert_drops( - tmp_path: Path, filters_dir: str, monkeypatch -) -> None: - """The draft writes both solution environments that the PartsOneSol filter drops. - - That filter matches a Div only when its first element stringifies to ``Solution``, - which pandoc writes for no solution environment, so convert exports the layout's - examples with no worked solution at all. Fixing it is t52; this says what each route - does with the same two environments in the meantime. - """ - monkeypatch.chdir(tmp_path) - draft_path, source, layout = _built(PARTS_ONE_SOL, tmp_path, filters_dir) - - fields = json.loads(draft_path.read_text())["fields"] - assert "The solution is copied across all parts." in fields["q1.solution"]["value"] - assert fields["q2.solution"]["value"] == "And here's the solution" - assert not [ - part - for question in runner(str(source), layout).questions - for part in question.parts - if part.worked_solution - ] - - def test_a_spec_that_swaps_part_and_solution_is_caught( tmp_path: Path, monkeypatch ) -> None: """A draft that says something else about a question fails, naming that question. - The four folders agree, so a comparison that could not tell them from a draft built - wrongly would pass them as well. The spec here reads each solution as the part and - each part as the solution. + Every folder agrees with convert save where its ``differs.txt`` says, so a + comparison that could not tell them from a draft built wrongly would pass them as + well. The spec here reads each solution as the part and each part as the solution. """ monkeypatch.chdir(tmp_path) source = tmp_path / "source.md" @@ -330,4 +312,5 @@ def test_a_spec_that_swaps_part_and_solution_is_caught( _same( Set.from_json(str(tmp_path / "out" / "set.zip")), runner(str(source), "PartsOneSol"), + [], ) diff --git a/tests/test_runner.py b/tests/test_runner.py index a7d30f0..c301899 100644 --- a/tests/test_runner.py +++ b/tests/test_runner.py @@ -11,6 +11,7 @@ import pytest from click.testing import CliRunner +from conftest import SOURCES_DIR from in2lambda.api.set import Set from in2lambda.filters import builtin_filters @@ -66,6 +67,18 @@ def test_runner_writes_importable_json( assert (set_dir / "media" / reference).is_file(), reference +def test_runner_converts_a_docx_holding_an_image() -> None: + r"""A .docx is a zip, and looking in it for a `\graphicspath` reads it as text. + + The image is what reaches that code: `image_path` looks for the file beside the + document, does not find it, and then reads the document for the directories a + `\graphicspath` names. Every .docx with a figure in it went through there. + """ + result = runner(str(SOURCES_DIR / "docx" / "source.docx"), "PartsOneSol") + + assert result.questions + + def test_cli_reports_problems_and_exports_anyway(tmp_path) -> None: """A problem is printed, and is a warning rather than a refusal to export.""" question_file = tmp_path / "questions.tex"