Make sure that tests fail in case there is an unhandled Python error (#1456)

Before this PR, the following test passed:

    def test_pyscript_hello(self):
        self.pyscript_run(
            """
            <script type="py">
                raise Exception("hello")
            </script>
            """)

What happens is that we intercept the Python exception and display a nice banner on the DOM, but the test itself passes. This is error prone: if we have Python exceptions on the page, the test should fail by default, and we should have a way to silence it in case those exceptions are expected.

This PR treats Python errors as we treat JS errors: unhandled exceptions cause the test to fail, but you can silence them by calling self.check_py_errors(), exactly as you can call self.check_js_errors().

pyscriptjs/tests/integration/support.py

@@ -208,6 +208,7 @@ class PyScriptTest:
 
         self.console = ConsoleMessageCollection(self.logger)
         self._js_errors = []
+        self._py_errors = []
         page.on("console", self._on_console)
         page.on("pageerror", self._on_pageerror)
 
@@ -243,17 +244,26 @@ class PyScriptTest:
         # page and they should not cause the test to fail, you should call
         # self.check_js_errors() in the test itself.
         self.check_js_errors()
+        self.check_py_errors()
 
     def _on_console(self, msg):
+        if msg.type == "error" and "Traceback (most recent call last)" in msg.text:
+            # this is a Python traceback, let's record it as a py_error
+            self._py_errors.append(msg.text)
         self.console.add_message(msg.type, msg.text)
 
     def _on_pageerror(self, error):
-        self.console.add_message("js_error", error.stack)
+        # apparently, playwright Error.stack contains all the info that we
-        self._js_errors.append(error)
+        # want: exception name, message and stacktrace. The docs say that
+        # error.stack is optional, so fallback to the standard repr if it's
+        # unavailable.
+        error_msg = error.stack or str(error)
+        self.console.add_message("js_error", error_msg)
+        self._js_errors.append(error_msg)
 
-    def check_js_errors(self, *expected_messages):
+    def _check_page_errors(self, kind, expected_messages):
         """
-        Check whether JS errors were reported.
+        Check whether the page raised any 'JS' or 'Python' error.
 
         expected_messages is a list of strings of errors that you expect they
         were raised in the page.  They are checked using a simple 'in' check,
@@ -261,40 +271,62 @@ class PyScriptTest:
             if expected_message in actual_error_message:
                 ...
 
-        If an error was expected but not found, it raises
+        If an error was expected but not found, it raises PageErrorsDidNotRaise.
-        DidNotRaiseJsError().
 
-        If there are MORE errors other than the expected ones, it raises JsErrors.
+        If there are MORE errors other than the expected ones, it raises PageErrors.
 
         Upon return, all the errors are cleared, so a subsequent call to
-        check_js_errors will not raise, unless NEW JS errors have been reported
+        check_{js,py}_errors will not raise, unless NEW errors have been reported
         in the meantime.
         """
+        assert kind in ("JS", "Python")
+        if kind == "JS":
+            actual_errors = self._js_errors[:]
+        else:
+            actual_errors = self._py_errors[:]
         expected_messages = list(expected_messages)
-        js_errors = self._js_errors[:]
 
         for i, msg in enumerate(expected_messages):
-            for j, error in enumerate(js_errors):
+            for j, error in enumerate(actual_errors):
-                if msg is not None and error is not None and msg in error.message:
+                if msg is not None and error is not None and msg in error:
                     # we matched one expected message with an error, remove both
                     expected_messages[i] = None
-                    js_errors[j] = None
+                    actual_errors[j] = None
 
-        # if everything is find, now expected_messages and js_errors contains
+        # if everything is find, now expected_messages and actual_errors contains
         # only Nones. If they contain non-None elements, it means that we
-        # either have messages which are expected-but-not-found or errors
+        # either have messages which are expected-but-not-found or
-        # which are found-but-not-expected.
+        # found-but-not-expected.
-        expected_messages = [msg for msg in expected_messages if msg is not None]
+        not_found = [msg for msg in expected_messages if msg is not None]
-        js_errors = [err for err in js_errors if err is not None]
+        unexpected = [err for err in actual_errors if err is not None]
-        self.clear_js_errors()
 
-        if expected_messages:
+        if kind == "JS":
+            self.clear_js_errors()
+        else:
+            self.clear_py_errors()
+
+        if not_found:
             # expected-but-not-found
-            raise JsErrorsDidNotRaise(expected_messages, js_errors)
+            raise PageErrorsDidNotRaise(kind, not_found, unexpected)
-
+        if unexpected:
-        if js_errors:
             # found-but-not-expected
-            raise JsErrors(js_errors)
+            raise PageErrors(kind, unexpected)
+
+    def check_js_errors(self, *expected_messages):
+        """
+        Check whether JS errors were reported.
+
+        See the docstring for _check_page_errors for more details.
+        """
+        self._check_page_errors("JS", expected_messages)
+
+    def check_py_errors(self, *expected_messages):
+        """
+        Check whether Python errors were reported.
+
+        See the docstring for _check_page_errors for more details.
+        """
+        self._check_page_errors("Python", expected_messages)
 
     def clear_js_errors(self):
         """
@@ -302,6 +334,9 @@ class PyScriptTest:
         """
         self._js_errors = []
 
+    def clear_py_errors(self):
+        self._py_errors = []
+
     def writefile(self, filename, content):
         """
         Very thin helper to write a file in the tmpdir
@@ -614,54 +649,37 @@ def wait_for_render(page, selector, pattern):
     assert py_rendered  # nosec
 
 
-class JsErrors(Exception):
+class PageErrors(Exception):
     """
-    Represent one or more exceptions which happened in JS.
+    Represent one or more exceptions which happened in JS or Python.
-
-    It's a thin wrapper around playwright.sync_api.Error, with two important
-    differences:
-
-    1. it has a better name: if you see JsError in a traceback, it's
-       immediately obvious that it's a JS exception.
-
-    2. Show also the JS stacktrace by default, contrarily to
-       playwright.sync_api.Error
     """
 
-    def __init__(self, errors):
+    def __init__(self, kind, errors):
+        assert kind in ("JS", "Python")
         n = len(errors)
         assert n != 0
-        lines = [f"JS errors found: {n}"]
+        lines = [f"{kind} errors found: {n}"]
-        for err in errors:
+        lines += errors
-            lines.append(self.format_playwright_error(err))
         msg = "\n".join(lines)
         super().__init__(msg)
         self.errors = errors
 
-    @staticmethod
-    def format_playwright_error(error):
-        # apparently, playwright Error.stack contains all the info that we
-        # want: exception name, message and stacktrace. The docs say that
-        # error.stack is optional, so fallback to the standard repr if it's
-        # unavailable.
-        return error.stack or str(error)
 
-
+class PageErrorsDidNotRaise(Exception):
-class JsErrorsDidNotRaise(Exception):
     """
-    Exception raised by check_js_errors when the expected JS error messages
+    Exception raised by check_{js,py}_errors when the expected JS or Python
-    are not found.
+    error messages are not found.
     """
 
-    def __init__(self, expected_messages, errors):
+    def __init__(self, kind, expected_messages, errors):
-        lines = ["The following JS errors were expected but could not be found:"]
+        assert kind in ("JS", "Python")
+        lines = [f"The following {kind} errors were expected but could not be found:"]
         for msg in expected_messages:
             lines.append("    - " + msg)
         if errors:
             lines.append("---")
-            lines.append("The following JS errors were raised but not expected:")
+            lines.append(f"The following {kind} errors were raised but not expected:")
-            for err in errors:
+            lines += errors
-                lines.append(JsErrors.format_playwright_error(err))
         msg = "\n".join(lines)
         super().__init__(msg)
         self.expected_messages = expected_messages

pyscriptjs/tests/integration/test_00_support.py

@@ -3,7 +3,12 @@ import textwrap
 
 import pytest
 
-from .support import JsErrors, JsErrorsDidNotRaise, PyScriptTest, with_execution_thread
+from .support import (
+    PageErrors,
+    PageErrorsDidNotRaise,
+    PyScriptTest,
+    with_execution_thread,
+)
 
 
 @with_execution_thread(None)
@@ -104,7 +109,7 @@ class TestSupport(PyScriptTest):
         """
         self.writefile("mytest.html", doc)
         self.goto("mytest.html")
-        with pytest.raises(JsErrors) as exc:
+        with pytest.raises(PageErrors) as exc:
             self.check_js_errors()
         # check that the exception message contains the error message and the
         # stack trace
@@ -147,7 +152,7 @@ class TestSupport(PyScriptTest):
         """
         self.writefile("mytest.html", doc)
         self.goto("mytest.html")
-        with pytest.raises(JsErrorsDidNotRaise) as exc:
+        with pytest.raises(PageErrorsDidNotRaise) as exc:
             self.check_js_errors(
                 "this is an error 1",
                 "this is an error 2",
@@ -176,7 +181,7 @@ class TestSupport(PyScriptTest):
         """
         self.writefile("mytest.html", doc)
         self.goto("mytest.html")
-        with pytest.raises(JsErrors) as exc:
+        with pytest.raises(PageErrors) as exc:
             self.check_js_errors()
         #
         msg = str(exc.value)
@@ -207,7 +212,7 @@ class TestSupport(PyScriptTest):
         """
         self.writefile("mytest.html", doc)
         self.goto("mytest.html")
-        with pytest.raises(JsErrors) as exc:
+        with pytest.raises(PageErrors) as exc:
             self.check_js_errors("expected 1", "expected 3")
         #
         msg = str(exc.value)
@@ -233,7 +238,7 @@ class TestSupport(PyScriptTest):
         """
         self.writefile("mytest.html", doc)
         self.goto("mytest.html")
-        with pytest.raises(JsErrorsDidNotRaise) as exc:
+        with pytest.raises(PageErrorsDidNotRaise) as exc:
             self.check_js_errors("this is not going to be found")
         #
         msg = str(exc.value)
@@ -348,7 +353,7 @@ class TestSupport(PyScriptTest):
         self.writefile("mytest.html", doc)
         # "Page loaded!" will never appear, of course.
         self.goto("mytest.html")
-        with pytest.raises(JsErrors) as exc:
+        with pytest.raises(PageErrors) as exc:
             self.wait_for_console("Page loaded!", timeout=200)
         assert "this is an error" in str(exc.value)
         assert isinstance(exc.value.__context__, TimeoutError)
@@ -358,7 +363,7 @@ class TestSupport(PyScriptTest):
         self.goto("mytest.html")
         with pytest.raises(TimeoutError):
             self.wait_for_console("Page loaded!", timeout=200, check_js_errors=False)
-        # we still got a JsErrors, so we need to manually clear it, else the
+        # we still got a PageErrors, so we need to manually clear it, else the
         # test fails at teardown
         self.clear_js_errors()
 
@@ -381,7 +386,7 @@ class TestSupport(PyScriptTest):
         """
         self.writefile("mytest.html", doc)
         self.goto("mytest.html")
-        with pytest.raises(JsErrors) as exc:
+        with pytest.raises(PageErrors) as exc:
             self.wait_for_console("Page loaded!", timeout=200)
         assert "this is an error" in str(exc.value)
         #

pyscriptjs/tests/integration/test_01_basic.py

@@ -2,7 +2,7 @@ import re
 
 import pytest
 
-from .support import JsErrors, PyScriptTest, skip_worker
+from .support import PageErrors, PyScriptTest, skip_worker
 
 
 class TestBasic(PyScriptTest):
@@ -76,6 +76,8 @@ class TestBasic(PyScriptTest):
         """
         )
         assert "hello pyscript" in self.console.log.lines
