Auto-ignore test fn calls in assign exception-based test cells (#867)

CHANGELOG.md

@@ -26,6 +26,7 @@
 * Fixed OK-test support for the `all_or_nothing` config per [#751](https://github.com/ucbds-infra/otter-grader/issues/751)
 * Added exception-based test support for the `all_or_nothing` config per [#751](https://github.com/ucbds-infra/otter-grader/issues/751)
 * Added Otter Assign support for the `all_or_nothing` config per [#751](https://github.com/ucbds-infra/otter-grader/issues/751)
+* Updated Otter Assign to only allow a function definition and statement to call the test function in exception-based test cells and automatically ignore the latter statement instead of requiring an explicit `# IGNORE` comment per [#516](https://github.com/ucbds-infra/otter-grader/issues/516)
 
 **v5.7.1:**
 

docs/otter_assign/notebook_format.rst

@@ -462,10 +462,11 @@ Exception-Based Test Cells
 
 To use Otter's exception-based tests, you must set ``tests: ok_format: false`` in your assignment 
 config. Your test cells should define
-a test case function as described :ref:`here <test_files_python_exception_based>`. You can run the
-test in the master notebook by calling the function, but you should make  sure that this call is 
-"ignored" by Otter Assign so that it's not included in the test file by appending ``# IGNORE`` to the
-end of line. You should *not* add the ``test_case`` decorator; Otter Assign will do this for you. 
+a test case function as described :ref:`here <test_files_python_exception_based>`. You can also run the
+test in the master notebook by calling the function. However, no other statements besides a single
+``def`` and (optionally) a call to the test function may be included in the cell; Otter enforces
+this by parsing the cell's code into an AST, which it uses to identify the test case function.
+You should *not* add the ``test_case`` decorator; Otter Assign will do this for you. 
 
 For example,
 
@@ -478,7 +479,7 @@ For example,
         assert len(arr) == 10
         assert (0 <= arr <= 1).all()
 
-    test_validity(arr)  # IGNORE
+    test_validity(arr)
 
 It is important to note that the exception-based test files are executed before the student's global
 environment is provided, so no work should be performed outside the test case function that relies
@@ -585,13 +586,13 @@ Ignoring Cells
 --------------
 
 For any cells that you don't want to be included in *either* of the output notebooks that are 
-present in the master notebook, include a line at the top of the cell with the ``## Ignore ##`` 
+present in the master notebook, include a line at the top of the cell with the ``# IGNORE`` 
 comment (case insensitive) just like with test cells. Note that this also works for Markdown cells 
 with the same syntax.
 
 .. code-block:: python
 
-    ## Ignore ##
+    # IGNORE
     print("This cell won't appear in the output.")
 
 

otter/assign/tests_manager.py

@@ -14,7 +14,6 @@
 
 from .assignment import Assignment
 from .question_config import QuestionConfig
-from .solutions import remove_ignored_lines
 from .utils import str_to_doctest
 from ..nbmeta_config import NBMetadataConfig, OK_FORMAT_VARNAME
 from ..test_files.abstract_test import TestCase, TestFile
@@ -38,6 +37,7 @@
     success_message="{{ tc.success_message }}"{% endif %}{% if tc.failure_message is not none %}, 
     failure_message="{{ tc.failure_message }}"{% endif %})
 {{ tc.body }}
+
 {% endfor %}
 """
 )
@@ -151,12 +151,22 @@ def read_test(self, cell: nbf.NotebookNode, question: QuestionConfig):
         success_message = config.get("success_message", None)
         failure_message = config.get("failure_message", None)
 
-        body: str = "\n".join(remove_ignored_lines(get_source(cell)[test_start_line + 1 :]))
+        test_source = get_source(cell)[test_start_line + 1 :]
+        test_ast = ast.parse("\n".join(test_source))
+        body: str = ""
         if self.assignment.tests.ok_format:
-            inp = ast.unparse(_AnnotationRemover().visit(ast.parse(body)))
+            inp = ast.unparse(_AnnotationRemover().visit(test_ast))
             body_lines = str_to_doctest(inp.split("\n"), [])
             body_lines.extend(output.split("\n"))
             body = "\n".join(body_lines)
+        else:
+            if len(test_ast.body) > 2 or not isinstance(test_ast.body[0], ast.FunctionDef):
+                raise ValueError(
+                    f"Error in question {question.name}: an exception-based test cell may have at "
+                    "most a function definition and a call to that function definition but a test "
+                    f"cell with {len(test_ast.body)} nodes in its AST was encountered"
+                )
+            body = ast.unparse(test_ast.body[0])
 
         self._add_test_case(
             question,

test/test_assign/files/exception-correct/autograder/tests/q8.py

@@ -11,6 +11,5 @@ def test_len(z):
 
 @test_case(points=None, hidden=True)
 def test_values(np, z):
-    assert np.allclose(z, [4.99342831, 3.7234714 , 5.29537708, 7.04605971, 3.53169325,
-       3.53172609, 7.15842563, 5.53486946, 3.06105123, 5.08512009])
+    assert np.allclose(z, [4.99342831, 3.7234714, 5.29537708, 7.04605971, 3.53169325, 3.53172609, 7.15842563, 5.53486946, 3.06105123, 5.08512009])
 

test/test_assign/files/exception-example.ipynb

@@ -90,7 +90,7 @@
     "    assert isinstance(x, int)\n",
     "    assert 0 < x < 100\n",
     "\n",
-    "test_type(x)  # IGNORE"
+    "test_type(x)"
    ]
   },
   {
@@ -107,7 +107,7 @@
     "def test_value(x):\n",
     "    assert x == 2\n",
     "\n",
-    "test_value(x)  # IGNORE"
+    "test_value(x)"
    ]
   },
   {
@@ -277,7 +277,7 @@
     "    assert nine == 9\n",
     "    assert square(16) == 256\n",
     "\n",
-    "test_public(nine, square)  # IGNORE"
+    "test_public(nine, square)"
    ]
   },
   {
@@ -290,7 +290,7 @@
     "def test_hidden(square):\n",
     "    assert square(1) == 1\n",
     "\n",
-    "test_hidden(square)  # IGNORE"
+    "test_hidden(square)"
    ]
   },
   {
@@ -602,7 +602,7 @@
     "def test_len(z):\n",
     "    assert len(z) == 10\n",
     "\n",
-    "test_len(z)  # IGNORE"
+    "test_len(z)"
    ]
   },
   {
@@ -616,7 +616,7 @@
     "    assert np.allclose(z, [4.99342831, 3.7234714 , 5.29537708, 7.04605971, 3.53169325,\n",
     "       3.53172609, 7.15842563, 5.53486946, 3.06105123, 5.08512009])\n",
     "\n",
-    "test_values(np, z)  # IGNORE"
+    "test_values(np, z)"
    ]
   },
   {