FIX to display function handling null element reference, and wrong target parameter values (#1784)

* blacked

* More robust code for display, with tests

Display now includes more robust controls when checking
the input target parameters, with appropriate exception raised
(i.e. ValueError or TypeError) whether target is either
an empty string, or a not-string, not-None type, respectively.

The TypeError aligns with other similar behaviour with other Pyton
functions (e.g. str.split with integer separator).

Also, now display raises a ValueError whether the target element
is not found or not existing.

All changes are supported by tests.

* traceback lines in check_py_error & removed clones

check_py_error function now automatically includes
check of the traceback in console errors.

This way tests in basic and display have been refactored,
and lots of duplicated code removed.

* removed useless console check lines.

If check_py_errors function is running, those
console log lines are useless to check.

pyscript.core/src/stdlib/pyscript/__init__.py

@@ -29,7 +29,14 @@
 #     pyscript.magic_js. This is the blessed way to access them from pyscript,
 #     as it works transparently in both the main thread and worker cases.
 
-from pyscript.magic_js import RUNNING_IN_WORKER, PyWorker, window, document, sync, current_target
+from pyscript.magic_js import (
+    RUNNING_IN_WORKER,
+    PyWorker,
+    window,
+    document,
+    sync,
+    current_target,
+)
 from pyscript.display import HTML, display
 
 try:
@@ -38,6 +45,5 @@ except:
     from pyscript.util import NotSupported
 
     when = NotSupported(
-        "pyscript.when",
+        "pyscript.when", "pyscript.when currently not available with this interpreter"
-        "pyscript.when currently not available with this interpreter"
     )

pyscript.core/src/stdlib/pyscript/display.py

@@ -148,9 +148,23 @@ def _write(element, value, append=False):
 def display(*values, target=None, append=True):
     if target is None:
         target = current_target()
+    elif not isinstance(target, str):
+        raise TypeError(f"target must be str or None, not {target.__class__.__name__}")
+    elif target == "":
+        raise ValueError("Cannot have an empty target")
+    elif target.startswith("#"):
+        # note: here target is str and not None!
+        # align with @when behavior
+        target = target[1:]
 
     element = document.getElementById(target)
 
+    # If target cannot be found on the page, a ValueError is raised
+    if element is None:
+        raise ValueError(
+            f"Invalid selector with id={target}. Cannot be found in the page."
+        )
+
     # if element is a <script type="py">, it has a 'target' attribute which
     # points to the visual element holding the displayed values. In that case,
     # use that.

pyscript.core/src/stdlib/pyscript/magic_js.py

@@ -7,8 +7,9 @@ if RUNNING_IN_WORKER:
     import polyscript
 
     PyWorker = NotSupported(
-        'pyscript.PyWorker',
+        "pyscript.PyWorker",
-        'pyscript.PyWorker works only when running in the main thread')
+        "pyscript.PyWorker works only when running in the main thread",
+    )
     window = polyscript.xworker.window
     document = window.document
     sync = polyscript.xworker.sync
@@ -21,11 +22,12 @@ if RUNNING_IN_WORKER:
 else:
     import _pyscript
     from _pyscript import PyWorker
+
     window = globalThis
     document = globalThis.document
     sync = NotSupported(
-        'pyscript.sync',
+        "pyscript.sync", "pyscript.sync works only when running in a worker"
-        'pyscript.sync works only when running in a worker')
+    )
 
     # in MAIN the current element target exist, just use it
     def current_target():

pyscript.core/tests/integration/test_01_basic.py

@@ -106,10 +106,6 @@ 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] == "PythonError: Traceback (most recent call last):"
-        #
         # check that we show the traceback in the page. Note that here we
         # display the "raw" python traceback, without the "[pyexec] Python
         # exception:" line (which is useful in the console, but not for the
@@ -138,10 +134,6 @@ class TestBasic(PyScriptTest):
 
         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] == "PythonError: Traceback (most recent call last):"
-
         ## error in DOM
         tb_lines = self.page.locator(".py-error").inner_text().splitlines()
         assert tb_lines[0] == "Traceback (most recent call last):"

pyscript.core/tests/integration/test_02_display.py

@@ -11,6 +11,7 @@ import pytest
 from PIL import Image
 
 from .support import (
+    PageErrors,
     PyScriptTest,
     filter_inner_text,
     filter_page_content,
@@ -72,6 +73,67 @@ class TestDisplay(PyScriptTest):
         mydiv = self.page.locator("#mydiv")
         assert mydiv.inner_text() == "hello world"
 
+    def test_target_parameter_with_sharp(self):
+        self.pyscript_run(
+            """
+            <script type="py">
+                from pyscript import display
+                display('hello world', target="#mydiv")
+            </script>
+            <div id="mydiv"></div>
+            """
+        )
+        mydiv = self.page.locator("#mydiv")
+        assert mydiv.inner_text() == "hello world"
+
+    def test_non_existing_id_target_raises_value_error(self):
+        self.pyscript_run(
+            """
+            <script type="py">
+                from pyscript import display
+                display('hello world', target="non-existing")
+            </script>
+            """
+        )
+        error_msg = (
+            f"Invalid selector with id=non-existing. Cannot be found in the page."
+        )
+        self.check_py_errors(f"ValueError: {error_msg}")
+
+    def test_empty_string_target_raises_value_error(self):
+        self.pyscript_run(
+            """
+            <script type="py">
+                from pyscript import display
+                display('hello world', target="")
+            </script>
+            """
+        )
+        self.check_py_errors(f"ValueError: Cannot have an empty target")
+
+    def test_non_string_target_values_raise_typerror(self):
+        self.pyscript_run(
+            """
+            <script type="py">
+                from pyscript import display
+                display("hello False", target=False)
+            </script>
+            """
+        )
+        error_msg = f"target must be str or None, not bool"
+        self.check_py_errors(f"TypeError: {error_msg}")
+
+        self.pyscript_run(
+            """
+            <script type="py">
+                from pyscript import display
+                display("hello False", target=123)
+            </script>
+            """
+        )
+        error_msg = f"target must be str or None, not int"
+        self.check_py_errors(f"TypeError: {error_msg}")
+
     @skip_worker("NEXT: display(target=...) does not work")
     def test_tag_target_attribute(self):
         self.pyscript_run(