Tests: upload screenshots also in case of failure.

[yakutovicha]

fixes #419

[danielhollas]

@yakutovicha thanks, this is an annoying problem indeed. I do wonder though if pytest has something like teardown methods that would ran automatically after each test? The solution in this PR is a lot of boilerplate.

[unkcpz]

Maybe this is related? https://stackoverflow.com/questions/12539897/need-py-test-to-log-assert-errors-in-log-file-from-python-logging-module/12547816#12547816 Seems a bit hacky to me, but it is the thing like teardown of pytest as I understand.

[yakutovicha]

@yakutovicha thanks, this is an annoying problem indeed. I do wonder though if pytest has something like teardown methods that would ran automatically after each test? The solution in this PR is a lot of boilerplate.

I am not aware of it, tbh. Without spending too much time on the problem, I could only come up with an alternative approach that uses decorator:

@upload_image_artifact("image-name.png")
def test_something(...):
    ...

Otherwise, one needs to do a bit of research. What do you guys think?

[danielhollas]

I would prefer to spend a bit of time on this if you're willing, since this seems like an important infrustructure.

This section of the pytest docs seems like it has answers:

https://docs.pytest.org/en/6.2.x/fixture.html#teardown-cleanup-aka-fixture-finalization

[danielhollas]

So I took this as an opportunity to dig into pytest a bit more and understand its magic. The link I shared above actually provides a possible solution to implement tear down methods.

Our case is complicated by the fact that we somehow need to pass the screenshot name to the teardown method. Here's the relevant section in docs and two SO questions were helpful:

https://docs.pytest.org/en/6.2.x/fixture.html#fixtures-can-introspect-the-requesting-test-context https://stackoverflow.com/questions/58549670/pass-a-parameter-to-a-pytest-fixture-for-teardown https://stackoverflow.com/questions/38488571/attributeerror-when-using-request-function-in-pytest-yield-fixture

In this PR in my app, https://github.com/ispg-group/aiidalab-ispg/pull/107, I am trying out two approaches:

1. Having a separate fixture. Screenshot name is passed via a custom variable, which is accessed from the pytest request context via request.function attribute. This is a bit hacky, but keeps this logic separated from other fixtures.

@pytest.fixture
def take_final_screenshot(request, selenium, screenshot_dir):
    yield
    try:
        filename = getattr(request.function, "final_screenshot")
        selenium.get_screenshot_as_file(f"{screenshot_dir}/{filename}")
    except AttributeError:
        # TODO: Emmit warning
        pass

def test_spectrum_app_init(selenium_driver, take_final_screenshot):
    driver = selenium_driver("spectrum_widget.ipynb", wait_time=30.0)
    driver.set_window_size(WINDOW_WIDTH, WINDOW_HEIGHT)
    final_screenshot = "spectrum-widget.png"
    driver.find_element(By.XPATH, "//button[text()='Download spectrum']")

Currently I couldn't make it work, but something like this presumable should work.

2. Extend the existing selenium_driver fixture with an extra optional parameter.

@pytest.fixture(scope="function")
def selenium_driver(selenium, notebook_service):
    final_screenshot_name = None

    def _selenium_driver(nb_path, wait_time=5.0, screenshot_name=None):
        url, token = notebook_service
        global final_screenshot_name
        final_screenshot_name = screenshot_name
        url_with_token = urljoin(
            url, f"apps/apps/aiidalab-ispg/{nb_path}?token={token}"
        )
        selenium.get(f"{url_with_token}")
        selenium.implicitly_wait(wait_time)  # must wait until the app loaded

        selenium.find_element(By.ID, "ipython-main-app")
        selenium.find_element(By.ID, "notebook-container")
        selenium.find_element(By.CLASS_NAME, "jupyter-widgets-view")

        return selenium

    yield _selenium_driver
    if final_screenshot_name is not None:
        selenium.get_screenshot_as_file(f"{screenshot_dir}/{final_screenshot_name}")

I think I might be doing something slightly wrong, as the teardowns down seem to working, but you can see the gist, and hopefully I can make it work.

[yakutovicha]

@danielhollas please feel free to close this PR and replace it with a better approach. FYI: tomorrow I am off - getting back on Monday.

[danielhollas]

TIL late binding in Python closures and the nonlocal keyword. :open_mouth:

Here's the code that actually works

@pytest.fixture(scope=function)
def selenium_driver(selenium, notebook_service, screenshot_dir):
    final_screenshot_name = None

    def _selenium_driver(nb_path, wait_time=5.0, screenshot_name=None):
        url, token = notebook_service
        nonlocal final_screenshot_name
        final_screenshot_name = screenshot_name
        url_with_token = urljoin(
            url, f"apps/apps/aiidalab-ispg/{nb_path}?token={token}"
        )
        selenium.get(f"{url_with_token}")
        selenium.implicitly_wait(wait_time)  # must wait until the app loaded

        selenium.find_element(By.ID, "ipython-main-app")
        selenium.find_element(By.ID, "notebook-container")
        selenium.find_element(By.CLASS_NAME, "jupyter-widgets-view")

        return selenium

    yield _selenium_driver
    if final_screenshot_name is not None:
        selenium.get_screenshot_as_file(f"{screenshot_dir}/{final_screenshot_name}")

def test_spectrum_app_init(selenium_driver):
    driver = selenium_driver(
        "spectrum_widget.ipynb", wait_time=30.0, screenshot_name="spectrum-widget.png"
    )
    driver.set_window_size(WINDOW_WIDTH, WINDOW_HEIGHT)
    driver.find_element(By.XPATH, "//button[text()='Download spectrum']")

or

@pytest.fixture
def final_screenshot(selenium, screenshot_dir):
    ctx = {"name": ""}
    yield ctx
    if not ctx["name"]:
        raise ValueError("You forgot to set the final screenshot name!")
    selenium.get_screenshot_as_file(f"{screenshot_dir}/{ctx['name']}")

def test_atmospec_app_init(selenium_driver, final_screenshot):
    driver = selenium_driver("atmospec.ipynb", wait_time=30.0)
    driver.set_window_size(WINDOW_WIDTH, WINDOW_HEIGHT)
    final_screenshot["name"] = "atmospec-app.png"
    driver.find_element(By.XPATH, "//button[text()='Refresh']")

I'll try to submit two competing PRs if I have time. If not on Monday.

[danielhollas]

Closing in favor of #421 or #422. @yakutovicha thank you for starting this.