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

Ensure TestClient HTTP methods return a context manager #1318

Merged

Conversation

alexhayes
Copy link
Contributor

@alexhayes alexhayes commented Oct 17, 2016

What do these changes do?

A user should be able to run a test as per the client documentation;

async with aiohttp.ClientSession() as session:
    async with session.get('https://api.github.com/events') as resp:
        print(resp.status)
        print(await resp.text())

However, because the test client doesn't return a context manager the above does not work (unless of course I'm doing something wrong).

This PR ensures the HTTP methods on TestClient return a context manager so that it mimics the same interface as a non-test client

Are there changes in behaviour for the user?

Yes, the TestClient will behave the same as the non-test client that their code will be using :)

Let's say or instance your code looked as follows;

# client request code
async request_hello(session):
    async with session.get('...') as resp:
        ...

Note the use of the context manager: async with session.get('...') as resp:

And your test was;

# pytest
from aiohttp import web

async def hello(request):
    return web.Response(text='Hello, world')

async def test_hello(test_client, loop):
    app = web.Application(loop=loop)
    app.router.add_get('/', hello)
    client = await test_client(app)
    resp = await request_hello(client)
    assert resp.status == 200
    ...

On master, this would fail, because TestClient does not return a context manager.

Related issue number

None.

Checklist

  • I think the code is well written
  • Unit tests for the changes exist
  • Documentation reflects the changes
  • Add yourself to CONTRIBUTORS.txt
  • Add a new entry to CHANGES.rst

@alexhayes alexhayes changed the title WIP - Ensure TestClient HTTP method returns a context manager WIP - Ensure TestClient HTTP methods return a context manager Oct 17, 2016
@alexhayes alexhayes force-pushed the feature/TestClientReturnsContextManager branch from 52e316c to 45c0597 Compare October 17, 2016 20:57
@alexhayes alexhayes changed the title WIP - Ensure TestClient HTTP methods return a context manager Ensure TestClient HTTP methods return a context manager Oct 17, 2016
@codecov-io
Copy link

codecov-io commented Oct 17, 2016

Current coverage is 98.51% (diff: 100%)

Merging #1318 into master will increase coverage by <.01%

@@             master      #1318   diff @@
==========================================
  Files            29         29          
  Lines          6515       6516     +1   
  Methods           0          0          
  Messages          0          0          
  Branches       1094       1094          
==========================================
+ Hits           6418       6419     +1   
  Misses           47         47          
  Partials         50         50          

Powered by Codecov. Last update 0033c48...9a40cc8

@asvetlov
Copy link
Member

Please resolve conflict in CHANGES.rst.
I'll merge the PR after this.

@alexhayes alexhayes force-pushed the feature/TestClientReturnsContextManager branch from 45c0597 to 9a40cc8 Compare October 19, 2016 11:28
@alexhayes
Copy link
Contributor Author

Rebased master onto my branch and force pushed.

@asvetlov asvetlov merged commit f8e2425 into aio-libs:master Oct 19, 2016
@asvetlov
Copy link
Member

Cool! Thanks!

@lock
Copy link

lock bot commented Oct 29, 2019

This thread has been automatically locked since there has not been
any recent activity after it was closed. Please open a new issue for
related bugs.

If you feel like there's important points made in this discussion,
please include those exceprts into that new issue.

@lock lock bot added the outdated label Oct 29, 2019
@lock lock bot locked as resolved and limited conversation to collaborators Oct 29, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants