Skip to content

gh-100690: Prevent prefix "called_" for attributes of safe mock objects #100691

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.

Sign up for GitHub

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

Merged
cjw296 merged 6 commits into from
Jan 6, 2023
Merged

gh-100690: Prevent prefix "called_" for attributes of safe mock objects #100691

Changes from 1 commit
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
154012f
Prevent calling assert functions without the prefix "assert_" in safe…
cklein Jan 3, 2023
26dc793
Merge remote-tracking branch 'python/main' into disallow_mock_method_…
cklein Jan 5, 2023
2bd97a1
Added tests to show that names from denylist still can be used with a…
cklein Jan 5, 2023
01946cb
Changed documentation
cklein Jan 6, 2023
e9b8f73
Reformatted NEWS
cklein Jan 6, 2023
674f5bc
Merge branch 'main' into disallow_mock_method_prefix
cjw296 Jan 6, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Merge remote-tracking branch 'python/main' into disallow_mock_method_… 
…prefix
  • Loading branch information
@cklein
cklein committed Jan 5, 2023
commit 26dc79344c3e65ecca3a6073ad1e115aabf75391
4 changes: 2 additions & 2 deletions Lib/unittest/mock.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -652,7 +652,7 @@ def __getattr__(self, name):
raise AttributeError("Mock object has no attribute %r" % name)
elif _is_magic(name):
raise AttributeError(name)
if not self._mock_unsafe:
if not self._mock_unsafe and (not self._mock_methods or name not in self._mock_methods):
if name.startswith(('assert', 'assret', 'asert', 'aseert', 'assrt')) or name in ATTRIB_DENY_LIST:
raise AttributeError(
f"{name!r} is not a valid assertion. Use a spec "
Expand Down Expand Up @@ -1062,7 +1062,7 @@ def _calls_repr(self, prefix="Calls"):
return f"\n{prefix}: {safe_repr(self.mock_calls)}."


# gh-100690 Denylist for forbidden method names in safe mode
# gh-100690 Denylist for forbidden attribute names in safe mode
Comment thread
merwok marked this conversation as resolved.
Outdated
ATTRIB_DENY_LIST = {name.removeprefix("assert_") for name in dir(NonCallableMock) if name.startswith("assert_")}
Copy link
Copy Markdown
Contributor

@cjw296 cjw296 Jan 5, 2023

Choose a reason for hiding this comment

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

Much prefer this approach! 🎉

Copy link
Copy Markdown
Member

@sobolevn sobolevn Jan 7, 2023

Choose a reason for hiding this comment

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

Sorry for being late to the party, but anyways :)

I think that ATTRIB_DENY_LIST is an implementation detail, so it should be _ATTRIB_DENY_LIST. And also, it is not suited for mutation, isn't it? So, it should be a frozenset instead :)

Copy link
Copy Markdown
Member

@sobolevn sobolevn Jan 7, 2023

Choose a reason for hiding this comment

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

Since this PR is already merged, I've opened #100819 with this proposal. Feel free to close if it should be public and mutable!

Copy link
Copy Markdown
Member

@merwok merwok Jan 7, 2023

Choose a reason for hiding this comment

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

It is already non-public by not being included in __all__, and the mutability is not really an issue.



Expand Down
You are viewing a condensed version of this merge commit. You can view the full changes here.