Skip to content

Fix passenv CI in tox ini and make tests insensitive to the presence of the CI env variable #13684

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 9 commits into from
Sep 2, 2025
+20 −2
Merged

Fix passenv CI in tox ini and make tests insensitive to the presence of the CI env variable #13684

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
6f89a1c
MAINT passenv CI in tox.ini
ogrisel Aug 28, 2025
e33850b
MAINT make tests insensitive to the presence of the CI env var
ogrisel Aug 28, 2025
1b5f077
Make test_line_with_reprcrash CI insensitive
ogrisel Aug 28, 2025
e3f4dad
Document change in the changelog
ogrisel Aug 28, 2025
c1976ad
Make test_error_messages_with_different_verbosity CI insensitive
ogrisel Aug 28, 2025
1efb477
Update changelog/13684.contrib.rst
ogrisel Aug 28, 2025
c34d9b5
Factorize monkeypatch.delenv('CI', raising=False) into remove_ci_env_…
ogrisel Sep 2, 2025
ef78906
Make enable autouse on the remove_ci_env_var fixture + add a marker t…
ogrisel Sep 2, 2025
1ce85ea
Re-add the skip mark on test_timeout
ogrisel Sep 2, 2025
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
1 change: 1 addition & 0 deletions changelog/13684.contrib.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Make pytest's own testsuite insensitive to the presence of the ``CI`` environment variable -- by :user:`ogrisel`.
6 changes: 5 additions & 1 deletion testing/python/approx.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
from _pytest.python_api import _recursive_sequence_map
import pytest
from pytest import approx
from pytest import MonkeyPatch


inf, nan = float("inf"), float("nan")
Expand Down Expand Up @@ -283,7 +284,10 @@ def test_error_messages_invalid_args(self, assert_approx_raises_regex):
]
)

def test_error_messages_with_different_verbosity(self, assert_approx_raises_regex):
def test_error_messages_with_different_verbosity(
self, assert_approx_raises_regex, monkeypatch: MonkeyPatch
):
monkeypatch.delenv("CI")
Copy link
Member

@nicoddemus nicoddemus Sep 1, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

I believe we want to remove this env var for all tests, correct?

If so, I suggest you add this autouse fixture to conftest.py:

@pytest.fixture(autouse=True)
def remove_ci_env_var(monkeypatch: MonkeyPatch) -> None:
    """
    Make the test suite insensitive if it is running in CI or not.
    
    If a test requires the variable, it should set it explicitly.
    """
    monkeypatch.delenv("CI", raising=False)

Then you can revert the changes done to the tests.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Actually, we might want to use this env var in #13679 to use a longer timeout value in some tests to decrease likelihood of failure on slow CI runners.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

