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

gh-71339: Add additional assertion methods for unittest #128707

Merged
merged 7 commits into from
Jan 14, 2025

Conversation

serhiy-storchaka
Copy link
Member

@serhiy-storchaka serhiy-storchaka commented Jan 10, 2025

Add the following methods:

  • assertHasAttr() and assertNotHasAttr()
  • assertIsSubclass() and assertNotIsSubclass()
  • assertStartsWith() and assertNotStartsWith()
  • assertEndsWith() and assertNotEndsWith()

Also improve error messages for assertIsInstance() and assertNotIsInstance().


📚 Documentation preview 📚: https://cpython-previews--128707.org.readthedocs.build/

Add the following methods:

* assertHasAttr() and assertNotHasAttr()
* assertIsSubclass() and assertNotIsSubclass()
* assertStartswith() and assertNotStartswith()
* assertEndswith() and assertNotEndswith()

Also improve error messages for assertIsInstance() and
assertNotIsInstance().
@hugovk
Copy link
Member

hugovk commented Jan 10, 2025

Great!

  • assertStartsWith() and assertEndsWith() has been renamed to assertStartswith() and assertEndswith().

I recommend assertStartsWith() and assertEndsWith(), both for readability, and for accessibility -- screen readers will have a better chance with the extra capitals.

It doesn't really matter that startswith is all lower case, because we're creating a new method name.

(And we already have assertIsInstance, which checks isinstance, and not assertIsinstance.)

@serhiy-storchaka
Copy link
Member Author

I wrote them initially as assertStartsWith() and assertEndsWith(), but renamed to assertStartswith() and assertEndswith() as the result of the discussion on the issue.

@serhiy-storchaka
Copy link
Member Author

I created a poll: https://discuss.python.org/t/assertstartwith-vs-assertstartwith/76701.

Copy link
Member

@JelleZijlstra JelleZijlstra left a comment

Choose a reason for hiding this comment

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

Looks good but I agree with Hugo that the name should be assertStartsWith.

@cjw296
Copy link
Contributor

cjw296 commented Jan 11, 2025

Looks like the poll is pretty unanimous :-)

The changes look great, but I'll admit I'm unlikely to use them as I tend to avoid the UnitTest base class where possible now.

@cjw296 cjw296 removed their request for review January 11, 2025 18:21
Doc/library/unittest.rst Outdated Show resolved Hide resolved
Doc/library/unittest.rst Outdated Show resolved Hide resolved
Doc/whatsnew/3.14.rst Outdated Show resolved Hide resolved
Co-authored-by: Hugo van Kemenade <[email protected]>
Copy link
Member

@vstinner vstinner left a comment

Choose a reason for hiding this comment

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

LGTM. Just a few minor coding style remarks.

@@ -1321,15 +1321,77 @@ def assertIsInstance(self, obj, cls, msg=None):
"""Same as self.assertTrue(isinstance(obj, cls)), with a nicer
default message."""
if not isinstance(obj, cls):
standardMsg = '%s is not an instance of %r' % (safe_repr(obj), cls)
if isinstance(cls, tuple):
standardMsg = '%s is not an instance of any of %r' % (safe_repr(obj), cls)
Copy link
Member

Choose a reason for hiding this comment

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

You may use f-strings :-)

Copy link
Member Author

Choose a reason for hiding this comment

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

Actually, '%s is not an instance of any of %r' % (safe_repr(obj), cls) and f'{safe_repr(obj)!s} is not an instance of any of {cls!r}' produce the same bytecode. So the difference is only in readability, which is at large part subjective. I was not sure that inlining expressions in f-strings would make the code more readable, but if you think so...

standardMsg = '%s is an instance of %r' % (safe_repr(obj), cls)
self.fail(self._formatMessage(msg, standardMsg))

def assertIsSubclass(self, cls, superclass, msg=None):
try:
r = issubclass(cls, superclass)
Copy link
Member

Choose a reason for hiding this comment

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

You may avoid variables of a single letter: use "res" or "result". Same remark for new functions below.

Copy link
Member

@picnixz picnixz left a comment

Choose a reason for hiding this comment

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

Just some docs nits and LGTM. I'm very happy to have assertHasAttr because I needed it a lot in my personal projects.

Doc/library/unittest.rst Show resolved Hide resolved
Doc/library/unittest.rst Show resolved Hide resolved
@serhiy-storchaka serhiy-storchaka merged commit 06cad77 into python:main Jan 14, 2025
38 checks passed
@serhiy-storchaka serhiy-storchaka deleted the extra-assertions branch January 14, 2025 08:02
@vstinner
Copy link
Member

Nice additions, thanks @serhiy-storchaka.

serhiy-storchaka added a commit to serhiy-storchaka/cpython that referenced this pull request Jan 14, 2025
…honGH-128707)

Add a mix-in class ExtraAssertions containing the following methods:

* assertHasAttr() and assertNotHasAttr()
* assertIsSubclass() and assertNotIsSubclass()
* assertStartsWith() and assertNotStartsWith()
* assertEndsWith() and assertNotEndsWith()

(cherry picked from commit 06cad77)
@bedevere-app
Copy link

bedevere-app bot commented Jan 14, 2025

GH-128815 is a backport of this pull request to the 3.13 branch.

serhiy-storchaka added a commit to serhiy-storchaka/cpython that referenced this pull request Jan 14, 2025
…honGH-128707)

Add a mix-in class ExtraAssertions containing the following methods:

* assertHasAttr() and assertNotHasAttr()
* assertIsSubclass() and assertNotIsSubclass()
* assertStartsWith() and assertNotStartsWith()
* assertEndsWith() and assertNotEndsWith()

(cherry picked from commit 06cad77)
serhiy-storchaka added a commit that referenced this pull request Jan 20, 2025
…-128707) (GH-128815)

Add a mix-in class ExtraAssertions containing the following methods:

* assertHasAttr() and assertNotHasAttr()
* assertIsSubclass() and assertNotIsSubclass()
* assertStartsWith() and assertNotStartsWith()
* assertEndsWith() and assertNotEndsWith()

(cherry picked from commit 06cad77)
serhiy-storchaka added a commit to serhiy-storchaka/cpython that referenced this pull request Jan 20, 2025
…t.support (pythonGH-128707) (pythonGH-128815)

Add a mix-in class ExtraAssertions containing the following methods:

* assertHasAttr() and assertNotHasAttr()
* assertIsSubclass() and assertNotIsSubclass()
* assertStartsWith() and assertNotStartsWith()
* assertEndsWith() and assertNotEndsWith()

(cherry picked from commit c6a566e)

Co-authored-by: Serhiy Storchaka <[email protected]>
(cherry picked from commit 06cad77)
serhiy-storchaka added a commit that referenced this pull request Jan 20, 2025
…-128707) (GH-128815) (GH-129059)

Add a mix-in class ExtraAssertions containing the following methods:

* assertHasAttr() and assertNotHasAttr()
* assertIsSubclass() and assertNotIsSubclass()
* assertStartsWith() and assertNotStartsWith()
* assertEndsWith() and assertNotEndsWith()

(cherry picked from commit c6a566e)

Co-authored-by: Serhiy Storchaka <[email protected]>
(cherry picked from commit 06cad77)
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.

7 participants