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

Propagate match_info in make_mocked_request #2331

Merged

Conversation

argaen
Copy link
Member

@argaen argaen commented Oct 16, 2017

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 document a new parameter here: http://aiohttp.readthedocs.io/en/stable/testing.html#aiohttp.test_utils.make_mocked_request

.. versionadded: 2.3 would be nice

@@ -564,7 +565,7 @@ def timeout(*args, **kw):
protocol, payload_writer, task, loop,
client_max_size=client_max_size)

match_info = UrlMappingMatchInfo({}, mock.Mock())
match_info = UrlMappingMatchInfo({} if match_info is sentinel else match_info, mock.Mock())
Copy link
Member

Choose a reason for hiding this comment

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

Isn't the line too long?

Copy link
Member Author

Choose a reason for hiding this comment

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

didn't give any problems to me, waiting for the automated checks to pass

Copy link
Member

Choose a reason for hiding this comment

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

You didn't run make test, travis blames exactly the line

Copy link
Member Author

Choose a reason for hiding this comment

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

Yup, didn't run because was failing on my machine for some reason, that's why I was letting travis do the job.

In case you had this before, that's what I'm getting: ValueError: option names {'--fast'} already added

Copy link
Member Author

Choose a reason for hiding this comment

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

uninstalling pytest-aiohttp worked for me

@argaen
Copy link
Member Author

argaen commented Oct 16, 2017

Added the doc about the new attribute but the docs of that method are with sphinx format. I'll check how do display that info in that format. Have to leave for 1/2 hours now.

@asvetlov
Copy link
Member

@asvetlov asvetlov merged commit a01c4ec into aio-libs:master Oct 17, 2017
@asvetlov
Copy link
Member

You shouldn't install pytest-aiohttp for developing aiohttp itself because it depends on aiohttp.
The test suite setups pytest-aiohttp fixtures in conftest.py without plugin installation, that's how things work.

@argaen argaen deleted the issue_2330/make_mocked_request_with_match_info branch October 17, 2017 12:20
@argaen
Copy link
Member Author

argaen commented Oct 17, 2017

Makes sense. I think I had it installed because of reusing venvs between aiohttp and pytest-aiohttp

@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
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.

2 participants