diff --git a/docs/source/quickstart.md b/docs/source/quickstart.md index 6087ba2..55455c5 100644 --- a/docs/source/quickstart.md +++ b/docs/source/quickstart.md @@ -73,6 +73,8 @@ $ in2lambda questions.tex -a solutions.tex PartsSepSol By default, this generates an `out` directory in the same place that the command was run in. It contains the zipped question files. +Before writing anything, in2lambda prints the problems it can detect that would stop the set importing or make it render wrongly — an answer that doesn't fit the box marking it, a figure the export won't contain, maths that KaTeX can't display. Each names the question, part and field to go and look at. They are warnings rather than errors: the `out` directory is written either way, since a problem found here may well be deliberate. + Check the [command line tool reference](reference/command-line) for more information. ## 3. Import into Lambda Feedback diff --git a/in2lambda/api/problem.py b/in2lambda/api/problem.py new file mode 100644 index 0000000..c930441 --- /dev/null +++ b/in2lambda/api/problem.py @@ -0,0 +1,24 @@ +"""Something wrong with a question set, and where in it to look.""" + +from dataclasses import dataclass + + +@dataclass(frozen=True) +class Problem: + """Something in2lambda can tell Lambda Feedback will refuse or render wrongly. + + ``location`` names the question, part and field to go and look at; ``message`` + says what is wrong with it. + + Examples: + >>> from in2lambda.api.problem import Problem + >>> print(Problem('Question 1 "Drag", part (a), answer box 1', "no option is marked correct")) + Question 1 "Drag", part (a), answer box 1: no option is marked correct + """ + + location: str + message: str + + def __str__(self) -> str: + """One line for the command line: ``: ``.""" + return f"{self.location}: {self.message}" diff --git a/in2lambda/api/set.py b/in2lambda/api/set.py index fbce164..7dc904e 100644 --- a/in2lambda/api/set.py +++ b/in2lambda/api/set.py @@ -3,6 +3,7 @@ from dataclasses import dataclass, field from typing import Any +from in2lambda.api.problem import Problem from in2lambda.api.question import Question from in2lambda.api.visibility_status import VisibilityController, VisibilityStatus @@ -102,6 +103,30 @@ def increment_current_question(self) -> None: """ self._current_question_index += 1 + def problems(self) -> list[Problem]: + r"""Everything in2lambda can tell Lambda Feedback would refuse or render wrongly. + + This is a report, not a refusal: the set can still be written out, since a + problem found here may well be deliberate. + + Returns: + One :class:`~in2lambda.api.problem.Problem` per problem found, each naming + the question, part and field to go and look at. + + Examples: + >>> from in2lambda.api.set import Set + >>> s = Set() + >>> s.add_question("Momentum", "The rocket is at $45^\\circ$.") + >>> s.current_question.images.append("no_such_file.png") + >>> for problem in s.problems(): + ... print(problem) + Question 1 "Momentum", main text: ^\circ does not display; write the degree sign ° instead + Question 1 "Momentum": there is no image file at no_such_file.png + """ + from in2lambda.validation import validate + + return validate(self) + def to_json(self, output_dir: str) -> None: """Turns this set into Lambda Feedback JSON/ZIP files. diff --git a/in2lambda/filters/markdown.py b/in2lambda/filters/markdown.py index be1d116..14bca99 100644 --- a/in2lambda/filters/markdown.py +++ b/in2lambda/filters/markdown.py @@ -7,7 +7,6 @@ import panflute as pf from beartype.typing import Callable, Optional -from rich_click import echo from in2lambda.api.set import Set from in2lambda.katex_convert.katex_convert import latex_to_katex @@ -171,10 +170,10 @@ def markdown_converter( case pf.Image: # TODO: Handle "pdf images" and svg files. - path = image_path(elem.url, tex_file) - if path is None: - echo(f"Warning: Couldn't find {elem.url}") - else: + # An image that can't be found is left out of the question's images, + # which in2lambda.validation then reports against the question and + # part it is referenced from. + if (path := image_path(elem.url, tex_file)) is not None: set.current_question.images.append(path) return pf.Str(f"![pictureTag]({elem.url})") diff --git a/in2lambda/katex_convert/katex_convert.py b/in2lambda/katex_convert/katex_convert.py index 735ec86..da5ea4a 100644 --- a/in2lambda/katex_convert/katex_convert.py +++ b/in2lambda/katex_convert/katex_convert.py @@ -77,41 +77,66 @@ def delete_functions(latex_string: str) -> str: return latex_string -def replace_functions(latex_string: str) -> str: - """Helper method of `latex_to_katex` that replaces some LaTeX expressions with an equivalent KaTeX one. +def unsupported_commands() -> dict[str, str | None]: + r"""Every LaTeX command KaTeX has no equivalent for, and what to write instead. - Args: - latex_string: A LaTeX string to be converted into valid KaTeX. + The keys and values are as ``delete_list.txt`` and ``replace_list.txt`` write them: + a regular expression and a :func:`re.sub` template, so a command's backslash is + escaped. ``None`` means the command has no equivalent and is simply dropped. + + Only the plain ``\name`` lines of ``delete_list.txt`` are included, the rest of it + matching whole environments, lengths and stray braces rather than a command. Returns: - The same LaTeX string with some commands replaced where necessary. + Each unsupported command mapped to its replacement, or to ``None`` if it has no + replacement. + + Examples: + >>> from in2lambda.katex_convert.katex_convert import unsupported_commands + >>> commands = unsupported_commands() + >>> commands["\\\\norm"] + '\\\\mathbf' + >>> commands["\\\\bigskip"] is None + True """ - replacement_dict = {} # Dictionary to store the formatted values + commands: dict[str, str | None] = {} + + with open(Path(__file__).with_name("delete_list.txt"), "r") as file: + for line in file: + if re.fullmatch(r"\\\\[a-zA-Z]+", line.strip()): + commands[line.strip()] = None with open(Path(__file__).with_name("replace_list.txt"), "r") as file: for line in file: - line = line.strip().replace(",", "") - key, value = line.split(":", 1) - key = key.strip() - value = value.strip() + key, value = line.strip().replace(",", "").split(":", 1) + commands[key.strip()] = value.strip() - pattern = re.compile(key) - match = pattern.search(value) + return commands - if match: - key = key + "(?![a-zA-Z])" - replacement_dict[key] = value +def replace_functions(latex_string: str) -> str: + """Helper method of `latex_to_katex` that replaces some LaTeX expressions with an equivalent KaTeX one. + + Args: + latex_string: A LaTeX string to be converted into valid KaTeX. + Returns: + The same LaTeX string with some commands replaced where necessary. + """ logger.info("") # replace the incompatible functions with their KaTeX equivalents using re.sub - for old, new in replacement_dict.items(): + for old, new in unsupported_commands().items(): + if new is None: # Deleted rather than replaced; see delete_functions. + continue + + # e.g. "\ang" must not match within its own replacement "\angle". + if re.search(old, new): + old = old + "(?![a-zA-Z])" + while re.search(old, latex_string): - match = re.search(old, latex_string) - if match: - logger.info(f"Replaced {old} with {new}") - latex_string = re.sub(old, new, latex_string) + logger.info(f"Replaced {old} with {new}") + latex_string = re.sub(old, new, latex_string) return latex_string diff --git a/in2lambda/main.py b/in2lambda/main.py index e424b58..50f8268 100644 --- a/in2lambda/main.py +++ b/in2lambda/main.py @@ -173,6 +173,11 @@ def runner( parsing_answers=True, ) + # Report before writing anything: the problems are the set's whether or not it is + # written out, and an author reading the command line should see them first. + for problem in set_obj.problems(): + click.echo(f"Warning: {problem}") + # Read the Python API format and convert to JSON. if output_dir is not None: set_obj.to_json(output_dir) diff --git a/in2lambda/validation/__init__.py b/in2lambda/validation/__init__.py index 21932e6..18ef5e4 100644 --- a/in2lambda/validation/__init__.py +++ b/in2lambda/validation/__init__.py @@ -1,37 +1,238 @@ -"""Pre-flight checks for the ``#``/``##`` and ``$``/``$$`` markdown delimiters. +"""Checks a question set for what Lambda Feedback would refuse or render wrongly. -The markdown that in2lambda converts - however it was produced - is a shared -contract with Lambda Feedback. These checks catch structural mistakes in that -markdown, currently unbalanced or misplaced math delimiters, before it is -converted. +A question can be perfectly valid JSON and still fail to import, or import and then +look wrong: an answer that does not fit the box marking it, an image the export will +not contain, maths KaTeX cannot render. Authors otherwise find this out by uploading +and looking. + +Everything here reports, never refuses: :func:`validate` returns what it found and the +export goes ahead regardless, since a problem may well be deliberate. """ +import re +from functools import cache +from pathlib import Path + +from in2lambda.api.problem import Problem +from in2lambda.api.question import Question +from in2lambda.api.response_area import ResponseArea +from in2lambda.api.set import Set +from in2lambda.katex_convert.katex_convert import unsupported_commands from in2lambda.validation.delimiters import MathDelimiterError, math_delimiter_checker -__all__ = ["MathDelimiterError", "math_delimiter_checker", "check_markdown"] +__all__ = ["MathDelimiterError", "Problem", "math_delimiter_checker", "validate"] + +_IMAGE = re.compile(r"!\[[^\]]*\]\(([^)]*)\)") +"""A markdown image, e.g. ``![pictureTag](question_000_Title_0001.png)``.""" + +_MATHS = re.compile(r"(? list[MathDelimiterError]: - """Run every markdown check and return the problems found. + +def validate(question_set: Set) -> list[Problem]: + r"""Everything in2lambda can tell is wrong with a set, in the order it is written. Args: - md_content: The markdown text to validate. + question_set: The set about to be exported. Returns: - A list of :class:`MathDelimiterError` members, one per problem found. - An empty list means the markdown passed every check. + One :class:`~in2lambda.api.problem.Problem` per problem found, each naming the + question, part and field to look at. An empty list means nothing was found - + not that the set will import, since only some mistakes can be seen from here. Examples: - >>> from in2lambda.validation import check_markdown - >>> check_markdown("Inline $x = y$ is fine.") - [] - >>> check_markdown("Unbalanced $x = y") - [] + >>> from in2lambda.api.set import Set + >>> from in2lambda.validation import validate + >>> s = Set() + >>> s.add_question("Angles", "Turn through $90^\\circ$.") + >>> [str(problem) for problem in validate(s)] + ['Question 1 "Angles", main text: ^\\circ does not display; write the degree sign ° instead'] + """ + problems: list[Problem] = [] + + for number, question in enumerate(question_set.questions, start=1): + where = f'Question {number} "{question.title}"' + problems += _markdown_problems( + question.main_text, question, f"{where}, main text" + ) + + for image in question.images: + if not Path(image).is_file(): + problems.append(Problem(where, f"there is no image file at {image}")) + + for index, part in enumerate(question.parts): + part_where = f"{where}, part ({chr(ord('a') + index)})" + for field, markdown in ( + ("text", part.text), + ("worked solution", part.worked_solution), + ("answer", part.answer), + ): + problems += _markdown_problems( + markdown, question, f"{part_where}, {field}" + ) + + for area_number, area in enumerate(part.response_areas, start=1): + area_where = f"{part_where}, answer box {area_number}" + problems += [ + Problem(area_where, message) for message in _area_problems(area) + ] + for field, markdown in ( + ("pre_text", area.pre_text), + ("post_text", area.post_text), + ("content_after", area.content_after), + ): + problems += _markdown_problems( + markdown, question, f"{area_where}, {field}" + ) + options = (area.config or {}).get("options") + if isinstance(options, list): + for option_number, option in enumerate(options, start=1): + problems += _markdown_problems( + option, question, f"{area_where}, option {option_number}" + ) + + return problems + + +def _markdown_problems( + markdown: str, question: Question, location: str +) -> list[Problem]: + """Every problem in one markdown field, reported against `location`. + + The question is needed because an image reference is only good if that image is + among the question's, and so will be written into the export's ``media/``. """ - problems: list[MathDelimiterError] = [] + problems: list[Problem] = [] - result = math_delimiter_checker(md_content) - if result is not MathDelimiterError.PASSED: - problems.append(result) + delimiters = math_delimiter_checker(markdown) + if delimiters is not MathDelimiterError.PASSED: + problems.append(Problem(location, delimiters.value)) + # Lambda Feedback finds an image in media/ by its file name alone. + media = {Path(image).name for image in question.images} + for reference in _IMAGE.findall(markdown): + if Path(reference).name not in media: + problems.append( + Problem(location, f"the export will not contain the image {reference}") + ) + + problems += _katex_problems(markdown, location) return problems + + +def _katex_problems(markdown: str, location: str) -> list[Problem]: + """Maths that KaTeX, which Lambda Feedback renders with, will not display.""" + problems: list[Problem] = [] + lacks = _katex_lacks() + + for span in _MATHS.finditer(markdown): + maths = span[1] if span[1] is not None else span[2] + for command in _COMMAND.findall(maths): + if command in lacks: + replacement = lacks[command] + problems.append( + Problem( + location, + ( + f"KaTeX does not render {command}; write {replacement} instead" + if replacement + else f"KaTeX does not render {command}" + ), + ) + ) + if _DEGREES.search(maths): + problems.append( + Problem( + location, + "^\\circ does not display; write the degree sign ° instead", + ) + ) + + return problems + + +@cache +def _katex_lacks() -> dict[str, str | None]: + """What KaTeX lacks, keyed by the command as it is written rather than as a regex. + + :func:`~in2lambda.katex_convert.katex_convert.unsupported_commands` gives the lists + as they are written, where a command's backslash is escaped for the replacing pass. + The entries that are not a single command, such as whole environments, simply never + match one. + """ + return { + pattern.replace("\\\\", "\\"): ( + replacement.replace("\\\\", "\\") if replacement else replacement + ) + for pattern, replacement in unsupported_commands().items() + } + + +def _area_problems(area: ResponseArea) -> list[str]: + """Where an answer box's answer does not fit the box, or what marks it. + + Only the three response type / evaluation function pairings the real exports use + (``tests/fixtures/exports/README.md``) are judged. Any other evaluation function + may expect an answer of any shape, and guessing at it would only cry wolf. + """ + messages = [] + + wants_list = [ + name + for name in (area.response_type, area.evaluation_function) + if name in ("MULTIPLE_CHOICE", "arrayEqual") + ] + wants_text = [ + name + for name in (area.response_type, area.evaluation_function) + if name + in ( + "MATH_SINGLE_LINE", + "NUMERIC_UNITS", + "symbolicEqual", + "comparePhysicalQuantities", + ) + ] + if wants_list and not isinstance(area.answer, list): + messages.append( + f"{' and '.join(wants_list)} needs one true/false answer per option, not text" + ) + if wants_text and not isinstance(area.answer, str): + messages.append( + f"{' and '.join(wants_text)} needs the answer as text, not a list" + ) + + if area.response_type == "MULTIPLE_CHOICE" and isinstance(area.answer, list): + config = area.config or {} + options = config.get("options") + if not isinstance(options, list): + messages.append("multiple choice has no options to answer") + elif len(options) != len(area.answer): + messages.append( + f"{len(options)} options but {len(area.answer)} true/false answers" + ) + + correct = area.answer.count(True) + if correct == 0: + messages.append("no option is marked correct") + elif correct > 1 and config.get("single"): + messages.append( + f"{correct} options are marked correct, but only one answer is allowed" + ) + + if area.response_type in ("MATH_SINGLE_LINE", "NUMERIC_UNITS") and isinstance( + area.answer, str + ): + if not area.answer.strip(): + messages.append(f"{area.response_type} has no answer") + elif area.response_type == "NUMERIC_UNITS" and not re.search( + r"\d", area.answer + ): + messages.append(f'NUMERIC_UNITS answer "{area.answer}" has no number in it') + + return messages diff --git a/tests/conftest.py b/tests/conftest.py index ac0da68..7e50eba 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -13,6 +13,12 @@ EXPORTS = sorted(path for path in EXPORTS_DIR.iterdir() if path.is_dir()) """Every export folder, found rather than listed so that adding one needs no code.""" +PROBLEMS_DIR = Path(__file__).parent / "fixtures" / "problems" +"""Hand-written exports, one per folder, each exhibiting one problem for the validator.""" + +PROBLEM_SETS = sorted(path for path in PROBLEMS_DIR.iterdir() if path.is_dir()) +"""Every folder of the above, found the same way: covering a check means adding one.""" + @pytest.fixture(scope="session") def filters_dir() -> str: diff --git a/tests/fixtures/problems/README.md b/tests/fixtures/problems/README.md new file mode 100644 index 0000000..59b2a46 --- /dev/null +++ b/tests/fixtures/problems/README.md @@ -0,0 +1,13 @@ +# Exports with one problem each + +Each folder here is a hand-written Lambda Feedback export exhibiting exactly one of the problems +`in2lambda.validation.validate` looks for, beside the `expected.txt` report it should produce: +one `str(Problem)` line per problem, which the test compares sorted. + +They are written by hand rather than exported by the platform, because the platform does not +produce broken sets. Real exports live in `../exports`, and the same test suite checks that none +of them is reported as having a problem. + +To cover a new check, add a folder. The set's `description` says what the folder is for. A folder +is loaded by `Set.from_json`, the same loader real exports go through rather than a lenient copy, +so it must carry every key that loader reads — they are listed in `../exports/README.md`. diff --git a/tests/fixtures/problems/array_equal_text_answer/expected.txt b/tests/fixtures/problems/array_equal_text_answer/expected.txt new file mode 100644 index 0000000..0c468c9 --- /dev/null +++ b/tests/fixtures/problems/array_equal_text_answer/expected.txt @@ -0,0 +1 @@ +Question 1 "Drag force", part (a), answer box 1: arrayEqual needs one true/false answer per option, not text diff --git a/tests/fixtures/problems/array_equal_text_answer/question_000_Drag_force.json b/tests/fixtures/problems/array_equal_text_answer/question_000_Drag_force.json new file mode 100644 index 0000000..b7b16de --- /dev/null +++ b/tests/fixtures/problems/array_equal_text_answer/question_000_Drag_force.json @@ -0,0 +1,48 @@ +{ + "orderNumber": 0, + "title": "Drag force", + "masterContent": "", + "publish": true, + "displayFinalAnswer": true, + "displayStructuredTutorial": true, + "displayWorkedSolution": true, + "displayChatbot": true, + "parts": [ + { + "orderNumber": 0, + "content": "Write down the drag force.", + "answerContent": "", + "workedSolution": {"content": "", "children": []}, + "responseAreas": [ + { + "orderNumber": 0, + "preResponseText": "", + "postResponseText": "", + "contentAfter": "", + "inputSymbols": [], + "displayInputSymbols": false, + "evaluationFunctionName": "arrayEqual", + "gradeParams": null, + "livePreview": false, + "includeInPdf": false, + "saveAllowed": false, + "separateFeedback": true, + "commonFeedbackColor": "#C4CDD5", + "correctFeedbackColor": "#22C55E", + "correctFeedbackPrefix": "Correct", + "incorrectFeedbackColor": "#ff5630", + "incorrectFeedbackPrefix": "Incorrect", + "tests": [], + "cases": [], + "response": { + "responseInput": { + "responseType": "MATH_SINGLE_LINE", + "answer": "(pi/6)*(rho)*(U**2)*(R**2)", + "config": null + } + } + } + ] + } + ] +} diff --git a/tests/fixtures/problems/array_equal_text_answer/set_Marked_as_a_list.json b/tests/fixtures/problems/array_equal_text_answer/set_Marked_as_a_list.json new file mode 100644 index 0000000..91c207c --- /dev/null +++ b/tests/fixtures/problems/array_equal_text_answer/set_Marked_as_a_list.json @@ -0,0 +1,7 @@ +{ + "name": "Marked as a list", + "description": "An answer written out as text, but marked by arrayEqual, which compares lists.", + "finalAnswerVisibility": "OPEN_WITH_WARNINGS", + "workedSolutionVisibility": "OPEN_WITH_WARNINGS", + "structuredTutorialVisibility": "OPEN" +} diff --git a/tests/fixtures/problems/degrees/expected.txt b/tests/fixtures/problems/degrees/expected.txt new file mode 100644 index 0000000..21713ff --- /dev/null +++ b/tests/fixtures/problems/degrees/expected.txt @@ -0,0 +1 @@ +Question 1 "Launch angle", part (a), answer: ^\circ does not display; write the degree sign ° instead diff --git a/tests/fixtures/problems/degrees/question_000_Launch_angle.json b/tests/fixtures/problems/degrees/question_000_Launch_angle.json new file mode 100644 index 0000000..4c24a34 --- /dev/null +++ b/tests/fixtures/problems/degrees/question_000_Launch_angle.json @@ -0,0 +1,19 @@ +{ + "orderNumber": 0, + "title": "Launch angle", + "masterContent": "", + "publish": true, + "displayFinalAnswer": true, + "displayStructuredTutorial": true, + "displayWorkedSolution": true, + "displayChatbot": true, + "parts": [ + { + "orderNumber": 0, + "content": "At what angle is the rocket launched?", + "answerContent": "The rocket is launched at $45^{\\circ}$ to the horizontal.", + "workedSolution": {"content": "", "children": []}, + "responseAreas": [] + } + ] +} diff --git a/tests/fixtures/problems/degrees/set_Degrees.json b/tests/fixtures/problems/degrees/set_Degrees.json new file mode 100644 index 0000000..8320a3f --- /dev/null +++ b/tests/fixtures/problems/degrees/set_Degrees.json @@ -0,0 +1,7 @@ +{ + "name": "Degrees", + "description": "A final answer in degrees written the usual LaTeX way, which does not display.", + "finalAnswerVisibility": "OPEN_WITH_WARNINGS", + "workedSolutionVisibility": "OPEN_WITH_WARNINGS", + "structuredTutorialVisibility": "OPEN" +} diff --git a/tests/fixtures/problems/math_list_answer/expected.txt b/tests/fixtures/problems/math_list_answer/expected.txt new file mode 100644 index 0000000..86c87e0 --- /dev/null +++ b/tests/fixtures/problems/math_list_answer/expected.txt @@ -0,0 +1 @@ +Question 1 "Speed of sound", part (a), answer box 1: MATH_SINGLE_LINE and symbolicEqual needs the answer as text, not a list diff --git a/tests/fixtures/problems/math_list_answer/question_000_Speed_of_sound.json b/tests/fixtures/problems/math_list_answer/question_000_Speed_of_sound.json new file mode 100644 index 0000000..24fd401 --- /dev/null +++ b/tests/fixtures/problems/math_list_answer/question_000_Speed_of_sound.json @@ -0,0 +1,48 @@ +{ + "orderNumber": 0, + "title": "Speed of sound", + "masterContent": "", + "publish": true, + "displayFinalAnswer": true, + "displayStructuredTutorial": true, + "displayWorkedSolution": true, + "displayChatbot": true, + "parts": [ + { + "orderNumber": 0, + "content": "Give the speed of sound in air.", + "answerContent": "", + "workedSolution": {"content": "", "children": []}, + "responseAreas": [ + { + "orderNumber": 0, + "preResponseText": "", + "postResponseText": "", + "contentAfter": "", + "inputSymbols": [], + "displayInputSymbols": false, + "evaluationFunctionName": "symbolicEqual", + "gradeParams": {"strict_syntax": false}, + "livePreview": false, + "includeInPdf": false, + "saveAllowed": false, + "separateFeedback": true, + "commonFeedbackColor": "#C4CDD5", + "correctFeedbackColor": "#22C55E", + "correctFeedbackPrefix": "Correct", + "incorrectFeedbackColor": "#ff5630", + "incorrectFeedbackPrefix": "Incorrect", + "tests": [], + "cases": [], + "response": { + "responseInput": { + "responseType": "MATH_SINGLE_LINE", + "answer": [true, false], + "config": null + } + } + } + ] + } + ] +} diff --git a/tests/fixtures/problems/math_list_answer/set_Listed_expression.json b/tests/fixtures/problems/math_list_answer/set_Listed_expression.json new file mode 100644 index 0000000..e0d34a4 --- /dev/null +++ b/tests/fixtures/problems/math_list_answer/set_Listed_expression.json @@ -0,0 +1,7 @@ +{ + "name": "Listed expression", + "description": "A maths box whose answer is a list of true/false rather than an expression.", + "finalAnswerVisibility": "OPEN_WITH_WARNINGS", + "workedSolutionVisibility": "OPEN_WITH_WARNINGS", + "structuredTutorialVisibility": "OPEN" +} diff --git a/tests/fixtures/problems/mc_none_correct/expected.txt b/tests/fixtures/problems/mc_none_correct/expected.txt new file mode 100644 index 0000000..e6bbe16 --- /dev/null +++ b/tests/fixtures/problems/mc_none_correct/expected.txt @@ -0,0 +1 @@ +Question 1 "No answer", part (a), answer box 1: no option is marked correct diff --git a/tests/fixtures/problems/mc_none_correct/question_000_No_answer.json b/tests/fixtures/problems/mc_none_correct/question_000_No_answer.json new file mode 100644 index 0000000..c9af267 --- /dev/null +++ b/tests/fixtures/problems/mc_none_correct/question_000_No_answer.json @@ -0,0 +1,52 @@ +{ + "orderNumber": 0, + "title": "No answer", + "masterContent": "", + "publish": true, + "displayFinalAnswer": true, + "displayStructuredTutorial": true, + "displayWorkedSolution": true, + "displayChatbot": true, + "parts": [ + { + "orderNumber": 0, + "content": "Is the flow laminar?", + "answerContent": "", + "workedSolution": {"content": "", "children": []}, + "responseAreas": [ + { + "orderNumber": 0, + "preResponseText": "", + "postResponseText": "", + "contentAfter": "", + "inputSymbols": [], + "displayInputSymbols": false, + "evaluationFunctionName": "arrayEqual", + "gradeParams": null, + "livePreview": false, + "includeInPdf": false, + "saveAllowed": false, + "separateFeedback": true, + "commonFeedbackColor": "#C4CDD5", + "correctFeedbackColor": "#22C55E", + "correctFeedbackPrefix": "Correct", + "incorrectFeedbackColor": "#ff5630", + "incorrectFeedbackPrefix": "Incorrect", + "tests": [], + "cases": [], + "response": { + "responseInput": { + "responseType": "MULTIPLE_CHOICE", + "answer": [false, false], + "config": { + "single": true, + "options": ["Yes", "No"], + "randomise": false + } + } + } + } + ] + } + ] +} diff --git a/tests/fixtures/problems/mc_none_correct/set_Nothing_correct.json b/tests/fixtures/problems/mc_none_correct/set_Nothing_correct.json new file mode 100644 index 0000000..46e9086 --- /dev/null +++ b/tests/fixtures/problems/mc_none_correct/set_Nothing_correct.json @@ -0,0 +1,7 @@ +{ + "name": "Nothing correct", + "description": "A multiple choice where no option is marked as the correct one.", + "finalAnswerVisibility": "OPEN_WITH_WARNINGS", + "workedSolutionVisibility": "OPEN_WITH_WARNINGS", + "structuredTutorialVisibility": "OPEN" +} diff --git a/tests/fixtures/problems/mc_option_count/expected.txt b/tests/fixtures/problems/mc_option_count/expected.txt new file mode 100644 index 0000000..027e3f6 --- /dev/null +++ b/tests/fixtures/problems/mc_option_count/expected.txt @@ -0,0 +1 @@ +Question 1 "Three options", part (a), answer box 1: 3 options but 2 true/false answers diff --git a/tests/fixtures/problems/mc_option_count/question_000_Three_options.json b/tests/fixtures/problems/mc_option_count/question_000_Three_options.json new file mode 100644 index 0000000..b8ca987 --- /dev/null +++ b/tests/fixtures/problems/mc_option_count/question_000_Three_options.json @@ -0,0 +1,52 @@ +{ + "orderNumber": 0, + "title": "Three options", + "masterContent": "", + "publish": true, + "displayFinalAnswer": true, + "displayStructuredTutorial": true, + "displayWorkedSolution": true, + "displayChatbot": true, + "parts": [ + { + "orderNumber": 0, + "content": "Which of these is a fluid?", + "answerContent": "", + "workedSolution": {"content": "", "children": []}, + "responseAreas": [ + { + "orderNumber": 0, + "preResponseText": "", + "postResponseText": "", + "contentAfter": "", + "inputSymbols": [], + "displayInputSymbols": false, + "evaluationFunctionName": "arrayEqual", + "gradeParams": null, + "livePreview": false, + "includeInPdf": false, + "saveAllowed": false, + "separateFeedback": true, + "commonFeedbackColor": "#C4CDD5", + "correctFeedbackColor": "#22C55E", + "correctFeedbackPrefix": "Correct", + "incorrectFeedbackColor": "#ff5630", + "incorrectFeedbackPrefix": "Incorrect", + "tests": [], + "cases": [], + "response": { + "responseInput": { + "responseType": "MULTIPLE_CHOICE", + "answer": [true, false], + "config": { + "single": true, + "options": ["Water", "Steel", "Air"], + "randomise": false + } + } + } + } + ] + } + ] +} diff --git a/tests/fixtures/problems/mc_option_count/set_Option_count.json b/tests/fixtures/problems/mc_option_count/set_Option_count.json new file mode 100644 index 0000000..d55976a --- /dev/null +++ b/tests/fixtures/problems/mc_option_count/set_Option_count.json @@ -0,0 +1,7 @@ +{ + "name": "Option count", + "description": "A multiple choice with three options but only two true/false answers.", + "finalAnswerVisibility": "OPEN_WITH_WARNINGS", + "workedSolutionVisibility": "OPEN_WITH_WARNINGS", + "structuredTutorialVisibility": "OPEN" +} diff --git a/tests/fixtures/problems/missing_image/expected.txt b/tests/fixtures/problems/missing_image/expected.txt new file mode 100644 index 0000000..2eacf8e --- /dev/null +++ b/tests/fixtures/problems/missing_image/expected.txt @@ -0,0 +1 @@ +Question 1 "Hydraulic scale", main text: the export will not contain the image question_000_Hydraulic_scale_0001.png diff --git a/tests/fixtures/problems/missing_image/question_000_Hydraulic_scale.json b/tests/fixtures/problems/missing_image/question_000_Hydraulic_scale.json new file mode 100644 index 0000000..e844259 --- /dev/null +++ b/tests/fixtures/problems/missing_image/question_000_Hydraulic_scale.json @@ -0,0 +1,19 @@ +{ + "orderNumber": 0, + "title": "Hydraulic scale", + "masterContent": "The scale is shown below.\n\n![pictureTag](question_000_Hydraulic_scale_0001.png){ width=60% }", + "publish": true, + "displayFinalAnswer": true, + "displayStructuredTutorial": true, + "displayWorkedSolution": true, + "displayChatbot": true, + "parts": [ + { + "orderNumber": 0, + "content": "How heavy is the load?", + "answerContent": "", + "workedSolution": {"content": "", "children": []}, + "responseAreas": [] + } + ] +} diff --git a/tests/fixtures/problems/missing_image/set_Missing_figure.json b/tests/fixtures/problems/missing_image/set_Missing_figure.json new file mode 100644 index 0000000..4792611 --- /dev/null +++ b/tests/fixtures/problems/missing_image/set_Missing_figure.json @@ -0,0 +1,7 @@ +{ + "name": "Missing figure", + "description": "A question referencing a figure that the export's media folder does not hold.", + "finalAnswerVisibility": "OPEN_WITH_WARNINGS", + "workedSolutionVisibility": "OPEN_WITH_WARNINGS", + "structuredTutorialVisibility": "OPEN" +} diff --git a/tests/fixtures/problems/unbalanced_maths/expected.txt b/tests/fixtures/problems/unbalanced_maths/expected.txt new file mode 100644 index 0000000..b5105fc --- /dev/null +++ b/tests/fixtures/problems/unbalanced_maths/expected.txt @@ -0,0 +1 @@ +Question 1 "Continuity", part (a), worked solution: unclosed inline $ ... $ diff --git a/tests/fixtures/problems/unbalanced_maths/question_000_Continuity.json b/tests/fixtures/problems/unbalanced_maths/question_000_Continuity.json new file mode 100644 index 0000000..d6f85fb --- /dev/null +++ b/tests/fixtures/problems/unbalanced_maths/question_000_Continuity.json @@ -0,0 +1,22 @@ +{ + "orderNumber": 0, + "title": "Continuity", + "masterContent": "", + "publish": true, + "displayFinalAnswer": true, + "displayStructuredTutorial": true, + "displayWorkedSolution": true, + "displayChatbot": true, + "parts": [ + { + "orderNumber": 0, + "content": "Show that mass is conserved.", + "answerContent": "", + "workedSolution": { + "content": "Mass in equals mass out, so $\\rho A U = constant.", + "children": [] + }, + "responseAreas": [] + } + ] +} diff --git a/tests/fixtures/problems/unbalanced_maths/set_Unbalanced_maths.json b/tests/fixtures/problems/unbalanced_maths/set_Unbalanced_maths.json new file mode 100644 index 0000000..784d57d --- /dev/null +++ b/tests/fixtures/problems/unbalanced_maths/set_Unbalanced_maths.json @@ -0,0 +1,7 @@ +{ + "name": "Unbalanced maths", + "description": "A worked solution opening an inline maths expression that it never closes.", + "finalAnswerVisibility": "OPEN_WITH_WARNINGS", + "workedSolutionVisibility": "OPEN_WITH_WARNINGS", + "structuredTutorialVisibility": "OPEN" +} diff --git a/tests/fixtures/problems/unsupported_command/expected.txt b/tests/fixtures/problems/unsupported_command/expected.txt new file mode 100644 index 0000000..0f50da1 --- /dev/null +++ b/tests/fixtures/problems/unsupported_command/expected.txt @@ -0,0 +1 @@ +Question 1 "Magnitude", part (a), text: KaTeX does not render \norm; write \mathbf instead diff --git a/tests/fixtures/problems/unsupported_command/question_000_Magnitude.json b/tests/fixtures/problems/unsupported_command/question_000_Magnitude.json new file mode 100644 index 0000000..4177428 --- /dev/null +++ b/tests/fixtures/problems/unsupported_command/question_000_Magnitude.json @@ -0,0 +1,19 @@ +{ + "orderNumber": 0, + "title": "Magnitude", + "masterContent": "", + "publish": true, + "displayFinalAnswer": true, + "displayStructuredTutorial": true, + "displayWorkedSolution": true, + "displayChatbot": true, + "parts": [ + { + "orderNumber": 0, + "content": "Find the magnitude $\\norm{v}$ of the velocity.", + "answerContent": "", + "workedSolution": {"content": "", "children": []}, + "responseAreas": [] + } + ] +} diff --git a/tests/fixtures/problems/unsupported_command/set_Unsupported_command.json b/tests/fixtures/problems/unsupported_command/set_Unsupported_command.json new file mode 100644 index 0000000..070ce76 --- /dev/null +++ b/tests/fixtures/problems/unsupported_command/set_Unsupported_command.json @@ -0,0 +1,7 @@ +{ + "name": "Unsupported command", + "description": "Maths using a LaTeX command that KaTeX, which the platform renders with, has no equivalent for.", + "finalAnswerVisibility": "OPEN_WITH_WARNINGS", + "workedSolutionVisibility": "OPEN_WITH_WARNINGS", + "structuredTutorialVisibility": "OPEN" +} diff --git a/tests/test_runner.py b/tests/test_runner.py index 7c39d51..e0b24a0 100644 --- a/tests/test_runner.py +++ b/tests/test_runner.py @@ -10,10 +10,11 @@ import os import pytest +from click.testing import CliRunner from in2lambda.api.set import Set from in2lambda.filters import builtin_filters -from in2lambda.main import runner +from in2lambda.main import cli, runner def _example(filters_dir: str, filter_name: str) -> str: @@ -56,3 +57,27 @@ def test_runner_writes_importable_json( assert question_json["title"] assert "masterContent" in question_json assert "parts" in question_json + + +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" + question_file.write_text( + "\\documentclass{article}\n" + "\\begin{document}\n" + "\\section{Buoyancy}\n" + "The apparatus is shown in \\includegraphics{absent.png}.\n" + "\\end{document}\n" + ) + out_dir = tmp_path / "out" + + result = CliRunner().invoke( + cli, [str(question_file), "PartsOneSol", "-o", str(out_dir)] + ) + + assert result.exit_code == 0 + assert ( + 'Warning: Question 1 "", main text: ' + "the export will not contain the image absent.png" in result.output + ) + assert (out_dir / "set.zip").is_file() diff --git a/tests/test_validation.py b/tests/test_validation.py index e624e4a..7b2ebcf 100644 --- a/tests/test_validation.py +++ b/tests/test_validation.py @@ -1,16 +1,21 @@ -"""Tests for the math-delimiter checker. +"""Tests for the checks run over a question set before it is exported. -Ported from ``conversion2025/tools and testing/validator_tests.py`` on the -``Summer2025`` branch and adapted to the :class:`MathDelimiterError` enum. +Each folder in ``fixtures/problems`` is a hand-written export exhibiting one problem, +beside the report it should produce, so covering another check means adding a folder +rather than a test. The real exports in ``fixtures/exports`` are the other half of it: +whatever the validator reports, it must not report a set the platform itself wrote. + +The markdown cases were ported from ``conversion2025/tools and testing/validator_tests.py`` +on the ``Summer2025`` branch. """ +from pathlib import Path + import pytest +from conftest import EXPORTS, PROBLEM_SETS -from in2lambda.validation import ( - MathDelimiterError, - check_markdown, - math_delimiter_checker, -) +from in2lambda.api.set import Set +from in2lambda.validation import MathDelimiterError, validate E = MathDelimiterError @@ -74,15 +79,46 @@ ] +def _messages(markdown: str) -> list[str]: + """What the validator says about a single piece of markdown.""" + question_set = Set() + question_set.add_question("Markdown", markdown) + return [problem.message for problem in validate(question_set)] + + +@pytest.mark.parametrize("problem_set", PROBLEM_SETS, ids=lambda path: path.name) +def test_expected_problems_are_reported(problem_set: Path) -> None: + """Each hand-written export produces exactly the report written beside it.""" + expected = (problem_set / "expected.txt").read_text().splitlines() + found = validate(Set.from_json(str(problem_set))) + + assert sorted(str(problem) for problem in found) == sorted(expected) + + +@pytest.mark.parametrize("export", EXPORTS, ids=lambda path: path.name) +def test_real_exports_have_no_problems(export: Path) -> None: + """A set the platform wrote and accepted back must never be reported.""" + assert validate(Set.from_json(str(export))) == [] + + @pytest.mark.parametrize("content", VALID) def test_valid_markdown_passes(content: str) -> None: - assert math_delimiter_checker(content) is E.PASSED - assert check_markdown(content) == [] + assert _messages(content) == [] @pytest.mark.parametrize("content, expected", INVALID) def test_invalid_markdown_is_reported( content: str, expected: MathDelimiterError ) -> None: - assert math_delimiter_checker(content) is expected - assert check_markdown(content) == [expected] + assert _messages(content) == [expected.value] + + +def test_image_that_is_not_on_disk_is_reported(tmp_path: Path) -> None: + """An image a question lists but that is not there would break the export.""" + question_set = Set() + question_set.add_question("Rocket", "![pictureTag](rocket.png)") + question_set.current_question.images.append(str(tmp_path / "rocket.png")) + + assert [problem.message for problem in question_set.problems()] == [ + f"there is no image file at {tmp_path / 'rocket.png'}" + ]