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

Support customizing portal in TestClient #1347

Closed
wants to merge 6 commits into from

Conversation

MatthewScholefield
Copy link

@MatthewScholefield MatthewScholefield commented Nov 28, 2021

Consider a situation where we would like to create a second instance of TestClient within a started app:

@pytest.fixture(scope='session')
def running_app() -> TestClient:
    with TestClient(app) as running_app:
        yield app

@pytest.fixture()
def user_client() -> TestClient:
    user_client = TestClient(app)
    user_client.headers['Authorization'] = 'Bearer abc123'
    return user_client

def test_auth(running_app: TestClient, user_client: TestClient):
    public_client = running_app
    assert public_client.get('/protected').status_code == 403
    assert user_client.get('/protected').status_code == 200

Previously, in starlette<0.15.0, this worked fine. However, since starlette>=0.15.0 this causes mayhem when something attaches to the event loop on startup and then is used within a request. The specific reason this doesn't work is because two different event loops are created as described here in #1315. This PR adds a new portal argument to TestClient which allows sharing the same BlockingPortal instance from anyio and transitively sharing the same event loop.

With this change, the example above just has to be slightly adapted as follows:

@pytest.fixture()
def user_client(running_app) -> TestClient:
    user_client = TestClient(app, portal=running_app.portal)
    user_client.headers['Authorization'] = 'Bearer abc123'
    return user_client

Feel free to suggest alternate solutions. I figured I'd create the PR for discussion (since the code change is simple enough).

Also, note that from what I understand of Trio, this problem wouldn't happen there because it doesn't have the concept of unscoped task creation like asyncio does.

@aminalaee
Copy link
Member

@MatthewScholefield Thank you for this, will you be able to add the required test?

@MatthewScholefield
Copy link
Author

Hi @aminalaee thanks for the ping. I've updated the test and it should now have 100% coverage.

@adriangb adriangb added the feature New feature or request label Feb 2, 2022
@tomchristie
Copy link
Member

when something attaches to the event loop on startup and then is used within a request

It sounds to me like this is the actual issue here. asyncio exposes some primitives that you really don't want to be using.

I'd be up for working carefully through the issue, but I don't think portal=... is something we want to be exposing as public API here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants