diff --git a/CHANGELOG.md b/CHANGELOG.md index 61a8864..67b86b3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -21,4 +21,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 an existing draft as its next source. Every block id and line range of a source after the first carries that source's number — `2/b3`, `2/s10:14` — and `1/b3` names the block `b3` names. A field quoted from a source records which source it came from, so that the same line number in two documents is two places. `in2lambda source show` prints each source under its number and its name. `in2lambda spec run` runs the spec over every source: the first source is laid out as the spec's `layout` says, and in any source after it the `question` selector picks out the marker written above each question's solutions while every other match is a solution, paired onto the questions and parts of the first source as `in2lambda convert -a` pairs an answers file. A draft now holds `sources`, a list of `{source, hash, blocks}` in the order they were frozen, in place of those three keys at the top level, so a draft written before this release is refused as a draft in2lambda did not write; `in2lambda source add --start-over` freezes the document again. Four changes to the Python API break existing scripts: `in2lambda.source.add` takes a list of files and the draft to freeze them into; `in2lambda.source.frozen` returns the markdown of every source and `in2lambda.draft.apply` takes the markdown of every source, in place 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 every caller constructing a `Field` must pass. - Every command that works on a draft takes `--draft`, naming either the draft or the source it was frozen from: `in2lambda source show`, each `in2lambda draft` command, `in2lambda spec run`, `in2lambda validate`, `in2lambda build` and `in2lambda render`. Left off, each command uses the one draft in the current directory, and where the directory holds more than one draft, the command is refused, naming them. `in2lambda spec run` resolves its SPEC from the draft's directory. The Python functions behind those commands take the draft's path in place of 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` returns the path of a document's draft, and `in2lambda.source.find` resolves `--draft` for the command line. - Importing `in2lambda.katex_convert` no longer writes a file called `log` into the working directory. That module reports what it changed in an expression to the `in2lambda.katex_convert` logger, which is silent unless the application configures logging. +- `in2lambda convert` now reads a .docx that holds an image. in2lambda looks in the document for the directories a `\graphicspath` names, and read the document as UTF-8 text to find them. A .docx is a zip file, so converting a Word document holding a figure raised `UnicodeDecodeError`. in2lambda now reads a document that is not UTF-8 text as naming no directory, which is what a .docx names. - The rest of 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/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/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/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..0a2c84c --- /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:8" + }, + "by": "tests", + "command": "part add" + }, + { + "args": { + "question": "q1", + "text": "s9:10" + }, + "by": "tests", + "command": "part add" + }, + { + "args": { + "at": 13, + "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..fa04f16 --- /dev/null +++ b/tests/fixtures/against_convert/README.md @@ -0,0 +1,58 @@ +# 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 + +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. +- **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 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/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..95371d4 --- /dev/null +++ b/tests/test_against_convert.py @@ -0,0 +1,316 @@ +"""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. 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 +as what it says. +""" + +import json +import shutil +import warnings +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, + EXPORTS, + SOURCES_DIR, + key_paths, + needs_compiler, + unexported_keys, +) + +import in2lambda.draft +import in2lambda.draft.report +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 + +# 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 +) + +_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]: + """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") + 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 = [ + 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 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. The folder's README says both. + """ + named = _IMAGE.sub(lambda reference: f"![]({Path(reference[1]).name})", markdown) + return " ".join(named.split()) + + +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 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. + """ + 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 _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. + + 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): + 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() + ] + + +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 set of the same document, bar the folder's differs.txt.""" + monkeypatch.chdir(tmp_path) + _, source, layout = _built(folder, tmp_path, filters_dir) + + _same( + Set.from_json(str(tmp_path / "out" / "set.zip")), + runner(str(source), layout), + _known(folder), + ) + + +@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_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. + + 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" + 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: 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"