Skip to content

[PR #13525/391324e1 backport][8.4.x] pytester: avoid unraisableexception gc collects in inline runs to speed up test suite #13526

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.

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
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
10 changes: 8 additions & 2 deletions src/_pytest/pytester.py
Original file line number Diff line number Diff line change
Expand Up @@ -1092,6 +1092,8 @@ def inline_run(
Typically we reraise keyboard interrupts from the child run. If
True, the KeyboardInterrupt exception is captured.
"""
from _pytest.unraisableexception import gc_collect_iterations_key

# (maybe a cpython bug?) the importlib cache sometimes isn't updated
# properly between file creation and inline_run (especially if imports
# are interspersed with file creation)
Expand All @@ -1115,12 +1117,16 @@ def inline_run(

rec = []

class Collect:
class PytesterHelperPlugin:
@staticmethod
def pytest_configure(config: Config) -> None:
rec.append(self.make_hook_recorder(config.pluginmanager))

plugins.append(Collect())
# The unraisable plugin GC collect slows down inline
# pytester runs too much.
config.stash[gc_collect_iterations_key] = 0

plugins.append(PytesterHelperPlugin())
ret = main([str(x) for x in args], plugins=plugins)
if len(rec) == 1:
reprec = rec.pop()
Expand Down
15 changes: 10 additions & 5 deletions src/_pytest/unraisableexception.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,10 +24,12 @@
from exceptiongroup import ExceptionGroup


def gc_collect_harder() -> None:
# A single collection doesn't necessarily collect everything.
# Constant determined experimentally by the Trio project.
for _ in range(5):
# This is a stash item and not a simple constant to allow pytester to override it.
gc_collect_iterations_key = StashKey[int]()


def gc_collect_harder(iterations: int) -> None:
for _ in range(iterations):
gc.collect()


Expand Down Expand Up @@ -84,9 +86,12 @@ def collect_unraisable(config: Config) -> None:
def cleanup(
*, config: Config, prev_hook: Callable[[sys.UnraisableHookArgs], object]
) -> None:
# A single collection doesn't necessarily collect everything.
# Constant determined experimentally by the Trio project.
gc_collect_iterations = config.stash.get(gc_collect_iterations_key, 5)
try:
try:
gc_collect_harder()
gc_collect_harder(gc_collect_iterations)
collect_unraisable(config)
finally:
sys.unraisablehook = prev_hook
Expand Down
3 changes: 2 additions & 1 deletion testing/test_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -2175,7 +2175,8 @@ class DummyPlugin:
plugins = config.invocation_params.plugins
assert len(plugins) == 2
assert plugins[0] is plugin
assert type(plugins[1]).__name__ == "Collect" # installed by pytester.inline_run()
# Installed by pytester.inline_run().
assert type(plugins[1]).__name__ == "PytesterHelperPlugin"

# args cannot be None
with pytest.raises(TypeError):
Expand Down
51 changes: 23 additions & 28 deletions testing/test_unraisableexception.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,5 @@
from __future__ import annotations

from collections.abc import Generator
import contextlib
import gc
import sys
from unittest import mock
Expand Down Expand Up @@ -229,19 +227,13 @@ def _set_gc_state(enabled: bool) -> bool:
return was_enabled


@contextlib.contextmanager
def _disable_gc() -> Generator[None]:
was_enabled = _set_gc_state(enabled=False)
try:
yield
finally:
_set_gc_state(enabled=was_enabled)


def test_refcycle_unraisable(pytester: Pytester) -> None:
# see: https://github.com/pytest-dev/pytest/issues/10404
pytester.makepyfile(
test_it="""
# Should catch the unraisable exception even if gc is disabled.
import gc; gc.disable()

import pytest

class BrokenDel:
Expand All @@ -256,23 +248,22 @@ def test_it():
"""
)

with _disable_gc():
result = pytester.runpytest()
result = pytester.runpytest_subprocess(
"-Wdefault::pytest.PytestUnraisableExceptionWarning"
)

# TODO: should be a test failure or error
assert result.ret == pytest.ExitCode.INTERNAL_ERROR
assert result.ret == 0

result.assert_outcomes(passed=1)
result.stderr.fnmatch_lines("ValueError: del is broken")


@pytest.mark.filterwarnings("default::pytest.PytestUnraisableExceptionWarning")
def test_refcycle_unraisable_warning_filter(pytester: Pytester) -> None:
# note that the host pytest warning filter is disabled and the pytester
# warning filter applies during config teardown of unraisablehook.
# see: https://github.com/pytest-dev/pytest/issues/10404
pytester.makepyfile(
test_it="""
# Should catch the unraisable exception even if gc is disabled.
import gc; gc.disable()

import pytest

class BrokenDel:
Expand All @@ -287,17 +278,18 @@ def test_it():
"""
)

with _disable_gc():
result = pytester.runpytest("-Werror")
result = pytester.runpytest_subprocess(
"-Werror::pytest.PytestUnraisableExceptionWarning"
)

# TODO: should be a test failure or error
assert result.ret == pytest.ExitCode.INTERNAL_ERROR
# TODO: Should be a test failure or error. Currently the exception
# propagates all the way to the top resulting in exit code 1.
assert result.ret == 1

result.assert_outcomes(passed=1)
result.stderr.fnmatch_lines("ValueError: del is broken")


@pytest.mark.filterwarnings("default::pytest.PytestUnraisableExceptionWarning")
def test_create_task_raises_unraisable_warning_filter(pytester: Pytester) -> None:
# note that the host pytest warning filter is disabled and the pytester
# warning filter applies during config teardown of unraisablehook.
Expand All @@ -306,6 +298,9 @@ def test_create_task_raises_unraisable_warning_filter(pytester: Pytester) -> Non
# the issue
pytester.makepyfile(
test_it="""
# Should catch the unraisable exception even if gc is disabled.
import gc; gc.disable()

import asyncio
import pytest

Expand All @@ -318,11 +313,11 @@ def test_scheduler_must_be_created_within_running_loop() -> None:
"""
)

with _disable_gc():
result = pytester.runpytest("-Werror")
result = pytester.runpytest_subprocess("-Werror")

# TODO: should be a test failure or error
assert result.ret == pytest.ExitCode.INTERNAL_ERROR
# TODO: Should be a test failure or error. Currently the exception
# propagates all the way to the top resulting in exit code 1.
assert result.ret == 1

result.assert_outcomes(passed=1)
result.stderr.fnmatch_lines("RuntimeWarning: coroutine 'my_task' was never awaited")
Expand Down
Loading