Add data_files setting and tests_dir fixture

.github/copilot-instructions.md

@@ -146,6 +146,10 @@ result = sandbox.query_function("student_func", arg1, arg2, timeout=5)
 these modules, so they must not contain answers. In workspace mode, their names are reserved: a student
 file or folder with one fails the submission with a message asking the student to rename it.
 
+`ConfigObject(data_files=["pagerank_large.txt"])` starts the sandbox in a new folder holding read-only
+copies of the listed tests-folder files, so code in the sandbox can read them by relative path. Tests read
+the originals through the `tests_dir` fixture. A missing listed file makes the submission ungradable.
+
 ### 5. Serialization
 
 - Uses `dill` for complex Python objects (numpy arrays, pandas DataFrames, matplotlib plots)

docker/run.sh

@@ -34,6 +34,15 @@ 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
 
+# Student code runs as ag, whose uid may match the owner of files mounted from the host. Stop it from
+# changing the tests and the submission, since tests read them after student code has run (for
+# example, a data file the tests_dir fixture reads to compute expected values).
+if ! { chown -R root:root "$MERGE_DIR" && chmod -R go-w "$MERGE_DIR"; }; then
+    echo '{"succeeded": false, "score": 0.0, "message": "The autograder could not protect its grading files. Please contact course staff and have them check the logs for this submission."}' > "$OUT_DIR/results.json"
+    echo "ERROR: could not protect $MERGE_DIR"
+    exit 1
+fi
+
 # 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

quick_start.md

@@ -123,7 +123,7 @@ def test_with_output(sandbox: StudentFixture) -> None:
 
 ### Available Fixtures
 
-Three fixtures are provided by the `pytest-prairielearn-grader` package:
+These fixtures are provided by the `pytest-prairielearn-grader` package:
 
 1. **`sandbox: StudentFixture`**: Provides sandboxed access to student code. Use this to query variables
    and call functions from the student's submission.
@@ -137,6 +137,10 @@ Three fixtures are provided by the `pytest-prairielearn-grader` package:
 4. **`data_json: DataFixture`**: Provides access to parameters from PrairieLearn's `data.json` file
    (generated via `pl-external-grader-variables` and other elements).
 
+5. **`tests_dir: Path`**: The question's tests folder, which holds `leading_code.py` and any data files.
+   Use it to read instructor files in a test, for example `np.loadtxt(tests_dir / "pagerank_large.txt")`,
+   instead of building the path from `__file__`.
+
 For implementation details, see the [fixture source code](https://github.com/prairielearn/grader-python-v2/blob/main/src/pytest_prairielearn_grader/fixture.py).
 
 ## Querying Student Code
@@ -349,6 +353,7 @@ All parameters are optional with sensible defaults:
 - **`import_blacklist`** (list[str] | None): Blocked import modules (blacklist mode)
 - **`builtin_whitelist`** (list[str] | None): Allowed builtin functions
 - **`helper_modules`** (list[str] | None): Modules from the tests folder that can be imported (see [Helper Modules](#helper-modules))
+- **`data_files`** (list[str] | None): Files from the tests folder that code in the sandbox can read by relative path (see [Data Files](#data-files))
 - **`names_for_user`** (list[str] | None): List of variable names to inject into student code
 - **`student_code_pattern`** (str, default="student_code\*.py"): Glob pattern for finding student files
 - **`starting_vars`** (dict[str, Any], default={}): Dictionary of variable values to provide for injection
@@ -393,6 +398,47 @@ import helper_function as hf
 > setting also doesn't make the rest of the tests folder private. `importlib` isn't blacklisted by
 > default, so student code can load files in that folder by path.
 
+#### Data Files
+
+In a workspace, code such as `np.loadtxt("pagerank_large.txt")` reads a file next to the notebook. To
+make the same code work during grading, put the file in the tests folder and name it in `data_files`:
+
+```python
+# tests/test_student.py, with the data in tests/pagerank_large.txt
+autograder_config = ConfigObject(
+    data_files=["pagerank_large.txt"],
+    student_code_pattern="Markov-Chains-3.ipynb",
+    notebook_cell_tag="#grade",
+)
+
+
+def test_pagerank(sandbox: StudentFixture, tests_dir: Path) -> None:
+    edges = np.loadtxt(tests_dir / "pagerank_large.txt")  # the instructor's original
+    ...
+```
+
+```python
+# tests/leading_code.py, or the student's code
+edges = np.loadtxt("pagerank_large.txt")
+```
+
+- **Paths:** each entry is relative to the tests folder, such as `"pagerank_large.txt"` or
+  `"data/points.csv"`, and keeps that relative path in the sandbox. Absolute paths and `..` aren't
+  accepted.
+- **Starting folder:** the sandbox starts in a new folder that holds only copies of the listed files.
+  Without `data_files`, the starting folder isn't changed.
+- **Read-only copies:** the copies and their folder are read-only for student code, and a fresh copy is
+  made for each sandbox. Tests should read the originals through the `tests_dir` fixture.
+- **Reading, not importing:** the folder isn't on the import path, so a listed `.py` file can't be
+  imported. Use `helper_modules` for that.
+- **Configuration errors:** if a listed file is missing, the submission is ungradable. The students
+  don't lose an attempt, and each test's message says what's wrong.
+- **Fixtures:** it works with `sandbox` and `module_sandbox`, for `.py` and `.ipynb` submissions, and
+  with `workspace_sandbox`. In workspace mode, relative paths resolve against the data files' folder,
+  not the student's workspace folder.
+
+> **Warning:** Students can read the listed files, so they must not contain answers.
+
 **How variable injection works**:
 
 - Only variables listed in `names_for_user` are injected into the student code namespace
@@ -1328,7 +1374,7 @@ def test_result_variable(sandbox: StudentFixture, feedback: FeedbackFixture) ->
 
 In production PrairieLearn, the submitted `gradedFiles` end up in `/grade/student/`. The grader
 searches for matching files there automatically. For local development, place the notebook in the
-scenario data directory next to your test file:
+tests folder next to your test file (the folder named after it):
 
 ```
 questions/my_notebook_question/
@@ -1389,7 +1435,7 @@ questions/my_workspace_question/
 └── tests/
     ├── test_student.py        # your test file
     ├── setup_code.py          # (optional)
-    └── test_student/          # ← mirrors the data directory layout
+    └── test_student/          # ← mirrors the tests folder layout
         └── student/           # ← local stand-in for /grade/student
             ├── calculator.py
             └── utils/

src/pytest_prairielearn_grader/config.py

@@ -3,10 +3,25 @@
 import keyword
 from dataclasses import dataclass
 from dataclasses import field
+from pathlib import PurePosixPath
+from pathlib import PureWindowsPath
 from typing import Any
 from typing import Literal
 
 
+def _is_plain_relative_path(name: str) -> bool:
+    """Whether name is a relative path to a file that stays inside the folder it's resolved against.
+
+    Entries always use "/", whatever the OS, so a config means the same thing on every machine.
+    Backslashes and drives are rejected because on Windows they'd be a separator or an anchor.
+    """
+    if not name.strip() or "\\" in name or PureWindowsPath(name).drive:
+        return False
+    path = PurePosixPath(name)
+    # PurePosixPath drops "." and empty parts, so "." and "" have no parts left.
+    return not path.is_absolute() and bool(path.parts) and ".." not in path.parts
+
+
 @dataclass(frozen=True, slots=True, kw_only=True)
 class ConfigObject:
     """
@@ -87,6 +102,24 @@ def test_student_code(sandbox):
     Example: ["helper_function"]
     """
 
+    data_files: list[str] | None = None
+    """Files from the tests folder that setup code, leading code and student code can read by relative path.
+
+    Each entry is a path relative to the folder that holds leading_code.py, such as
+    "pagerank_large.txt" or "data/points.csv". Absolute paths and ".." aren't accepted.
+
+    The sandbox starts in a fresh folder that holds a copy of each listed file at the same
+    relative path, so ``np.loadtxt("pagerank_large.txt")`` works as it does in a Jupyter
+    workspace. Student code can't change the copies, and changes to them couldn't reach the
+    originals anyway, which tests read through the ``tests_dir`` fixture. Without this setting
+    the sandbox's starting folder isn't changed.
+
+    A listed file that doesn't exist makes the submission ungradable, with a message for the
+    instructor. Students can read the listed files, so they must not contain answers.
+
+    Example: ["pagerank_large.txt"]
+    """
+
     builtin_whitelist: list[str] | None = None
     """List of allowed Python builtin functions that student code can use.
 
@@ -264,6 +297,18 @@ def __post_init__(self) -> None:
                 if not isinstance(module_name, str) or not module_name.isidentifier() or keyword.iskeyword(module_name):
                     raise ValueError(f"helper_modules must contain plain module names without dots, paths or '.py', got: {module_name!r}")
 
+        # Validate data_files contains relative paths that stay inside the tests folder
+        if self.data_files is not None:
+            if not isinstance(self.data_files, list):
+                raise TypeError(f"data_files must be a list, got {type(self.data_files).__name__}")
+            if not self.data_files:
+                raise ValueError("data_files cannot be empty (use None for no data files)")
+            for file_name in self.data_files:
+                if not isinstance(file_name, str) or not _is_plain_relative_path(file_name):
+                    raise ValueError(f"data_files must contain relative paths inside the tests folder, without '..', got: {file_name!r}")
+            if len({PurePosixPath(name) for name in self.data_files}) != len(self.data_files):
+                raise ValueError(f"data_files lists the same file more than once: {self.data_files!r}")
+
         # Validate builtin_whitelist contains non-empty strings
         if self.builtin_whitelist is not None:
             if not isinstance(self.builtin_whitelist, list):

src/pytest_prairielearn_grader/fixture.py

@@ -1,9 +1,12 @@
 import json
 import logging
 import os
+import shutil
 import socket
+import stat
 import subprocess
 import sys
+import tempfile
 from importlib.resources import files
 from pathlib import Path
 from typing import Any
@@ -33,6 +36,13 @@
 SCRIPT_PATH = str(files("pytest_prairielearn_grader").joinpath("_student_code_runner.py"))
 DEFAULT_TIMEOUT = 1.0
 
+# Modes for the data files folder (see _make_data_files_dir). Everyone can read the files and list the
+# folders, and nobody can change them; only the owner (root, in the Docker image) can make them writable again.
+_READ_ONLY_FILE_MODE = stat.S_IRUSR | stat.S_IRGRP | stat.S_IROTH  # r--r--r--
+_READ_ONLY_DIR_MODE = _READ_ONLY_FILE_MODE | stat.S_IXUSR | stat.S_IXGRP | stat.S_IXOTH  # r-xr-xr-x
+# Lets the owner delete the folder's contents at cleanup.
+_OWNER_WRITABLE_DIR_MODE = _READ_ONLY_DIR_MODE | stat.S_IWUSR  # rwxr-xr-x
+
 logger = logging.getLogger(__name__)
 
 
@@ -76,6 +86,33 @@ def _extract_notebook_code(notebook_path: Path, cell_tag: str | None = None) ->
     return "\n\n".join(code_parts)
 
 
+def _make_data_files_dir(data_files: dict[str, Path]) -> Path:
+    """Copy each data file to its relative path in a new folder that only its owner can change.
+
+    The folder is made by the grader process, so in the Docker image it's owned by root and the
+    sandbox user can read the copies but not change, add or remove anything.
+    """
+    root = Path(tempfile.mkdtemp(prefix="pl-data-files-"))
+    try:
+        for relative_path, source in data_files.items():
+            destination = root / relative_path
+            destination.parent.mkdir(parents=True, exist_ok=True)
+            shutil.copyfile(source, destination)
+            destination.chmod(_READ_ONLY_FILE_MODE)
+        for directory in [root, *(path for path in root.rglob("*") if path.is_dir())]:
+            directory.chmod(_READ_ONLY_DIR_MODE)
+    except BaseException:
+        _remove_data_files_dir(root)
+        raise
+    return root
+
+
+def _remove_data_files_dir(root: Path) -> None:
+    for directory in [root, *(path for path in root.rglob("*") if path.is_dir())]:
+        directory.chmod(_OWNER_WRITABLE_DIR_MODE)
+    shutil.rmtree(root, ignore_errors=True)
+
+
 class StudentFiles(NamedTuple):
     leading_file: Path
     trailing_file: Path
@@ -139,6 +176,8 @@ class _SandboxBase:
     builtin_whitelist: list[str] | None
     names_for_user_list: list[str] | None
     worker_username: str | None
+    data_files: dict[str, Path] | None
+    _data_files_dir: Path | None
     _accumulated_stdout: list[str]
     _server_failure: str | None
 
@@ -150,13 +189,16 @@ def __init__(
         builtin_whitelist: list[str] | None,
         names_for_user_list: list[str] | None,
         worker_username: str | None,
+        data_files: dict[str, Path] | None = None,
     ) -> None:
         self.import_whitelist = import_whitelist
         self.import_blacklist = import_blacklist
         self.starting_vars = starting_vars
         self.builtin_whitelist = builtin_whitelist
         self.names_for_user_list = names_for_user_list
         self.worker_username = worker_username
+        self.data_files = data_files
+        self._data_files_dir = None
         self.process = None
         self.student_socket = None
         self._accumulated_stdout = []
@@ -213,11 +255,17 @@ def try_drop_privileges() -> None:
             if self.worker_username is not None:
                 drop_privileges(self.worker_username)
 
+        # With data files, the sandbox starts in a folder holding copies of them, so student code can
+        # read them by relative path. Without them, the starting folder is left as it was.
+        if self.data_files is not None:
+            self._data_files_dir = _make_data_files_dir(self.data_files)
+
         self.process = subprocess.Popen(
             args=(sys.executable, SCRIPT_PATH),
             stdin=subprocess.PIPE,
             stdout=subprocess.PIPE,
             stderr=subprocess.PIPE,
+            cwd=self._data_files_dir,
             preexec_fn=None if sys.platform == "win32" else try_drop_privileges,
         )
         self._assert_process_running()
@@ -337,6 +385,9 @@ def _cleanup(self) -> None:
             self.process.terminate()
             self.process.wait()
             self.process = None
+        if self._data_files_dir is not None:
+            _remove_data_files_dir(self._data_files_dir)
+            self._data_files_dir = None
 
 
 class StudentFixture(_SandboxBase):
@@ -356,8 +407,11 @@ def __init__(
         worker_username: str | None,
         notebook_cell_tag: str | None = None,
         helper_modules: dict[str, str] | None = None,
+        data_files: dict[str, Path] | None = None,
     ) -> None:
-        super().__init__(import_whitelist, import_blacklist, starting_vars, builtin_whitelist, names_for_user_list, worker_username)
+        super().__init__(
+            import_whitelist, import_blacklist, starting_vars, builtin_whitelist, names_for_user_list, worker_username, data_files
+        )
         self.leading_file = file_names.leading_file
         self.trailing_file = file_names.trailing_file
         self.student_code_file = file_names.student_code_file
@@ -466,8 +520,11 @@ def __init__(
         worker_username: str | None,
         exec_entry: str | None = None,
         helper_modules: dict[str, str] | None = None,
+        data_files: dict[str, Path] | None = None,
     ) -> None:
-        super().__init__(import_whitelist, import_blacklist, starting_vars, builtin_whitelist, names_for_user_list, worker_username)
+        super().__init__(
+            import_whitelist, import_blacklist, starting_vars, builtin_whitelist, names_for_user_list, worker_username, data_files
+        )
         self.workspace_dir = workspace_dir
         self.setup_code_file = setup_code_file
         self.exec_entry = exec_entry