To be more specific: there is an existing test named test_timeout in test_faulhandler.py that relies on the presence of this variable to adjust the value of the timeout to make it less likely to fail in case the CI runner is too slow (see #7022). However, this strategy never worked as intended because the CI variable propagation was always blocked by the existing configuration in tox.ini.

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, I wasn't sure what the impact would be in other places so I didn't suggest making it an autouse fixture.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I will update the PR to introduce the fixture but not make it auto-use for now.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@nicoddemus @webknjaz @RonnyPfannschmidt done in c34d9b5.

Once merged, I can follow up with a PR to update test_timeout to remove the skip marker and then trigger the tests on the CI a few times in a row to check that it can properly fix #7022 without having to skip the test entirely on the CI. Then I will update #13679 accordingly.

Copy link
Member

@nicoddemus nicoddemus Sep 2, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

To be more specific: there is an existing test named test_timeout in test_faulhandler.py that relies on the presence of this variable to adjust the value of the timeout to make it less likely to fail in case the CI runner is too slow

Ahh I see.

In that case we could then create a new mark that skips deleting the env var if present in a test.

@pytest.fixture(autouse=True)
def remove_ci_env_var(monkeypatch: MonkeyPatch, request: pytest.FixtureRequest) -> None:
    """
    Make the test insensitive if it is running in CI or not.

    Use `@pytest.mark.keep_ci_var` in a test to avoid applying this fixture, letting the test
    see the real `CI` variable (if present).
    """
    has_keep_ci_mark = request.node.get_closest_marker("keep_ci_var") is not None
    if not has_keep_ci_mark:
        monkeypatch.delenv("CI", raising=False)

We can apply this mark to test_timeout and any other test that requires seeing the real CI variable.

@ogrisel I understand this is PR is a side quest just to work on #13679, but would mind give the autouse fixture a go? I think the time to try this approach is now that we have someone on hands on the issue.

If there are too many breakages and you decide to not deal with them now, feel free to revert the autouse change and merge as is currently.

Copy link
Member

Choose a reason for hiding this comment

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

I'm leaving pre-approved. Would appreciate you giving the autouse approach one more try before merging though.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Alright, let me give this a try.

np = pytest.importorskip("numpy")
for v in [0, 1, 2]:
# Verbosity level doesn't affect the error message for scalars
Expand Down
32 changes: 24 additions & 8 deletions testing/test_assertion.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -110,11 +110,14 @@ def test(check_first):
assert 0
result.stdout.fnmatch_lines([expected])

def test_rewrite_assertions_pytester_plugin(self, pytester: Pytester) -> None:
def test_rewrite_assertions_pytester_plugin(
self, pytester: Pytester, monkeypatch: MonkeyPatch
) -> None:
"""
Assertions in the pytester plugin must also benefit from assertion
rewriting (#1920).
"""
monkeypatch.delenv("CI")
pytester.makepyfile(
"""
pytest_plugins = ['pytester']
Expand Down Expand Up @@ -452,8 +455,9 @@ def test_multiline_text_diff(self) -> None:
assert "- eggs" in diff
assert "+ spam" in diff

def test_bytes_diff_normal(self) -> None:
def test_bytes_diff_normal(self, monkeypatch: MonkeyPatch) -> None:
"""Check special handling for bytes diff (#5260)"""
monkeypatch.delenv("CI")
diff = callequal(b"spam", b"eggs")

assert diff == [
Expand Down Expand Up @@ -530,20 +534,24 @@ def test_list(self) -> None:
),
],
)
def test_iterable_full_diff(self, left, right, expected) -> None:
def test_iterable_full_diff(
self, left, right, expected, monkeypatch: MonkeyPatch
) -> None:
"""Test the full diff assertion failure explanation.

When verbose is False, then just a -v notice to get the diff is rendered,
when verbose is True, then ndiff of the pprint is returned.
"""
monkeypatch.delenv("CI")
expl = callequal(left, right, verbose=0)
assert expl is not None
assert expl[-1] == "Use -v to get more diff"
verbose_expl = callequal(left, right, verbose=1)
assert verbose_expl is not None
assert "\n".join(verbose_expl).endswith(textwrap.dedent(expected).strip())

def test_iterable_quiet(self) -> None:
def test_iterable_quiet(self, monkeypatch: MonkeyPatch) -> None:
monkeypatch.delenv("CI")
expl = callequal([1, 2], [10, 2], verbose=-1)
assert expl == [
"[1, 2] == [10, 2]",
Expand Down Expand Up @@ -713,7 +721,8 @@ def test_dict(self) -> None:
assert expl is not None
assert len(expl) > 1

def test_dict_omitting(self) -> None:
def test_dict_omitting(self, monkeypatch: MonkeyPatch) -> None:
monkeypatch.delenv("CI")
lines = callequal({"a": 0, "b": 1}, {"a": 1, "b": 1})
assert lines is not None
assert lines[2].startswith("Omitting 1 identical item")
Expand Down Expand Up @@ -981,7 +990,10 @@ def test_dataclasses(self, pytester: Pytester) -> None:
consecutive=True,
)

def test_recursive_dataclasses(self, pytester: Pytester) -> None:
def test_recursive_dataclasses(
self, pytester: Pytester, monkeypatch: MonkeyPatch
) -> None:
monkeypatch.delenv("CI")
p = pytester.copy_example("dataclasses/test_compare_recursive_dataclasses.py")
result = pytester.runpytest(p)
result.assert_outcomes(failed=1, passed=0)
Expand Down Expand Up @@ -1213,7 +1225,9 @@ def __eq__(self, other): # pragma: no cover


class TestAssert_reprcompare_namedtuple:
def test_namedtuple(self) -> None:
def test_namedtuple(self, monkeypatch: MonkeyPatch) -> None:
monkeypatch.delenv("CI")

class NT(NamedTuple):
a: Any
b: Any
Expand All @@ -1236,7 +1250,9 @@ class NT(NamedTuple):
"Use -v to get more diff",
]

def test_comparing_two_different_namedtuple(self) -> None:
def test_comparing_two_different_namedtuple(self, monkeypatch: MonkeyPatch) -> None:
monkeypatch.delenv("CI")

class NT1(NamedTuple):
a: Any
b: Any
Expand Down
1 change: 1 addition & 0 deletions testing/test_terminal.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2536,6 +2536,7 @@ class X:


def test_line_with_reprcrash(monkeypatch: MonkeyPatch) -> None:
monkeypatch.delenv("CI")
mocked_verbose_word = "FAILED"

mocked_pos = "some::nodeid"
Expand Down
1 change: 1 addition & 0 deletions tox.ini
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ passenv =
PYTEST_ADDOPTS
TERM
SETUPTOOLS_SCM_PRETEND_VERSION_FOR_PYTEST
CI
setenv =
_PYTEST_TOX_DEFAULT_POSARGS={env:_PYTEST_TOX_POSARGS_DOCTESTING:} {env:_PYTEST_TOX_POSARGS_LSOF:} {env:_PYTEST_TOX_POSARGS_XDIST:} {env:_PYTEST_FILES:}

Expand Down
Loading