镜像站点 · 本页由第三方 GitHub 只读镜像提供,非 GitHub 官方站点,不接受任何登录或凭据输入。前往 github.com
Skip to content

Handle patch.multiple mocks individually - #655

Open
mehdimirzafaruk wants to merge 3 commits into
pytest-dev:mainfrom
mehdimirzafaruk:fix/patch-multiple-registration
Open

mehdimirzafaruk wants to merge 3 commits into
pytest-dev:mainfrom
mehdimirzafaruk:fix/patch-multiple-registration

Conversation

@mehdimirzafaruk

Copy link
Copy Markdown

patch.multiple currently registers its result dictionary rather than the individual mocks. This means the context-manager warning is not installed and mocker.stop() cannot stop one returned mock.

Register each underlying patch separately while preserving the returned dictionary of mocks. This also keeps resetall() and stopall() behavior consistent with other patch helpers.

Tests: tox -e py312; pre-commit run --all-files

Fixes #638

Comment thread src/pytest_mock/plugin.py Outdated
patchers = [patcher, *patcher.additional_patchers]
patcher.additional_patchers = []
mocked = {}
for patcher in patchers:

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

one edge case w/ starting these patchers separately: if the second or third patch fails, the earlier ones have already been applied. unlike the original combined patcher, this path doesn't roll them back before raising. could we clean up the patchers started by this call on failure? a test where one target is invalid after a valid one would cover it.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Good catch, thank you! Added rollback cleanup on failure so earlier patchers started during the call are stopped and unregistered before re-raising, along with a regression test for an invalid target attribute.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

that's the failure case I was worried about. the earlier patches get rolled back now instead of being left active. good fix!

This branch has not been deployed

No deployments
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.

Mocks created by mocker.patch.multiple are not handled like other patches

2 participants