From 59e1712a0862357a748f998892c827e3fa83a3ef Mon Sep 17 00:00:00 2001 From: Eliot Robson Date: Thu, 5 Nov 2026 11:24:30 -0500 Subject: [PATCH 1/6] Add plot-serializer to the full image Plot grading depends on plot_serializer, which was only a dev dependency, so it failed in the full image. Also correct the docs: plots must be built with MatplotlibSerializer, because a plain Figure isn't serialized. Co-Authored-By: Claude Opus 5.5 --- docker/requirements-full.txt | 1 + quick_start.md | 28 +++++++++++++++++++++++----- 2 files changed, 24 insertions(+), 5 deletions(-) diff --git a/docker/requirements-full.txt b/docker/requirements-full.txt index 68e332d..5340f2c 100644 --- a/docker/requirements-full.txt +++ b/docker/requirements-full.txt @@ -10,6 +10,7 @@ numpy==2.5.4 openpyxl==3.1.5 pandas==3.1.0 Pillow==13.0.0 +plot-serializer==0.2.3 PuLP==4.2.0 Pygments==2.21.0 pytest==9.2.0 diff --git a/quick_start.md b/quick_start.md index d045a30..13198ba 100644 --- a/quick_start.md +++ b/quick_start.md @@ -264,7 +264,7 @@ result = sandbox.query_function("slow_computation", data, query_timeout=2.0) **Note**: Student code must define the queried symbols, and return values must be JSON-serializable. Supported types include: `int`, `float`, `str`, `list`, `dict`, `bool`, `None`, numpy arrays, pandas DataFrames, -and matplotlib figures. +and plots built with `MatplotlibSerializer` (see [Testing Matplotlib Plots](#testing-matplotlib-plots)). ## Partial Credit and Feedback @@ -950,9 +950,27 @@ since students can learn from reading full tracebacks. ### Testing Matplotlib Plots -The grader supports automatic serialization and deserialization of matplotlib figures: +To grade a plot, student code builds it with `MatplotlibSerializer` from the `plot-serializer` package and returns the serializer. The test receives a matplotlib `Figure`. + +`plot-serializer` is included in the `:latest` image but not in `:lite`. Returning a plain matplotlib `Figure` from student code isn't supported. + +```python +# Student code +from plot_serializer.matplotlib.serializer import MatplotlibSerializer + + +def create_plot(data): + serializer = MatplotlibSerializer() + fig, ax = serializer.subplots() + ax.plot(data) + ax.set_xlabel("Time") + ax.set_ylabel("Value") + ax.set_title("Data Visualization") + return serializer +``` ```python +# Test code import matplotlib matplotlib.use("Agg") # Non-GUI backend for server environments @@ -965,7 +983,7 @@ from pytest_prairielearn_grader.fixture import StudentFixture @pytest.mark.grading_data(name="Plot Test", points=5) def test_student_plot(sandbox: StudentFixture) -> None: """Test that student creates a correct plot.""" - # Student function returns a matplotlib figure + # Student function returns a MatplotlibSerializer, which arrives here as a Figure plot = sandbox.query_function("create_plot", data) assert isinstance(plot, Figure) @@ -984,7 +1002,7 @@ def test_student_plot(sandbox: StudentFixture) -> None: The grader automatically serializes/deserializes: -- Matplotlib figures +- Plots built with `MatplotlibSerializer` (received as a `Figure`) - NumPy arrays - Pandas DataFrames - Standard Python types (int, float, str, list, dict, bool, None) @@ -1019,7 +1037,7 @@ Two image variants are published. Pick the smallest one that includes what your | Tag | Size (approx.) | Includes | Best for | | --------- | -------------- | ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | ----------------------------------------------------------------------------------------------- | -| `:latest` | ~400–500 MB | Full scientific stack: `numpy`, `pandas`, `scipy`, `matplotlib` + `matplotcheck2`, `seaborn`, `scikit-learn`, `sympy`, `networkx`, `nltk`, `nbformat`, and more | Any course that needs scientific computing, data science, plotting, or Jupyter notebook grading | +| `:latest` | ~400–500 MB | Full scientific stack: `numpy`, `pandas`, `scipy`, `matplotlib` + `matplotcheck2` + `plot-serializer`, `seaborn`, `scikit-learn`, `sympy`, `networkx`, `nltk`, `nbformat`, and more | Any course that needs scientific computing, data science, plotting, or Jupyter notebook grading | | `:lite` | ~60–80 MB | Grader core only: `pytest-prairielearn-grader`, `Pygments`, `beautifulsoup4`, `defusedxml`, `requests` | Introductory CS courses where student code uses only the Python standard library | To use the lite image, change the `image` field: From 45533581c8a1a9e10679e83eac9384159852a90b Mon Sep 17 00:00:00 2001 From: Eliot Robson Date: Thu, 5 Nov 2026 11:24:30 -0500 Subject: [PATCH 2/6] Load PrairieLearn's data.json in the grader images PrairieLearn writes /grade/data/data.json, but the data_json fixture only read data.json next to the test file and nothing put it there, so tests using data_json failed with a NoneType error and correct submissions scored 0. run.sh now passes the path with --data-json-path. The option uses the --data-json-path=VALUE form because pytest's first pass over the arguments would treat a separate value as a test path, which moves rootdir and loses the results file. Without the option, or if the file doesn't exist, the fixture reads data.json next to the test file. data.json can hold correct_answers, so run.sh makes it root-owned with owner-only modes before student code runs, and stops grading if that fails. Root ownership matters because the student uid can match the host owner of mounted files, and owner-only modes matter because student code keeps root's supplementary groups. The fixture now raises a clear error for a data.json it can't parse or read, instead of treating it like a missing file. run.sh takes the job directory as an optional argument so tests can run it, and no longer exports JOB_DIR or MERGE_DIR. Co-Authored-By: Claude Opus 5.5 --- docker/run.sh | 33 ++++- quick_start.md | 2 + src/pytest_prairielearn_grader/plugin.py | 33 +++-- .../test_docker_layout/scenario.py | 1 - .../test_invalid_data_json/__init__.py | 0 .../test_invalid_data_json/data.json | 1 + .../expected_outcome.json | 15 ++ .../test_invalid_data_json/scenario.py | 5 + .../test_invalid_data_json/student_code.py | 1 + tests/test_run_script.py | 131 ++++++++++++++++++ 10 files changed, 206 insertions(+), 16 deletions(-) create mode 100644 tests/scenario_root/test_invalid_data_json/__init__.py create mode 100644 tests/scenario_root/test_invalid_data_json/data.json create mode 100644 tests/scenario_root/test_invalid_data_json/expected_outcome.json create mode 100644 tests/scenario_root/test_invalid_data_json/scenario.py create mode 100644 tests/scenario_root/test_invalid_data_json/student_code.py create mode 100644 tests/test_run_script.py diff --git a/docker/run.sh b/docker/run.sh index 2fbc49a..c7f7709 100644 --- a/docker/run.sh +++ b/docker/run.sh @@ -4,20 +4,22 @@ # INIT ########################## -if [ ! -d /grade ]; then - echo "ERROR: /grade not found! Mounting may have failed." +# the parent directory containing everything about this grading job +JOB_DIR="${1:-/grade}" + +if [ ! -d "$JOB_DIR" ]; then + echo "ERROR: $JOB_DIR not found! Mounting may have failed." exit 1 fi -# the parent directory containing everything about this grading job -export JOB_DIR='/grade' # the job subdirectories STUDENT_DIR=$JOB_DIR'/student' TEST_DIR=$JOB_DIR'/tests' OUT_DIR=$JOB_DIR'/results' +DATA_DIR=$JOB_DIR'/data' # where we will copy everything -export MERGE_DIR=$JOB_DIR'/run' +MERGE_DIR=$JOB_DIR'/run' # now set up the stuff so that our run.sh can work mkdir $MERGE_DIR @@ -33,6 +35,23 @@ mv "$STUDENT_DIR" "$MERGE_DIR/test_student/student" mv $TEST_DIR/* $MERGE_DIR/test_student/ mv $MERGE_DIR/test_student/test_student.py $MERGE_DIR/test_student.py +# PrairieLearn writes data.json to the data directory, and pytest is told to read it from there. +# It can hold correct_answers, so it's for the grader only. It must be owned by root with +# owner-only modes: student code keeps root's supplementary groups, and its uid may match the owner +# of files mounted from the host. +protect_data_json() { + if [ "$(id -u)" = 0 ]; then + chown -R root:root "$DATA_DIR" || return 1 + fi + chmod 700 "$DATA_DIR" && chmod 600 "$DATA_DIR/data.json" +} + +if [ -f "$DATA_DIR/data.json" ] && ! protect_data_json; then + echo '{"succeeded": false, "score": 0.0, "message": "The autograder could not protect its grading data. Please contact course staff and have them check the logs for this submission."}' > "$OUT_DIR/results.json" + echo "ERROR: could not protect $DATA_DIR/data.json" + exit 1 +fi + # Do not allow ag user to modify, rename, or delete any existing files #chmod -R 755 "$MERGE_DIR" #chmod 1777 "$MERGE_DIR" @@ -44,8 +63,10 @@ mv $MERGE_DIR/test_student/test_student.py $MERGE_DIR/test_student.py echo "[run] starting autograder" # run the autograder as a limited user called ag +# Pass --data-json-path with '=': pytest's first pass over the arguments, before plugin options are +# registered, would treat a separate value as a test path. # Use --worker-username to drop privileges to the 'ag' user in the student code sandbox -uv run pytest -p prairielearn-grader --color=no --log-cli-level=DEBUG --worker-username ag "$MERGE_DIR" +uv run pytest -p prairielearn-grader --color=no --log-cli-level=DEBUG --worker-username ag --data-json-path="$DATA_DIR/data.json" "$MERGE_DIR" # TODO change the default output name mv "$MERGE_DIR/autograder_results.json" "$OUT_DIR/results.json" diff --git a/quick_start.md b/quick_start.md index 13198ba..d4e6d29 100644 --- a/quick_start.md +++ b/quick_start.md @@ -603,6 +603,8 @@ def generate(data): These parameters are automatically passed to the autograder via `/grade/data/data.json` and enforced during student code execution. +In the grader images, `run.sh` passes this path to pytest with `--data-json-path` and makes the file readable only by root, so student code can't open it. The file can contain `correct_answers`. Without that option, for example when running locally, the fixture reads `data.json` next to your test file. The sandbox still receives `params` through `__data_params`, so don't rely on values in `params` staying hidden from student code. + #### Testing Import/Builtin Restrictions Locally To test your restrictions in the test scenarios: diff --git a/src/pytest_prairielearn_grader/plugin.py b/src/pytest_prairielearn_grader/plugin.py index 00b15c5..8cb96fe 100644 --- a/src/pytest_prairielearn_grader/plugin.py +++ b/src/pytest_prairielearn_grader/plugin.py @@ -67,19 +67,28 @@ def get_datadir(test_module: ModuleType) -> Path | None: return data_dir +def _find_data_json(request: pytest.FixtureRequest) -> Path: + """Prefer the file named by --data-json-path, and otherwise use data.json next to the test file.""" + data_json_path = request.config.getoption("--data-json-path") + if data_json_path is not None and Path(data_json_path).is_file(): + return Path(data_json_path) + + datadir = get_datadir(request.module) + assert datadir is not None + return datadir / "data.json" + + @pytest.fixture(scope="module") def data_json(request: pytest.FixtureRequest) -> dict[str, Any] | None: + data_file = _find_data_json(request) + logger.debug(f"Loading data.json from {data_file}") try: - datadir = get_datadir(request.module) - assert datadir is not None - data_file = datadir / "data.json" - logger.debug(f"Loading data.json from {data_file}") return json.loads(data_file.read_text(encoding="utf-8")) - except Exception as e: - logger.debug(f"No data.json found or failed to load: {e}") - - # If the data file is not found or cannot be read, return None - return None + except FileNotFoundError: + logger.debug(f"No data.json found at {data_file}") + return None + except (OSError, ValueError) as e: + raise RuntimeError(f"Could not load {data_file.name}: {e}") from e # if datadir is None or not datadir.is_dir(): # raise ValueError(f"Data directory '{datadir}' not found or is not a directory.") @@ -677,6 +686,12 @@ def pytest_addoption(parser: pytest.Parser) -> None: """ group = parser.getgroup("pytest_pl_autograder", "Prairielearn Python Autograder Options") + group.addoption( + "--data-json-path", + action="store", + default=None, + help="Path to the data.json file for this grading job. If it doesn't exist, data.json next to the test file is used.", + ) group.addoption( "--worker-username", action="store", diff --git a/tests/scenario_root/test_docker_layout/scenario.py b/tests/scenario_root/test_docker_layout/scenario.py index 22b17df..ed52db4 100644 --- a/tests/scenario_root/test_docker_layout/scenario.py +++ b/tests/scenario_root/test_docker_layout/scenario.py @@ -10,7 +10,6 @@ student/ student_code.py <- moved from /grade/student/ by run.sh setup_code.py <- from /grade/tests/ (not present here) - data.json <- from /grade/tests/ (not present here) The plugin's _find_student_files fallback discovers student_code.py inside the student/ subdirectory automatically when no files match the pattern at the root. diff --git a/tests/scenario_root/test_invalid_data_json/__init__.py b/tests/scenario_root/test_invalid_data_json/__init__.py new file mode 100644 index 0000000..e69de29 diff --git a/tests/scenario_root/test_invalid_data_json/data.json b/tests/scenario_root/test_invalid_data_json/data.json new file mode 100644 index 0000000..42e2ff7 --- /dev/null +++ b/tests/scenario_root/test_invalid_data_json/data.json @@ -0,0 +1 @@ +{"params": diff --git a/tests/scenario_root/test_invalid_data_json/expected_outcome.json b/tests/scenario_root/test_invalid_data_json/expected_outcome.json new file mode 100644 index 0000000..2762715 --- /dev/null +++ b/tests/scenario_root/test_invalid_data_json/expected_outcome.json @@ -0,0 +1,15 @@ +{ + "expected_data_object": { + "score": 0.0, + "tests": [ + { + "max_points": 1, + "points_frac": 0.0, + "points": 0.0, + "test_id": "test_invalid_data_json.py::test_query[student_code]", + "outcome": "error", + "message": "Could not load data.json: Expecting value" + } + ] + } +} diff --git a/tests/scenario_root/test_invalid_data_json/scenario.py b/tests/scenario_root/test_invalid_data_json/scenario.py new file mode 100644 index 0000000..1950ffa --- /dev/null +++ b/tests/scenario_root/test_invalid_data_json/scenario.py @@ -0,0 +1,5 @@ +from pytest_prairielearn_grader.fixture import StudentFixture + + +def test_query(sandbox: StudentFixture) -> None: + assert sandbox.query("ANSWER") == 42 diff --git a/tests/scenario_root/test_invalid_data_json/student_code.py b/tests/scenario_root/test_invalid_data_json/student_code.py new file mode 100644 index 0000000..b0f8b2d --- /dev/null +++ b/tests/scenario_root/test_invalid_data_json/student_code.py @@ -0,0 +1 @@ +ANSWER = 42 diff --git a/tests/test_run_script.py b/tests/test_run_script.py new file mode 100644 index 0000000..b7df0fc --- /dev/null +++ b/tests/test_run_script.py @@ -0,0 +1,131 @@ +"""Runs docker/run.sh against a temporary job directory, with the layout PrairieLearn creates.""" + +import json +import os +import stat +import subprocess +import sys +from pathlib import Path + +import pytest + +RUN_SH = Path(__file__).parents[1] / "docker" / "run.sh" + +pytestmark = pytest.mark.skipif(sys.platform == "win32", reason="run.sh only runs on Unix systems") + +TEST_FILE = """ +from pytest_prairielearn_grader.fixture import DataFixture, StudentFixture + + +def test_params(sandbox: StudentFixture, data_json: DataFixture) -> None: + assert data_json["params"]["expected"] == 42 + assert sandbox.query("ANSWER") == 42 + assert sandbox.query_setup("from_setup") == 42 +""" + +SETUP_CODE = """ +from_setup = __data_params["expected"] +""" + +# Stands in for `uv run`, and drops the option that needs root and an 'ag' user. +FAKE_UV = f"""#!{sys.executable} +import os +import sys + +args = sys.argv[2:] +index = args.index("--worker-username") +del args[index : index + 2] +os.execv(sys.executable, [sys.executable, "-m", *args]) +""" + + +def mode(path: Path) -> int: + return stat.S_IMODE(path.stat().st_mode) + + +@pytest.fixture +def job_dir(tmp_path: Path) -> Path: + job = tmp_path / "grade" + (job / "tests").mkdir(parents=True) + (job / "student").mkdir() + (job / "tests" / "test_student.py").write_text(TEST_FILE) + (job / "tests" / "setup_code.py").write_text(SETUP_CODE) + (job / "student" / "student_code.py").write_text("ANSWER = 42\n") + return job + + +def write_data_json(job: Path, content: str) -> Path: + data_file = job / "data" / "data.json" + data_file.parent.mkdir() + data_file.write_text(content) + data_file.chmod(0o644) + return data_file + + +def run_grader(job: Path, tmp_path: Path) -> subprocess.CompletedProcess[str]: + bin_dir = tmp_path / "bin" + bin_dir.mkdir() + fake_uv = bin_dir / "uv" + fake_uv.write_text(FAKE_UV) + fake_uv.chmod(0o755) + env = {**os.environ, "PATH": f"{bin_dir}{os.pathsep}{os.environ['PATH']}"} + return subprocess.run(["/bin/sh", str(RUN_SH), str(job)], env=env, capture_output=True, text=True, check=False) + + +def score(job: Path) -> float: + return json.loads((job / "results" / "results.json").read_text())["score"] + + +def test_data_json_reaches_grader_and_sandbox(job_dir: Path, tmp_path: Path) -> None: + write_data_json(job_dir, json.dumps({"params": {"expected": 42}, "correct_answers": {"x": 1}})) + + run_grader(job_dir, tmp_path) + + assert score(job_dir) == 1.0 + + +def test_data_json_is_owner_only_and_not_copied(job_dir: Path, tmp_path: Path) -> None: + data_file = write_data_json(job_dir, json.dumps({"params": {"expected": 42}})) + + run_grader(job_dir, tmp_path) + + assert mode(data_file) == 0o600 + assert mode(data_file.parent) == 0o700 + assert not (job_dir / "run" / "test_student" / "data.json").exists() + + +def test_prairielearn_data_json_takes_precedence_over_tests(job_dir: Path, tmp_path: Path) -> None: + (job_dir / "tests" / "data.json").write_text(json.dumps({"params": {"expected": 0}})) + write_data_json(job_dir, json.dumps({"params": {"expected": 42}})) + + run_grader(job_dir, tmp_path) + + assert score(job_dir) == 1.0 + + +def test_data_json_from_tests_used_without_prairielearn_file(job_dir: Path, tmp_path: Path) -> None: + (job_dir / "tests" / "data.json").write_text(json.dumps({"params": {"expected": 42}})) + + run_grader(job_dir, tmp_path) + + assert score(job_dir) == 1.0 + + +def test_invalid_prairielearn_data_json_is_an_error(job_dir: Path, tmp_path: Path) -> None: + write_data_json(job_dir, '{"params": ') + + run_grader(job_dir, tmp_path) + + results = json.loads((job_dir / "results" / "results.json").read_text()) + assert results["score"] == 0.0 + assert "Could not load data.json" in results["tests"][0]["message"] + + +def test_missing_data_json_still_grades(job_dir: Path, tmp_path: Path) -> None: + (job_dir / "tests" / "test_student.py").write_text("def test_runs() -> None:\n assert True\n") + (job_dir / "tests" / "setup_code.py").unlink() + + run_grader(job_dir, tmp_path) + + assert score(job_dir) == 1.0 + assert not (job_dir / "run" / "test_student" / "data.json").exists() From 44158e9c5012ac69961be9ff60aab2383184465d Mon Sep 17 00:00:00 2001 From: Eliot Robson Date: Thu, 5 Nov 2026 11:30:04 -0500 Subject: [PATCH 3/6] Remove the run.sh test and its job directory argument run.sh isn't Python, so it doesn't get its own test file. The optional job directory argument only existed for that test, so /grade is fixed in the script again, and protect_data_json no longer has to handle running as non-root. Co-Authored-By: Claude Opus 5.5 --- docker/run.sh | 15 ++--- tests/test_run_script.py | 131 --------------------------------------- 2 files changed, 6 insertions(+), 140 deletions(-) delete mode 100644 tests/test_run_script.py diff --git a/docker/run.sh b/docker/run.sh index c7f7709..1102d55 100644 --- a/docker/run.sh +++ b/docker/run.sh @@ -4,14 +4,13 @@ # INIT ########################## -# the parent directory containing everything about this grading job -JOB_DIR="${1:-/grade}" - -if [ ! -d "$JOB_DIR" ]; then - echo "ERROR: $JOB_DIR not found! Mounting may have failed." +if [ ! -d /grade ]; then + echo "ERROR: /grade not found! Mounting may have failed." exit 1 fi +# the parent directory containing everything about this grading job +JOB_DIR='/grade' # the job subdirectories STUDENT_DIR=$JOB_DIR'/student' TEST_DIR=$JOB_DIR'/tests' @@ -40,9 +39,7 @@ mv $MERGE_DIR/test_student/test_student.py $MERGE_DIR/test_student.py # owner-only modes: student code keeps root's supplementary groups, and its uid may match the owner # of files mounted from the host. protect_data_json() { - if [ "$(id -u)" = 0 ]; then - chown -R root:root "$DATA_DIR" || return 1 - fi + chown -R root:root "$DATA_DIR" || return 1 chmod 700 "$DATA_DIR" && chmod 600 "$DATA_DIR/data.json" } @@ -63,9 +60,9 @@ fi echo "[run] starting autograder" # run the autograder as a limited user called ag +# Use --worker-username to drop privileges to the 'ag' user in the student code sandbox # Pass --data-json-path with '=': pytest's first pass over the arguments, before plugin options are # registered, would treat a separate value as a test path. -# Use --worker-username to drop privileges to the 'ag' user in the student code sandbox uv run pytest -p prairielearn-grader --color=no --log-cli-level=DEBUG --worker-username ag --data-json-path="$DATA_DIR/data.json" "$MERGE_DIR" # TODO change the default output name diff --git a/tests/test_run_script.py b/tests/test_run_script.py deleted file mode 100644 index b7df0fc..0000000 --- a/tests/test_run_script.py +++ /dev/null @@ -1,131 +0,0 @@ -"""Runs docker/run.sh against a temporary job directory, with the layout PrairieLearn creates.""" - -import json -import os -import stat -import subprocess -import sys -from pathlib import Path - -import pytest - -RUN_SH = Path(__file__).parents[1] / "docker" / "run.sh" - -pytestmark = pytest.mark.skipif(sys.platform == "win32", reason="run.sh only runs on Unix systems") - -TEST_FILE = """ -from pytest_prairielearn_grader.fixture import DataFixture, StudentFixture - - -def test_params(sandbox: StudentFixture, data_json: DataFixture) -> None: - assert data_json["params"]["expected"] == 42 - assert sandbox.query("ANSWER") == 42 - assert sandbox.query_setup("from_setup") == 42 -""" - -SETUP_CODE = """ -from_setup = __data_params["expected"] -""" - -# Stands in for `uv run`, and drops the option that needs root and an 'ag' user. -FAKE_UV = f"""#!{sys.executable} -import os -import sys - -args = sys.argv[2:] -index = args.index("--worker-username") -del args[index : index + 2] -os.execv(sys.executable, [sys.executable, "-m", *args]) -""" - - -def mode(path: Path) -> int: - return stat.S_IMODE(path.stat().st_mode) - - -@pytest.fixture -def job_dir(tmp_path: Path) -> Path: - job = tmp_path / "grade" - (job / "tests").mkdir(parents=True) - (job / "student").mkdir() - (job / "tests" / "test_student.py").write_text(TEST_FILE) - (job / "tests" / "setup_code.py").write_text(SETUP_CODE) - (job / "student" / "student_code.py").write_text("ANSWER = 42\n") - return job - - -def write_data_json(job: Path, content: str) -> Path: - data_file = job / "data" / "data.json" - data_file.parent.mkdir() - data_file.write_text(content) - data_file.chmod(0o644) - return data_file - - -def run_grader(job: Path, tmp_path: Path) -> subprocess.CompletedProcess[str]: - bin_dir = tmp_path / "bin" - bin_dir.mkdir() - fake_uv = bin_dir / "uv" - fake_uv.write_text(FAKE_UV) - fake_uv.chmod(0o755) - env = {**os.environ, "PATH": f"{bin_dir}{os.pathsep}{os.environ['PATH']}"} - return subprocess.run(["/bin/sh", str(RUN_SH), str(job)], env=env, capture_output=True, text=True, check=False) - - -def score(job: Path) -> float: - return json.loads((job / "results" / "results.json").read_text())["score"] - - -def test_data_json_reaches_grader_and_sandbox(job_dir: Path, tmp_path: Path) -> None: - write_data_json(job_dir, json.dumps({"params": {"expected": 42}, "correct_answers": {"x": 1}})) - - run_grader(job_dir, tmp_path) - - assert score(job_dir) == 1.0 - - -def test_data_json_is_owner_only_and_not_copied(job_dir: Path, tmp_path: Path) -> None: - data_file = write_data_json(job_dir, json.dumps({"params": {"expected": 42}})) - - run_grader(job_dir, tmp_path) - - assert mode(data_file) == 0o600 - assert mode(data_file.parent) == 0o700 - assert not (job_dir / "run" / "test_student" / "data.json").exists() - - -def test_prairielearn_data_json_takes_precedence_over_tests(job_dir: Path, tmp_path: Path) -> None: - (job_dir / "tests" / "data.json").write_text(json.dumps({"params": {"expected": 0}})) - write_data_json(job_dir, json.dumps({"params": {"expected": 42}})) - - run_grader(job_dir, tmp_path) - - assert score(job_dir) == 1.0 - - -def test_data_json_from_tests_used_without_prairielearn_file(job_dir: Path, tmp_path: Path) -> None: - (job_dir / "tests" / "data.json").write_text(json.dumps({"params": {"expected": 42}})) - - run_grader(job_dir, tmp_path) - - assert score(job_dir) == 1.0 - - -def test_invalid_prairielearn_data_json_is_an_error(job_dir: Path, tmp_path: Path) -> None: - write_data_json(job_dir, '{"params": ') - - run_grader(job_dir, tmp_path) - - results = json.loads((job_dir / "results" / "results.json").read_text()) - assert results["score"] == 0.0 - assert "Could not load data.json" in results["tests"][0]["message"] - - -def test_missing_data_json_still_grades(job_dir: Path, tmp_path: Path) -> None: - (job_dir / "tests" / "test_student.py").write_text("def test_runs() -> None:\n assert True\n") - (job_dir / "tests" / "setup_code.py").unlink() - - run_grader(job_dir, tmp_path) - - assert score(job_dir) == 1.0 - assert not (job_dir / "run" / "test_student" / "data.json").exists() From 6985f266a13ee2917fc754d6856d4e7530e688c8 Mon Sep 17 00:00:00 2001 From: Eliot Robson Date: Thu, 5 Nov 2026 11:33:17 -0500 Subject: [PATCH 4/6] Return None from data_json when there's nowhere to look Moving the lookup out of the try block meant a test module without a file raised an AssertionError instead of getting None, as it did before. Co-Authored-By: Claude Opus 5.5 --- src/pytest_prairielearn_grader/plugin.py | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/src/pytest_prairielearn_grader/plugin.py b/src/pytest_prairielearn_grader/plugin.py index 8cb96fe..81b96da 100644 --- a/src/pytest_prairielearn_grader/plugin.py +++ b/src/pytest_prairielearn_grader/plugin.py @@ -67,28 +67,30 @@ def get_datadir(test_module: ModuleType) -> Path | None: return data_dir -def _find_data_json(request: pytest.FixtureRequest) -> Path: +def _find_data_json(request: pytest.FixtureRequest) -> Path | None: """Prefer the file named by --data-json-path, and otherwise use data.json next to the test file.""" data_json_path = request.config.getoption("--data-json-path") if data_json_path is not None and Path(data_json_path).is_file(): return Path(data_json_path) datadir = get_datadir(request.module) - assert datadir is not None - return datadir / "data.json" + return None if datadir is None else datadir / "data.json" @pytest.fixture(scope="module") def data_json(request: pytest.FixtureRequest) -> dict[str, Any] | None: - data_file = _find_data_json(request) - logger.debug(f"Loading data.json from {data_file}") try: + data_file = _find_data_json(request) + if data_file is None: + logger.debug("No location to load data.json from") + return None + logger.debug(f"Loading data.json from {data_file}") return json.loads(data_file.read_text(encoding="utf-8")) except FileNotFoundError: - logger.debug(f"No data.json found at {data_file}") + logger.debug("No data.json found") return None except (OSError, ValueError) as e: - raise RuntimeError(f"Could not load {data_file.name}: {e}") from e + raise RuntimeError(f"Could not load data.json: {e}") from e # if datadir is None or not datadir.is_dir(): # raise ValueError(f"Data directory '{datadir}' not found or is not a directory.") From 1487e6317e8ef9106768a2144ec9f029bac475e8 Mon Sep 17 00:00:00 2001 From: Eliot Robson Date: Thu, 5 Nov 2026 11:33:28 -0500 Subject: [PATCH 5/6] Restore the data.json line in the docker layout scenario run.sh still moves a data.json from /grade/tests to that location, and the fixture still falls back to it. Co-Authored-By: Claude Opus 5.5 --- tests/scenario_root/test_docker_layout/scenario.py | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/scenario_root/test_docker_layout/scenario.py b/tests/scenario_root/test_docker_layout/scenario.py index ed52db4..22b17df 100644 --- a/tests/scenario_root/test_docker_layout/scenario.py +++ b/tests/scenario_root/test_docker_layout/scenario.py @@ -10,6 +10,7 @@ student/ student_code.py <- moved from /grade/student/ by run.sh setup_code.py <- from /grade/tests/ (not present here) + data.json <- from /grade/tests/ (not present here) The plugin's _find_student_files fallback discovers student_code.py inside the student/ subdirectory automatically when no files match the pattern at the root. From a02fa7fdedbd0868c51b0188f67586c6474d639b Mon Sep 17 00:00:00 2001 From: Eliot Robson Date: Thu, 5 Nov 2026 11:35:42 -0500 Subject: [PATCH 6/6] Pass --worker-username with = in run.sh pytest's first pass over the arguments treats a separate value as a possible test path, so use the same form as --data-json-path. Co-Authored-By: Claude Opus 5.5 --- docker/run.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docker/run.sh b/docker/run.sh index 1102d55..9d7aa4a 100644 --- a/docker/run.sh +++ b/docker/run.sh @@ -61,9 +61,9 @@ echo "[run] starting autograder" # run the autograder as a limited user called ag # Use --worker-username to drop privileges to the 'ag' user in the student code sandbox -# Pass --data-json-path with '=': pytest's first pass over the arguments, before plugin options are +# Pass plugin options with '=': pytest's first pass over the arguments, before plugin options are # registered, would treat a separate value as a test path. -uv run pytest -p prairielearn-grader --color=no --log-cli-level=DEBUG --worker-username ag --data-json-path="$DATA_DIR/data.json" "$MERGE_DIR" +uv run pytest -p prairielearn-grader --color=no --log-cli-level=DEBUG --worker-username=ag --data-json-path="$DATA_DIR/data.json" "$MERGE_DIR" # TODO change the default output name mv "$MERGE_DIR/autograder_results.json" "$OUT_DIR/results.json"