diff --git a/pytest_trio/_tests/test_fixture_ordering.py b/pytest_trio/_tests/test_fixture_ordering.py index a180958..0a218d3 100644 --- a/pytest_trio/_tests/test_fixture_ordering.py +++ b/pytest_trio/_tests/test_fixture_ordering.py @@ -1,11 +1,14 @@ import pytest -# Tests that: -# - leaf_fix gets set up first and torn down last -# - the two fix_concurrent_{1,2} fixtures run their setup/teardown code -# at the same time -- their execution can be interleaved. def test_fixture_basic_ordering(testdir): + """ + Tests that: + - leaf_fix gets set up first and torn down last + - the two fix_{1,2} fixtures run their setup/teardown code + in the expected order + fix_1 setup -> fix_2 setup -> fix_2 teardown -> fix_1 teardown + """ testdir.makepyfile( """ import pytest @@ -26,48 +29,189 @@ async def leaf_fix(): teardown_events.append("leaf_fix teardown") assert teardown_events == [ - "fix_concurrent_1 teardown 1", - "fix_concurrent_2 teardown 1", - "fix_concurrent_1 teardown 2", - "fix_concurrent_2 teardown 2", + "fix_2 teardown 1", + "fix_2 teardown 2", + "fix_1 teardown 1", + "fix_1 teardown 2", "leaf_fix teardown", ] @pytest.fixture - async def fix_concurrent_1(leaf_fix, seq): + async def fix_1(leaf_fix, seq): async with seq(0): - setup_events.append("fix_concurrent_1 setup 1") - async with seq(2): - setup_events.append("fix_concurrent_1 setup 2") + setup_events.append("fix_1 setup 1") + async with seq(1): + setup_events.append("fix_1 setup 2") yield - async with seq(4): - teardown_events.append("fix_concurrent_1 teardown 1") async with seq(6): - teardown_events.append("fix_concurrent_1 teardown 2") + teardown_events.append("fix_1 teardown 1") + async with seq(7): + teardown_events.append("fix_1 teardown 2") @pytest.fixture - async def fix_concurrent_2(leaf_fix, seq): - async with seq(1): - setup_events.append("fix_concurrent_2 setup 1") + async def fix_2(leaf_fix, seq): + async with seq(2): + setup_events.append("fix_2 setup 1") async with seq(3): - setup_events.append("fix_concurrent_2 setup 2") + setup_events.append("fix_2 setup 2") yield + async with seq(4): + teardown_events.append("fix_2 teardown 1") async with seq(5): - teardown_events.append("fix_concurrent_2 teardown 1") - async with seq(7): - teardown_events.append("fix_concurrent_2 teardown 2") + teardown_events.append("fix_2 teardown 2") @pytest.mark.trio - async def test_root(fix_concurrent_1, fix_concurrent_2): + async def test_root(fix_1, fix_2): assert setup_events == [ "leaf_fix setup", - "fix_concurrent_1 setup 1", - "fix_concurrent_2 setup 1", - "fix_concurrent_1 setup 2", - "fix_concurrent_2 setup 2", + "fix_1 setup 1", + "fix_1 setup 2", + "fix_2 setup 1", + "fix_2 setup 2", + ] + assert teardown_events == [] + + """ + ) + + result = testdir.runpytest() + result.assert_outcomes(passed=1) + + +def test_fixture_complicated_dag_ordering(testdir): + """ + This test involves several fixtures forming a pretty + complicated DAG, make sure we got the topological sort in order. + """ + testdir.makepyfile( + """ + import pytest + from pytest_trio import trio_fixture + + setup_events = [] + teardown_events = [] + + @trio_fixture + async def fix_6(fix_7): + setup_events.append("fix_6 setup") + yield + teardown_events.append("fix_6 teardown") + + @pytest.fixture + async def fix_7(): + setup_events.append("fix_7 setup") + yield + teardown_events.append("fix_7 teardown") + assert teardown_events == [ + "fix_4 teardown", + "fix_5 teardown", + "fix_3 teardown", + "fix_1 teardown", + "fix_2 teardown", + "fix_6 teardown", + "fix_7 teardown", + ] + @pytest.fixture + async def fix_4(fix_5, fix_6, fix_7): + setup_events.append("fix_4 setup") + yield + teardown_events.append("fix_4 teardown") + + @pytest.fixture + async def fix_5(fix_3): + setup_events.append("fix_5 setup") + yield + teardown_events.append("fix_5 teardown") + + @pytest.fixture + async def fix_3(fix_1, fix_6): + setup_events.append("fix_3 setup") + yield + teardown_events.append("fix_3 teardown") + + @pytest.fixture + async def fix_1(fix_2, fix_7): + setup_events.append("fix_1 setup") + yield + teardown_events.append("fix_1 teardown") + + @pytest.fixture + async def fix_2(fix_6, fix_7): + setup_events.append("fix_2 setup") + yield + teardown_events.append("fix_2 teardown") + + @pytest.mark.trio + async def test_root(fix_1, fix_2, fix_3, fix_4, fix_5, fix_6, fix_7): + assert setup_events == [ + "fix_7 setup", + "fix_6 setup", + "fix_2 setup", + "fix_1 setup", + "fix_3 setup", + "fix_5 setup", + "fix_4 setup", ] assert teardown_events == [] + """ + ) + result = testdir.runpytest() + result.assert_outcomes(passed=1) + + +def test_contextvars_modification_follows_fixture_ordering(testdir): + """ + Tests that fixtures are being set up and tore down synchronously. + Specifically this ensures that fixtures that modify context variables + doesn't lead to a race condition. + This tries to simluate thing like trio_asyncio.open_loop that modifies + the contextvar. + + Main assertion is that 2 async tasks in teardown (Resource.__aexit__) + doesn't crash. + """ + testdir.makepyfile( + """ + import contextvars + import pytest + import trio + from contextlib import asynccontextmanager + current_value = contextvars.ContextVar("variable", default=None) + + @asynccontextmanager + async def variable_setter(): + old_value = current_value.set("value") + try: + await trio.sleep(0) + yield + finally: + current_value.reset(old_value) + + class Resource(): + async def __aenter__(self): + await trio.sleep(0) + + async def __aexit__(self, *_): + # We need to yield and run another trio task + await trio.sleep(0) + assert current_value.get() is not None + + @pytest.fixture + async def resource(): + async with variable_setter() as loop: + async with Resource(): + yield + + @pytest.fixture + async def trio_asyncio_loop(): + async with variable_setter() as loop: + yield loop + + @pytest.mark.trio + async def test_root(trio_asyncio_loop, resource): + await trio.sleep(0) + assert True """ ) @@ -127,18 +271,53 @@ async def test_root(fix2, nursery): result.assert_outcomes(passed=1) +def test_error_message_upon_circular_dependency(testdir): + """ + Make sure that the error message is produced if there's + a circular dependency on the fixtures + """ + testdir.makepyfile( + """ + import pytest + from pytest_trio import trio_fixture + + @trio_fixture + def seq(leaf_fix): + pass + + @pytest.fixture + async def leaf_fix(seq): + pass + + @pytest.mark.trio + async def test_root(leaf_fix, seq): + pass + """ + ) + + result = testdir.runpytest() + result.assert_outcomes(errors=1) + result.stdout.fnmatch_lines( + ["*recursive dependency involving fixture 'leaf_fix' detected*"] + ) + + def test_error_collection(testdir): - # We want to make sure that pytest ultimately reports all the different + # We want to make sure that pytest ultimately reports all the # exceptions. We call .upper() on all the exceptions so that we have # tokens to look for in the output corresponding to each exception, where # those tokens don't appear at all the source (so we can't get a false # positive due to pytest printing out the source file). - # We sleep at the beginning of all the fixtures b/c currently if any - # fixture crashes, we skip setting up unrelated fixtures whose setup - # hasn't even started yet. Maybe we shouldn't? But for now the sleeps make - # sure that all the fixtures have started before any of them start - # crashing. + # We sleep at the beginning of all the fixtures to give opportunity + # for all fixtures to start the setup. Maybe we shouldn't? + # But for now the sleeps make sure that all the fixtures have + # started setting up before any of them start crashing. + + # We only expect the crash output of the first fixture that crashes + # during the setup. This is because the setup are synchronous. + # Once the fixture has crashed the test contex, the others would + # immediately return and wouldn't even complete the setup process testdir.makepyfile( """ import pytest @@ -191,15 +370,7 @@ def test_followup(): result = testdir.runpytest() result.assert_outcomes(passed=1, failed=1) - result.stdout.fnmatch_lines_random( - [ - "*CRASH_NONGEN*", - "*CRASH_EARLY_AGEN*", - "*CRASH_LATE_AGEN*", - "*CRASH_BACKGROUND_EARLY*", - "*CRASH_BACKGROUND_LATE*", - ] - ) + result.stdout.fnmatch_lines(["*CRASH_NONGEN*"]) @pytest.mark.parametrize("bgmode", ["nursery fixture", "manual nursery"]) diff --git a/pytest_trio/plugin.py b/pytest_trio/plugin.py index 1a56a83..f80f64f 100644 --- a/pytest_trio/plugin.py +++ b/pytest_trio/plugin.py @@ -161,8 +161,13 @@ def __init__(self, name, func, pytest_kwargs, is_test=False): self._func = func self._pytest_kwargs = pytest_kwargs self._is_test = is_test - self._teardown_done = trio.Event() - + # Previous and next fixture in terms of fixture topological order + # During set up, each fixture waits for + # its previous_fixture to set its setup_done value + self.previous_fixture = None + # During tear down, each fixture waits for + # itsnext_fixture to set its teardown_done value + self.next_fixture = None # These attrs are all accessed from other objects: # Downstream users read this value. self.fixture_value = None @@ -170,20 +175,31 @@ def __init__(self, name, func, pytest_kwargs, is_test=False): # Invariant: if this is set, then either fixture_value is usable *or* # test_ctx.crashed is True. self.setup_done = trio.Event() - # Downstream users *modify* this value, by adding their _teardown_done - # events to it, so we know who we need to wait for before tearing - # down. - self.user_done_events = set() + # This event notifies downstream users that we're done setting up. + # Same invariant as setup_done + self.teardown_done = trio.Event() - def register_and_collect_dependencies(self): + def collect_dependencies(self): # Returns the set of all TrioFixtures that this fixture depends on, - # directly or indirectly, and sets up all their user_done_events. - deps = set() - deps.add(self) + # directly or indirectly, in a topological order + deps = [] for value in self._pytest_kwargs.values(): if isinstance(value, TrioFixture): - value.user_done_events.add(self._teardown_done) - deps.update(value.register_and_collect_dependencies()) + for dependency in value.collect_dependencies(): + if dependency not in deps: + deps.append(dependency) + deps.append(self) + return deps + + def collect_register_dependencies(self): + # Collect dependencies then register previous and next fixture + # for each fixture (in terms of its topological order) + deps = self.collect_dependencies() + for index, dep in enumerate(deps): + if index > 0: + dep.previous_fixture = deps[index - 1] + if index < len(deps) - 1: + dep.next_fixture = deps[index + 1] return deps @asynccontextmanager @@ -199,7 +215,7 @@ async def _fixture_manager(self, test_ctx): test_ctx.crash(self, exc) finally: self.setup_done.set() - self._teardown_done.set() + self.teardown_done.set() async def run(self, test_ctx, contextvars_ctx): __tracebackhide__ = True @@ -219,11 +235,13 @@ async def run(self, test_ctx, contextvars_ctx): # teardone_done event, and crashing the context if there's an # unhandled exception. async with self._fixture_manager(test_ctx) as nursery_fixture: + if self.previous_fixture: + await self.previous_fixture.setup_done.wait() + # Resolve our kwargs resolved_kwargs = {} for name, value in self._pytest_kwargs.items(): if isinstance(value, TrioFixture): - await value.setup_done.wait() if value.fixture_value is NURSERY_FIXTURE_PLACEHOLDER: resolved_kwargs[name] = nursery_fixture else: @@ -244,7 +262,6 @@ async def run(self, test_ctx, contextvars_ctx): if self._is_test: # Tests are exactly like fixtures, except that they to be # regular async functions. - assert not self.user_done_events func_value = None assert not test_ctx.crashed await self._func(**resolved_kwargs) @@ -293,15 +310,15 @@ async def run(self, test_ctx, contextvars_ctx): # about cancellation. yield_outcome = outcome.Value(None) try: - for event in self.user_done_events: - await event.wait() + if self.next_fixture: + await self.next_fixture.teardown_done.wait() except BaseException as exc: assert isinstance(exc, trio.Cancelled) yield_outcome = outcome.Error(exc) test_ctx.crash(self, None) with trio.CancelScope(shield=True): - for event in self.user_done_events: - await event.wait() + if self.next_fixture: + await self.next_fixture.teardown_done.wait() # Do our teardown if isasyncgen(func_value): @@ -405,7 +422,7 @@ async def _bootstrap_fixtures_and_run_test(**kwargs): contextvars_ctx.run(canary.set, "in correct context") async with trio.open_nursery() as nursery: - for fixture in test.register_and_collect_dependencies(): + for fixture in test.collect_register_dependencies(): nursery.start_soon( fixture.run, test_ctx, contextvars_ctx, name=fixture.name )