Skip to content
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

reenable pytest -p unraisableexception and -p threadexception #1898

Merged
Merged
Show file tree
Hide file tree
Changes from 4 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
2 changes: 1 addition & 1 deletion ci.sh
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,7 @@ else
cd empty

INSTALLDIR=$(python -c "import os, trio; print(os.path.dirname(trio.__file__))")
cp ../setup.cfg $INSTALLDIR
cp ../pyproject.toml $INSTALLDIR
# We have to copy .coveragerc into this directory, rather than passing
# --cov-config=../.coveragerc to pytest, because codecov.sh will run
# 'coverage xml' to generate the report that it uses, and that will only
Expand Down
7 changes: 7 additions & 0 deletions pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -50,3 +50,10 @@ directory = "misc"
name = "Miscellaneous internal changes"
showcontent = true

[tool.pytest.ini_options]
addopts = ["--strict-markers", "--strict-config"]
altendky marked this conversation as resolved.
Show resolved Hide resolved
xfail_strict = true
faulthandler_timeout = 60
markers = ["redistributors_should_skip: tests that should be skipped by downstream redistributors"]
junit_family = "xunit2"
filterwarnings = ["error"]
Copy link
Member

Choose a reason for hiding this comment

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

The -W error in ci.sh wouldn't be needed anymore, right?

Copy link
Member Author

@graingert graingert Mar 5, 2021

Choose a reason for hiding this comment

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

I'm pretty sure it's redundant now, but does no harm there

Copy link
Member

Choose a reason for hiding this comment

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

It'll be confusing when someone wants it off and changes one spot and it's still active. Or when they are looking to Trio as an example.

9 changes: 0 additions & 9 deletions setup.cfg

This file was deleted.

42 changes: 23 additions & 19 deletions trio/_core/tests/test_asyncgen.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
from functools import partial
from async_generator import aclosing
from ... import _core
from .tutil import gc_collect_harder, buggy_pypy_asyncgens
from .tutil import gc_collect_harder, buggy_pypy_asyncgens, restore_unraisablehook


def test_asyncgen_basics():
Expand Down Expand Up @@ -94,8 +94,9 @@ async def agen():
record.append("crashing")
raise ValueError("oops")

await agen().asend(None)
gc_collect_harder()
with restore_unraisablehook():
await agen().asend(None)
gc_collect_harder()
await _core.wait_all_tasks_blocked()
assert record == ["crashing"]
exc_type, exc_value, exc_traceback = caplog.records[0].exc_info
Expand Down Expand Up @@ -170,6 +171,7 @@ async def async_main():
assert record == ["innermost"] + list(range(100))


@restore_unraisablehook()
def test_last_minute_gc_edge_case():
saved = []
record = []
Expand Down Expand Up @@ -267,17 +269,18 @@ async def awaits_after_yield():
yield 42
await _core.cancel_shielded_checkpoint()

await step_outside_async_context(well_behaved())
gc_collect_harder()
assert capsys.readouterr().err == ""
with restore_unraisablehook():
await step_outside_async_context(well_behaved())
gc_collect_harder()
assert capsys.readouterr().err == ""

await step_outside_async_context(yields_after_yield())
gc_collect_harder()
assert "ignored GeneratorExit" in capsys.readouterr().err
await step_outside_async_context(yields_after_yield())
gc_collect_harder()
assert "ignored GeneratorExit" in capsys.readouterr().err

await step_outside_async_context(awaits_after_yield())
gc_collect_harder()
assert "awaited something during finalization" in capsys.readouterr().err
await step_outside_async_context(awaits_after_yield())
gc_collect_harder()
assert "awaited something during finalization" in capsys.readouterr().err


@pytest.mark.skipif(buggy_pypy_asyncgens, reason="pypy 7.2.0 is buggy")
Expand Down Expand Up @@ -307,10 +310,11 @@ async def async_main():
await _core.wait_all_tasks_blocked()
assert record == ["trio collected ours"]

old_hooks = sys.get_asyncgen_hooks()
sys.set_asyncgen_hooks(my_firstiter, my_finalizer)
try:
_core.run(async_main)
finally:
assert sys.get_asyncgen_hooks() == (my_firstiter, my_finalizer)
sys.set_asyncgen_hooks(*old_hooks)
with restore_unraisablehook():
old_hooks = sys.get_asyncgen_hooks()
sys.set_asyncgen_hooks(my_firstiter, my_finalizer)
try:
_core.run(async_main)
finally:
assert sys.get_asyncgen_hooks() == (my_firstiter, my_finalizer)
sys.set_asyncgen_hooks(*old_hooks)
4 changes: 3 additions & 1 deletion trio/_core/tests/test_guest_mode.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@

import trio
import trio.testing
from .tutil import gc_collect_harder, buggy_pypy_asyncgens
from .tutil import gc_collect_harder, buggy_pypy_asyncgens, restore_unraisablehook
from ..._util import signal_raise

# The simplest possible "host" loop.
Expand Down Expand Up @@ -277,6 +277,7 @@ def after_io_wait(self, timeout):
assert trivial_guest_run(trio_main) == "ok"


@restore_unraisablehook()
def test_guest_warns_if_abandoned():
# This warning is emitted from the garbage collector. So we have to make
# sure that our abandoned run is garbage. The easiest way to do this is to
Expand Down Expand Up @@ -506,6 +507,7 @@ async def trio_main(in_host):
sys.implementation.name == "pypy" and sys.version_info >= (3, 7),
reason="async generator issue under investigation",
)
@restore_unraisablehook()
def test_guest_mode_asyncgens():
import sniffio

Expand Down
4 changes: 4 additions & 0 deletions trio/_core/tests/test_run.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
gc_collect_harder,
ignore_coroutine_never_awaited_warnings,
buggy_pypy_asyncgens,
restore_unraisablehook,
)

