Repository: pyodide/pytest-pyodide (language: python)

Issue:
Host tests are skipped by default
Currently, host tests are skipped by default. For instance in Pyodide,
```
pytest packages/_tests/test_packages_common.py -k parse_package -v
```
all tests are skipped.

This is pretty dangerous and should not happen.

In particular, it even passing `--rt=host` produces another issue in the above command, since it looks like we did changes with this test skipped so it's now broken,
```
INTERNALERROR> Traceback (most recent call last):
INTERNALERROR>   File "/home/rth/miniconda3/envs/pyodide-env/lib/python3.10/site-packages/_pytest/main.py", line 268, in wrap_session
INTERNALERROR>     session.exitstatus = doit(config, session) or 0
INTERNALERROR>   File "/home/rth/miniconda3/envs/pyodide-env/lib/python3.10/site-packages/_pytest/main.py", line 321, in _main
INTERNALERROR>     config.hook.pytest_collection(session=session)
INTERNALERROR>   File "/home/rth/miniconda3/envs/pyodide-env/lib/python3.10/site-packages/pluggy/_hooks.py", line 265, in __call__
INTERNALERROR>     return self._hookexec(self.name, self.get_hookimpls(), kwargs, firstresult)
INTERNALERROR>   File "/home/rth/miniconda3/envs/pyodide-env/lib/python3.10/site-packages/pluggy/_manager.py", line 80, in _hookexec
INTERNALERROR>     return self._inner_hookexec(hook_name, methods, kwargs, firstresult)
INTERNALERROR>   File "/home/rth/miniconda3/envs/pyodide-env/lib/python3.10/site-packages/pluggy/_callers.py", line 60, in _multicall
INTERNALERROR>     return outcome.get_result()
INTERNALERROR>   File "/home/rth/miniconda3/envs/pyodide-env/lib/python3.10/site-packages/pluggy/_result.py", line 60, in get_result
INTERNALERROR>     raise ex[1].with_traceback(ex[2])
INTERNALERROR>   File "/home/rth/miniconda3/envs/pyodide-env/lib/python3.10/site-packages/pluggy/_callers.py", line 39, in _multicall
INTERNALERROR>     res = hook_impl.function(*args)
INTERNALERROR>   File "/home/rth/miniconda3/envs/pyodide-env/lib/python3.10/site-packages/_pytest/main.py", line 332, in pytest_collection
INTERNALERROR>     session.perform_collect()
INTERNALERROR>   File "/home/rth/miniconda3/envs/pyodide-env/lib/python3.10/site-packages/_pytest/main.py", line 660, in perform_collect
INTERNALERROR>     hook.pytest_collection_modifyitems(
INTERNALERROR>   File "/home/rth/miniconda3/envs/pyodide-env/lib/python3.10/site-packages/pluggy/_hooks.py", line 265, in __call__
INTERNALERROR>     return self._hookexec(self.name, self.get_hookimpls(), kwargs, firstresult)
INTERNALERROR>   File "/home/rth/miniconda3/envs/pyodide-env/lib/python3.10/site-packages/pluggy/_manager.py", line 80, in _hookexec
INTERNALERROR>     return self._inner_hookexec(hook_name, methods, kwargs, firstresult)
INTERNALERROR>   File "/home/rth/miniconda3/envs/pyodide-env/lib/python3.10/site-packages/pluggy/_callers.py", line 60, in _multicall
INTERNALERROR>     return outcome.get_result()
INTERNALERROR>   File "/home/rth/miniconda3/envs/pyodide-env/lib/python3.10/site-packages/pluggy/_result.py", line 60, in get_result
INTERNALERROR>     raise ex[1].with_traceback(ex[2])
INTERNALERROR>   File "/home/rth/miniconda3/envs/pyodide-env/lib/python3.10/site-packages/pluggy/_callers.py", line 39, in _multicall
INTERNALERROR>     res = hook_impl.function(*args)
INTERNALERROR>   File "/home/rth/src/pyodide/conftest.py", line 138, in pytest_collection_modifyitems
INTERNALERROR>     maybe_skip_test(item, delayed=True)
INTERNALERROR>   File "/home/rth/src/pyodide/conftest.py", line 84, in maybe_skip_test
INTERNALERROR>     raise AssertionError(
INTERNALERROR> AssertionError: Couldn't parse package name from test_import[host-distlib]. This should not happen!
```


Discussion / hints:
So the problem is the default runtime is [node](https://github.com/pyodide/pytest-pyodide/blob/cf46ef981da3d519a3307116803235bad0dda4fc/pytest_pyodide/hook.py#L61) in which case the [host tests are skipped](https://github.com/pyodide/pytest-pyodide/blob/cf46ef981da3d519a3307116803235bad0dda4fc/pytest_pyodide/hook.py#L122). 

~~I think we should probably remove the "host" runtime altogether and always run host tests.~~ Or there should be another CLI option to skip host tests explicitly. The fact that  all regular tests are going to be skipped by installing this package is really surprising to the user. And even for us, it looks like in pyodide we are now skipping a good part of host tests because of this behavior.

cc @ryanking13 
> I think we should probably remove the "host" runtime altogether and always run host tests. Or there should be another CLI option to skip host tests explicitly. The fact that all regular tests are going to be skipped by installing this package is really surprising to the user. And even for us, it looks like in pyodide we are now skipping a good part of host tests because of this behavior.

Probably we should change the behavior such as:

- `--rt=chrome`: run chrome + host tests
- `--rt=host`: run host tests only
+1. but I can also imagine a case where you want to run the chrome tests excluding host tests, and we can probably add an extra flag for that if necessary.

Fix patch:
diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml
index 97b8a562..bc8afb2c 100644
--- a/.github/workflows/main.yml
+++ b/.github/workflows/main.yml
@@ -59,6 +59,12 @@ jobs:
           {runner: selenium, runtime: node, node-version: 14},
           {runner: selenium, runtime: node, node-version: 16},
           {runner: selenium, runtime: node, node-version: 18},
+          {runner: selenium, runtime: firefox-no-host, firefox-version: latest, geckodriver-version: latest },
+          {
+            runner: selenium, runtime: chrome firefox,
+            firefox-version: latest, geckodriver-version: latest,
+            chrome-version: latest, chromedriver-version: latest,
+          },
           {runner: selenium, runtime: host},
           # playwright browser versions are pinned to playwright version
           {runner: playwright, runtime: firefox, playwright-version: 1.22.0, node-version: 18},
@@ -81,7 +87,7 @@ jobs:
 
       - name: Install node
         uses: actions/setup-node@v3
-        if: ${{ matrix.test-config.runtime == 'node' || matrix.test-config.runner == 'playwright' }}
+        if: ${{ contains(matrix.test-config.runtime, 'node') || matrix.test-config.runner == 'playwright' }}
         with:
           node-version: ${{ matrix.test-config.node-version }}
 
@@ -102,28 +108,28 @@ jobs:
 
       - name: Install firefox
         uses: browser-actions/setup-firefox@latest
-        if: ${{ matrix.test-config.runner == 'selenium' && matrix.test-config.runtime == 'firefox' }}
+        if: ${{ matrix.test-config.runner == 'selenium' && contains(matrix.test-config.runtime, 'firefox') }}
         with:
           firefox-version: ${{ matrix.test-config.firefox-version }}
 
       - name: Install geckodriver
         uses: browser-actions/setup-geckodriver@latest
-        if: ${{ matrix.test-config.runner == 'selenium' && matrix.test-config.runtime == 'firefox' }}
+        if: ${{ matrix.test-config.runner == 'selenium' && contains(matrix.test-config.runtime, 'firefox') }}
         with:
           geckodriver-version: ${{ matrix.test-config.geckodriver-version }}
 
       - name: Install chrome
         uses: browser-actions/setup-chrome@latest
-        if: ${{ matrix.test-config.runner == 'selenium' && matrix.test-config.runtime == 'chrome' }}
+        if: ${{ matrix.test-config.runner == 'selenium' && contains(matrix.test-config.runtime, 'chrome') }}
         with:
           chrome-version: ${{ matrix.test-config.geckodriver-version }}
 
       - name: Install chromedriver
-        if: ${{ matrix.test-config.runner == 'selenium' && matrix.test-config.runtime == 'chrome' }}
+        if: ${{ matrix.test-config.runner == 'selenium' && contains(matrix.test-config.runtime, 'chrome') }}
         uses: nanasess/setup-chromedriver@v1
 
       - name: Enable Safari Driver
-        if: ${{ matrix.test-config.runner == 'selenium' && matrix.test-config.runtime == 'safari' && contains(runner.os, 'macos') }}
+        if: ${{ matrix.test-config.runner == 'selenium' && contains(matrix.test-config.runtime, 'safari') && contains(runner.os, 'macos') }}
         run: |
           sudo safaridriver --enable
           # Only one Safari browser instance can be active at any given time
@@ -153,7 +159,7 @@ jobs:
             --cov=pytest_pyodide \
             --dist-dir=./dist/ \
             --runner=${{ matrix.test-config.runner }} \
-            --rt=${{ matrix.test-config.runtime }}
+            --rt ${{ matrix.test-config.runtime }}
 
       - uses: codecov/codecov-action@v3
         if: ${{ github.event.repo.name == 'pyodide/pytest-pyodide' || github.event_name == 'pull_request' }}
diff --git a/CHANGELOG.md b/CHANGELOG.md
index c03e056e..bc182e62 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -1,3 +1,8 @@
+## [0.22.2] - 2002.09.08
+
+- Host tests will now run by default. If you want to disable running host tests, add `-no-host` suffix in the `--runtime` option. ([#33](https://github.com/pyodide/pytest-pyodide/pull/33))
+- Multiple browser testing is now available by passing `--runtime` option multiple times. ([#33](https://github.com/pyodide/pytest-pyodide/pull/33))
+
 ## [0.22.1] - 2002.09.06
 
 - Re-order safari tests to make sure only one simultaneous session exists during the test ([#29](https://github.com/pyodide/pytest-pyodide/pull/29))
diff --git a/README.md b/README.md
index 887e1368..4806e1dd 100644
--- a/README.md
+++ b/README.md
@@ -118,11 +118,14 @@ Possible options for `--runtime` are:
 - firefox
 - chrome
 - safari
-- all (chrome + firefox + safari + node)
 - host (do not run browser-based tests)
 
 ```sh
 pytest --runtime firefox
+pytest --runtime firefox --runtime chrome
+
+# Adding -no-host suffix will disable running host tests
+pytest --runtime chrome-no-host
 ```
 
 ## Running tests with Playwright (optional)


Test patch:
diff --git a/pytest_pyodide/hook.py b/pytest_pyodide/hook.py
index a787ccdf..0c401d5e 100644
--- a/pytest_pyodide/hook.py
+++ b/pytest_pyodide/hook.py
@@ -14,6 +14,31 @@
 from .utils import parse_xfail_browsers
 
 RUNTIMES = ["firefox", "chrome", "safari", "node"]
+RUNTIMES_AND_HOST = RUNTIMES + ["host"]
+RUNTIMES_NO_HOST = [f"{runtime}-no-host" for runtime in RUNTIMES]
+
+
+def _filter_runtimes(runtime: list[str]) -> tuple[bool, set[str]]:
+    # Always run host test, unless 'no-host' is given.
+    run_host = True
+
+    # remove duplicates
+    runtime_set = set(runtime)
+
+    runtime_filtered = set()
+    for rt in runtime_set:
+        if rt.endswith("-no-host"):
+            run_host = False
+            rt = rt.replace("-no-host", "")
+
+        runtime_filtered.add(rt)
+
+    # If '--rt chrome-no-host --rt host' is given, we run host tests.
+    run_host = run_host or ("host" in runtime_filtered)
+
+    runtime_filtered.discard("host")
+
+    return run_host, runtime_filtered
 
 
 def pytest_configure(config):
@@ -38,6 +63,9 @@ def pytest_configure(config):
         "xfail_browsers: xfail a test in specific browsers",
     )
 
+    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")
 
 
@@ -61,9 +89,10 @@ def pytest_addoption(parser):
         "--rt",
         "--runtime",
         dest="runtime",
-        default="node",
-        choices=RUNTIMES + ["all", "host"],
-        help="Select runtime, firefox, chrome, node, all, or host (default: %(default)s)",
+        nargs="+",
+        default=["node"],
+        choices=RUNTIMES_AND_HOST + RUNTIMES_NO_HOST,
+        help="Select runtime (default: %(default)s)",
     )
 
 
@@ -106,26 +135,10 @@ def pytest_pycollect_makemodule(module_path: Path, path: Any, parent: Any) -> No
 
 def pytest_generate_tests(metafunc: Any) -> None:
     if "runtime" in metafunc.fixturenames:
-        runtime = metafunc.config.option.runtime
-
-        if runtime == "all":
-            runtime = RUNTIMES
-
-        metafunc.parametrize("runtime", [runtime], scope="module")
+        metafunc.parametrize("runtime", pytest.pyodide_runtimes, scope="module")
 
 
 def pytest_collection_modifyitems(items: list[Any]) -> None:
-    for item in items:
-        if not hasattr(item, "fixturenames"):
-            # Some items like DoctestItem has no fixture
-            continue
-        if item.config.option.runtime == "host" and "runtime" in item.fixturenames:
-            item.add_marker(pytest.mark.skip(reason="Non-host test"))
-        elif (
-            item.config.option.runtime != "host" and "runtime" not in item.fixturenames
-        ):
-            item.add_marker(pytest.mark.skip(reason="Host test"))
-
     # Run all Safari standalone tests first
     # Since Safari doesn't support more than one simultaneous session, we run all
     # selenium_standalone Safari tests first. We preserve the order of other
@@ -158,6 +171,18 @@ def _get_item_position(item):
     items[:] = sorted(items, key=_get_item_position)
 
 
+@pytest.hookimpl(tryfirst=True)
+def pytest_runtest_setup(item):
+    if not hasattr(item, "fixturenames"):
+        # Some items like DoctestItem has no fixture
+        return
+
+    if not pytest.pyodide_runtimes and "runtime" in item.fixturenames:  # type: ignore[truthy-bool]
+        pytest.skip(reason="Non-host test")
+    elif not pytest.pyodide_run_host_test and "runtime" not in item.fixturenames:  # type: ignore[truthy-bool]
+        pytest.skip("Host test")
+
+
 @pytest.hookimpl(hookwrapper=True)
 def pytest_runtest_call(item):
 
diff --git a/tests/test_testing.py b/tests/test_testing.py
index 710d64d6..7afcafb1 100644
--- a/tests/test_testing.py
+++ b/tests/test_testing.py
@@ -1,5 +1,7 @@
 import pathlib
 
+import pytest
+
 
 def test_web_server_secondary(selenium, web_server_secondary):
     host, port, logs = web_server_secondary
@@ -7,14 +9,16 @@ def test_web_server_secondary(selenium, web_server_secondary):
     assert selenium.server_port != port
 
 
-def test_host(request):
-    runtime = request.config.option.runtime
-    assert runtime == "host", "this test should only run when runtime is host"
+def test_host():
+    assert (
+        pytest.pyodide_run_host_test
+    ), "this test should only run when host test is enabled"
 
 
-def test_runtime(selenium, request):
-    runtime = request.config.option.runtime
-    assert runtime != "host", "this test should only run when runtime is not host"
+def test_runtime(selenium):
+    assert (
+        pytest.pyodide_runtimes
+    ), "this test should only run when runtime is specified"
 
 
 def test_doctest():