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

Mocker.copy: Use correct type when copying mocker #160

Merged

Conversation

allanlewis
Copy link
Contributor

As per the sole commit:

Currently, the copy method hard-codes the Mocker class as the return type. In order to allow this to work with sub-classes of Mocker, this commit replaces it with the type of the instance.

Currently, the `copy` method hard-codes the `Mocker` class as the return type.
In order to allow this to work with sub-classes of `Mocker`, this commit
replaces it with the type of the instance.
@allanlewis
Copy link
Contributor Author

@jamielennox Do you have any comments on this change?

@jamielennox
Copy link
Owner

Only that I swear I had dealt with this one a few weeks ago.

Very sorry it took so long.

I'll admit I find it weird that you'd want to subclass this object. Can you give an example of why?

Even given this there's no reason to not use the real class type.

@jamielennox jamielennox merged commit a44af2b into jamielennox:master Apr 12, 2021
@allanlewis allanlewis deleted the use-correct-type-when-copying-mock branch April 12, 2021 13:05
@allanlewis
Copy link
Contributor Author

I'll admit I find it weird that you'd want to subclass this object. Can you give an example of why?

I subclassed requests_mock.Mocker to make a specialised MessageMocker to mock responses to inter-service messages in my microservice app. This issue caught me out because copies were Mockers instead of MessageMockers.

Thanks for merging this 🙏

@allanlewis
Copy link
Contributor Author

@jamielennox Do you have any plans to make a new release? I'd very much like this feature, although obviously I can use a Git version for the time being. (I'm not sure what else has changed since v1.8.0.)

@jamielennox
Copy link
Owner

@allanlewis i've merged one or two other PRs, i'm trying to finish off #162 and get github-actions to do the version publish. So should be this week.

@jamielennox
Copy link
Owner

Released as 1.9.0

Thanks for the help.

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

2 participants