+        self.check_py_errors("Exception: this is an error")
+        #
         # check that we sent the traceback to the console
         tb_lines = self.console.error.lines[-1].splitlines()
         assert tb_lines[0] == "[pyexec] Python exception:"
@@ -107,6 +109,8 @@ class TestBasic(PyScriptTest):
             "Exception: this is an error inside handler", match_substring=True
         )
 
+        self.check_py_errors("Exception: this is an error inside handler")
+
         ## error in console
         tb_lines = self.console.error.lines[-1].splitlines()
         assert tb_lines[0] == "[pyexec] Python exception:"
@@ -178,20 +182,21 @@ class TestBasic(PyScriptTest):
         self.pyscript_run(
             """
             <py-config>
-                packages = ["nonexistendright"]
+                packages = ["i-dont-exist"]
             </py-config>
             """,
             wait_for_pyscript=False,
         )
 
         expected_alert_banner_msg = (
-            "(PY1001): Unable to install package(s) 'nonexistendright'. "
+            "(PY1001): Unable to install package(s) 'i-dont-exist'. "
             "Unable to find package in PyPI. Please make sure you have "
             "entered a correct package name."
         )
 
         alert_banner = self.page.wait_for_selector(".alert-banner")
         assert expected_alert_banner_msg in alert_banner.inner_text()
