From 82968dbc1830e9fdcc8665dc5f30a1acb78ee88a Mon Sep 17 00:00:00 2001 From: Hood Chatham Date: Fri, 17 Nov 2023 12:53:26 -0800 Subject: [PATCH 1/3] MAINT Clean up hooks and run_tests_inside_pyodide This makes run_tests_inside_pyodide work via fixtures and removes the custom `get_browser` setup. It also moves some logic from `hook.py` into `decorator.py` and `run_tests_inside_pyodide` --- pytest_pyodide/decorator.py | 41 +++++- pytest_pyodide/doctest.py | 10 +- pytest_pyodide/hook.py | 159 +++++---------------- pytest_pyodide/run_tests_inside_pyodide.py | 143 +++++------------- 4 files changed, 107 insertions(+), 246 deletions(-) diff --git a/pytest_pyodide/decorator.py b/pytest_pyodide/decorator.py index 12765c2..cc455ba 100644 --- a/pytest_pyodide/decorator.py +++ b/pytest_pyodide/decorator.py @@ -6,17 +6,56 @@ from collections.abc import Callable, Collection from copy import deepcopy from io import BytesIO +from pathlib import Path from typing import Any, Protocol import pytest +from _pytest.assertion.rewrite import AssertionRewritingHook, rewrite_asserts from .copy_files_to_pyodide import copy_files_to_emscripten_fs -from .hook import ORIGINAL_MODULE_ASTS, REWRITTEN_MODULE_ASTS from .runner import _BrowserBaseRunner from .utils import package_is_built as _package_is_built MaybeAsyncFuncDef = ast.FunctionDef | ast.AsyncFunctionDef +ORIGINAL_MODULE_ASTS: dict[str, ast.Module] = {} +REWRITTEN_MODULE_ASTS: dict[str, ast.Module] = {} + + +# Handling for pytest assertion rewrites +# First we find the pytest rewrite config. It's an attribute of the pytest +# assertion rewriting meta_path_finder, so we locate that to get the config. + + +def _get_pytest_rewrite_config() -> Any: + for meta_path_finder in sys.meta_path: + if isinstance(meta_path_finder, AssertionRewritingHook): + break + else: + return None + return meta_path_finder.config + + +# Now we need to parse the ast of the files, rewrite the ast, and store the +# original and rewritten ast into dictionaries. `run_in_pyodide` will look the +# ast up in the appropriate dictionary depending on whether or not it is using +# pytest assert rewrites. + +REWRITE_CONFIG = _get_pytest_rewrite_config() +del _get_pytest_rewrite_config + + +def record_module_ast(module_path): + strfn = str(module_path) + if strfn in ORIGINAL_MODULE_ASTS: + return + source = Path(module_path).read_bytes() + tree = ast.parse(source, filename=strfn) + ORIGINAL_MODULE_ASTS[strfn] = tree + tree2 = deepcopy(tree) + rewrite_asserts(tree2, source, strfn, REWRITE_CONFIG) + REWRITTEN_MODULE_ASTS[strfn] = tree2 + def package_is_built(package_name: str): return _package_is_built(package_name, pytest.pyodide_dist_dir) # type: ignore[arg-type] diff --git a/pytest_pyodide/doctest.py b/pytest_pyodide/doctest.py index 89e2cef..ad9ad9f 100644 --- a/pytest_pyodide/doctest.py +++ b/pytest_pyodide/doctest.py @@ -1,4 +1,3 @@ -import ast from collections.abc import Callable from copy import copy from doctest import DocTest, DocTestRunner, register_optionflag @@ -17,17 +16,12 @@ from _pytest.scope import Scope from pytest import Collector -from . import run_in_pyodide -from .hook import ORIGINAL_MODULE_ASTS +from .decorator import record_module_ast, run_in_pyodide __all__ = ["patch_doctest_runner", "collect_doctests"] +record_module_ast(__file__) -# Record the ast of this file so we can use run_in_pyodide in here -# TODO: maybe extract this as a utility function for clarity? -ORIGINAL_MODULE_ASTS[__file__] = ast.parse( - Path(__file__).read_bytes(), filename=__file__ -) # make doctest aware of our `doctest: +RUN_IN_PYODIDE`` optionflag RUN_IN_PYODIDE = register_optionflag("RUN_IN_PYODIDE") diff --git a/pytest_pyodide/hook.py b/pytest_pyodide/hook.py index 5a63d9c..6a0ebb3 100644 --- a/pytest_pyodide/hook.py +++ b/pytest_pyodide/hook.py @@ -3,26 +3,17 @@ will look in here for hooks to execute. """ -import ast import re -import sys from argparse import BooleanOptionalAction -from copy import copy, deepcopy from pathlib import Path -from typing import Any, cast +from typing import Any import pytest -from _pytest.assertion.rewrite import AssertionRewritingHook, rewrite_asserts -from _pytest.python import ( - pytest_pycollect_makemodule as orig_pytest_pycollect_makemodule, -) from pytest import Collector, Session -from .copy_files_to_pyodide import copy_files_to_emscripten_fs from .run_tests_inside_pyodide import ( - close_pyodide_browsers, - get_browser_pyodide, - run_test_in_pyodide, + run_in_pyodide_generate_tests, + run_in_pyodide_runtest_call, ) from .utils import parse_xfail_browsers @@ -111,7 +102,6 @@ def pytest_configure(config): def pytest_unconfigure(config): - close_pyodide_browsers() try: ( pytest.pyodide_run_host_test, @@ -186,41 +176,16 @@ def pytest_collect_file(file_path: Path, parent: Collector): return collect_doctests(file_path, parent, doctestmodules) -# Handling for pytest assertion rewrites -# First we find the pytest rewrite config. It's an attribute of the pytest -# assertion rewriting meta_path_finder, so we locate that to get the config. - - -def _get_pytest_rewrite_config() -> Any: - for meta_path_finder in sys.meta_path: - if isinstance(meta_path_finder, AssertionRewritingHook): - break - else: - return None - return meta_path_finder.config - - -# Now we need to parse the ast of the files, rewrite the ast, and store the -# original and rewritten ast into dictionaries. `run_in_pyodide` will look the -# ast up in the appropriate dictionary depending on whether or not it is using -# pytest assert rewrites. - -REWRITE_CONFIG = _get_pytest_rewrite_config() -del _get_pytest_rewrite_config +def pytest_pycollect_makemodule(module_path: Path, parent: Collector) -> None: + from .decorator import record_module_ast -ORIGINAL_MODULE_ASTS: dict[str, ast.Module] = {} -REWRITTEN_MODULE_ASTS: dict[str, ast.Module] = {} + record_module_ast(module_path) + # orig_pytest_pycollect_makemodule(module_path, parent) -def pytest_pycollect_makemodule(module_path: Path, parent: Collector) -> None: - source = module_path.read_bytes() - strfn = str(module_path) - tree = ast.parse(source, filename=strfn) - ORIGINAL_MODULE_ASTS[strfn] = tree - tree2 = deepcopy(tree) - rewrite_asserts(tree2, source, strfn, REWRITE_CONFIG) - REWRITTEN_MODULE_ASTS[strfn] = tree2 - orig_pytest_pycollect_makemodule(module_path, parent) +def pytest_generate_tests(metafunc: Any) -> None: + if metafunc.config.option.run_in_pyodide: + run_in_pyodide_generate_tests(metafunc) STANDALONE_FIXTURES = [ @@ -238,29 +203,7 @@ def _has_standalone_fixture(item): return False -def modifyitems_run_in_pyodide(items: list[Any]): - # TODO: get rid of this - # if we are running tests in pyodide, then run all tests for each runtime - new_items = [] - # if pyodide_runtimes is not a singleton this is buggy... - # pytest_collection_modifyitems is only allowed to filter and reorder items, - # not to add new ones... - for runtime in pytest.pyodide_runtimes: # type: ignore[attr-defined] - if runtime == "host": - continue - for x in items: - x = copy(x) - x.pyodide_runtime = runtime - new_items.append(x) - items[:] = new_items - return - - def pytest_collection_modifyitems(items: list[Any]) -> None: - # TODO: is this the best way to figure out if run_in_pyodide was requested? - if items and items[0].config.option.run_in_pyodide: - modifyitems_run_in_pyodide(items) - # 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 @@ -281,80 +224,44 @@ def _get_item_position(item): @pytest.hookimpl(tryfirst=True) def pytest_runtest_setup(item): - if item.config.option.run_in_pyodide: - if not hasattr(item, "fixturenames"): - return - if pytest.pyodide_runtimes and "runtime" in item.fixturenames: - pytest.skip(reason="pyodide specific test, can't run in pyodide") - else: - # Pass this test to pyodide runner - # First: make sure that pyodide has the test folder copied over - item_path = Path(item.path) - copy_files = list(item_path.parent.rglob("*")) - # If we have a pyodide build dist folder with wheels in, copy those over - # and install the wheels in pyodide so we can import this package for tests - dist_path = Path.cwd() / "dist" - if dist_path.exists(): - copy_files.extend(list(dist_path.glob("*.whl"))) - - copy_files_with_destinations = [] - for src in copy_files: - dest = Path("test_files") / src.relative_to(Path.cwd()) - copy_files_with_destinations.append((src, dest)) - - class RequestType: - config = item.config - node = item - - selenium = get_browser_pyodide( - request=cast(pytest.FixtureRequest, RequestType), - runtime=item.pyodide_runtime, - ) - copy_files_to_emscripten_fs( - copy_files_with_destinations, selenium, install_wheels=True - ) - else: - if not hasattr(item, "fixturenames"): - # Some items like DoctestItem have no fixture - return - if not pytest.pyodide_runtimes and "runtime" in item.fixturenames: - pytest.skip(reason="Non-host test") - elif not pytest.pyodide_run_host_test and "runtime" not in item.fixturenames: - pytest.skip("Host test") + if not item.config.option.run_in_pyodide: + maybe_skip_item(item) -@pytest.hookimpl(hookwrapper=True) -def pytest_runtest_call(item): - if item.config.option.run_in_pyodide: +def maybe_skip_item(item): + if not hasattr(item, "fixturenames"): + # Some items like DoctestItem have no fixture + return - def _run_in_pyodide(self): - class RequestType: - config = item.config - node = item + if item.config.option.run_in_pyodide: + if "runtime" in item.fixturenames: + pytest.skip(reason="pyodide specific test, can't run in pyodide") + return - selenium = get_browser_pyodide( - request=cast(pytest.FixtureRequest, RequestType), - runtime=item.pyodide_runtime, - ) - run_test_in_pyodide(self.nodeid, selenium) + if not pytest.pyodide_runtimes and "runtime" in item.fixturenames: + pytest.skip(reason="Non-host test") + elif not pytest.pyodide_run_host_test and "runtime" not in item.fixturenames: + pytest.skip("Host test") - item.runtest = _run_in_pyodide.__get__(item, item.__class__) - yield - return +def xfail_browsers_marker_impl(item): browser = None - for fixture in item._fixtureinfo.argnames: + for fixture in item.fixturenames: if fixture.startswith("selenium"): browser = item.funcargs[fixture] break if not browser: - yield return xfail_msg = parse_xfail_browsers(item).get(browser.browser, None) if xfail_msg is not None: pytest.xfail(xfail_msg) - yield - return + +@pytest.hookimpl(tryfirst=True) +def pytest_runtest_call(item): + if item.config.option.run_in_pyodide: + run_in_pyodide_runtest_call(item) + else: + xfail_browsers_marker_impl(item) diff --git a/pytest_pyodide/run_tests_inside_pyodide.py b/pytest_pyodide/run_tests_inside_pyodide.py index 7ee6e10..deacc5f 100644 --- a/pytest_pyodide/run_tests_inside_pyodide.py +++ b/pytest_pyodide/run_tests_inside_pyodide.py @@ -1,106 +1,45 @@ -import re -import sys import xml.etree.ElementTree as ET -from dataclasses import dataclass -from typing import ContextManager +from pathlib import Path import pytest -from .server import spawn_web_server +from .copy_files_to_pyodide import copy_files_to_emscripten_fs -class ContextManagerUnwrapper: - """Class to take a context manager (e.g. a pytest fixture or something) - and unwrap it so that it can be used for the whole of the module. +def run_in_pyodide_generate_tests(metafunc): + metafunc.fixturenames += ("runtime", "selenium") + metafunc.parametrize("runtime", pytest.pyodide_runtimes, scope="module") - This is a bit of a hack, but it allows us to use some of our pytest fixtures - without having to be inside a pytest context. Avoids significant duplication - of the standard pytest_pyodide code here. - """ - - def __init__(self, ctx_manager: ContextManager): - self.ctx_manager = ctx_manager - self.value = ctx_manager.__enter__() - - def get_value(self): - return self.value - - def __del__(self): - self.close() - - def close(self): - if self.ctx_manager is not None: - self.ctx_manager.__exit__(None, None, None) - self.value = None +def copy_files(selenium, path: Path): + # Pass this test to pyodide runner + # First: make sure that pyodide has the test folder copied over + copy_files = [x for x in path.parent.rglob("*") if x.name != "__pycache__"] + # If we have a pyodide build dist folder with wheels in, copy those over + # and install the wheels in pyodide so we can import this package for tests + dist_path = Path.cwd() / "dist" + if dist_path.exists(): + copy_files.extend(list(dist_path.glob("*.whl"))) -@dataclass -class _SeleniumInstance: - selenium: ContextManagerUnwrapper - server: ContextManagerUnwrapper + copy_files_with_destinations = [] + for src in copy_files: + dest = Path("test_files") / src.relative_to(Path.cwd()) + copy_files_with_destinations.append((src, dest)) - -_seleniums: dict[str, _SeleniumInstance] = {} -_playwright_browser_list = None -_playwright_browser_generator = None - - -def get_browser_pyodide(request: pytest.FixtureRequest, runtime: str): - """Start a browser running with pyodide, ready to run pytest - calls. If the same runtime is already running, it will - just return that. - """ - global _playwright_browser_generator, _playwright_browser_list - from .fixture import _playwright_browsers, selenium_common - - if ( - request.config.option.runner.lower() == "playwright" - and _playwright_browser_generator is None - ): - _playwright_browser_generator = _playwright_browsers(request) - _playwright_browser_list = _playwright_browser_generator.__next__() - if runtime in _seleniums: - return _seleniums[runtime].selenium.get_value() - web_server_main = ContextManagerUnwrapper( - spawn_web_server(request.config.option.dist_dir) - ) - # open pyodide - _seleniums[runtime] = _SeleniumInstance( - selenium=ContextManagerUnwrapper( - selenium_common( - request, - runtime, - web_server_main.get_value(), - browsers=_playwright_browser_list, - ) - ), - server=web_server_main, + copy_files_to_emscripten_fs( + copy_files_with_destinations, selenium, install_wheels=True ) - return _seleniums[runtime].selenium.get_value() +def run_in_pyodide_runtest_call(item): + selenium = item._request.getfixturevalue("selenium") + item.path.relative_to(Path.cwd()) + copy_files(selenium, item.path) -def _remove_pytest_capture_title( - capture_element: ET.Element | None, title_name: str -) -> str | None: - """ - pytest captures (even in xml) have a title line - like ------ Capture out ------- + def runtest(): + run_test_in_pyodide(item.nodeid, selenium) - This helper removes that line. - """ - if capture_element is None: - return None - capture_text = capture_element.text - if not capture_text: - return None - lines = capture_text.splitlines() - assert lines[0].find(" " + title_name + " ") - ret_data = "\n".join(lines[1:]) - if re.search(r"\S", ret_data): - return ret_data - else: - return None + item.runtest = runtest def run_test_in_pyodide(node_tree_id, selenium, ignore_fail=False): @@ -114,8 +53,10 @@ def run_test_in_pyodide(node_tree_id, selenium, ignore_fail=False): roughly the same as they would when you are running pytest locally. """ all_args = [ - node_tree_id, - "--color=no", + node_tree_id.removesuffix(f"[{selenium.browser}]").replace( + f"[{selenium.browser}-", "[" + ), + "--color=yes", "--junitxml", "test_output.xml", "-o", @@ -132,37 +73,17 @@ def run_test_in_pyodide(node_tree_id, selenium, ignore_fail=False): output_xml """ ) + selenium.clean_logs() # get the error from junitxml root = ET.fromstring(ret_xml) fails = root.findall("*/testcase[failure]") for fail in fails: - stdout = fail.find("./system-out") - stderr = fail.find("./system-err") failure = fail.find("failure") if failure is not None and failure.text: fail_txt = failure.text else: fail_txt = "" - stdout_text = _remove_pytest_capture_title(stdout, "Captured Out") - stderr_text = _remove_pytest_capture_title(stderr, "Captured Err") - if stdout_text is not None: - print(stdout_text) - if stderr_text is not None: - sys.stderr.write(stderr_text) if not ignore_fail: pytest.fail(fail_txt, pytrace=False) return False return True - - -def close_pyodide_browsers(): - """Close the browsers that are currently open with - pyodide runtime initialised. - - This is done at the end of testing so that we can run more - than one test without launching browsers each time. - """ - global _seleniums, _playwright_browser_list, _playwright_browser_generator - for x in _seleniums.values(): - x.selenium.close() - _seleniums.clear() From 6dd7b167526ba9b024043c9a3ac6ed68dd3b3408 Mon Sep 17 00:00:00 2001 From: Hood Chatham Date: Fri, 17 Nov 2023 13:04:42 -0800 Subject: [PATCH 2/3] More cleanup --- pytest_pyodide/fixture.py | 2 -- pytest_pyodide/hook.py | 12 +++--------- 2 files changed, 3 insertions(+), 11 deletions(-) diff --git a/pytest_pyodide/fixture.py b/pytest_pyodide/fixture.py index 6ffd78a..a77b600 100644 --- a/pytest_pyodide/fixture.py +++ b/pytest_pyodide/fixture.py @@ -143,8 +143,6 @@ def wrapper(*args, **kwargs): return use_variant -standalone = rename_fixture("selenium", "selenium_standalone") - @pytest.fixture(scope="function") def selenium_standalone(request, runtime, web_server_main, playwright_browsers): diff --git a/pytest_pyodide/hook.py b/pytest_pyodide/hook.py index 6a0ebb3..3c7cc3a 100644 --- a/pytest_pyodide/hook.py +++ b/pytest_pyodide/hook.py @@ -15,6 +15,7 @@ run_in_pyodide_generate_tests, run_in_pyodide_runtest_call, ) +from . import fixture from .utils import parse_xfail_browsers RUNTIMES = ["firefox", "chrome", "safari", "node"] @@ -188,16 +189,9 @@ def pytest_generate_tests(metafunc: Any) -> None: run_in_pyodide_generate_tests(metafunc) -STANDALONE_FIXTURES = [ - "selenium_standalone", - "selenium_standalone_noload", - "selenium_webworker_standalone", -] - - def _has_standalone_fixture(item): - for fixture in item._request.fixturenames: - if fixture in STANDALONE_FIXTURES: + for fixture in item.fixturenames: + if fixture.startswith("selenium") and "standalone" in fixture: return True return False From 0ea369cc1520b585b241288319631b9feb539ed6 Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Fri, 17 Nov 2023 21:06:59 +0000 Subject: [PATCH 3/3] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- pytest_pyodide/fixture.py | 1 - pytest_pyodide/hook.py | 1 - 2 files changed, 2 deletions(-) diff --git a/pytest_pyodide/fixture.py b/pytest_pyodide/fixture.py index a77b600..4946fb9 100644 --- a/pytest_pyodide/fixture.py +++ b/pytest_pyodide/fixture.py @@ -143,7 +143,6 @@ def wrapper(*args, **kwargs): return use_variant - @pytest.fixture(scope="function") def selenium_standalone(request, runtime, web_server_main, playwright_browsers): with selenium_common( diff --git a/pytest_pyodide/hook.py b/pytest_pyodide/hook.py index 3c7cc3a..31730e1 100644 --- a/pytest_pyodide/hook.py +++ b/pytest_pyodide/hook.py @@ -15,7 +15,6 @@ run_in_pyodide_generate_tests, run_in_pyodide_runtest_call, ) -from . import fixture from .utils import parse_xfail_browsers RUNTIMES = ["firefox", "chrome", "safari", "node"]