Opened 4 years ago

Closed 4 years ago

Last modified 4 years ago

#6350 enhancement closed fixed (fixed)

Add a few unittest2 assertions to TestCase

Reported by: Julian Berman Owned by: Tom Prince
Priority: normal Milestone:
Component: trial Keywords:
Cc: Jonathan Lange Branch: branches/assertIs-6350
branch-diff, diff-cov, branch-cov, buildbot
Author: tomprince

Description

This begins to add some of unittest2's assertions to TestCase. Some of these are needed (assertIs and assertIsNot) because they will be replacing assertIdentical and assertNotIdentical.

Others are just plain dumb (assertIsNone) though unittest2 has them, and eventually it would be nice to have full unittest2 compatibility (once again, unittest2 is the name for pyunit in 2.7+). So these are here for now to help that move forward. Eventually they will be removed and only defined on Python 2.6.

Attachments (3)

assertIs.patch (5.1 KB) - added by Julian Berman 4 years ago.
assertIs-v2.patch (2.6 KB) - added by Julian Berman 4 years ago.
assertIs-v3.patch (2.8 KB) - added by Julian Berman 4 years ago.

Download all attachments as: .zip

Change History (20)

comment:1 Changed 4 years ago by DefaultCC Plugin

Cc: Jonathan Lange added

Changed 4 years ago by Julian Berman

Attachment: assertIs.patch added

comment:2 Changed 4 years ago by Jean-Paul Calderone

eventually it would be nice to have full unittest2 compatibility

I'm not so sure about this. Keep in mind that trial TestCase is a subclass of unittest.TestCase. It inherits all the methods unittest.TestCase defines. It is hardly a surprise that unittest.TestCase on Python 2.6 does not have the new features of unittest.TestCase which were added in Python 2.7. Generally speaking, this is not a problem. Developers writing code for Python 2.6 are aware that they will be getting Python 2.6 functionality, not Python 2.7 functionality.

If there are specific cases where some neat Python 2.7 unittest functionality would actually help implement some tests for Twisted, by all means extend trial to provide this functionality on Python 2.6 as well. However, functionality should be driven by real use cases (because only through real use does code provide actual value). In the abstract (that is, without real use cases), "full compatibility" is meaningless (only unittest2 is fully compatible with unittest2) and not obviously desirable.

comment:3 Changed 4 years ago by Jean-Paul Calderone

Other areas of trial which could benefit greatly from attention (in place of "full compatibility" with unittest2):

And if you want to focus on stdlib interoperability, then #6345 would help solve at least one person's real-world issue.

comment:4 Changed 4 years ago by Julian Berman

Well. When I say "full compatibility", it'd be nice at least if trial were able to take advantage of unittest2 if present. E.g. if most of the time I write 2.7+ code, when I want to use a trial TestCase I'm forced to use 2.6 style assertions.

If I have unittest2 installed though, I'd like trial to pick it up and inherit from unittest2.TestCase rather than unittest, same as if I were writing non-twisted test code, where I could use 2.7 style assertions even though I was supporting 2.6 too.

comment:5 Changed 4 years ago by Julian Berman

Though assertIsNone is essentially totally pointless given that even in pyunit it doesn't give any better of an error than assertIs(foo, None). So the real world use case here is that as things stand, when I write non-twisted code, I'm able to write 2.7+ style assertions and then have them work on 2.6.

I can't do so when I write twisted code, and it'd be nice if I could.

comment:6 Changed 4 years ago by Jean-Paul Calderone

So the real world use case here is that as things stand, when I write non-twisted code, I'm able to write 2.7+ style assertions and then have them work on 2.6.

This is the problem with making TestCase the center of extensibility for making assertions. unittest2 is just one example of a divergence in the class hierarchy that can't be resolved except by duplicating lots of code all over the place.

It seems to me that adding testtools style matcher support would solve this problem once and for all.

comment:7 Changed 4 years ago by Julian Berman

OK well, we don't especially need to have this conversation now I guess. To get us unblocked on #6220 here's just the addition of assertIs and assertIsNot, which I hope you at least agree on (that we should use the unittest2 names for assertions we already have but under different names).

Changed 4 years ago by Julian Berman

Attachment: assertIs-v2.patch added

comment:8 Changed 4 years ago by Tom Prince

Keywords: review removed
Owner: set to Julian Berman
  1. The tests don't check that the assertions use is instead of ==:
    (object() == object()) is False
    
  2. The test docstrings should document what the test is doing, and what result it expects from that (see #6301). We want to document the intent of the test, so that a reviewer, or future reader can see if the test is testing what is expected.

comment:9 Changed 4 years ago by Julian Berman

Keywords: review added
Owner: Julian Berman deleted

Sigh. These were copied out of the stdlib test suite :).

Fixed, see latest patch. Thanks.

Changed 4 years ago by Julian Berman

Attachment: assertIs-v3.patch added

comment:10 Changed 4 years ago by Jean-Paul Calderone

Keywords: review removed
Owner: set to Julian Berman

Sigh. These were copied out of the stdlib test suite :).

Um. I am quite concerned about this. You cannot copy code out of other projects and contribute it to Twisted without annotating its origin. Doing so is potentially a violation of the Python license and potentially makes distributing Twisted itself illegal.

Taking this ticket out of review until this issue is addressed definitively.

comment:11 Changed 4 years ago by Julian Berman

Keywords: review added

There is no directly copied code in here and there never was.

The assertion in the test (an assertEqual on two things that happen to be objects()) was the same as what the stdlib happens to be doing, so all I meant was that the stdlib didn't care about specifically going out of its way to complicate things.

comment:12 Changed 4 years ago by Julian Berman

Owner: Julian Berman deleted

comment:13 Changed 4 years ago by Tom Prince

Author: tomprince
Branch: branches/assertIs-6350

(In [37931]) Branching to assertIs-6350.

comment:14 Changed 4 years ago by Tom Prince

(In [37932]) Apply assertIs-v3.patch from Julian.

Refs: #6350

comment:15 Changed 4 years ago by Tom Prince

Keywords: review removed
Owner: set to Tom Prince

This looks good. I've added a topfile.

comment:16 Changed 4 years ago by Tom Prince

Resolution: fixed
Status: newclosed

(In [37934]) Merge assertIs-6350: Add assertIs and assertIsNot assertions.

Author: Julian Reviewers: tom.prince Fixes: #6350

These are the names used in python 2.7 and python 3. Adding these is part of picking a single spelling for each assertion.

comment:17 Changed 4 years ago by Julian Berman

Great, thanks as usual Tom. See you on the flipside of #6220 and friends.

Note: See TracTickets for help on using tickets.