Three small but important bugfixes (#25)

* - Do not auto-use the `session_browser` fixture

Not all contexts are easily equipped to handle more than one context, and need the ability to define their own.

- Ensure that if the SELENIUM_SERVER environment variable is not provided, it is set to None; ensure no whitespace leaks through.

- Fix default report_title, which was usually None at the time the fixture ran.

* Update pyproject.toml version to 4.0.1-alpha.0

* Fix conflicting use cases for selenium_server argument

No wonder I was always so confused by how this was getting set.

* Clean up one last autouse of the session_browser

* Update pyproject.toml version to 4.0.1-alpha.1

Co-authored-by: tomthorogood <[email protected]>

pyproject.toml

@@ -1,6 +1,9 @@
 [tool.poetry]
 name = "uw-webdriver-recorder"
-version = "4.0.0"
+# This version string is typically managed by the CI workflow,
+# and is changed anytime `poetry version [new version]` is run.
+# Do not revert this manually.
+version = "4.0.1-alpha.1"
 description = "A pytest plugin for recording screenshots of selenium interactions, with other convenient features too."
 authors = ["Tom Thorogood <[email protected]>"]
 license = "Apache Software License 3.0"

webdriver_recorder/plugin.py

@@ -13,10 +13,11 @@
 
 import jinja2
 import pytest
+from _pytest.fixtures import FixtureRequest
 from pydantic import BaseModel, root_validator
 from selenium import webdriver
 
-from .browser import BrowserError, Chrome, Remote
+from .browser import BrowserError, BrowserRecorder, Chrome, Remote
 
 TEMPLATE_FILE = os.path.join(
     os.path.abspath(os.path.dirname(__file__)), "report.template.html"
@@ -33,7 +34,7 @@ def pytest_addoption(parser):
         "--selenium-server",
         action="store",
         dest="selenium_server",
-        default=os.environ.get("SELENIUM_SERVER"),
+        default=None,
         help="Remote selenium webdriver to connect to (eg localhost:4444)",
     )
     group.addoption(
@@ -112,7 +113,15 @@ def __init__(self, report, excinfo, doc):
 
 @pytest.fixture(scope="session")
 def selenium_server(request) -> Optional[str]:
-    return request.config.getoption("selenium_server", None)
+    """Returns a non-empty string or None"""
+    return (
+        # CLI arg takes precedence
+        request.config.getoption("selenium_server")
+        # Otherwise, we look for a non-empty string
+        or os.environ.get('SELENIUM_SERVER', '').strip()
+        # If the result is still Falsey, we always return None.
+        or None
+    )
 
 
 @pytest.fixture(scope="session")
@@ -146,7 +155,7 @@ def session_browser(selenium_server, chrome_options):
 
 
 @pytest.fixture(scope="session")
-def browser_context(session_browser) -> Callable[..., Chrome]:
+def browser_context(request: FixtureRequest) -> Callable[..., Chrome]:
     """
     This fixture allows you to create a fresh context for a given
     browser instance.
@@ -168,7 +177,11 @@ def test_something(browser_context):
     def inner(
         browser: Optional[Chrome] = None, cookie_urls: Optional[List[str]] = None
     ):
-        browser = browser or session_browser
+        if not browser:
+            # Only loads this fixture if no override is present
+            # to avoid creating session_browsers if the
+            # dependent does not to.
+            browser = request.getfixturevalue('session_browser')
         browser.open_tab()
         cookie_urls = cookie_urls or []
         try:
@@ -191,7 +204,13 @@ def class_browser(request, browser_context) -> Chrome:
 
 
 @pytest.fixture
-def browser(browser_context) -> Chrome:
+def browser(browser_context) -> BrowserRecorder:
+    """
+    The default browser fixture. This default behavior will lazily
+    instantiate a `session_browser` if one does not exist. To
+    override this behavior and create your own context, you can
+    redefine this fixture.
+    """
     with browser_context() as browser:
         yield browser
 
@@ -217,7 +236,7 @@ def report_generator(generate_report, report_dir) -> str:
 
 
 @pytest.fixture(autouse=True)
-def report_test(report_generator, request, session_browser):
+def report_test(report_generator, request):
     """
     Print the results to report_file after a test run. Without this, the results of the test will not be saved.
     You can ensure this is always run by including the following in your conftest.py:
@@ -273,7 +292,7 @@ def report_test(report_test):
         link=slug,
         doc=doc,
         nodeid=nodeid,
-        pngs=session_browser.pngs,
+        pngs=BrowserRecorder.pngs,
         failure=failure,
         time1=time1,
         time2=time2,
@@ -286,7 +305,7 @@ def report_test(report_test):
         fd.write(header.json())
     with open(filename, "w") as fd:
         fd.write(report.json())
-    session_browser.pngs.clear()
+    BrowserRecorder.pngs.clear()
 
 
 @pytest.hookimpl(tryfirst=True, hookwrapper=True)
@@ -313,12 +332,12 @@ def lettergen():
 
 
 @pytest.fixture(scope="session")
-def report_title(request, session_browser) -> str:
-    return request.config.getoption("report_title", session_browser.current_url)
+def report_title(request) -> str:
+    return request.config.getoption("report_title", default="Webdriver Recorder Summary")
 
 
 @pytest.fixture(scope="session", autouse=True)
-def generate_report(template_filename, session_browser, report_title, report_dir):
+def generate_report(template_filename, report_title, report_dir):
     """
     Uses the included HTML template to generate the final report, using the results found in `report_dir`. Can be
     called explicitly in order to do this at any time.