+        self.check_py_errors("Can't fetch metadata for 'i-dont-exist'")
 
     @skip_worker("FIXME: the banner doesn't appear")
     def test_no_python_wheel(self):
@@ -211,6 +216,7 @@ class TestBasic(PyScriptTest):
 
         alert_banner = self.page.wait_for_selector(".alert-banner")
         assert expected_alert_banner_msg in alert_banner.inner_text()
+        self.check_py_errors("Can't find a pure Python 3 wheel for 'opsdroid'")
 
     def test_dynamically_add_py_script_tag(self):
         self.pyscript_run(
@@ -240,7 +246,7 @@ class TestBasic(PyScriptTest):
         assert self.console.log.lines[-1] == "hello from foo"
 
     def test_py_script_src_not_found(self):
-        with pytest.raises(JsErrors) as exc:
+        with pytest.raises(PageErrors) as exc:
             self.pyscript_run(
                 """
                 <py-script src="foo.py"></py-script>
@@ -249,9 +255,7 @@ class TestBasic(PyScriptTest):
         assert "Failed to load resource" in self.console.error.lines[0]
 
         error_msgs = str(exc.value)
-
         expected_msg = "(PY0404): Fetching from URL foo.py failed with error 404"
-
         assert expected_msg in error_msgs
         assert self.assert_banner_message(expected_msg)
 

pyscriptjs/tests/integration/test_02_display.py

@@ -124,6 +124,7 @@ class TestDisplay(PyScriptTest):
         """
         )
         self.page.locator("text=Click me").click()
+        self.check_py_errors("Implicit target not allowed here")
         ## error in console
         tb_lines = self.console.error.lines[-1].splitlines()
         assert tb_lines[0] == "[pyexec] Python exception:"

pyscriptjs/tests/integration/test_event_handling.py

@@ -191,4 +191,5 @@ class TestEventHandler(PyScriptTest):
                 f"match banner text '{banner_text}'"
             )
 
