Skip to content

Commit

Permalink
Prevent from overwriting global configurations (#105)
Browse files Browse the repository at this point in the history
This fixes a test failure that started to occur after #101.

```
! _pytest.outcomes.Exit: playwright failed to launch
```

The cause of the problem was that the combination of the `pytester` fixture and the `pytest_configure` hook is not very good.
We use `pytest_configure` to manage global settings, but this function is called again when we run sub-tests using pytester, resulting in new global settings being overwritten.

So I added a check to prevent overwriting global settings once it is set.
  • Loading branch information
ryanking13 authored Aug 9, 2023
1 parent 57dafeb commit e253295
Show file tree
Hide file tree
Showing 2 changed files with 30 additions and 31 deletions.
12 changes: 9 additions & 3 deletions pytest_pyodide/hook.py
Original file line number Diff line number Diff line change
Expand Up @@ -92,9 +92,15 @@ def pytest_configure(config):
)

run_host, runtimes = _filter_runtimes(config.option.runtime)
pytest.pyodide_run_host_test = run_host
pytest.pyodide_runtimes = runtimes
pytest.pyodide_dist_dir = config.getoption("--dist-dir")

# using `pytester` fixture seems to call this hook again with different options
# so this is a workaround to avoid overwriting the values
if not hasattr(pytest, "pyodide_run_host_test"):
pytest.pyodide_run_host_test = run_host
if not hasattr(pytest, "pyodide_runtimes"):
pytest.pyodide_runtimes = runtimes
if not hasattr(pytest, "pyodide_dist_dir"):
pytest.pyodide_dist_dir = config.getoption("--dist-dir")


@pytest.hookimpl(tryfirst=True)
Expand Down
49 changes: 21 additions & 28 deletions tests/test_options.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
import pytest

from pytest_pyodide.hook import _filter_runtimes


def test_dist_dir(pytester):
dist_dir = "dist"
Expand Down Expand Up @@ -47,49 +49,40 @@ def test_option(request):
assert result.ret == 4


@pytest.mark.parametrize(
"_runtime",
[
"chrome",
"firefox",
"safari",
"node",
"firefox,chrome",
],
)
def test_runtime(pytester, _runtime):
runtimes = _runtime.split(",")

pytester.makepyfile(
f"""
import pytest
def test_option():
assert pytest.pyodide_runtimes == set({runtimes!r})
"""
)

result = pytester.runpytest("--runtime", _runtime)
result.assert_outcomes(passed=1)


@pytest.mark.parametrize(
"_runtime",
[
"invalid",
],
)
def test_invalid_runtime(pytester, _runtime):
runtimes = _runtime.split(",")
_runtime.split(",")

pytester.makepyfile(
f"""
"""
import pytest
def test_option():
assert pytest.pyodide_runtimes == set({runtimes!r})
assert True
"""
)

# TODO: catch internal errors directly?
with pytest.raises(ValueError, match="Pytest terminal summary report not found"):
result = pytester.runpytest("--runtime", _runtime)
result.assert_outcomes(errors=1)


@pytest.mark.parametrize(
"_runtime,expected",
[
("chrome", (True, {"chrome"})),
("firefox", (True, {"firefox"})),
("node", (True, {"node"})),
("chrome,firefox", (True, {"chrome", "firefox"})),
("chrome-no-host,firefox", (False, {"chrome", "firefox"})),
("host", (True, set())),
("chrome-no-host,host", (True, {"chrome"})),
],
)
def test_filter_runtimes(_runtime, expected):
_filter_runtimes(_runtime) == expected

0 comments on commit e253295

Please sign in to comment.