diff --git a/source/vscode/resources/qdk-learning/courses/chemistry-qpe/_course_lib.py b/source/vscode/resources/qdk-learning/courses/chemistry-qpe/_course_lib.py index 36280b65d8c..33e7ae86d9b 100644 --- a/source/vscode/resources/qdk-learning/courses/chemistry-qpe/_course_lib.py +++ b/source/vscode/resources/qdk-learning/courses/chemistry-qpe/_course_lib.py @@ -16,6 +16,7 @@ from typing import Callable +from IPython.core.getipython import get_ipython from IPython.display import HTML, display # A learner exercise is a no-argument function whose result is checked. @@ -31,8 +32,21 @@ class ExerciseError(AssertionError): """Raised when an exercise is not yet correct.""" - def _render_traceback_(self) -> list[str]: - return [] + +def _hide_traceback() -> None: + """Show only the failure banner for a wrong answer. + + The cell still ends in an error, which marks the exercise as incomplete, + but real errors raised by learner code keep their traceback. + """ + shell = get_ipython() + if shell is None: + return + + shell.set_custom_exc((ExerciseError,), lambda *args, **kwargs: None) + + +_hide_traceback() def _register(name: str, checker: Checker) -> str: diff --git a/source/vscode/test/course-notebooks/notebook_runner.py b/source/vscode/test/course-notebooks/notebook_runner.py index b72c228a907..f05876d0867 100644 --- a/source/vscode/test/course-notebooks/notebook_runner.py +++ b/source/vscode/test/course-notebooks/notebook_runner.py @@ -65,24 +65,29 @@ def clear_notebook_outputs(notebook: NotebookNode) -> None: cell.metadata.pop("execution", None) -def collect_cell_failures(notebook: NotebookNode) -> list[CellFailure]: +def collect_cell_failures( + notebook: NotebookNode, + execution_errors: dict[int, NotebookNode], +) -> list[CellFailure]: failures: list[CellFailure] = [] - for cell_number, cell in enumerate(notebook.cells, start=1): + for cell_index, cell in enumerate(notebook.cells): if cell.cell_type != "code": continue + cell_number = cell_index + 1 tags = set(cell.metadata.get("tags", [])) source_line = _first_source_line(cell.source) if SKIP_TEST_TAG in tags: continue - errors = [ + error = execution_errors.get(cell_index) + visible_errors = [ output for output in cell.get("outputs", []) if output.get("output_type") == "error" ] if EXERCISE_TAG in tags: - if not errors: + if error is None: failures.append( CellFailure( cell_number, @@ -90,24 +95,32 @@ def collect_cell_failures(notebook: NotebookNode) -> list[CellFailure]: "exercise cell did not raise ExerciseError", ) ) - elif errors[0].get("ename") != "ExerciseError": + elif error.get("ename") != "ExerciseError": failures.append( CellFailure( cell_number, source_line, - "exercise cell raised " + _format_error(errors[0]), + "exercise cell raised " + _format_error(error), + ) + ) + elif visible_errors: + failures.append( + CellFailure( + cell_number, + source_line, + "exercise cell displayed duplicate error output", ) ) continue - failures.extend( - CellFailure( - cell_number, - source_line, - "unexpected error: " + _format_error(error), + if error is not None: + failures.append( + CellFailure( + cell_number, + source_line, + "unexpected error: " + _format_error(error), + ) ) - for error in errors - ) return failures @@ -118,6 +131,12 @@ def run_notebook( ) -> NotebookRunReport: notebook = nbformat.read(notebook_path, as_version=4) clear_notebook_outputs(notebook) + execution_errors: dict[int, NotebookNode] = {} + + def record_cell_error( + *, cell: NotebookNode, cell_index: int, execute_reply: NotebookNode + ) -> None: + execution_errors[cell_index] = execute_reply["content"] started = perf_counter() kernel_manager = AsyncKernelManager( @@ -135,6 +154,7 @@ def run_notebook( resources={"metadata": {"path": str(notebook_path.parent)}}, skip_cells_with_tag=SKIP_TEST_TAG, store_widget_state=False, + on_cell_error=record_cell_error, ).execute(cleanup_kc=True) elapsed_seconds = perf_counter() - started @@ -162,7 +182,7 @@ def run_notebook( executed_cells, skipped_cells, slow_cells, - tuple(collect_cell_failures(notebook)), + tuple(collect_cell_failures(notebook, execution_errors)), ) print_notebook_report(report) return report diff --git a/source/vscode/test/course-notebooks/test_notebook_runner.py b/source/vscode/test/course-notebooks/test_notebook_runner.py index 9745acea72d..9ab0ff8c345 100644 --- a/source/vscode/test/course-notebooks/test_notebook_runner.py +++ b/source/vscode/test/course-notebooks/test_notebook_runner.py @@ -13,68 +13,114 @@ def _notebook(*cells: NotebookNode) -> NotebookNode: def _code_cell( *, tags: Iterable[str] = (), - error: tuple[str, str] | None = None, ) -> NotebookNode: - cell = nbformat.v4.new_code_cell("answer = 42", metadata={"tags": list(tags)}) - if error is not None: - name, value = error - cell.outputs = [ - nbformat.v4.new_output( - "error", - ename=name, - evalue=value, - traceback=[], - ) - ] - return cell - - -def test_exercise_requires_exercise_error() -> None: - notebook = _notebook( - _code_cell(tags=["exercise"], error=("ExerciseError", "try again")) + return nbformat.v4.new_code_cell( + "answer = 42", + metadata={"tags": list(tags)}, ) - assert collect_cell_failures(notebook) == [] + +def _execution_error(name: str, value: str) -> NotebookNode: + return NotebookNode( + { + "ename": name, + "evalue": value, + "traceback": [], + } + ) + + +def test_hidden_exercise_error_satisfies_policy() -> None: + cell = _code_cell(tags=["exercise"]) + cell.outputs = [ + nbformat.v4.new_output( + "display_data", + data={"text/html": "try again"}, + metadata={}, + ) + ] + notebook = _notebook(cell) + + failures = collect_cell_failures( + notebook, + {0: _execution_error("ExerciseError", "try again")}, + ) + + assert failures == [] + assert [output.output_type for output in cell.outputs] == ["display_data"] + + +def test_displayed_exercise_error_fails_policy() -> None: + cell = _code_cell(tags=["exercise"]) + cell.outputs = [ + nbformat.v4.new_output( + "display_data", + data={"text/html": "try again"}, + metadata={}, + ), + nbformat.v4.new_output( + "error", + ename="ExerciseError", + evalue="try again", + traceback=[], + ), + ] + notebook = _notebook(cell) + + failures = collect_cell_failures( + notebook, + {0: _execution_error("ExerciseError", "try again")}, + ) + + assert len(failures) == 1 + assert failures[0].message == "exercise cell displayed duplicate error output" def test_exercise_that_succeeds_fails_policy() -> None: notebook = _notebook(_code_cell(tags=["exercise"])) - failures = collect_cell_failures(notebook) + failures = collect_cell_failures(notebook, {}) assert len(failures) == 1 assert failures[0].message == "exercise cell did not raise ExerciseError" def test_exercise_with_wrong_error_fails_policy() -> None: - notebook = _notebook( - _code_cell(tags=["exercise"], error=("ValueError", "bad value")) - ) + notebook = _notebook(_code_cell(tags=["exercise"])) - failures = collect_cell_failures(notebook) + failures = collect_cell_failures( + notebook, + {0: _execution_error("ValueError", "bad value")}, + ) assert len(failures) == 1 assert failures[0].message == "exercise cell raised ValueError: bad value" def test_ordinary_cell_error_fails_policy() -> None: - notebook = _notebook(_code_cell(error=("RuntimeError", "broken"))) + notebook = _notebook(_code_cell()) - failures = collect_cell_failures(notebook) + failures = collect_cell_failures( + notebook, + {0: _execution_error("RuntimeError", "broken")}, + ) assert len(failures) == 1 assert failures[0].message == "unexpected error: RuntimeError: broken" def test_skip_test_cell_is_not_evaluated() -> None: - notebook = _notebook( - _code_cell(tags=["skip-test"], error=("RuntimeError", "ignored")) + notebook = _notebook(_code_cell(tags=["skip-test"])) + + failures = collect_cell_failures( + notebook, + {0: _execution_error("RuntimeError", "ignored")}, ) - assert collect_cell_failures(notebook) == [] + assert failures == [] def test_skipped_exercise_is_not_evaluated() -> None: notebook = _notebook(_code_cell(tags=["exercise", "skip-test"])) - assert collect_cell_failures(notebook) == [] + assert collect_cell_failures(notebook, {}) == []