Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions .github/workflows/ci.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,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 --skipslow

pytest-extra:
name: Extra unit tests
Expand Down Expand Up @@ -162,7 +162,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 --skipslow src/openfermion/resource_estimates

pytest-compat:
name: Python compatibility tests
Expand Down Expand Up @@ -190,7 +190,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 --skipslow

coverage:
name: Python code coverage tests
Expand Down
2 changes: 1 addition & 1 deletion CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -293,7 +293,7 @@ We use [pytest](https://docs.pytest.org) to run our tests and
in the `check/` subdirectory to run these programs.

* During development, periodically check that code changes do not break anything. For fast checks,
run `check/pytest -m "not slow" -n auto PATH ...`, where `PATH ...` is one or more directories
run `check/pytest --skipslow -n auto PATH ...`, where `PATH ...` is one or more directories
or `_test.py` files to test. (The `-n auto` option makes pytest use parallel processes; omit it
if it causes problems on your system.)

Expand Down
9 changes: 9 additions & 0 deletions conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -66,3 +66,12 @@ def set_threadpool_limits():
yield
else:
yield


def pytest_addoption(parser: pytest.Parser) -> None:
parser.addoption("--skipslow", action="store_true", help="skips slow tests")


def pytest_runtest_setup(item: pytest.Item) -> None:
if "slow" in item.keywords and item.config.getoption("skipslow"):
pytest.skip("skipped because of --skipslow option")
Comment on lines +75 to +77

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

Using "slow" in item.keywords is a pytest anti-pattern. item.keywords contains not only the markers applied to a test, but also the names of the test function, class, and module (and their components split by underscores). This means any test with the word slow in its name, class, or file name (e.g., test_slow_algorithm) will be incorrectly skipped when --skipslow is passed, even if it is not marked with @pytest.mark.slow.

Instead, use item.get_closest_marker("slow") to safely and precisely check for the presence of the marker.

Suggested change
def pytest_runtest_setup(item: pytest.Item) -> None:
if "slow" in item.keywords and item.config.getoption("skipslow"):
pytest.skip("skipped because of --skipslow option")
def pytest_runtest_setup(item: pytest.Item) -> None:
if item.get_closest_marker("slow") is not None and item.config.getoption("skipslow"):
pytest.skip("skipped because of --skipslow option")

2 changes: 1 addition & 1 deletion pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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 (skip with '--skipslow')""",
]
filterwarnings = [
"ignore:Skipped assert_qasm_is_consistent_with_unitary because qiskit.*:UserWarning",
Expand Down
47 changes: 47 additions & 0 deletions src/openfermion/config_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -170,3 +170,50 @@ def test_set_threading_limits(self):
with patch("openfermion.config.get_available_cpu_count", return_value=8):
set_threading_limits()
self.assertEqual(os.environ.get("OMP_NUM_THREADS"), "7")


class ConftestTest(unittest.TestCase):

def test_pytest_addoption(self):
import conftest
from unittest.mock import MagicMock

parser = MagicMock()
conftest.pytest_addoption(parser)
parser.addoption.assert_called_once_with(
"--skipslow", action="store_true", help="skips slow tests"
)

def test_pytest_runtest_setup_skips(self):
import conftest
import pytest
from unittest.mock import MagicMock

# Create mock item representing a slow test when skipslow is True.
item = MagicMock()
item.keywords = {"slow"}
item.config.getoption.return_value = True

with self.assertRaises(pytest.skip.Exception):
conftest.pytest_runtest_setup(item)

item.config.getoption.assert_called_once_with("skipslow")

def test_pytest_runtest_setup_does_not_skip_if_not_slow(self):
import conftest
from unittest.mock import MagicMock

# Test case 1: Not marked as 'slow', skipslow is True.
item = MagicMock()
item.keywords = set()
item.config.getoption.return_value = True

# Should not raise an exception.
conftest.pytest_runtest_setup(item)

# Test case 2: Marked as 'slow', skipslow is False.
item = MagicMock()
item.keywords = {"slow"}
item.config.getoption.return_value = False

conftest.pytest_runtest_setup(item)
Comment on lines +187 to +219

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

Since we are updating pytest_runtest_setup to use item.get_closest_marker("slow") instead of checking item.keywords, we should update the unit tests to mock get_closest_marker accordingly.

    def test_pytest_runtest_setup_skips(self):
        import conftest
        import pytest
        from unittest.mock import MagicMock

        # Create mock item representing a slow test when skipslow is True.
        item = MagicMock()
        item.get_closest_marker.return_value = MagicMock()
        item.config.getoption.return_value = True

        with self.assertRaises(pytest.skip.Exception):
            conftest.pytest_runtest_setup(item)

        item.get_closest_marker.assert_called_once_with("slow")
        item.config.getoption.assert_called_once_with("skipslow")

    def test_pytest_runtest_setup_does_not_skip_if_not_slow(self):
        import conftest
        from unittest.mock import MagicMock

        # Test case 1: Not marked as 'slow', skipslow is True.
        item = MagicMock()
        item.get_closest_marker.return_value = None
        item.config.getoption.return_value = True

        # Should not raise an exception.
        conftest.pytest_runtest_setup(item)

        # Test case 2: Marked as 'slow', skipslow is False.
        item = MagicMock()
        item.get_closest_marker.return_value = MagicMock()
        item.config.getoption.return_value = False

        conftest.pytest_runtest_setup(item)

Loading