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

Wrap assert methods of mock module to clean up traceback #27

Merged
merged 1 commit into from
Jan 27, 2016

Conversation

Chronial
Copy link
Contributor

Fixes #26

@Chronial
Copy link
Contributor Author

This is proving to be more difficult than expected ^^.

@Chronial Chronial force-pushed the feature/patch-assert branch 3 times, most recently from a21efc9 to 92eb505 Compare January 27, 2016 05:44
@Chronial
Copy link
Contributor Author

There we go, working now.

_mock_module_originals.clear()


def pytest_configure(config):
Copy link
Member

Choose a reason for hiding this comment

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

I think for completeness we should also implement pytest_unconfigure to undo all the patches.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, I had that – but then the tests break, because the tests actually run a second pytest in the same process. That pytest will also call unconfigure, and the patching will be gone for all following tests.

But see line 209 – that does exactly what you're asking for.

Copy link
Member

Choose a reason for hiding this comment

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

I'm a little uneasy of doing this wrapping automatically for everyone which uses the plugin... there are a tons of test suites, some of them doing already some deep dark magic on their on, who knows what this might break? I agree with you it is unlikely, but I think we should add an option to disable this. How about an ini option:

[pytest]
mock_traceback_monkeypatch = false

The default would be true, but users can disable it at will at least. What do you think?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, I agree – it's quite agressive and I felt the pain of this while writing it. But as I can't see any other way to make sure that the patching is there when somebody uses mock in a test, I think a config option would probably be a good way to handle problems. I'll add one.

Copy link
Member

Choose a reason for hiding this comment

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

Thanks! 😄

Copy link
Member

Choose a reason for hiding this comment

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

But see line 209 – that does exactly what you're asking for.

Oops, missed that, thanks!

@nicoddemus
Copy link
Member

Hey thanks a lot for the PR! The code looks great!

Other than my other comments, the only thing left to do is mention this new feature in the README.

@Chronial Chronial force-pushed the feature/patch-assert branch 3 times, most recently from b3921e4 to 773f8d9 Compare January 27, 2016 12:02
@Chronial
Copy link
Contributor Author

done 😄

@nicoddemus
Copy link
Member

Thanks a lot! I will merge this tonight and make a new release. 😄

@The-Compiler
Copy link
Member

👍

@nicoddemus nicoddemus merged commit 773f8d9 into pytest-dev:master Jan 27, 2016
nicoddemus added a commit that referenced this pull request Jan 27, 2016
@nicoddemus
Copy link
Member

Released 0.10.0 😄

Thanks again!

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.

3 participants