-        assert any(msg in line for line in self.console.error.lines)
+        assert msg in self.console.error.lines[-1]
+        self.check_py_errors(msg)

pyscriptjs/tests/integration/test_py_repl.py

@@ -183,6 +183,8 @@ class TestPyRepl(PyScriptTest):
         tb_lines = err_pre.inner_text().splitlines()
         assert tb_lines[0] == "Traceback (most recent call last):"
         assert tb_lines[-1] == "Exception: this is an error"
+        #
+        self.check_py_errors("this is an error")
 
     @skip_worker("FIXME: display()")
     def test_multiple_repls(self):
@@ -228,6 +230,8 @@ class TestPyRepl(PyScriptTest):
         out_div = self.page.wait_for_selector("#py-internal-0-repl-output")
         assert "hello world" not in out_div.inner_text()
         assert "ZeroDivisionError" in out_div.inner_text()
+        #
+        self.check_py_errors("ZeroDivisionError")
 
     @skip_worker("FIXME: js.document")
     def test_hide_previous_error_after_successful_run(self):
@@ -253,6 +257,8 @@ class TestPyRepl(PyScriptTest):
         # test runner can be too fast, the line below should wait for output to change
         out_div = self.page.wait_for_selector("#py-internal-0-repl-output")
         assert out_div.inner_text() == "hello"
+        #
+        self.check_py_errors("this is an error")
 
     def test_output_attribute_does_not_exist(self):
         """

pyscriptjs/tests/integration/test_style.py

@@ -1,6 +1,6 @@
 from playwright.sync_api import expect
 
-from .support import PyScriptTest
+from .support import PyScriptTest, skip_worker
 
 
 class TestStyle(PyScriptTest):
@@ -24,6 +24,7 @@ class TestStyle(PyScriptTest):
         expect(self.page.locator("py-script")).to_be_hidden()
         expect(self.page.locator("py-repl")).to_be_hidden()
 
+    @skip_worker("FIXME: display()")
     def test_pyscript_defined(self):
         """Test elements have visibility that should"""
         self.pyscript_run(