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

bpo-41877 Check for asert, aseert, assrt in mocks #23165

Merged
merged 2 commits into from
Nov 5, 2020
Merged

bpo-41877 Check for asert, aseert, assrt in mocks #23165

merged 2 commits into from
Nov 5, 2020

Conversation

vabr-g
Copy link
Contributor

@vabr-g vabr-g commented Nov 5, 2020

Currently, a Mock object which is not unsafe will raise an
AttributeError if an attribute with the prefix assert or assret is
accessed on it. This protects against misspellings of real assert
method calls, which lead to tests passing silently even if the tested
code does not satisfy the intended assertion.

Recently a check was done in a large code base and three more frequent
ways of misspelling assert were found causing harm: asert, aseert,
assrt. These are now added to the existing check.

https://bugs.python.org/issue41877

Currently, a Mock object which is not unsafe will raise an
AttributeError if an attribute with the prefix assert or assret is
accessed on it. This protects against misspellings of real assert
method calls, which lead to tests passing silently even if the tested
code does not satisfy the intended assertion.

Recently a check was done in a large code base and three more frequent
ways of misspelling assert were found causing harm: asert, aseert,
assrt. These are now added to the existing check.
Copy link
Member

@gpshead gpshead left a comment

Choose a reason for hiding this comment

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

I like this PR. The discussion on the bug is valid but this is practical for existing unittest.mock practices used around the world today and will help prevent issues while the longer term "where do we want test assertions to go in Python" conversations continue.

@gpshead
Copy link
Member

gpshead commented Nov 5, 2020

Thanks! I agree that while larger discussions of how Python's unittest and unittest.mock system design allowed this to happen in the first place are good, lets not let perfect be the enemy of the good. This is still an improvement for real world issues in code today; the larger topic will continue to be discussed.

@gpshead gpshead merged commit 4662fa9 into python:master Nov 5, 2020
shihai1991 added a commit to shihai1991/cpython that referenced this pull request Nov 6, 2020
* origin/master:
  bpo-42179: Doc/tutorial: Remove mention of __cause__ (pythonGH-23162)
  bpo-26389: Allow passing an exception object in the traceback module (pythonGH-22610)
  bpo-42260: PyConfig_Read() only parses argv once (pythonGH-23168)
  bpo-42260: Add _PyConfig_FromDict() (pythonGH-23167)
  bpo-41877 Check for asert, aseert, assrt in mocks (pythonGH-23165)
  [docs] fix wrongly named AsyncContextDecorator (pythonGH-23164)
  bpo-42262: Add Py_NewRef() and Py_XNewRef() (pythonGH-23152)
  bpo-42266: Handle monkey-patching descriptors in LOAD_ATTR cache (pythonGH-23157)
  bpo-40816 Add AsyncContextDecorator class (pythonGH-20516)
shihai1991 added a commit to shihai1991/cpython that referenced this pull request Nov 7, 2020
…lots1

* origin/master: (80 commits)
  bpo-42282: Fold constants inside named expressions (pythonGH-23190)
  bpo-41028: Doc: Move switchers to docsbuild-scripts. (pythonGH-20969)
  bpo-42133: update parts of the stdlib to fall back to `__spec__.loader` when `__loader__` is missing (python#22929)
  Remove outdated reference to pywin32 from platform module (pythonGH-22005)
  bpo-41832: PyType_FromModuleAndSpec() now accepts NULL tp_doc (pythonGH-23123)
  Minor grammar edits for the descriptor howto guide (GH-python#23175)
  bpo-42179: Doc/tutorial: Remove mention of __cause__ (pythonGH-23162)
  bpo-26389: Allow passing an exception object in the traceback module (pythonGH-22610)
  bpo-42260: PyConfig_Read() only parses argv once (pythonGH-23168)
  bpo-42260: Add _PyConfig_FromDict() (pythonGH-23167)
  bpo-41877 Check for asert, aseert, assrt in mocks (pythonGH-23165)
  [docs] fix wrongly named AsyncContextDecorator (pythonGH-23164)
  bpo-42262: Add Py_NewRef() and Py_XNewRef() (pythonGH-23152)
  bpo-42266: Handle monkey-patching descriptors in LOAD_ATTR cache (pythonGH-23157)
  bpo-40816 Add AsyncContextDecorator class (pythonGH-20516)
  bpo-42260: Add _PyInterpreterState_SetConfig() (pythonGH-23158)
  Disable peg generator tests when building with PGO (pythonGH-23141)
  bpo-1635741: _sqlite3 uses PyModule_AddObjectRef() (pythonGH-23148)
  bpo-1635741: Fix PyInit_pyexpat() error handling (pythonGH-22489)
  bpo-42260: Main init modify sys.flags in-place (pythonGH-23150)
  ...
@vabr-g vabr-g deleted the more-assert-typos-issue-41877 branch November 8, 2020 10:35
adorilson pushed a commit to adorilson/cpython that referenced this pull request Mar 13, 2021
Currently, a Mock object which is not unsafe will raise an
AttributeError if an attribute with the prefix assert or assret is
accessed on it. This protects against misspellings of real assert
method calls, which lead to tests passing silently even if the tested
code does not satisfy the intended assertion.

Recently a check was done in a large code base (Google) and three
more frequent ways of misspelling assert were found causing harm:
asert, aseert, assrt. These are now added to the existing check.
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.

4 participants