Complete roundtrip of PL <--> OPB MD conversion

.github/workflows/publish.yml

@@ -72,7 +72,9 @@ jobs:
         uses: actions/upload-artifact@v4
         with:
           name: test-generated-files-${{ github.event_name }}
-          path: tests/test_question_templates/question_generated_outputs/
+          path: |
+            tests/test_question_templates/question_generated_outputs/
+            tests/test_question_templates/question_return_generated_outputs/
           if-no-files-found: ignore
           retention-days: 5
 
@@ -130,7 +132,7 @@ jobs:
           echo "::notice::Installing ${{ matrix.dist.name }}: $path_to_file"
           python -m pip install --user "$path_to_file[tests]"
           python -m pip list
-      - name: Run tests against installed wheel
+      - name: Run tests against installed ${{ matrix.dist.name }}
         run: rm -rf src/ && pytest tests/
 
 

pyproject.toml

@@ -25,6 +25,8 @@ problem-bank-helpers = "^0.3.2"
 typing-extensions = "^4.12.2"
 black = "^24.8.0"
 gitpython = "^3.1.43"
+lxml = "^5.2.0,<5.3.0"
+beautifulsoup4 = {extras = ["lxml"], version = "^4.12.3"}
 exceptiongroup = {version = "^1.2.2", python = "<3.11"}
 questionary = {version = "^2.0.1", optional = true}
 nltk = {version = "^3.8.1", optional = true}
@@ -56,4 +58,4 @@ process_q = "problem_bank_scripts.scripts.process_q:main"
 checkq = "problem_bank_scripts.scripts.check_question:main"
 
 [tool.pytest.ini_options]
-filterwarnings = ["error", "ignore::DeprecationWarning:problem_bank_helpers"]
+filterwarnings = ["error", "ignore::DeprecationWarning:problem_bank_helpers", "ignore:The 'strip_cdata':DeprecationWarning"]

src/problem_bank_scripts/problem_bank_scripts.py

@@ -86,7 +86,7 @@ def defdict_to_dict(defdict, finaldict):
 
         elif hasattr(v, "dtype"):
             try:
-                finaldict[k] = v.item()
+                finaldict[k] = v.item() # pyright: ignore[reportAttributeAccessIssue]
             except Exception:
                 finaldict[k] = v
         else:
@@ -397,8 +397,7 @@ def write_info_json(output_path, parsed_question, modified_time: str | None = No
         modified_time (str | None, optional): Last commit timestamp or modified timestamp of the file
     """
 
-    # Deal with optional tags in info.json
-    # optional = ""
+    ## *** IMPORTANT: If more auto-tags or optional keys are added, make sure to update the `pl_to_md` function to take them into account properly for the roundtrip ***
 
     optional_keys = {
         "gradingMethod",
@@ -456,8 +455,34 @@ def write_info_json(output_path, parsed_question, modified_time: str | None = No
             msg = f"workspaceOptions.port must be an integer, got {type(info_json['workspaceOptions']['port'])!r} instead"
             raise TypeError(msg)
 
+    comment_keys = (
+        "author",
+        "source",
+        "template_version",
+        "outcomes",
+        "difficulty",
+        "randomization",
+        "taxonomy",
+        "span",
+        "length",
+    )
+
+    comment = {key: parsed_question["header"][key] for key in comment_keys if key in parsed_question["header"]}
+
+    # Get keys that need to get added to the comment from the question body (Rubric, Solution, Comments)
+
+    comment |= {
+        key: parsed_question["body_parts"][key].split("\n\n", 1)[-1].strip()
+        for key in ("Rubric", "Solution", "Comments")
+        if key in parsed_question["body_parts"]
+    }
+
+    # Add the comment to the info_json, under a metadata key in the comment object of info_json
+
+    info_json["comment"] = {"METADATA": comment}
+
     if modified_time:
-        info_json["comment"] = {"lastModified": modified_time}
+        info_json["comment"]["lastModified"] = modified_time  # pyright: ignore[reportArgumentType]
 
     # End add tags
     with pathlib.Path(output_path / "info.json").open("w") as output_file:
@@ -491,9 +516,7 @@ def assemble_server_py(parsed_question, location):
     for function, code in server_dict.items():
         indented_code = code.replace("\n", "\n    ")
         # With the custom header, add functions to server.py as-is
-        if function == "custom":
-            server_py += f"{code}"
-        elif function == "imports":
+        if function in {"custom", "imports"}:
             continue
         else:
             if code:
@@ -512,6 +535,9 @@ def assemble_server_py(parsed_question, location):
 
 """
 
+    if "custom" in server_dict:
+        server_py += f"{server_dict['custom']}"
+
     return server_py
 
 
@@ -1061,4 +1087,3 @@ def validate_header(header_dict):
     if topics.get(topic := header_dict["topic"], None) is None:
         msg = f"topic '{topic}' is not listed in the learning outcomes"
         raise ValueError(msg)
-

tests/conftest.py

@@ -0,0 +1,41 @@
+import json
+import pathlib
+
+import fastjsonschema
+import pytest
+
+
+@pytest.fixture(scope="session")
+def paths():
+    """Sets the paths of where to find inputs, generated outputs, and expected outputs.
+
+    Returns:
+        Nothing, it's a fixture that is run before every test.
+    """
+    return {
+        "inputDest": pathlib.Path("tests/test_question_templates/question_inputs/"),
+        "outputDest": pathlib.Path("tests/test_question_templates/question_generated_outputs/"),
+        "compareDest": pathlib.Path("tests/test_question_templates/question_expected_outputs/"),
+        "returnCompareDest": pathlib.Path("tests/test_question_templates/question_return_expected_outputs/"),
+        "returnOutputDest": pathlib.Path("tests/test_question_templates/question_return_generated_outputs/"),
+    }
+
+
+@pytest.fixture(scope="session")
+def validate_info_json():
+    """Generates a schema validator for info.json files.
+
+    Returns:
+        Nothing, it's a fixture that is run before every test.
+    """
+    with open("tests/infoSchema.json") as file:
+        return fastjsonschema.compile(json.load(file))
+
+@pytest.fixture(scope="session", autouse=True)
+def monkeypatch_prairielearn():
+    """Monkeypatches the prairielearn module into sys.modules to make it accessible."""
+
+    import sys
+
+    from problem_bank_scripts import prairielearn
+    sys.modules["prairielearn"] = prairielearn

tests/test_md_to_pl.py

@@ -0,0 +1,180 @@
+from __future__ import annotations
+
+import filecmp
+import json
+import pathlib
+
+import pytest
+
+from problem_bank_scripts import process_question_md, process_question_pl
+
+
+# Generate a list of all problems in the test problems directory
+files = sorted(file.name for file in pathlib.Path("tests/test_question_templates/question_inputs/").iterdir() if file.name != ".DS_Store")
+
+_tested_questions = set()
+
+
+def run_prairie_learn_generator(paths: dict[str, pathlib.Path], question: str, devmode: bool):
+    """Helper function that runs the PrairieLearn generator on a question.
+
+    This allows us to deduplicate the code for running the generator.
+
+    Args:
+        paths (dict): definition of the output and input paths
+        question (str): the name of the question to test, set by the parametrize decorator
+        devmode (bool): whether to run the generator in devmode
+    """
+    if (question, devmode) in _tested_questions:
+        return  # don't parse the same question twice
+    _tested_questions.add((question, devmode))
+    outputPath = paths["outputDest"].joinpath(f"prairielearn{'-dev' if devmode else ''}/")
+
+    baseFile = paths["inputDest"] / question / f"{question}.md"
+    folder = baseFile.parent.stem
+    outputFolder = outputPath.joinpath(folder)
+    if question in {"q03_dropdown", "q05_multi-part_feedback"}:
+        with pytest.warns(FutureWarning, match="The 'pl-dropdown' tag is deprecated."):
+            process_question_pl(baseFile, outputFolder.joinpath(baseFile.name), devmode)
+    else:
+        process_question_pl(baseFile, outputFolder.joinpath(baseFile.name), devmode)
+
+
+@pytest.mark.parametrize(
+    ("question", "devmode"),
+    [
+        pytest.param(file, dev, id=(f"dev-{file}" if dev else f"nodev-{file}"))
+        for file in files
+        for dev in [False, True]
+    ],
+)
+def test_prairie_learn(paths: dict[str, pathlib.Path], question: str, devmode: bool, subtests):
+    """Tests the PrairieLearn `process_question_pl()`
+
+    Args:
+        paths (dict): set by the fixture paths()
+        question (str): the name of the question to test, set by the parametrize decorator
+        devmode (bool): whether to run the generator in devmode
+    """
+    run_prairie_learn_generator(paths, question, devmode)
+    outputPath = paths["outputDest"].joinpath(f"prairielearn{'-dev' if devmode else ''}/")
+    comparePath = paths["compareDest"].joinpath(f"prairielearn{'-dev' if devmode else ''}/")
+    baseFile = paths["inputDest"].joinpath(f"{question}/{question}.md")
+    folder = baseFile.parent.stem
+
+    for file in sorted(comparePath.joinpath(f"{folder}/").glob("**/*")):
+        isFile = file.is_file()
+        hiddenFile = not file.name.startswith(".")
+        assetFile = file.name == "question.html" or not file.name.endswith(
+            (".png", ".jpg", ".jpeg", ".gif", ".html", ".DS_Store")
+        )
+
+        infoJSON = not file.name.endswith("info.json")
+
+        if isFile and hiddenFile and assetFile and infoJSON:
+            with subtests.test("Check Generated File Matches Expected", file=file.name):
+                folder = file.parent.name
+                outputFolder = outputPath.joinpath(folder)
+
+                try:
+                    filecmp.cmp(file, outputPath / file.relative_to(comparePath))
+                except FileNotFoundError:
+                    print(file, folder, outputFolder, outputPath / file.relative_to(comparePath))
+
+                assert filecmp.cmp(
+                    file, outputPath / file.relative_to(comparePath)
+                ), f"File: {'/'.join(file.parts[-2:])} did not match with expected output."
+
+
+@pytest.mark.parametrize(
+    ("question", "devmode"),
+    [
+        pytest.param(file, dev, id=(f"nodev-{file}" if dev else f"dev-{file}"))
+        for file in files
+        for dev in [False, True]
+    ],
+)
+def test_info_json(paths: dict[str, pathlib.Path], question: str, devmode: bool, validate_info_json, subtests):
+    """Tests the PrairieLearn `process_question_pl()` info.json file
+
+    Args:
+        paths (dict): set by the fixture paths()
+        question (str): the name of the question to test, set by the parametrize decorator
+        devmode (bool): whether to run the generator in devmode
+    """
+    run_prairie_learn_generator(paths, question, devmode)
+    output_info_json = paths["outputDest"].joinpath(f"prairielearn{'-dev' if devmode else ''}/{question}/info.json")
+    compare_info_json = paths["compareDest"].joinpath(f"prairielearn{'-dev' if devmode else ''}/{question}/info.json")
+    generated_json = json.loads(output_info_json.read_bytes())
+    expected_json = json.loads(compare_info_json.read_bytes())
+    validate_info_json(generated_json)
+    del generated_json["uuid"]  # uuid is semi-randomly generated, so we can't compare reliably it
+    del expected_json["uuid"]
+    for key in expected_json:
+        with subtests.test("Check Generated info.json", key=key):
+            generated = generated_json[key]
+            if isinstance(generated, list):
+                generated = sorted(generated)
+            expected = expected_json[key]
+            if isinstance(expected, list):
+                expected = sorted(expected)
+            assert expected == generated, f"info.json key {key!r} for {question} did not match with expected output."
+
+
+@pytest.mark.parametrize("question", files)
+def test_public(paths: dict[str, pathlib.Path], question: str, subtests):
+    """Tests the PrairieLearn `process_question_md()`
+
+    Args:
+        paths (dict): set by the fixture paths()
+        question (str): the name of the question to test, set by the parametrize decorator
+    """
+    outputPath = paths["outputDest"].joinpath("public/")
+    comparePath = paths["compareDest"].joinpath("public/")
+
+    baseFile = paths["inputDest"] / question / f"{question}.md"
+    folder = baseFile.parent.stem
+    outputFolder = outputPath.joinpath(folder)
+    process_question_md(baseFile, outputFolder.joinpath(baseFile.name), instructor=False)
+
+    for file in sorted(comparePath.joinpath(f"{folder}/").glob("**/*")):
+        isFile = file.is_file()
+        notHiddenFile = not file.name.startswith(".")
+        notImageFile = not file.name.endswith(".png")
+        if isFile and notHiddenFile and notImageFile:
+            with subtests.test("Check Generated File Matches Expected", file=file.name):
+                folder = file.parent.name
+                outputFolder = outputPath.joinpath(folder)
+                assert filecmp.cmp(
+                    file, outputPath / file.relative_to(comparePath), shallow=False
+                ), f"File: {'/'.join(file.parts[-2:])} did not match with expected output."
+
+
+@pytest.mark.parametrize("question", files)
+def test_instructor(paths: dict[str, pathlib.Path], question: str, subtests):
+    """Tests the PrairieLearn `process_question_md(instructor=True)`
+
+    Args:
+        paths (dict): set by the fixture paths()
+        question (str): the name of the question to test, set by the parametrize decorator
+    """
+    outputPath = paths["outputDest"].joinpath("instructor")  # the path to where the newly generated file will be stored
+    comparePath = paths["compareDest"].joinpath("instructor")  # the path to where the existing files to be compared are stored
+
+    baseFile = paths["inputDest"] / question / f"{question}.md"
+    folder = baseFile.parent.stem
+    outputFolder = outputPath.joinpath(folder)
+    process_question_md(baseFile, outputFolder.joinpath(baseFile.name), instructor=True)
+
+    for file in sorted(comparePath.joinpath(f"{folder}/").glob("**/*")):
+        isFile = file.is_file()
+        notHiddenFile = not file.name.startswith(".")
+        notImageFile = not file.name.endswith(".png")
+
+        if isFile and notHiddenFile and notImageFile:
+            with subtests.test("Check Generated File Matches Expected", file=file.name):
+                folder = file.parent.name
+                outputFolder = outputPath.joinpath(folder)
+                assert filecmp.cmp(
+                    file, outputPath / file.relative_to(comparePath), shallow=False
+                ), f"File: {'/'.join(file.parts[-2:])} did not match with expected output."