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

DRAFT, New TestClientApp implementation #979

Closed
pfreixes opened this issue Jul 21, 2016 · 6 comments
Closed

DRAFT, New TestClientApp implementation #979

pfreixes opened this issue Jul 21, 2016 · 6 comments
Labels

Comments

@pfreixes
Copy link
Contributor

This commit [1] is a DRAFT proposal to get a new implementation of the current make_mocked_request.

This new approximation is formulated to be used for testing and keeping the interface
quite close to that one used by the test_client fixture where a full HTTP service is used.

As an example of how tests can be coded right now:

@pytest.mark.run_loop
@asyncio.coroutine
def test_test_client_app(loop, app):
    client = app.test_client()
    response = yield from client.get("/")
    assert (yield from response.text()) == "Hello, world"

Looking forward for comments and feedback.

[1] pfreixes@7956a1a

@asvetlov
Copy link
Member

Why do you want to skip network layer on testing your own application?

Using real server with localhost networking solves the issue very well.
make_mocked_request() exists for very limited usage, 99% of your test should go through localhost loopback without network emulation.

@pfreixes
Copy link
Contributor Author

Regarding make_mocked_request right now can be perceived as another way, rather than the test_client fixture, to test your views. IMHO the use of this object can give a false sensation of security - perhaps app routing is not take into account or make impossible formulate many situations - perhaps the URL params are not supported.

As a "side effect" it also can be seen as a replacement of the test_client fixture that tries to minimize the use of none related resources such as network [1] or reducing the time needed to load them.

To sum up, this DRAFT wants to open a discussion to get an unified test way to test the different views belonging to an aiohttp application.

[1] http://stackoverflow.com/questions/1257560/when-is-a-test-not-a-unit-test

@asvetlov
Copy link
Member

Well, the example in http://aiohttp.readthedocs.io/en/stable/testing.html#faking-request-object is wrong.

The make_mocked_request is intended to be used for testing transport layer and may be useful in middleware tests.

I suspect the most part of unittests for aiohttp usage on application level will connect to real local database, perform real queries etc.

In other words for web-handler testing functional tests are more suitable than pure-unittests.
I suggest to use mock objects and derivatives like make_mocked_request as little as you can.

@pfreixes
Copy link
Contributor Author

Well for integration tests or acceptance current test stack is plenty of util, but integration/acceptance are just tens over hundred of UTs regarding a well distributed amount of tests [1].

Have the amount of dependencies for you UTs, rather than the functional ones, as minimum as possible is always a good point for many raesons, even the network dependency.

I disagree about reduce the mock objects as little as you can, it is not an option it is a needed when you code each UTs, in that case the mock objects are mainly used to characterize the environment of your UTs and allowing to them get a deterministic behavior. Adding external dependencies UTs will become less deterministic.

The view layer, regarding perhaps a MVC model, where the aiohttps views live have to be covered with all of the needed tests. For example check mandatory fields, check error handling, and so on. Only with a good coverage the amount of functional tests will keep at some tens, you wont need to check all the cases regarding mandatory fields when these scenarios are already covered by your UTs.

I got your point about don't use make_mocked_request instead of use the full stack composed by a full aiohttp app and its dependencies, but IMHO this is not the right way to code the the test suite belonging to your aiohttp views.

Regarding the documentation, will be good to know the opinion of @jettify that was the author. I would like to get more opinions.

[1] http://martinfowler.com/bliki/TestPyramid.html

@asvetlov
Copy link
Member

I have an answer but It's long enough.

First, why libraries like Django or Flask have a fake tool for performing send queries to views without touching network layer? That's because they are WSGI frameworks.
In WSGI world is impossible to run both tested and testing code in the same thread. Starting two threads is slow and cumbersome, especially when you need thread synchronization.

From other hand with asyncio co-existing testing and tested code in the same thread and even in the same function works perfectly fine!

Expenses for opening new localhost socket and starting server on it are very low. Passing test data through real network allows to avoid subtle bugs when test behaves a little bit different than real code.

Tornado framework use the same approach for testing as aiohttp has BTW.

What am I thinking about unitests, test isolation and retting rid of access to DB and network?
That's a brilliant concept.
But please write isolated tests not for web-handlers (views) but for business-logic objects. Do you have a separate layer for business logic handling, isn't it? On this layer you never need to cope with HTTP request and response but operate with entities like User, Product, Transaction, Account -- nothing from HTTP world.

Anyway, if you really need mocked approach for testing aiohttp server -- please do it outside of the main library. Feel free to push your tool into separate lib and publish it on PyPI if needed.

@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

No branches or pull requests

2 participants