Allow upload of submission PDF

CHANGELOG.md

@@ -4,6 +4,8 @@
 
 * Updated Otter Assign to throw a `ValueError` when invalid Python code is encountered in test cells per [#756](https://github.com/ucbds-infra/otter-grader/issues/756)
 * Fixed an error causing intercell seeding code to be added to cells using cell magic commands which caused syntax errors per [#754](https://github.com/ucbds-infra/otter-grader/issues/754)
+* Add support for submitting the PDF uploaded in a submission for manual grading instead of generating a new one per [#764](https://github.com/ucbds-infra/otter-grader/issues/764)
+* Validate that a course ID and assignment ID were provided before attempting PDF upload
 
 **v5.2.3:**
 

otter/run/run_autograder/autograder_config.py

@@ -69,12 +69,12 @@ class AutograderConfig(fica.Config):
 
     course_id = fica.Key(
         description="a Gradescope course ID for uploading a PDF of the notebook",
-        default="None",
+        default=None,
     )
 
     assignment_id = fica.Key(
         description="a Gradescope assignment ID for uploading a PDF of the notebook",
-        default="None",
+        default=None,
     )
 
     filtering = fica.Key(
@@ -163,5 +163,12 @@ class AutograderConfig(fica.Config):
         default=False,
     )
 
+    use_submission_pdf = fica.Key(
+        description="use the PDF in the submission zip file instead of exporting a new one; if " \
+            "no PDF is present, a new one is generated anyway; assumes there is only 1 PDF file " \
+            "in the submission",
+        default=False,
+    )
+
     _otter_run = False
     """whether this autograder run is being run by Otter Run (i.e. without containerization)"""

otter/run/run_autograder/runners/abstract_runner.py

@@ -6,6 +6,7 @@
 import tempfile
 
 from abc import ABC, abstractmethod
+from glob import glob
 
 from ..autograder_config import AutograderConfig
 from ..utils import OtterRuntimeError, print_output, write_blank_page_to_stare_at_before_you
@@ -100,7 +101,11 @@
                 appended if needed)
         """
         try:
-            pdf_path = self.write_pdf(submission_path)
+            subm_pdfs = glob("*.pdf")
+            if self.ag_config.use_submission_pdf and subm_pdfs:
+                pdf_path = subm_pdfs[0]
+            else:
+                pdf_path = self.write_pdf(submission_path)
 
             if submit:
                 self.submit_pdf(client, pdf_path)
@@ -132,6 +137,12 @@
         for user in metadata.get("users", []):
             student_emails.append(user["email"])
 
+        # validate that a course and assignment ID were specified
+        if self.ag_config.course_id is None:
+            raise OtterRuntimeError("PDF upload course ID not specified")
+        if self.ag_config.assignment_id is None:
+            raise OtterRuntimeError("PDF upload assignment ID not specified")
+
         for student_email in student_emails:
             res = client.upload_pdf_submission(
                 self.ag_config.course_id,

test/test_check/test_notebook.py

@@ -218,7 +218,8 @@ def test_export_with_no_pdf_ack(
     mocked_dt.datetime.now.return_value = timestmap
     mocked_export.return_value = FILE_MANAGER.get_path(f"{NB_PATH_STEM}.pdf")
 
-    grader.export()
+    with pytest.warns(UserWarning, match="Could not locate a PDF to include"):
+        grader.export()
 
     mocked_export.assert_called_with(NB_PATH, filtering=True, pagebreaks=True)
 

test/test_run/files/autograder/source/otter_config.json

@@ -1 +1 @@
-{"autograder_dir": "test/test_run/files/autograder", "plugins": [{"otter.plugins.builtin.RateLimiting": {"days": 1, "allowed_submissions": 1}}], "warn_missing_pdf": true, "token": "abc123", "show_hidden": false, "force_public_test_summary": false}
+{"autograder_dir": "test/test_run/files/autograder", "plugins": [{"otter.plugins.builtin.RateLimiting": {"days": 1, "allowed_submissions": 1}}], "token": "abc123", "show_hidden": false, "force_public_test_summary": false, "course_id": "12345", "assignment_id": "67890"}

test/test_run/files/autograder/submission_metadata.json

@@ -1,4 +1,9 @@
 {
     "created_at": "2020-01-01T01:01:01.000-0700",
-    "previous_submissions": []
+    "previous_submissions": [],
+    "users": [
+        {
+            "email": "student@univ.edu"
+        }
+    ]
 }

test/test_run/test_integration.py

@@ -11,6 +11,7 @@
 from contextlib import contextmanager, nullcontext
 from unittest import mock
 
+from otter.generate.token import APIClient
 from otter.run.run_autograder import main as run_autograder
 from otter.run.run_autograder.utils import OtterRuntimeError
 from otter.utils import NBFORMAT_VERSION
@@ -223,6 +224,38 @@ def test_pdf_generation_failure(mocked_export, get_config_path, load_config, exp
         f"Actual results did not matched expected:\n{actual_results}"
 
 
+@mock.patch.object(APIClient, "upload_pdf_submission")
+@mock.patch("otter.run.run_autograder.runners.python_runner.export_notebook")
+def test_use_submission_pdf(
+    mocked_export,
+    mocked_upload_pdf_submission,
+    get_config_path,
+    load_config,
+    expected_results,
+):
+    config = load_config()
+    config["use_submission_pdf"] = True
+    config["token"] = "abc123"
+
+    FILE_MANAGER.open("autograder/submission/fails2and6H.pdf", "wb+").close()
+
+    with alternate_config(get_config_path(), config):
+        run_autograder(config['autograder_dir'])
+
+    with FILE_MANAGER.open("autograder/results/results.json") as f:
+        actual_results = json.load(f)
+
+    mocked_export.assert_not_called()
+    mocked_upload_pdf_submission.assert_called_with(
+        config["course_id"],
+        config["assignment_id"],
+        "student@univ.edu", # from submission_metadata.json in autograder dir
+        "fails2and6H.pdf",
+    )
+    assert actual_results == expected_results, \
+        f"Actual results did not matched expected:\n{actual_results}"
+
+
 def test_force_public_test_summary(get_config_path, load_config):
     config = load_config()
 

test/utils.py

@@ -25,11 +25,8 @@ def __init__(self, test_file_path):
     def get_path(self, path):
         return str(self.dir / path)
 
-    @contextmanager
     def open(self, path, *args, **kwargs):
-        f = open(self.get_path(path), *args, **kwargs)
-        yield f
-        f.close()
+        return open(self.get_path(path), *args, **kwargs)
 
     def assert_path_exists(self, path, file_okay=True, dir_okay=True):
         path = self.get_path(path)