diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index e318f796d..df0f0e692 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -224,7 +224,7 @@ jobs: - name: Run pytest run: | echo '::add-matcher::.github/problem-matchers/pytest.json' - check/pytest -n auto -m "not slow" + check/pytest -n auto pytest-extra: if: needs.changes.outputs.python == 'true' @@ -259,7 +259,7 @@ jobs: - name: Run pytest run: | echo '::add-matcher::.github/problem-matchers/pytest.json' - check/pytest -n auto -m "not slow" src/openfermion/resource_estimates + check/pytest -n auto src/openfermion/resource_estimates pytest-compat: if: needs.changes.outputs.python == 'true' @@ -288,7 +288,7 @@ jobs: - name: Run pytest run: | echo '::add-matcher::.github/problem-matchers/pytest.json' - check/pytest -n auto -m "not slow" + check/pytest -n auto coverage: if: needs.changes.outputs.python == 'true' diff --git a/.github/workflows/nightly-pytest.yaml b/.github/workflows/nightly-pytest.yaml index 5951bc11a..b11b1ff9b 100644 --- a/.github/workflows/nightly-pytest.yaml +++ b/.github/workflows/nightly-pytest.yaml @@ -95,7 +95,7 @@ jobs: run: echo '::add-matcher::.github/problem-matchers/pytest.json' - name: Run Pytest - run: check/pytest + run: check/pytest --run-slow - name: Print debugging info upon job failure if: failure() diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 60fadd681..1c3174276 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -292,12 +292,14 @@ tests, follow these general principles: We use [pytest](https://docs.pytest.org) to run our tests and [pytest-cov](https://pytest-cov.readthedocs.io) to compute coverage. -* While developing, periodically check that changes do not break anything. For fast checks, use - `pytest -m "not slow" PATH`, where `PATH` is a directory or pytest file to test. +* While developing, periodically check that changes do not break anything. By default, pytest + skips tests marked `slow`; use `pytest PATH`, where `PATH` is a directory or pytest file to + test. -* After finishing a task, run `check/pytest` to test all of the OpenFermion code. If your system - has multiple processor cores, you can add the option `-n auto` to make pytest use multiple - parallel processes for a speed increase. (Beware, though, that this is resource-intensive.) +* After finishing a task, run `check/pytest --run-slow` to test all of the OpenFermion code. If + your system has multiple processor cores, you can add the option `-n auto` to make pytest use + multiple parallel processes for a speed increase. (Beware, though, that this is + resource-intensive.) We don't require 100% coverage, but coverage should be very high, and any uncovered code must be annotated with `# pragma: no cover`. To ignore coverage of a single line, place `# pragma: no cover` @@ -311,7 +313,7 @@ After a task is finished, run each of the following to make sure everything pass * `check/format-incremental` (and `check/format-incremental --apply` to auto-fix format problems) * `check/pylint -j 0` * `check/mypy` -* `check/pytest -n auto` +* `check/pytest -n auto --run-slow` * `check/pytest-and-incremental-coverage` ### Pull requests and code reviews diff --git a/check/pytest b/check/pytest index 6e48b56b3..4878f8ac0 100755 --- a/check/pytest +++ b/check/pytest @@ -23,7 +23,7 @@ # # You may specify pytest flags and specific files to test. The file paths # must be relative to the repository root. If no files are specified, everything -# is tested. +# except tests marked slow is tested. Pass --run-slow to include slow tests. ################################################################################ # Get the working directory to the repo root. diff --git a/check/pytest-and-incremental-coverage b/check/pytest-and-incremental-coverage index ed6c1e506..2bfe9fcb1 100755 --- a/check/pytest-and-incremental-coverage +++ b/check/pytest-and-incremental-coverage @@ -70,6 +70,7 @@ fi check/pytest . \ -n auto \ --actually-quiet \ + --run-slow \ --cov \ --cov-report=annotate pytest_result=$? diff --git a/conftest.py b/conftest.py new file mode 100644 index 000000000..0b70cf7ed --- /dev/null +++ b/conftest.py @@ -0,0 +1,40 @@ +# Copyright 2025 Google LLC +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# https://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +import re +from typing import Any + +_SLOW_MARKER_RE = re.compile(r"(? None: + parser.addoption("--run-slow", action="store_true", default=False, help="run tests marked slow") + + +def pytest_collection_modifyitems(config: Any, items: list[Any]) -> None: + markexpr = config.getoption("-m", default="") + if config.getoption("--run-slow") or _SLOW_MARKER_RE.search(markexpr): + return + + selected = [] + deselected = [] + for item in items: + if "slow" in item.keywords: + deselected.append(item) + else: + selected.append(item) + + if deselected: + config.hook.pytest_deselected(items=deselected) + items[:] = selected diff --git a/pyproject.toml b/pyproject.toml index 36d7951b2..1540ad74d 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -25,7 +25,7 @@ disable_warnings = ["no-sysmon"] [tool.pytest.ini_options] markers = [ - """slow: marks tests as slow (deselect with '-m "not slow"')""", + """slow: marks tests as slow (run with '--run-slow' or '-m slow')""", ] filterwarnings = [ "ignore:Skipped assert_qasm_is_consistent_with_unitary because qiskit.*:UserWarning", diff --git a/src/openfermion/testing/pytest_config_test.py b/src/openfermion/testing/pytest_config_test.py new file mode 100644 index 000000000..cfe3debfd --- /dev/null +++ b/src/openfermion/testing/pytest_config_test.py @@ -0,0 +1,105 @@ +# Copyright 2025 Google LLC +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# https://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +import importlib.util +import pathlib +from typing import Any + +_CONFTEST_PATH = pathlib.Path(__file__).parents[3] / "conftest.py" +_CONFTEST_SPEC = importlib.util.spec_from_file_location("openfermion_pytest_config", _CONFTEST_PATH) +assert _CONFTEST_SPEC is not None +assert _CONFTEST_SPEC.loader is not None +conftest = importlib.util.module_from_spec(_CONFTEST_SPEC) +_CONFTEST_SPEC.loader.exec_module(conftest) + + +class _FakeHook: + def __init__(self) -> None: + self.deselected: list[Any] = [] + + def pytest_deselected(self, items: list[Any]) -> None: + self.deselected = list(items) + + +class _FakeConfig: + def __init__(self, *, run_slow: bool = False, markexpr: str = "") -> None: + self._run_slow = run_slow + self._markexpr = markexpr + self.hook = _FakeHook() + + def getoption(self, name: str, default: Any = None) -> Any: + if name == "--run-slow": + return self._run_slow + if name == "-m": + return self._markexpr + return default + + +class _FakeItem: + def __init__(self, *, slow: bool = False) -> None: + self.keywords = {"slow": True} if slow else {} + + +def test_fake_config_returns_default_for_unknown_options() -> None: + config = _FakeConfig() + + assert config.getoption("--unknown", default="fallback") == "fallback" + + +def test_default_collection_skips_slow_tests() -> None: + config = _FakeConfig() + slow_item = _FakeItem(slow=True) + fast_item = _FakeItem() + items = [slow_item, fast_item] + + conftest.pytest_collection_modifyitems(config, items) + + assert items == [fast_item] + assert config.hook.deselected == [slow_item] + + +def test_run_slow_keeps_slow_tests() -> None: + config = _FakeConfig(run_slow=True) + slow_item = _FakeItem(slow=True) + fast_item = _FakeItem() + items = [slow_item, fast_item] + + conftest.pytest_collection_modifyitems(config, items) + + assert items == [slow_item, fast_item] + assert config.hook.deselected == [] + + +def test_explicit_slow_mark_expression_keeps_slow_tests() -> None: + config = _FakeConfig(markexpr="slow") + slow_item = _FakeItem(slow=True) + fast_item = _FakeItem() + items = [slow_item, fast_item] + + conftest.pytest_collection_modifyitems(config, items) + + assert items == [slow_item, fast_item] + assert config.hook.deselected == [] + + +def test_unrelated_mark_expression_keeps_default_slow_filter() -> None: + config = _FakeConfig(markexpr="not slowpoke") + slow_item = _FakeItem(slow=True) + fast_item = _FakeItem() + items = [slow_item, fast_item] + + conftest.pytest_collection_modifyitems(config, items) + + assert items == [fast_item] + assert config.hook.deselected == [slow_item]