python-trio / pytest-trio

Pytest plugin for trio
Other
54 stars 25 forks source link

Should we be sharing contextvars.Context between fixtures? #59

Open njsmith opened 6 years ago

njsmith commented 6 years ago

In #50, we started using a gross hack to make it so within each test invocation, all fixtures + the test itself all shared a single contextvars.Context. This simulates them all running in a single task (except that some can happen concurrently, see #57).

Is this the optimal behavior? I guess the options are:

The difference is in code like:

mycv = contextvars.ContextVar("mycv")
@trio_fixture
def fix():
    mycv.set("a")
    yield
    assert mycv.get() == "a"

@pytest.mark.trio
async def test(fix):
     mycv.set("b")

Right now, the assert fails, because the fixture can "see" the set call in test, because they share a Context. If we used separate contexts with copy-on-startup, then fix would not be able to see changes made by downstream fixtures. You can think of it as arranging for ContextVar changes to be automatically rolled back during the teardown process. This is conceptually attractive, but I don't know if it matters, or is even beneficial, in practice.

Also, even if we keep the current semantics, we should find a way to stop doing the gross hack that we're doing right now. This might mean enhancing trio, e.g. by adding a context= argument to nursery.start_soon.

njsmith commented 6 years ago

Here's a consideration: if PEP 568 happens and we start storing cancel state inside the contextvars.Context, then we definitely won't be able to share contexts between fixtures running simultaneously.

Some discussion: https://github.com/python-trio/trio/issues/264#issuecomment-418989328

oremanj commented 4 years ago

An interesting manifestation of this that I ran into recently:

trio-asyncio uses contextvars to identify which asyncio loop is in scope. In theory, you can have multiple trio-asyncio loops in your program and they won't conflict with each other. But if you have multiple fixtures (or a combo of one fixture + main test) that use trio-asyncio internally, and each does their own open_loop(), the sharing of contextvars means they'll stomp on each other.