Skip to content

bpo-33516: Add support for __round__ to MagicMock#6880

Merged
vstinner merged 1 commit into
python:masterfrom
amyreese:round
May 22, 2018
Merged

bpo-33516: Add support for __round__ to MagicMock#6880
vstinner merged 1 commit into
python:masterfrom
amyreese:round

Conversation

@amyreese

@amyreese amyreese commented May 15, 2018

Copy link
Copy Markdown
Contributor

Adds support for __round__ to MagicMock.

https://bugs.python.org/issue33516

@vstinner vstinner left a comment

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.

Would it be possible to add a short unit test to Lib/unittest/test/testmock/testmagicmethods.py please?

It may be add nice to add tests for trunc and floor as well.

@vstinner

Copy link
Copy Markdown
Member

For Travis CI, your PR has be bitten by https://bugs.python.org/issue33531 :-(

@amyreese
amyreese force-pushed the round branch 3 times, most recently from f9cd107 to 92c03e2 Compare May 17, 2018 15:32
@amyreese

Copy link
Copy Markdown
Contributor Author

Added unit tests for round/trunc/floor/ceil, and fixed docs build.

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.

The result of round(), trunc(), etc. are a number, not a boolean. You should use assertEqual(..., value) instead.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Updated to assert equal to mock.__dunder__() calls.

@vstinner vstinner left a comment

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.

LGTM, except of the minor issue in the NEWS entry.

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.

Oh, you didn't mention the module. Please add "unittest.mock: " prefix to this NEWS entry.

@amyreese

Copy link
Copy Markdown
Contributor Author

Updated/reworded news entry to link to unittest.mock.MagicMock.

@vstinner
vstinner merged commit 6c4fab0 into python:master May 22, 2018
@vstinner

Copy link
Copy Markdown
Member

Thanks @jreese, the change now LGTM. The final version is better than the first version ;-)

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