make_mocked_request's default app should be a MagicMock

[butla]

Long story short

App is, among other things, a container for objects with an application-wide use. The default right now is to populate the app in the Request returned by make_mocked_request with unittest.mock.Mock, which doesn't support indexing, thus crashing when values in the app are accessed, unless MagicMock (or a dictionary) is explicitly put as app in the the call to make_mocked_request. I think the default should be to create the app as a MagicMock.

I couldn't test if that would brake any tests, because running tox was just spewing missing .h file errors at me and wouldn't run anything.

Expected behaviour

>>> from aiohttp.test_utils import make_mocked_request
>>> r = make_mocked_request('GET', '/bla')
>>> r.app['some_value']
<MagicMock name='mock.__getitem__()' id='139715764977336'>

Actual behaviour

>>> from aiohttp.test_utils import make_mocked_request
>>> r = make_mocked_request('GET', '/bla')
>>> r.app['some_value']
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
TypeError: 'Mock' object does not support item assignment
'Mock' object does not support item assignment

Your environment

Aiohttp server: 3.3.2
OS: Kubuntu 17.10 x64

[asvetlov]

Well, make sense.
Would you create a pull request for the proposed fix?
A simple unit test is required.

[butla]

@asvetlov I'd need to know how to set up the tests, because trying to run tox on master failed miserably. Or should I base the PR on a tagged release commit?

[webknjaz]

@butla, tox is not yet supported (basically I'm trying to convince Andrew to use it as an underlying layer here, in a smaller project aio-libs/multidict#263), so you'll have to relay on make (see Makefile), also, you can find inspiration in .travis.yml.
Would this help?

[asvetlov]

@butla https://docs.aiohttp.org/en/stable/contributing.html has some info about contributions into the project. A fork from the current master should work perfectly fine.

@webknjaz sorry for the delay about tox decision. Will do it soon.

[butla]

@webknjaz As for travis.yml - that's the most complex YAML file I've ever seen, I've no idea what's going on :) But I'm guessing I should just run make test inside of a virtual environment, correct? @asvetlov You want me to put that into CONTRIBUTING? It only says "Make sure all tests passed" which isn't very specific.

[asvetlov]

Yes, make test into a virtual env is how I work on aiohttp everyday.

Filing CONTRIBUTING is up to you.
I appreciate everybody who worked on the project and like to enumerate them.
But if you don't like the file or too lazy -- you can skip it.

[webknjaz]

@butla that basically means that CI should remain green. So you can skip local testing if you have difficulties of setting it up.

P.S. In Travis's config I use lots of anchors to avoid copy-paste, which would be even harder to track. Meanwhile, I'm thinking of a solution, which would simplify this stuff and maybe make that readable :)

[butla]

I think I'm seeing the same error when I run make test in a 3.6.3 virtualenv as I was when I running tox, that is:

aiohttp/_http_parser.c:590:10: fatal error: ../vendor/http-parser/http_parser.h: No such file or directory                                                                                                       
     #include "../vendor/http-parser/http_parser.h"  
              ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~  
    compilation terminated.                          
    error: command 'x86_64-linux-gnu-gcc' failed with exit status 1  

[butla]

@webknjaz Well, the CI's red right now, from what I see on the main project page...

[webknjaz]

@butla you should sync git submodules to get http-parser sources on your machine.
As for CI, yes, it needs to be fixed, but that's just OS X stuff probably broken.

[webknjaz]

Apparently, CI log is out of 4MB limit for those jobs, that's why they've been terminated by Travis. I hope that with the introduction of tox-centric flow we'll be able to split that into smaller parts..
Your PR won't fail there, because we don't run OS X jobs in this case, only by cron or triggered manually.

[asvetlov]

Fixed by #3174

[lock]

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].
[new issue]: https://github.com/aio-libs/aiohttp/issues/new