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

Show file tree
Hide file tree
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
Changed documentation
  • Loading branch information
@cklein
cklein committed Jan 6, 2023
commit 01946cb02964f45985c5ec83f52663bfd115aa9b
10 changes: 6 additions & 4 deletions Lib/unittest/mock.py
View file
Open in desktop
Original file line number Diff line number Diff line change
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 attribute names in safe mode
# Denylist for forbidden attribute names in safe mode
ATTRIB_DENY_LIST = {name.removeprefix("assert_") for name in dir(NonCallableMock) if name.startswith("assert_")}

@cjw296 cjw296 Jan 5, 2023

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Much prefer this approach! 🎉

@sobolevn sobolevn Jan 7, 2023

Copy link
Copy Markdown
Member

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 :)

@sobolevn sobolevn Jan 7, 2023

Copy link
Copy Markdown
Member

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!

@merwok merwok Jan 7, 2023

Copy link
Copy Markdown
Member

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 Expand Up @@ -1235,9 +1235,11 @@ class or instance) that acts as the specification for the mock object. If
`return_value` attribute.

* `unsafe`: By default, accessing any attribute whose name starts with
*assert*, *assret*, *asert*, *aseert*, *assrt*, or an attribute name that matches
an assertion method without the prefix *assert_* will raise an
AttributeError. Passing `unsafe=True` will allow access to these attributes.
*assert*, *assret*, *asert*, *aseert*, or *assrt* raises an AttributeError.
Additionally, an AttributeError is raised when accessing
attributes that match the name of an assertion method without the prefix
`assert_`, e.g. accessing `called_once` instead of `assert_called_once`.
Passing `unsafe=True` will allow access to these attributes.

* `wraps`: Item for the mock object to wrap. If `wraps` is not None then
calling the Mock will pass the call through to the wrapped object
Expand Down
4 changes: 2 additions & 2 deletions Misc/NEWS.d/next/Library/2023-01-02-16-59-49.gh-issue-100690.2EgWPS.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
Mock objects which are not unsafe will now raise an AttributeError if an
attribute that matches the name of an existing assertion is accessed, e.g. ``called_once`` instead of ``assert_called_once``.
Mock objects which are not unsafe will now raise an AttributeError when accessing an
attribute that matches the name of an assertion but without the prefix ``assert_``, e.g. accessing ``called_once`` instead of ``assert_called_once``.
This is in addition to this already happening for accessing attributes with prefixes assert, assret, asert, aseert, and assrt.
Comment thread
merwok marked this conversation as resolved.
Outdated