From fa901fae41552115fa41d0f7b4cf9d6573c63f4f Mon Sep 17 00:00:00 2001 From: "Peter B. Johnson" Date: Mon, 21 Sep 2026 09:45:29 +0100 Subject: [PATCH 1/3] implement: Compare two sets as a library function and a command (t55) --- CHANGELOG.md | 1 + in2lambda/compare.py | 156 +++++++++++++++++++++++ in2lambda/main.py | 54 ++++++++ tests/fixtures/against_convert/README.md | 3 + tests/test_against_convert.py | 139 ++------------------ tests/test_cli.py | 1 + tests/test_compare.py | 140 ++++++++++++++++++++ 7 files changed, 365 insertions(+), 129 deletions(-) create mode 100644 in2lambda/compare.py create mode 100644 tests/test_compare.py diff --git a/CHANGELOG.md b/CHANGELOG.md index c379426..3651034 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -25,4 +25,5 @@ - `in2lambda convert FILE PartsOneSol` now exports the worked solution a document writes in a `solution` environment. Pandoc writes that environment as a Div whose classes hold `solution`, and the filter recognised only a Div whose first block reads `Solution`, so a document using the environment exported every question with an empty worked solution. A Div whose first block reads `Solution` is still recognised. - 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. +- `in2lambda compare BUILT_ZIP EXPORT_DIR` compares two Lambda Feedback sets, each given as a folder or a zip, so that a set in2lambda wrote can be checked against the export it should reproduce. Each question's main text is compared, and each part's text and worked solution, and every difference is printed naming the question, the part and the field. Three differences in wording are taken off both sides first: a run of whitespace is compared as one space, an image is compared by the file's name, and a part holding neither text nor a worked solution is dropped where it is the question's only part. `--known FILE` names the differences the two sets are known to have, one line per difference with the ticket that would close it written after ` # `, and `in2lambda compare` exits 1 where the differences found are not the differences that file names. `in2lambda.compare.differences` and `in2lambda.compare.known` are the two functions behind the command. - 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/compare.py b/in2lambda/compare.py new file mode 100644 index 0000000..2520e17 --- /dev/null +++ b/in2lambda/compare.py @@ -0,0 +1,156 @@ +"""Compares two sets question by question, naming every place they say something else. + +`in2lambda convert` writes a set, `in2lambda build` writes a set from a draft, and Lambda +Feedback exports a set. :func:`differences` compares any two of them in question and part +order - each question's main text, and each part's text and worked solution - and returns +one line per difference, naming the question, the part and the field as +`in2lambda.validation` names them. + +Three differences in wording are not differences in what a question says, and are taken +off both sides before comparing: + +- **Whitespace.** Every run of whitespace is compared as one space, because a draft + quotes the lines pandoc wrapped where `in2lambda convert` writes a paragraph on one + line. +- **Image references.** An image is compared by the file's name, because + `in2lambda convert` writes every image as ``![pictureTag](path)`` where a draft keeps + the alt text the document wrote, and an export names each file as ``media/`` holds it + where the set `in2lambda convert` returns holds the path the document wrote. +- **A lone empty part.** A single part holding neither text nor a worked solution is + dropped from both sides, because a question written without parts or solution exports + as one part holding nothing, where `in2lambda convert` writes no part at all. + +:func:`known` reads the differences two sets are known to have from a file: one line per +difference as :func:`differences` words it, with the ticket that would close it written +after `` # ``. +""" + +from itertools import zip_longest +from pathlib import Path +from typing import Any, Optional + +from in2lambda.api.question import Question +from in2lambda.api.set import Set +from in2lambda.json_convert.json_convert import _IMAGE +from in2lambda.validation import _location + +_TICKET = " # " +"""What a line of a differs.txt names the ticket closing it after.""" + + +def _text(markdown: str) -> str: + """A field with the differences in wording that are not differences taken off. + + Every run of whitespace becomes one space, and every image reference is written as + the file's name alone. The module docstring says why. + """ + 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.""" + parts = [(_text(part.text), _text(part.worked_solution)) for part in question.parts] + return [] if parts == [("", "")] else parts + + +def _only(left: Optional[Any], thing: str, left_name: str, right_name: str) -> str: + """Which of the two sets holds a question or a part the other one does not.""" + if left is None: + return f"{right_name} wrote this {thing} and {left_name} did not" + return f"{left_name} wrote this {thing} and {right_name} did not" + + +def _differing( + where: str, left: str, right: str, left_name: str, right_name: str +) -> list[str]: + """The line naming a field the two sets write differently, or no line at all.""" + if left == right: + return [] + return [f"{where}: {left_name} says {left!r} and {right_name} says {right!r}"] + + +def differences( + built: Set, + expected: Set, + left_name: str = "the draft", + right_name: str = "convert", +) -> list[str]: + """Every place the two sets say something different, in question and part order. + + Args: + built: The set being checked, such as the one `in2lambda build` wrote. + expected: The set it should reproduce, such as a Lambda Feedback export. + left_name: What to call `built` in each line. + right_name: What to call `expected` in each line. + + Returns: + One line per difference, naming the question, the part and the field as + `in2lambda.validation` names them and quoting what each set says there. + + Examples: + >>> from in2lambda.api.set import Set + >>> from in2lambda.compare import differences + >>> built, expected = Set(), Set() + >>> built.add_question(main_text="The rocket is at\\n45 degrees.") + >>> expected.add_question(main_text="The rocket is at 45 degrees.") + >>> differences(built, expected) + [] + >>> expected.add_question(main_text="Find the impulse.") + >>> differences(built, expected) + ['Question 2 "": convert wrote this question and the draft did not'] + """ + found = [] + questions = zip_longest(built.questions, expected.questions) + for number, (built_question, expected_question) in enumerate(questions, start=1): + if built_question is None or expected_question is None: + found.append( + f"{_location(number, '')}: " + f"{_only(built_question, 'question', left_name, right_name)}" + ) + continue + found += _differing( + _location(number, "", field="main text"), + _text(built_question.main_text), + _text(expected_question.main_text), + left_name, + right_name, + ) + parts = zip_longest(_parts(built_question), _parts(expected_question)) + for index, (built_part, expected_part) in enumerate(parts): + if built_part is None or expected_part is None: + found.append( + f"{_location(number, '', index)}: " + f"{_only(built_part, 'part', left_name, right_name)}" + ) + continue + for field, built_value, expected_value in zip( + ("text", "worked solution"), built_part, expected_part + ): + found += _differing( + _location(number, "", index, field), + built_value, + expected_value, + left_name, + right_name, + ) + return found + + +def known(path: str | Path) -> list[str]: + """The differences two sets are known to have, as a differs.txt file holds them. + + Args: + path: The file to read. A file that does not exist names no difference, so that + a folder of fixtures holds one only where the two sets differ. + + Returns: + Each line as :func:`differences` words it, with the ticket written after `` # `` + taken off and blank lines dropped. + """ + path = Path(path) + if not path.is_file(): + return [] + return [ + line.split(_TICKET)[0] for line in path.read_text().splitlines() if line.strip() + ] diff --git a/in2lambda/main.py b/in2lambda/main.py index d235c9c..cc88165 100644 --- a/in2lambda/main.py +++ b/in2lambda/main.py @@ -11,6 +11,7 @@ import rich_click as click +import in2lambda.compare import in2lambda.draft import in2lambda.draft.export import in2lambda.draft.report @@ -606,5 +607,58 @@ def render(output_dir: str, draft: Optional[str]) -> None: click.echo(f"Wrote {pdf}") +@cli.command("compare") +@click.argument("built_zip", type=click.Path(exists=True)) +@click.argument("export_dir", type=click.Path(exists=True)) +@click.option( + "--known", + "known_path", + type=click.Path(exists=True, dir_okay=False), + help="File naming the differences the two sets are known to have, one per line.", +) +def compare(built_zip: str, export_dir: str, known_path: Optional[str]) -> None: + """Compares the set in BUILT_ZIP with the set in EXPORT_DIR, and prints each difference. + + Each argument is a Lambda Feedback set, as a folder or as a zip. Each question's main + text is compared, and each part's text and worked solution, and every difference is + printed naming the question, the part and the field. Three differences in wording are + taken off both sides first: a run of whitespace is compared as one space, an image is + compared by the file's name, and a lone empty part is dropped. in2lambda.compare says + why. --known names a file of the differences the two sets are known to have, one per + line as this command prints it, with a ticket written after " # ". in2lambda compare + exits 1 where the differences found are not the differences --known names. + """ + with _message_not_traceback(): + found = in2lambda.compare.differences( + Set.from_json(built_zip), + Set.from_json(export_dir), + left_name=built_zip, + right_name=export_dir, + ) + for line in found: + click.echo(line) + + expected = in2lambda.compare.known(known_path) if known_path else [] + if found == expected: + if not found: + click.echo("Identical.") + return + # Echoed rather than put in the message, because a difference is a long line and + # the message is printed in a box that wraps it. + for line in expected: + if line not in found: + click.echo(f"Not found: {line}") + if not known_path: + raise click.ClickException( + "The two sets differ in the places printed above. Pass --known FILE to " + "name the differences the two sets are known to have." + ) + raise click.ClickException( + f"The differences printed above are not the differences {known_path} names. " + f"Write one line of {known_path} per difference found, with the ticket that " + 'would close it after " # ".' + ) + + if __name__ == "__main__": cli() diff --git a/tests/fixtures/against_convert/README.md b/tests/fixtures/against_convert/README.md index 184b986..a202bba 100644 --- a/tests/fixtures/against_convert/README.md +++ b/tests/fixtures/against_convert/README.md @@ -23,6 +23,9 @@ document itself, as the ranges in `fixtures/sources` are. They were produced wit ## What the comparison ignores +The comparison is `in2lambda.compare.differences`, whose docstring states these rules, so +that this README and the code do not drift apart. + 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: diff --git a/tests/test_against_convert.py b/tests/test_against_convert.py index 95371d4..c393a69 100644 --- a/tests/test_against_convert.py +++ b/tests/test_against_convert.py @@ -16,9 +16,7 @@ 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 @@ -33,12 +31,10 @@ import in2lambda.draft import in2lambda.draft.report -from in2lambda.api.question import Question from in2lambda.api.set import Set +from in2lambda.compare import differences, known 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. @@ -48,9 +44,6 @@ "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. @@ -104,118 +97,6 @@ def _built(folder: Path, tmp_path: Path, filters_dir: str) -> tuple[Path, Path, 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 @@ -224,11 +105,9 @@ def test_the_draft_route_agrees_with_convert( 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), - ) + assert differences( + Set.from_json(str(tmp_path / "out" / "set.zip")), runner(str(source), layout) + ) == known(folder / "differs.txt") @each_folder @@ -309,8 +188,10 @@ def test_a_spec_that_swaps_part_and_solution_is_caught( 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"), - [], + assert ( + differences( + Set.from_json(str(tmp_path / "out" / "set.zip")), + runner(str(source), "PartsOneSol"), + ) + == [] ) diff --git a/tests/test_cli.py b/tests/test_cli.py index fe706f6..d1cf9dc 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -129,6 +129,7 @@ def test_completing_the_old_form_offers_the_subcommand() -> None: assert [candidate.value for candidate in completions] == [ "build", + "compare", "convert", "draft", "render", diff --git a/tests/test_compare.py b/tests/test_compare.py new file mode 100644 index 0000000..73c7343 --- /dev/null +++ b/tests/test_compare.py @@ -0,0 +1,140 @@ +"""What `in2lambda.compare` folds out before comparing, and what it reports. + +`test_against_convert` compares two real sets with these functions. The sets here are +built in the test, one field apart, so that each normalisation is covered on its own. +""" + +from pathlib import Path + +from click.testing import CliRunner +from conftest import EXPORTS_DIR + +from in2lambda.api.part import Part +from in2lambda.api.set import Set +from in2lambda.compare import differences, known +from in2lambda.main import cli + +_EXPORT = str(EXPORTS_DIR / "me2_introduction") +"""A real export, compared with itself by the command-line tests.""" + + +def _set(main_text: str = "", *parts: tuple[str, str]) -> Set: + """One question holding `main_text` and a part per (text, worked solution) pair.""" + question_set = Set() + question_set.add_question(main_text=main_text) + for text, worked_solution in parts: + question_set.current_question.parts.append( + Part(text=text, worked_solution=worked_solution) + ) + return question_set + + +def test_a_run_of_whitespace_is_one_space() -> None: + """A field wrapped over two lines says what the same field on one line says.""" + assert ( + differences(_set("The piston\nis large."), _set("The piston is large.")) == [] + ) + + +def test_an_image_is_compared_by_the_file_name() -> None: + """The alt text and the directory differ between the routes; the file name does not.""" + assert ( + differences(_set("![fig](figures/a.png)"), _set("![pictureTag](a.png)")) == [] + ) + + assert differences(_set("![fig](figures/a.png)"), _set("![pictureTag](b.png)")) == [ + "Question 1 \"\", main text: the draft says '![](a.png)' and convert says " + "'![](b.png)'" + ] + + +def test_a_lone_empty_part_is_dropped() -> None: + """A question exported as one empty part matches a question written with no part.""" + assert differences(_set("Find the load.", ("", "")), _set("Find the load.")) == [] + + assert differences( + _set("Find the load.", ("", ""), ("", "")), _set("Find the load.") + ) == [ + 'Question 1 "", part (a): the draft wrote this part and convert did not', + 'Question 1 "", part (b): the draft wrote this part and convert did not', + ] + + +def test_a_difference_names_the_question_the_part_and_the_field() -> None: + """Each line quotes what both sets say, under the names the arguments give.""" + built = _set("Find the load.", ("State the pressure.", "$F = pA$")) + expected = _set("Find the load.", ("State the pressure.", "$F = 2pA$")) + + assert differences(built, expected) == [ + "Question 1 \"\", part (a), worked solution: the draft says '$F = pA$' and " + "convert says '$F = 2pA$'" + ] + assert differences(built, expected, "set.zip", "the export") == [ + "Question 1 \"\", part (a), worked solution: set.zip says '$F = pA$' and " + "the export says '$F = 2pA$'" + ] + + +def test_known_reads_the_lines_without_their_tickets(tmp_path: Path) -> None: + """A differs.txt names the differences two sets have; a missing file names none.""" + path = tmp_path / "differs.txt" + path.write_text( + "Question 1 \"\", main text: the draft says 'a' and convert says 'b' # t12\n" + "\n" + 'Question 2 "": convert wrote this question and the draft did not # t13\n' + ) + + assert known(path) == [ + "Question 1 \"\", main text: the draft says 'a' and convert says 'b'", + 'Question 2 "": convert wrote this question and the draft did not', + ] + assert known(tmp_path / "no_such_file.txt") == [] + + +def test_a_known_difference_is_the_difference_found(tmp_path: Path) -> None: + """The one difference between two sets is the line a differs.txt holds for them.""" + path = tmp_path / "differs.txt" + path.write_text( + "Question 1 \"\", main text: the draft says 'Find the load.' and convert says " + "'Find the force.' # t99\n" + ) + + assert differences(_set("Find the load."), _set("Find the force.")) == known(path) + + +def test_the_command_reports_a_set_compared_with_itself_as_identical() -> None: + """`in2lambda compare` on one export twice finds nothing to report, and exits 0.""" + result = CliRunner().invoke(cli, ["compare", _EXPORT, _EXPORT]) + + assert result.exit_code == 0, result.output + assert "Identical." in result.output + + +def test_the_command_refuses_a_known_difference_that_is_not_there( + tmp_path: Path, +) -> None: + """A line of the --known file that the comparison does not find is a failure.""" + path = tmp_path / "differs.txt" + path.write_text("Question 1 \"\", main text: the export says 'a' # t99\n") + + result = CliRunner().invoke( + cli, ["compare", _EXPORT, _EXPORT, "--known", str(path)] + ) + + assert result.exit_code == 1 + assert "Not found: Question 1 \"\", main text: the export says 'a'" in result.output + + +def test_the_command_prints_each_difference_between_two_sets(tmp_path: Path) -> None: + """Two sets that differ are reported line by line, and the command exits 1.""" + built = tmp_path / "built" + _set("Find the load.").to_json(str(built)) + expected = tmp_path / "expected" + _set("Find the force.").to_json(str(expected)) + + result = CliRunner().invoke( + cli, ["compare", str(built / "set"), str(expected / "set")] + ) + + assert result.exit_code == 1 + assert "Find the load." in result.output and "Find the force." in result.output From 842b1f55b5eb1aefcc7b846e2efcf147b0510f09 Mon Sep 17 00:00:00 2001 From: "Peter B. Johnson" Date: Mon, 21 Sep 2026 10:09:32 +0100 Subject: [PATCH 2/3] implement: Compare two sets as a library function and a command (t55) --- in2lambda/main.py | 19 +++++++++++++++++-- tests/test_compare.py | 20 ++++++++++++++++++++ 2 files changed, 37 insertions(+), 2 deletions(-) diff --git a/in2lambda/main.py b/in2lambda/main.py index cc88165..a4a3a29 100644 --- a/in2lambda/main.py +++ b/in2lambda/main.py @@ -607,6 +607,21 @@ def render(output_dir: str, draft: Optional[str]) -> None: click.echo(f"Wrote {pdf}") +def _set_at(path: str) -> Set: + """The set at `path`, or a message naming `path` where it holds no set. + + `Set.from_json` raises `ValueError` where a folder or a zip holds no ``set_*.json``, + and `compare` reads two paths, so the message names which of the two is at fault. + """ + try: + return Set.from_json(path) + except ValueError: + raise click.ClickException( + f"{path} is not a Lambda Feedback set. A set is a folder or a zip holding " + "one set_*.json file beside a question_*.json file per question." + ) from None + + @cli.command("compare") @click.argument("built_zip", type=click.Path(exists=True)) @click.argument("export_dir", type=click.Path(exists=True)) @@ -630,8 +645,8 @@ def compare(built_zip: str, export_dir: str, known_path: Optional[str]) -> None: """ with _message_not_traceback(): found = in2lambda.compare.differences( - Set.from_json(built_zip), - Set.from_json(export_dir), + _set_at(built_zip), + _set_at(export_dir), left_name=built_zip, right_name=export_dir, ) diff --git a/tests/test_compare.py b/tests/test_compare.py index 73c7343..756dfab 100644 --- a/tests/test_compare.py +++ b/tests/test_compare.py @@ -138,3 +138,23 @@ def test_the_command_prints_each_difference_between_two_sets(tmp_path: Path) -> assert result.exit_code == 1 assert "Find the load." in result.output and "Find the force." in result.output + + +def test_the_command_refuses_a_path_that_is_not_a_set( + tmp_path: Path, monkeypatch +) -> None: + """A file, and a folder holding no set_*.json, are named in a message. + + `Set.from_json` raises `ValueError` for either, and the command names which of the + two paths it read is not a set instead of printing that traceback. + """ + monkeypatch.setenv("COLUMNS", "200") # So the message is not wrapped mid-sentence. + not_a_set = tmp_path / "README.md" + not_a_set.write_text("A document, not an export.\n") + + for path in (str(not_a_set), str(tmp_path)): + result = CliRunner().invoke(cli, ["compare", _EXPORT, path]) + + assert result.exit_code == 1 + assert not isinstance(result.exception, ValueError), result.output + assert f"{path} is not a Lambda Feedback set" in result.output From 9caea4dafb48e99403c3416f65bfe9d2bf4c9f3c Mon Sep 17 00:00:00 2001 From: "Peter B. Johnson" Date: Mon, 21 Sep 2026 10:33:22 +0100 Subject: [PATCH 3/3] implement: Compare two sets as a library function and a command (t55) --- in2lambda/compare.py | 4 ++-- in2lambda/main.py | 6 ++++-- tests/test_compare.py | 16 +++++++++++----- 3 files changed, 17 insertions(+), 9 deletions(-) diff --git a/in2lambda/compare.py b/in2lambda/compare.py index 2520e17..439ce30 100644 --- a/in2lambda/compare.py +++ b/in2lambda/compare.py @@ -76,7 +76,7 @@ def differences( left_name: str = "the draft", right_name: str = "convert", ) -> list[str]: - """Every place the two sets say something different, in question and part order. + r"""Every place the two sets say something different, in question and part order. Args: built: The set being checked, such as the one `in2lambda build` wrote. @@ -92,7 +92,7 @@ def differences( >>> from in2lambda.api.set import Set >>> from in2lambda.compare import differences >>> built, expected = Set(), Set() - >>> built.add_question(main_text="The rocket is at\\n45 degrees.") + >>> built.add_question(main_text="The rocket is at\n45 degrees.") >>> expected.add_question(main_text="The rocket is at 45 degrees.") >>> differences(built, expected) [] diff --git a/in2lambda/main.py b/in2lambda/main.py index a4a3a29..af77230 100644 --- a/in2lambda/main.py +++ b/in2lambda/main.py @@ -5,6 +5,7 @@ import os import shlex import warnings +import zipfile from collections.abc import Callable # Rather than typing's, which beartype warns on. from contextlib import contextmanager from typing import Any, Optional @@ -611,11 +612,12 @@ def _set_at(path: str) -> Set: """The set at `path`, or a message naming `path` where it holds no set. `Set.from_json` raises `ValueError` where a folder or a zip holds no ``set_*.json``, - and `compare` reads two paths, so the message names which of the two is at fault. + and `zipfile.BadZipFile` where a path named ``.zip`` is not a zip at all. `compare` + reads two paths, so the message names which of the two is at fault. """ try: return Set.from_json(path) - except ValueError: + except (ValueError, zipfile.BadZipFile): raise click.ClickException( f"{path} is not a Lambda Feedback set. A set is a folder or a zip holding " "one set_*.json file beside a question_*.json file per question." diff --git a/tests/test_compare.py b/tests/test_compare.py index 756dfab..aaf8dbb 100644 --- a/tests/test_compare.py +++ b/tests/test_compare.py @@ -4,6 +4,7 @@ built in the test, one field apart, so that each normalisation is covered on its own. """ +import zipfile from pathlib import Path from click.testing import CliRunner @@ -143,18 +144,23 @@ def test_the_command_prints_each_difference_between_two_sets(tmp_path: Path) -> def test_the_command_refuses_a_path_that_is_not_a_set( tmp_path: Path, monkeypatch ) -> None: - """A file, and a folder holding no set_*.json, are named in a message. + """A file, a folder holding no set_*.json, and a .zip that is not a zip. - `Set.from_json` raises `ValueError` for either, and the command names which of the - two paths it read is not a set instead of printing that traceback. + `Set.from_json` raises `ValueError` for the first two and `zipfile.BadZipFile` for + the third, and the command names which of the two paths it read is not a set + instead of printing either traceback. """ monkeypatch.setenv("COLUMNS", "200") # So the message is not wrapped mid-sentence. not_a_set = tmp_path / "README.md" not_a_set.write_text("A document, not an export.\n") + not_a_zip = tmp_path / "set.zip" + not_a_zip.write_text("A document named as a zip.\n") - for path in (str(not_a_set), str(tmp_path)): + for path in (str(not_a_set), str(tmp_path), str(not_a_zip)): result = CliRunner().invoke(cli, ["compare", _EXPORT, path]) assert result.exit_code == 1 - assert not isinstance(result.exception, ValueError), result.output + assert not isinstance( + result.exception, (ValueError, zipfile.BadZipFile) + ), result.output assert f"{path} is not a Lambda Feedback set" in result.output