GeezerVison

The World Wide Web, restored to 800 pixels of glory.

GeezerVison is showing github.com — Trivial: Improving error reporting when assert_has_calls fails by davidair · Pull Request #8205 · python/cpython

Conversation

davidair commented Jul 9, 2018

Copy link

Copy Markdown

Contributor

No description provided.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions


          Improving error reporting when assert_has_calls fails

37a6f26

the-knights-who-say-ni added the CLA signed label

Jul 9, 2018

bedevere-bot added the awaiting review label

Jul 9, 2018

davidair commented Jul 9, 2018

Copy link

Copy Markdown

Contributor Author

@gpshead

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

gpshead requested review from 1st1 and gpshead

July 9, 2018 19:06

gpshead commented Jul 9, 2018

Copy link

Copy Markdown

Member

I pinged @1st1 for a second pair of eyeballs and to ponder if there are other places we should improve the assertion error messages. This is a trivial change, not really worthy of a BPO issue. A news entry can be added if we accept this.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

gpshead added the skip issue label

Jul 9, 2018

gpshead self-assigned this

Jul 9, 2018

1st1 reviewed

Jul 9, 2018

View reviewed changes

Comment thread

Lib/unittest/mock.py Outdated

		raise AssertionError(
		'%r not all found in call list' % (tuple(not_found),)
		'%r not all found in call list: %r' % (tuple(not_found),
		all_calls)

1st1 Jul 9, 2018

Copy link

Copy Markdown

Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

I'd also include self._mock_name in the error message, like in https://github.com/python/cpython/pull/8205/files#diff-ff75b1b83c21770847ade91fa5bb2525R829

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

davidair Jul 10, 2018

Copy link

Copy Markdown

Contributor Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

What should the message look like? Would it be something along the lines of "%r does not contain all of %r in its call list, found %r instead", where the first format parameter would be self._mock_name or 'mock'?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

gpshead Aug 17, 2018

Copy link

Copy Markdown

Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

I believe so, yes. I just committed and edit from the github UI doing that. awaiting CI test results.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

davidair and others added 3 commits

July 10, 2018 18:55


          Adding news

d3e1799


          improve error message based on review comments.

e285e51


          news entry wordsmithing

c5bf049

gpshead merged commit 2b32da2 into python:master

Aug 17, 2018

bedevere-bot removed the awaiting review label

Aug 17, 2018

carljm added a commit to carljm/cpython that referenced this pull request

Aug 19, 2018


          Merge branch 'master' into cached-property

977ece0

* master: (107 commits)
  bpo-22057: Clarify eval() documentation (pythonGH-8812)
  bpo-34318: Convert deprecation warnings to errors in assertRaises() etc. (pythonGH-8623)
  bpo-22602: Raise an exception in the UTF-7 decoder for ill-formed sequences starting with "+". (pythonGH-8741)
  bpo-34415: Updated logging.Formatter docstring. (pythonGH-8811)
  bpo-34432: doc Mention complex and decimal.Decimal on str.format not about locales (pythonGH-8808)
  bpo-34381: refer to 'Running & Writing Tests' in README.rst (pythonGH-8797)
  Improve error message when mock.assert_has_calls fails (pythonGH-8205)
  Warn not to set SIGPIPE to SIG_DFL (python#6773)
  bpo-34419: selectmodule.c does not compile on HP-UX due to bpo-31938 (pythonGH-8796)
  bpo-34418: Fix HTTPErrorProcessor documentation (pythonGH-8793)
  bpo-34391: Fix ftplib test for TLS 1.3 (pythonGH-8787)
  bpo-34217: Use lowercase for windows headers (pythonGH-8472)
  bpo-34395: Fix memory leaks caused by incautious usage of PyMem_Resize(). (pythonGH-8756)
  bpo-34405: Updated to OpenSSL 1.1.0i for Windows builds. (pythonGH-8775)
  bpo-34384: Fix os.readlink() on Windows (pythonGH-8740)
  closes bpo-34400: Fix undefined behavior in parsetok(). (pythonGH-4439)
  bpo-34399: 2048 bits RSA keys and DH params (python#8762)
  Make regular expressions in test_tasks.py raw strings. (pythonGH-8759)
  smtplib documentation fixes (pythonGH-8708)
  Fix misindented yaml in logging how to example (pythonGH-8604)
  ...

dimaqq reviewed

Aug 29, 2018

View reviewed changes

Comment thread

Lib/unittest/mock.py

		'%r not all found in call list' % (tuple(not_found),)
		'%r does not contain all of %r in its call list, '
		'found %r instead' % (self._mock_name or 'mock',
		tuple(not_found), all_calls)

dimaqq Aug 29, 2018

Copy link

Copy Markdown

Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Not sure about tuple here, given that both not_found and all_calls are lists as evident from lines 855, 857.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

gpshead Aug 30, 2018

Copy link

Copy Markdown

Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

While this change preserved the behavior of the existing code. I don't really think it matters, the error message will be understandable regardless.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

encukou pushed a commit to encukou/cpython that referenced this pull request

Sep 29, 2020


          00121-add-Modules-to-build-path.patch

815049a

Upstream r79310 removed the "Modules" directory from sys.path when Python is
running from the build directory on POSIX to fix a unit test (issue python#8205).
This seems to have broken the compileall.py done in "make install": it cannot
find shared library extension modules at this point in the build (sys.path
does not contain DESTDIR/usr/lib(64)/python-2.7/lib-dynload for some reason),
leading to the build failing with:
Traceback (most recent call last):
  File "/home/david/rpmbuild/BUILDROOT/python-2.7-0.1.rc2.fc14.x86_64/usr/lib64/python2.7/compileall.py", line 17, in <module>
    import struct
  File "/home/david/rpmbuild/BUILDROOT/python-2.7-0.1.rc2.fc14.x86_64/usr/lib64/python2.7/struct.py", line 1, in <module>
   from _struct import *
ImportError: No module named _struct
This patch adds the build Modules directory to build path.

stratakis pushed a commit to stratakis/cpython that referenced this pull request

Jul 18, 2022


          00121-add-Modules-to-build-path.patch

6a621e2

Upstream r79310 removed the "Modules" directory from sys.path when Python is
running from the build directory on POSIX to fix a unit test (issue python#8205).
This seems to have broken the compileall.py done in "make install": it cannot
find shared library extension modules at this point in the build (sys.path
does not contain DESTDIR/usr/lib(64)/python-2.7/lib-dynload for some reason),
leading to the build failing with:
Traceback (most recent call last):
  File "/home/david/rpmbuild/BUILDROOT/python-2.7-0.1.rc2.fc14.x86_64/usr/lib64/python2.7/compileall.py", line 17, in <module>
    import struct
  File "/home/david/rpmbuild/BUILDROOT/python-2.7-0.1.rc2.fc14.x86_64/usr/lib64/python2.7/struct.py", line 1, in <module>
   from _struct import *
ImportError: No module named _struct
This patch adds the build Modules directory to build path.

Reviewers

1st1

1st1 left review comments

gpshead

Awaiting requested review from gpshead

+1 more reviewer

dimaqq

dimaqq left review comments

Reviewers whose approvals may not affect merge requirements

Uh oh!

There was an error while loading. Please reload this page.

Assignees

gpshead

Labels

skip issue

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

6 participants

Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.

You are visitor 01402520 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.