from ... import _core
Expand Down Expand Up @@ -844,6 +845,7 @@ async def stubborn_sleeper():
assert record == ["sleep", "woke", "cancelled"]


@restore_unraisablehook()
def test_broken_abort():
async def main():
# These yields are here to work around an annoying warning -- we're
Expand All @@ -870,6 +872,7 @@ async def main():
gc_collect_harder()


@restore_unraisablehook()
def test_error_in_run_loop():
# Blow stuff up real good to check we at least get a TrioInternalError
async def main():
Expand Down Expand Up @@ -2154,6 +2157,7 @@ def abort_fn(_):
assert abort_fn_called


@restore_unraisablehook()
def test_async_function_implemented_in_C():
# These used to crash because we'd try to mutate the coroutine object's
# cr_frame, but C functions don't have Python frames.
Expand Down
36 changes: 25 additions & 11 deletions trio/_core/tests/test_thread_cache.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,9 @@
from queue import Queue
import time
import sys
from contextlib import contextmanager

from .tutil import slow, gc_collect_harder
from .tutil import slow, gc_collect_harder, disable_threading_excepthook
from .. import _thread_cache
from .._thread_cache import start_thread_soon, ThreadCache

Expand Down Expand Up @@ -99,6 +100,17 @@ def test_idle_threads_exit(monkeypatch):
assert not seen_thread.is_alive()


@contextmanager
def _join_started_threads():
before = frozenset(threading.enumerate())
try:
yield
finally:
for thread in threading.enumerate():
if thread not in before:
thread.join()


def test_race_between_idle_exit_and_job_assignment(monkeypatch):
# This is a lock where the first few times you try to acquire it with a
# timeout, it waits until the lock is available and then pretends to time
Expand Down Expand Up @@ -138,13 +150,15 @@ def release(self):

monkeypatch.setattr(_thread_cache, "Lock", JankyLock)

tc = ThreadCache()
done = threading.Event()
tc.start_thread_soon(lambda: None, lambda _: done.set())
done.wait()
# Let's kill the thread we started, so it doesn't hang around until the
# test suite finishes. Doesn't really do any harm, but it can be confusing
# to see it in debug output. This is hacky, and leaves our ThreadCache
# object in an inconsistent state... but it doesn't matter, because we're
# not going to use it again anyway.
tc.start_thread_soon(lambda: None, lambda _: sys.exit())
with disable_threading_excepthook(), _join_started_threads():
tc = ThreadCache()
done = threading.Event()
tc.start_thread_soon(lambda: None, lambda _: done.set())
done.wait()
# Let's kill the thread we started, so it doesn't hang around until the
# test suite finishes. Doesn't really do any harm, but it can be confusing
# to see it in debug output. This is hacky, and leaves our ThreadCache
# object in an inconsistent state... but it doesn't matter, because we're
# not going to use it again anyway.

tc.start_thread_soon(lambda: None, lambda _: sys.exit())
3 changes: 2 additions & 1 deletion trio/_core/tests/test_windows.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
# Mark all the tests in this file as being windows-only
pytestmark = pytest.mark.skipif(not on_windows, reason="windows only")

from .tutil import slow, gc_collect_harder
from .tutil import slow, gc_collect_harder, restore_unraisablehook
from ... import _core, sleep, move_on_after
from ...testing import wait_all_tasks_blocked

Expand Down Expand Up @@ -111,6 +111,7 @@ def pipe_with_overlapped_read():
kernel32.CloseHandle(ffi.cast("HANDLE", write_handle))


@restore_unraisablehook()
def test_forgot_to_register_with_iocp():
with pipe_with_overlapped_read() as (write_fp, read_handle):
with write_fp:
Expand Down
35 changes: 35 additions & 0 deletions trio/_core/tests/tutil.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
# Utilities for testing
import socket as stdlib_socket
import threading
import os
import sys
from typing import TYPE_CHECKING
Expand Down Expand Up @@ -80,6 +81,40 @@ def ignore_coroutine_never_awaited_warnings():
gc_collect_harder()


def _noop(*args, **kwargs):
pass


if sys.version_info >= (3, 8):

@contextmanager
def restore_unraisablehook():
altendky marked this conversation as resolved.
Show resolved Hide resolved
sys.unraisablehook, prev = sys.__unraisablehook__, sys.unraisablehook
try:
yield
finally:
sys.unraisablehook = prev

@contextmanager
def disable_threading_excepthook():
threading.excepthook, prev = _noop, threading.excepthook
Copy link
Member Author

@graingert graingert Feb 11, 2021

Choose a reason for hiding this comment

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

sadly threading.__excepthook__ is only in 3.10, so I disable the hook rather than restore the old one

Copy link
Member

Choose a reason for hiding this comment

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

I don't know... if we were already testing 3.10 then maybe it would be good practice to put an if in here to use threading.__excepthook__ when possible and then when we dropped 3.9 there would be a coverage loss and we'd remove the work around. But we aren't testing against 3.10... so... a comment? Or perhaps this is too far forward looking to be appropriate.

Copy link
Member

Choose a reason for hiding this comment

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

Probably shouldn't hold this up. Maybe I'll add something in #1921.

try:
yield
finally:
threading.excephtook = prev
graingert marked this conversation as resolved.
Show resolved Hide resolved


else:

@contextmanager
def restore_unraisablehook(): # pragma: no cover
yield

@contextmanager
def disable_threading_excepthook(): # pragma: no cover
yield


# template is like:
# [1, {2.1, 2.2}, 3] -> matches [1, 2.1, 2.2, 3] or [1, 2.2, 2.1, 3]
def check_sequence_matches(seq, template):
Expand Down