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

Easier session scoped mock (compatible with nested mocks) #133

Merged
merged 7 commits into from
May 2, 2020
Merged

Easier session scoped mock (compatible with nested mocks) #133

merged 7 commits into from
May 2, 2020

Conversation

clslgrnc
Copy link
Contributor

@clslgrnc clslgrnc commented Apr 3, 2020

Resolve conflicts between #116 and #110.

@clslgrnc clslgrnc changed the title Akeeman patch Easier session scoped mock (compatible with nested mocks) Apr 3, 2020
@clslgrnc
Copy link
Contributor Author

clslgrnc commented Apr 3, 2020

b8746ce is necessary to make real_http work as well as interactions between global and session mocks.
Indeed #116 changed call to _original_send, that always takes session as an argument, into a call to self._last_send for which session is implicit when mocking a specific session.
Likewise, restoration of get_adapter was not always done on the right object.

@clslgrnc
Copy link
Contributor Author

clslgrnc commented Apr 3, 2020

There are still a few issues with nested global and session mocks like in the following example:

    url = 'mock://example.test/'
    with requests_mock.Mocker() as global_mock:
        global_mock.get(url, text='global')
        session = requests.Session()
        with requests_mock.Mocker(session=session) as session_mock:
            session_mock.get(url, real_http=True)
            text = session.get(url).text

What should happen?

  • [x] text is "global" since that global mock was in place at session creation.
  • [ ] an error since the given url is invalid and global and session mock are independents
  • [ ] global and session mocks should not be mixed up and we should update doc

I feel like the first option is the best, but what currently happens is the second one.


Edit: last commit solved the issue

Adding a wrapper function adds another layer to step through when
debugging. Really all we want to do there is bind it to the instance so
just do that up front.

This is one of those moments you should just drop python 2.
Session is really the only argument i can think we'd allow to take on
Mocker(). Also type check it. I know python, but still.
requests_mock/mocker.py Outdated Show resolved Hide resolved
@jamielennox jamielennox added this to the 1.8.0 milestone May 2, 2020
@jamielennox
Copy link
Owner

I did tack a small change on here just to clean it up so we can merge it rather than go back and forward in review. I hope this is ok for you both.

Thanks you @clslgrnc @akeeman

@jamielennox jamielennox merged commit 1b9a732 into jamielennox:master May 2, 2020
@clslgrnc clslgrnc deleted the akeeman_patch branch June 23, 2020 12:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants