From 4650b06042b7b3a06e97734c1d95ea26988b58d0 Mon Sep 17 00:00:00 2001 From: Eliot Robson Date: Wed, 11 Nov 2026 00:56:29 -0500 Subject: [PATCH 1/2] Fix ruff errors and make the ruff step fail the Lint job Co-Authored-By: Claude Opus 5.5 --- .github/workflows/lint.yaml | 2 -- pyproject.toml | 16 +++++++++++++++- .../_student_code_runner.py | 2 +- src/pytest_prairielearn_grader/config.py | 4 +--- src/pytest_prairielearn_grader/fixture.py | 5 +++-- src/pytest_prairielearn_grader/utils.py | 6 +++--- .../test_fixture_exception/scenario.py | 1 + .../test_function_not_found/student_code.py | 2 +- .../test_initialization_timeout/scenario.py | 6 +++--- .../test_matplotlib_plots/student_code.py | 6 +++--- tests/scenario_root/test_quiz_2_1/scenario.py | 3 ++- tests/scenario_root/test_quiz_2_1/setup_code.py | 5 +++-- tests/scenario_root/test_quiz_3_1/scenario.py | 4 ++-- .../test_quiz_3_1/student_code_0.py | 2 +- .../test_quiz_3_1/student_code_100.py | 2 +- .../test_quiz_3_1/student_code_66.py | 2 +- tests/scenario_root/test_timeout/scenario.py | 6 +++--- .../student/calculator.py | 2 +- .../student/raises_attribute_error_on_load.py | 2 +- tests/test_autograder_scenarios.py | 6 ++---- tests/test_serialize.py | 6 ++---- 21 files changed, 50 insertions(+), 40 deletions(-) diff --git a/.github/workflows/lint.yaml b/.github/workflows/lint.yaml index 5f38052..43c0d44 100644 --- a/.github/workflows/lint.yaml +++ b/.github/workflows/lint.yaml @@ -23,9 +23,7 @@ jobs: - name: Check lockfile run: uv lock --check - name: Lint with ruff - continue-on-error: true run: | - # Fail if codebase contains any of these issues uv run ruff check . uv run ruff format --check . - name: Static Typechecking with mypy diff --git a/pyproject.toml b/pyproject.toml index cadd473..38bd974 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -53,6 +53,14 @@ fallback-version = "0.0.0" line-length = 140 src = ["src", "tests"] target-version = "py310" +extend-exclude = [ + # Notebook submissions, whose cells start with tags such as "#grade" that the formatter would change. + "tests/scenario_root/**/*.ipynb", + # These files test how the grader handles syntax errors, so they can't be parsed. + "tests/scenario_root/test_helper_modules_syntax_error/helper_function.py", + "tests/scenario_root/test_module_sandbox_init_error/student_code.py", + "tests/scenario_root/test_syntax_error_ungradable/student_code.py", +] [tool.ruff.lint] @@ -61,7 +69,6 @@ ignore = [ "S101", # flake8-bandit assert "S308", # flake8-bandit suspicious-mark-safe-usage "E501", # pycodestyle line-too-long - "PT004", "PT011", "PT013", "S603", @@ -88,6 +95,13 @@ select = [ "W", # pycodestyle warnings ] +[tool.ruff.lint.per-file-ignores] +# numpy and pandas aren't installed in the lite image, so they're imported only when needed. +"src/pytest_prairielearn_grader/json_utils.py" = ["PLC0415"] +# Stand-ins for student code: they use names that the sandbox injects, import inside functions to +# test import restrictions, and evaluate expressions to test the sandbox. +"tests/scenario_root/**" = ["F821", "PLC0415", "B018"] + [tool.ruff.lint.flake8-pytest-style] fixture-parentheses = false mark-parentheses = false diff --git a/src/pytest_prairielearn_grader/_student_code_runner.py b/src/pytest_prairielearn_grader/_student_code_runner.py index 7e5b842..b7246e6 100644 --- a/src/pytest_prairielearn_grader/_student_code_runner.py +++ b/src/pytest_prairielearn_grader/_student_code_runner.py @@ -238,7 +238,7 @@ def load(self, name: str) -> types.ModuleType: # Add it before running it so that helper modules can import each other, as in sys.modules. self._loaded[name] = module try: - exec(compile(source, file_name, "exec"), module.__dict__) + exec(compile(source, file_name, "exec"), module.__dict__) # noqa: S102 except Exception as e: del self._loaded[name] raise ImportError(f"Helper module '{name}' could not be loaded: {type(e).__name__}: {e}") from e diff --git a/src/pytest_prairielearn_grader/config.py b/src/pytest_prairielearn_grader/config.py index f80804a..390a527 100644 --- a/src/pytest_prairielearn_grader/config.py +++ b/src/pytest_prairielearn_grader/config.py @@ -375,9 +375,7 @@ def __post_init__(self) -> None: # Validate output_level valid_output_levels = ("none", "message", "traceback", "friendly") if self.output_level is not None and self.output_level not in valid_output_levels: - raise ValueError( - f"output_level must be one of {valid_output_levels} or None, got: {self.output_level!r}" - ) + raise ValueError(f"output_level must be one of {valid_output_levels} or None, got: {self.output_level!r}") def to_dict(self) -> dict[str, Any]: """ diff --git a/src/pytest_prairielearn_grader/fixture.py b/src/pytest_prairielearn_grader/fixture.py index 253ebe5..35e496d 100644 --- a/src/pytest_prairielearn_grader/fixture.py +++ b/src/pytest_prairielearn_grader/fixture.py @@ -63,7 +63,7 @@ def _extract_notebook_code(notebook_path: Path, cell_tag: str | None = None) -> ImportError: If ``nbformat`` is not installed. """ try: - import nbformat # type: ignore[import-untyped] + import nbformat # type: ignore[import-untyped] # noqa: PLC0415 except ImportError: raise ImportError( "nbformat is required for Jupyter notebook grading support. Install it with: pip install 'pytest-prairielearn-grader[notebook]'" @@ -272,7 +272,8 @@ def try_drop_privileges() -> None: def _connect_to_server(self, timeout: float) -> None: """Read the host/port printed by the subprocess and connect the socket.""" - assert self.process is not None and self.process.stdout is not None + assert self.process is not None + assert self.process.stdout is not None host, port = self.process.stdout.readline().decode().strip().split(",") self.student_socket = socket.socket(socket.AF_INET, socket.SOCK_STREAM) self.student_socket.settimeout(timeout) diff --git a/src/pytest_prairielearn_grader/utils.py b/src/pytest_prairielearn_grader/utils.py index d89d100..cda4c3e 100644 --- a/src/pytest_prairielearn_grader/utils.py +++ b/src/pytest_prairielearn_grader/utils.py @@ -207,7 +207,7 @@ def deserialize_object_unsafe(base64_string: str) -> object: dilled_bytes = base64.b64decode(base64_encoded_bytes) # 3. Deserialize the object using dill - return dill.loads(dilled_bytes) + return dill.loads(dilled_bytes) # noqa: S301 def get_builtins(builtin_whitelist: list[str] | None) -> dict[str, Any]: @@ -331,7 +331,7 @@ def drop_privileges(user_name: str) -> None: raise NotImplementedError("Dropping privileges is not supported on Windows.") # pwd module is only available on Unix systems - import pwd + import pwd # noqa: PLC0415 try: # Get the UID and GID for the target user (e.g., 'nobody') @@ -339,7 +339,7 @@ def drop_privileges(user_name: str) -> None: target_uid = pwnam.pw_uid target_gid = pwnam.pw_gid except KeyError: - raise ValueError(f"User '{user_name}' not found.") + raise ValueError(f"User '{user_name}' not found.") from None # 1. Set the real and effective GID # NOTE: Set GID before UID, as changing UID may restrict GID changes diff --git a/tests/scenario_root/test_fixture_exception/scenario.py b/tests/scenario_root/test_fixture_exception/scenario.py index 07a2489..ffda8c8 100644 --- a/tests/scenario_root/test_fixture_exception/scenario.py +++ b/tests/scenario_root/test_fixture_exception/scenario.py @@ -1,4 +1,5 @@ import pytest + from pytest_prairielearn_grader.fixture import StudentFixture diff --git a/tests/scenario_root/test_function_not_found/student_code.py b/tests/scenario_root/test_function_not_found/student_code.py index 2ffff84..08e9e40 100644 --- a/tests/scenario_root/test_function_not_found/student_code.py +++ b/tests/scenario_root/test_function_not_found/student_code.py @@ -23,4 +23,4 @@ def raises_import_error(): def raises_name_error(): - return undefined_variable # noqa: F821 + return undefined_variable diff --git a/tests/scenario_root/test_initialization_timeout/scenario.py b/tests/scenario_root/test_initialization_timeout/scenario.py index b2eaf54..0176b8a 100644 --- a/tests/scenario_root/test_initialization_timeout/scenario.py +++ b/tests/scenario_root/test_initialization_timeout/scenario.py @@ -1,10 +1,10 @@ -# Module-level initialization timeout (used as default) -initialization_timeout = 0.5 - import pytest from pytest_prairielearn_grader.fixture import StudentFixture +# Module-level initialization timeout (used as default) +initialization_timeout = 0.5 + @pytest.mark.grading_data(name="uses_module_timeout", points=2) def test_with_module_timeout(sandbox: StudentFixture) -> None: diff --git a/tests/scenario_root/test_matplotlib_plots/student_code.py b/tests/scenario_root/test_matplotlib_plots/student_code.py index 72784f9..99c50d0 100644 --- a/tests/scenario_root/test_matplotlib_plots/student_code.py +++ b/tests/scenario_root/test_matplotlib_plots/student_code.py @@ -8,7 +8,7 @@ def create_line_plot(): """Create a simple line plot with proper labels and title.""" serializer = MatplotlibSerializer() - fig, ax = serializer.subplots() + _fig, ax = serializer.subplots() x = np.linspace(0, 2 * np.pi, 100) y = np.sin(x) ax.plot(x, y, label="sin(x)", color="blue", linewidth=2) @@ -23,7 +23,7 @@ def create_line_plot(): def create_scatter_plot(): """Create a scatter plot with random data.""" serializer = MatplotlibSerializer() - fig, ax = serializer.subplots() + _fig, ax = serializer.subplots() np.random.seed(42) x = np.random.randn(50) y = 2 * x + np.random.randn(50) * 0.5 @@ -37,7 +37,7 @@ def create_scatter_plot(): def create_bar_chart(): """Create a bar chart.""" serializer = MatplotlibSerializer() - fig, ax = serializer.subplots() + _fig, ax = serializer.subplots() categories = ["A", "B", "C", "D"] values = [23, 45, 56, 78] ax.bar(categories, values, color=["red", "green", "blue", "yellow"]) diff --git a/tests/scenario_root/test_quiz_2_1/scenario.py b/tests/scenario_root/test_quiz_2_1/scenario.py index a1e9af9..149f7eb 100644 --- a/tests/scenario_root/test_quiz_2_1/scenario.py +++ b/tests/scenario_root/test_quiz_2_1/scenario.py @@ -67,7 +67,8 @@ def test_0(sandbox: StudentFixture, feedback: FeedbackFixture, data_json: DataFi assert st_result is not None, "Your function did not return any value. Ensure you have a return statement." - assert isinstance(st_result, list) and len(st_result) == 3, "Your function should return a tuple with exactly three elements." + assert isinstance(st_result, list), "Your function should return a tuple with exactly three elements." + assert len(st_result) == 3, "Your function should return a tuple with exactly three elements." points = 0 diff --git a/tests/scenario_root/test_quiz_2_1/setup_code.py b/tests/scenario_root/test_quiz_2_1/setup_code.py index efafff1..5c9bf72 100644 --- a/tests/scenario_root/test_quiz_2_1/setup_code.py +++ b/tests/scenario_root/test_quiz_2_1/setup_code.py @@ -1,7 +1,8 @@ +import sympy as sp + + def not_allowed(*args, **kwargs): raise RuntimeError("Calling sympy.series is not allowed in this question. You can still use sympy to compute derivatives") -import sympy as sp - sp.series = not_allowed diff --git a/tests/scenario_root/test_quiz_3_1/scenario.py b/tests/scenario_root/test_quiz_3_1/scenario.py index 4a72476..06f8d38 100644 --- a/tests/scenario_root/test_quiz_3_1/scenario.py +++ b/tests/scenario_root/test_quiz_3_1/scenario.py @@ -31,7 +31,7 @@ def compute_starting_parameters() -> tuple[list[str], list[ParameterDict]]: continue hours_set.add(hours) rng.shuffle(members) - sale_metadata.append({"hours": list(zip(members, hours)), "units_sold": rng.integers(100, 2001)}) + sale_metadata.append({"hours": list(zip(members, hours, strict=True)), "units_sold": rng.integers(100, 2001)}) return members, sale_metadata @@ -64,7 +64,7 @@ def test_0(sandbox: StudentFixture, feedback: FeedbackFixture) -> None: ref_member_ratings, _, _ = compute_answers(members, sale_metadata) if student_member_ratings is not None: - if type(student_member_ratings) != dict: + if not isinstance(student_member_ratings, dict): feedback.add_message("member_ratings is not a dictionary") else: for member, rating in student_member_ratings.items(): diff --git a/tests/scenario_root/test_quiz_3_1/student_code_0.py b/tests/scenario_root/test_quiz_3_1/student_code_0.py index f863d88..b3e2066 100644 --- a/tests/scenario_root/test_quiz_3_1/student_code_0.py +++ b/tests/scenario_root/test_quiz_3_1/student_code_0.py @@ -20,6 +20,6 @@ def solve_ratings(members, sale_metadata): ratings = la.solve(theMatrix, sales) highest_performing = np.max(ratings) lowest_performing = np.min(ratings) - member_ratings = dict(zip(members, ratings)) + member_ratings = dict(zip(members, ratings, strict=True)) return member_ratings, highest_performing, lowest_performing diff --git a/tests/scenario_root/test_quiz_3_1/student_code_100.py b/tests/scenario_root/test_quiz_3_1/student_code_100.py index 9f5d20f..24afe62 100644 --- a/tests/scenario_root/test_quiz_3_1/student_code_100.py +++ b/tests/scenario_root/test_quiz_3_1/student_code_100.py @@ -13,6 +13,6 @@ def solve_ratings(members, sale_metadata): ratings = la.solve(theMatrix, sales) highest_performing = members[np.argmax(ratings)] lowest_performing = members[np.argmin(ratings)] - member_ratings = dict(zip(members, ratings)) + member_ratings = dict(zip(members, ratings, strict=True)) return member_ratings, highest_performing, lowest_performing diff --git a/tests/scenario_root/test_quiz_3_1/student_code_66.py b/tests/scenario_root/test_quiz_3_1/student_code_66.py index 4e531b5..ea425f3 100644 --- a/tests/scenario_root/test_quiz_3_1/student_code_66.py +++ b/tests/scenario_root/test_quiz_3_1/student_code_66.py @@ -13,6 +13,6 @@ def solve_ratings(members, sale_metadata): ratings = la.solve(theMatrix, sales) highest_performing = np.max(ratings) lowest_performing = np.min(ratings) - member_ratings = dict(zip(members, ratings)) + member_ratings = dict(zip(members, ratings, strict=True)) return member_ratings, highest_performing, lowest_performing diff --git a/tests/scenario_root/test_timeout/scenario.py b/tests/scenario_root/test_timeout/scenario.py index c0751f8..d841f53 100644 --- a/tests/scenario_root/test_timeout/scenario.py +++ b/tests/scenario_root/test_timeout/scenario.py @@ -1,12 +1,12 @@ -# Module-level initialization timeout (used as default when no marker is present) -initialization_timeout = 0.5 - import pytest from pytest_prairielearn_grader.fixture import FeedbackFixture from pytest_prairielearn_grader.fixture import StudentFixture from pytest_prairielearn_grader.utils import FunctionStatusCode +# Module-level initialization timeout (used as default when no marker is present) +initialization_timeout = 0.5 + @pytest.mark.grading_data(name="initialization_timeout", points=2) @pytest.mark.sandbox_timeout(0.05) diff --git a/tests/scenario_root/test_workspace_function_not_found/student/calculator.py b/tests/scenario_root/test_workspace_function_not_found/student/calculator.py index 7230100..c4b4b14 100644 --- a/tests/scenario_root/test_workspace_function_not_found/student/calculator.py +++ b/tests/scenario_root/test_workspace_function_not_found/student/calculator.py @@ -14,4 +14,4 @@ def raises_import_error(): def imports_missing_module(): - import no_such_module_inside_function # noqa: F401, PLC0415 + import no_such_module_inside_function # noqa: F401 diff --git a/tests/scenario_root/test_workspace_function_not_found/student/raises_attribute_error_on_load.py b/tests/scenario_root/test_workspace_function_not_found/student/raises_attribute_error_on_load.py index 4b97f9c..c5a4d3d 100644 --- a/tests/scenario_root/test_workspace_function_not_found/student/raises_attribute_error_on_load.py +++ b/tests/scenario_root/test_workspace_function_not_found/student/raises_attribute_error_on_load.py @@ -1,2 +1,2 @@ missing = None -missing.attribute # noqa: B018 +missing.attribute diff --git a/tests/test_autograder_scenarios.py b/tests/test_autograder_scenarios.py index 096b817..259cce1 100644 --- a/tests/test_autograder_scenarios.py +++ b/tests/test_autograder_scenarios.py @@ -1,3 +1,4 @@ +import json import math import platform import shutil @@ -9,7 +10,6 @@ pytest_plugins = ("pytester",) platform # noqa: B018 -import json # --- Configuration --- # The root directory where your scenario folders are located @@ -116,9 +116,7 @@ def test_autograder_scenario_with_pytester(pytester: pytest.Pytester, scenario_d # Check gradable field expected_gradable = expected_data_obj.get("gradable", True) actual_gradable = results_obj.get("gradable", True) - assert expected_gradable == actual_gradable, ( - f"Gradable mismatch: expected {expected_gradable}, got {actual_gradable}" - ) + assert expected_gradable == actual_gradable, f"Gradable mismatch: expected {expected_gradable}, got {actual_gradable}" # If submission is ungradable, check format_errors and message but skip score/test checks if not expected_gradable: diff --git a/tests/test_serialize.py b/tests/test_serialize.py index 9a5c9fa..bc1f592 100644 --- a/tests/test_serialize.py +++ b/tests/test_serialize.py @@ -1,12 +1,13 @@ -from pathlib import Path import subprocess import sys +from pathlib import Path from typing import Any from typing import cast import numpy as np import pandas as pd import pytest +import tomllib from pytest_prairielearn_grader.json_utils import from_json from pytest_prairielearn_grader.json_utils import from_server_json @@ -111,9 +112,6 @@ def test_from_server_json_networkx_graph() -> None: def test_optional_dependencies_are_declared() -> None: - import tomllib - from pathlib import Path - project = tomllib.loads((Path(__file__).parents[1] / "pyproject.toml").read_text())["project"] requirements = project["dependencies"] From 001effc0cefd66680b94cec3d586ac43a13c7528 Mon Sep 17 00:00:00 2001 From: Eliot Robson Date: Wed, 11 Nov 2026 01:00:53 -0500 Subject: [PATCH 2/2] Escape the match pattern in the blacklisted-function test Co-Authored-By: Claude Opus 5.5 --- tests/test_import_blacklist.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/test_import_blacklist.py b/tests/test_import_blacklist.py index 6632173..d04911b 100644 --- a/tests/test_import_blacklist.py +++ b/tests/test_import_blacklist.py @@ -1,5 +1,6 @@ """How import_blacklist entries match the module an import would load.""" +import re import sys import types @@ -39,7 +40,7 @@ def test_block_blacklisted_functions(monkeypatch: pytest.MonkeyPatch) -> None: _block_blacklisted_functions(["os", "fake_linalg.inv", "fake_linalg.EPSILON", "missing_module.f"]) - with pytest.raises(RuntimeError, match="'fake_linalg.inv' is blacklisted and cannot be used."): + with pytest.raises(RuntimeError, match=re.escape("'fake_linalg.inv' is blacklisted and cannot be used.")): attributes["inv"]([[1.0]]) assert attributes["solve"]([[1.0]], [2.0]) == [2.0] assert attributes["EPSILON"] == 1e-8