refactor and cleanup tests

Author: rmorshea

Diff for: idom/testing.py

@@ -1,3 +1,4 @@
+import time
 from contextlib import contextmanager, AbstractContextManager
 from typing import Callable, Tuple, Iterator, Type, Optional, Union, Any
 
@@ -9,17 +10,18 @@
 import idom
 from idom.core.element import AbstractElement, ElementConstructor
 from idom.server.sanic import SanicRenderServer, PerClientStateServer
-from idom.server.prefab import hotswap_server
+from idom.server.prefab import hotswap_server, multiview_server, MultiViewMount
 from idom.server.utils import find_available_port
 from idom.utils import Ref
 
 
 DisplayFunction = Callable[[Union[ElementConstructor, AbstractElement], str], None]
 
 
-def create_selenium_display_context(
-    headless: bool, driver_timeout: float = 3.0
-) -> Iterator[DisplayFunction]:
+@contextmanager
+def open_selenium_chrome_driver_and_display_context(
+    headless: bool, driver_timeout: float = 3.0, wait_for_server_start: float = 1.0
+) -> Iterator[Tuple[Chrome, DisplayFunction]]:
     host = "127.0.0.1"
     port = find_available_port(host)
     server_url = f"http://{host}:{port}"
@@ -29,17 +31,21 @@ def create_selenium_display_context(
         page_load_timeout=driver_timeout,
         implicit_wait_timeout=driver_timeout,
     ) as driver:
-        with open_sanic_mount_and_server(
+        with open_sanic_hotswap_mount_and_server(
             server_type=PerClientStateServer, host=host, port=port
         ) as (mount, server):
-            return create_selenium_page_get_and_display_context(
-                driver, server, server_url, mount
-            )[1]
+            time.sleep(wait_for_server_start)
+            yield (
+                driver,
+                create_selenium_page_get_and_display_context(
+                    driver, server, server_url, mount
+                )[1],
+            )
 
 
 def create_selenium_page_get_and_display_context(
     driver: WebDriver,
-    server: "_RenderServerSavesLastError",
+    server: "SanicRenderServerWithLastError",
     server_url: str,
     element_mount_function: Callable[..., None],
 ) -> Tuple[Callable[[str], None], "AbstractContextManager[DisplayFunction]"]:
@@ -86,24 +92,46 @@ def display_context():
 
 
 @contextmanager
-def open_sanic_mount_and_server(
+def open_sanic_multiview_mount_and_server(
     server_type: Type[SanicRenderServer],
     host: str,
     port: int,
+    debug: bool = False,
+    app: Optional[Sanic] = None,
+) -> Iterator[Tuple[MultiViewMount, SanicRenderServer]]:
+    server_type = create_sanic_server_type_for_testing(server_type)
+    try:
+        yield multiview_server(
+            server_type,
+            host,
+            port,
+            server_options={"cors": True},
+            run_options={"debug": debug},
+            app=app,
+        )
+    finally:
+        if server_type.last_server_error_for_idom_testing.current is not None:
+            raise server_type.last_server_error_for_idom_testing.current  # pragma: no cover
+
+
+@contextmanager
+def open_sanic_hotswap_mount_and_server(
+    server_type: Type[SanicRenderServer],
+    host: str,
+    port: int,
+    sync_views: bool = False,
+    debug: bool = False,
     app: Optional[Sanic] = None,
 ) -> Iterator[Tuple[Callable[..., None], SanicRenderServer]]:
+    server_type = create_sanic_server_type_for_testing(server_type)
     try:
         yield hotswap_server(
-            (
-                server_type
-                if issubclass(server_type, _RenderServerSavesLastError)
-                else create_sanic_server_type_for_testing(server_type)
-            ),
+            server_type,
             host,
             port,
             server_options={"cors": True},
-            run_options={},
-            sync_views=False,
+            run_options={"debug": debug},
+            sync_views=sync_views,
             app=app,
         )
     finally:
@@ -113,11 +141,15 @@ def open_sanic_mount_and_server(
 
 def create_sanic_server_type_for_testing(
     server_type: Type[SanicRenderServer],
-) -> Type["_RenderServerSavesLastError"]:
-    return type(
-        server_type.__name__,
-        (_RenderServerSavesLastError, server_type),
-        {"last_server_error_for_idom_testing": Ref(None)},
+) -> Type["SanicRenderServerWithLastError"]:
+    return (
+        server_type
+        if issubclass(server_type, SanicRenderServerWithLastError)
+        else type(
+            server_type.__name__,
+            (SanicRenderServerWithLastError, server_type),
+            {"last_server_error_for_idom_testing": Ref(None)},
+        )
     )
 
 
@@ -143,7 +175,7 @@ def open_selenium_chrome_driver(
         driver.quit()
 
 
-class _RenderServerSavesLastError(SanicRenderServer):
+class SanicRenderServerWithLastError(SanicRenderServer):
     """A server that updates the ``last_server_error`` fixture"""
 
     last_server_error_for_idom_testing: Ref[Optional[Exception]]

Diff for: requirements/extras.txt

@@ -2,6 +2,9 @@
 sanic <19.12.0
 sanic-cors >=0.9.9
 
+# extra=testing
+selenium
+
 # extra=matplotlib
 matplotlib
 

Diff for: tests/conftest.py

@@ -2,9 +2,8 @@
 import inspect
 import time
 from contextlib import ExitStack
-from typing import Callable, Any, Type, Tuple, Iterator, Iterable, Union
+from typing import Callable, Any, Tuple, Iterator, Iterable, Union
 
-import sanic
 from loguru import logger
 import pytest
 from _pytest.logging import caplog as _caplog, LogCaptureFixture  # noqa
@@ -23,18 +22,12 @@
 from idom.server.sanic import PerClientStateServer
 from idom.testing import (
     create_selenium_page_get_and_display_context,
-    open_sanic_mount_and_server,
+    open_sanic_hotswap_mount_and_server,
     open_selenium_chrome_driver,
-    create_sanic_server_type_for_testing,
+    SanicRenderServerWithLastError,
 )
 
 
-# Default is an error because we want to know whether we are setting the last
-# error while testing. A refactor could miss the code path that catches serve
-# errors.
-default_error = NotImplementedError()
-
-
 def pytest_collection_modifyitems(items: Iterable[Any]) -> None:
     for item in items:
         if isinstance(item, pytest.Function):
@@ -113,13 +106,13 @@ def driver(create_driver: Callable[[], Chrome]) -> Chrome:
 
 
 @pytest.fixture(scope="module")
-def create_driver(pytestconfig: Config):
+def create_driver(driver_is_headless):
     """A Selenium web driver"""
     with ExitStack() as exit_stack:
 
         def create():
             return exit_stack.enter_context(
-                open_selenium_chrome_driver(headless=bool(pytestconfig.option.headless))
+                open_selenium_chrome_driver(headless=driver_is_headless)
             )
 
         yield create
@@ -139,49 +132,32 @@ def server(
 ) -> Iterator[AbstractRenderServer]:
     """An IDOM server"""
     server = mount_and_server[1]
+    if not isinstance(server, SanicRenderServerWithLastError):
+        raise TypeError(
+            "Servers for testing must be SanicRenderServerWithLastError instances"
+        )
     time.sleep(1)  # wait for server to start
     yield server
     server.stop()
 
 
 @pytest.fixture(scope="module")
 def mount_and_server(
-    application: sanic.Sanic,
-    fixturized_server_type: Type[AbstractRenderServer],
-    host: str,
-    port: int,
-    last_server_error: idom.Ref[Exception],
+    host: str, port: int
 ) -> Tuple[Callable[..., None], AbstractRenderServer]:
     """An IDOM layout mount function and server as a tuple
 
     The ``mount`` and ``server`` fixtures use this.
     """
-    with open_sanic_mount_and_server(
-        fixturized_server_type, host=host, port=port, app=application
+    with open_sanic_hotswap_mount_and_server(
+        PerClientStateServer, host=host, port=port
     ) as mount_and_server:
         yield mount_and_server
 
 
 @pytest.fixture(scope="module")
-def application():
-    return sanic.Sanic()
-
-
-@pytest.fixture(scope="module")
-def last_server_error(fixturized_server_type):
-    """A ``Ref`` containing the last server error. This must be populated by ``server_type``"""
-    return fixturized_server_type.last_server_error_for_idom_testing
-
-
-@pytest.fixture(scope="module")
-def fixturized_server_type(server_type):
-    return create_sanic_server_type_for_testing(server_type)
-
-
-@pytest.fixture(scope="module")
-def server_type() -> Type[AbstractRenderServer]:
-    """The type of server the ``mount_and_server`` fixture will use to initialize a server"""
-    return PerClientStateServer
+def last_server_error(server):
+    return server.last_server_error_for_idom_testing
 
 
 @pytest.fixture(scope="module")
@@ -210,6 +186,11 @@ def client_implementation():
         idom.client.current = original
 
 
+@pytest.fixture(scope="session")
+def driver_is_headless(pytestconfig: Config):
+    return bool(pytestconfig.option.headless)
+
+
 @pytest.fixture(scope="session", autouse=True)
 def _restore_client(pytestconfig: Config) -> Iterator[None]:
     """Restore the client's state before and after testing

Diff for: tests/test_core/test_hooks.py

@@ -120,22 +120,22 @@ async def on_click(event):
     count = driver.find_element_by_id("count-view")
 
     driver_wait.until(lambda d: constructor_call_count.current == 1)
-    assert count.get_attribute("innerHTML") == "0"
+    driver_wait.until(lambda d: count.get_attribute("innerHTML") == "0")
 
     inner.click()
 
     driver_wait.until(lambda d: constructor_call_count.current == 1)
-    assert count.get_attribute("innerHTML") == "1"
+    driver_wait.until(lambda d: count.get_attribute("innerHTML") == "1")
 
     outer.click()
 
     driver_wait.until(lambda d: constructor_call_count.current == 2)
-    assert count.get_attribute("innerHTML") == "0"
+    driver_wait.until(lambda d: count.get_attribute("innerHTML") == "0")
 
     inner.click()
 
     driver_wait.until(lambda d: constructor_call_count.current == 2)
-    assert count.get_attribute("innerHTML") == "1"
+    driver_wait.until(lambda d: count.get_attribute("innerHTML") == "1")
 
 
 def test_set_state_with_reducer_instead_of_value(driver, display):

Diff for: tests/test_server/test_sanic/test_base.py

@@ -1,21 +1,23 @@
 import asyncio
 
 import pytest
+from sanic import Sanic
 
 import idom
+from idom.server.sanic import PerClientStateServer
+from idom.testing import open_sanic_multiview_mount_and_server
 
 
 @pytest.fixture(scope="module")
-def mount_and_server(fixturized_server_type, host, port, last_server_error):
-    return idom.server.multiview_server(
-        fixturized_server_type,
+def mount_and_server(host, port):
+    with open_sanic_multiview_mount_and_server(
+        PerClientStateServer,
         host,
         port,
-        server_options={"last_server_error": last_server_error},
-        run_options={"debug": True},
-        # use the default app for the server just to get test coverage there
-        app=None,
-    )
+        # test that we can use a custom app instance
+        app=Sanic(),
+    ) as mount_and_server:
+        yield mount_and_server
 
 
 def test_serve_has_loop_attribute(server):

Diff for: tests/test_server/test_sanic/test_multiview_server.py

@@ -1,23 +1,18 @@
 import pytest
 
 import idom
-from idom.server import multiview_server
+from idom.server.sanic import PerClientStateServer
+from idom.testing import open_sanic_multiview_mount_and_server
 
 from tests.driver_utils import no_such_element
 
 
 @pytest.fixture(scope="module")
-def mount_and_server(
-    fixturized_server_type, host, port, last_server_error, application
-):
-    return multiview_server(
-        fixturized_server_type,
-        host,
-        port,
-        server_options={"last_server_error": last_server_error},
-        run_options={"debug": True},
-        app=application,
-    )
+def mount_and_server(host, port):
+    with open_sanic_multiview_mount_and_server(
+        PerClientStateServer, host, port
+    ) as mount_and_server:
+        yield mount_and_server
 
 
 def test_multiview_server(driver_get, driver, mount, server):