Skip to content

make_mocked_request: subscriptable default app#3174

Merged
webknjaz merged 1 commit intoaio-libs:masterfrom
butla:master
Sep 12, 2018
Merged

make_mocked_request: subscriptable default app#3174
webknjaz merged 1 commit intoaio-libs:masterfrom
butla:master

Conversation

@butla
Copy link
Contributor

@butla butla commented Aug 5, 2018

What do these changes do?

App in the default request created with make_mocked_request won't crash when accessed with [].

Are there changes in behavior for the user?

Nope.

Related issue number

#3134

@codecov-io
Copy link

codecov-io commented Aug 5, 2018

Codecov Report

Merging #3174 into master will increase coverage by <.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #3174      +/-   ##
==========================================
+ Coverage   98.07%   98.07%   +<.01%     
==========================================
  Files          43       43              
  Lines        7853     7860       +7     
  Branches     1354     1354              
==========================================
+ Hits         7702     7709       +7     
  Misses         59       59              
  Partials       92       92
Impacted Files Coverage Δ
aiohttp/test_utils.py 99.27% <100%> (+0.01%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 99914c8...765c4eb. Read the comment docs.

@asvetlov
Copy link
Member

asvetlov commented Aug 6, 2018

The PR doesn't crash, it's true.
I wonder is it helpful?

req.app['key'] = 'val'
assert req.app['key'] == 'val'

fails.
Adding a mock for app.__getitem__ works but it looks complicated.

What do you think?

@butla
Copy link
Contributor Author

butla commented Aug 6, 2018

Well, it'd be useful for some of the tests I have, where something is accessed from the app, but whatever gets accessed doesn't get used in a meaningful way for the logic.

To make it more useful I could implement __setitem__ and __getitem__ inside _create_app_mock more or less like this:

def set_dict(app, key, value):
    app.__app_dict[key] = value

def get_dict(app, key):
     return app.__app_dict[key]

app = MagicMock()
app.__app_dict = {}
app.__getitem__ = get_dict
app.__setitem__ = set_dict

@asvetlov
Copy link
Member

asvetlov commented Aug 7, 2018

yes, it is more robust

@butla
Copy link
Contributor Author

butla commented Aug 20, 2018

@asvetlov I've updated the code.

Copy link
Member

@asvetlov asvetlov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add ca change note into CHANGES folder

@butla
Copy link
Contributor Author

butla commented Sep 12, 2018

@asvetlov Added.

@lock
Copy link

lock bot commented Oct 28, 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].
[new issue]: https://github.com/aio-libs/aiohttp/issues/new

@lock lock bot added the outdated label Oct 28, 2019
@lock lock bot locked as resolved and limited conversation to collaborators Oct 28, 2019
@psf-chronographer psf-chronographer bot added the bot:chronographer:provided There is a change note present in this PR label Oct 28, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

bot:chronographer:provided There is a change note present in this PR outdated

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants