Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(128)

#9554: test_argparse.py: use new unittest features

Can't Edit
Can't Publish+Mail
Start Review
Created:
9 years, 3 months ago by denver
Modified:
5 years, 6 months ago
Reviewers:
ezio.melotti, berker.peksag
CC:
bethard, eric.smith, Benjamin Peterson, ezio.melotti, eric.araujo, r.david.murray, sandro.tosi, BreamoreBoy, denversc, devnull_psf.upfronthosting.co.za, berkerpeksag, storchaka, raduv
Visibility:
Public.

Patch Set 1 #

Patch Set 2 #

Patch Set 3 #

Patch Set 4 #

Patch Set 5 #

Total comments: 6
Unified diffs Side-by-side diffs Delta from patch set Stats Patch
Lib/test/test_argparse.py View 1 2 3 4 7 chunks +27 lines, -66 lines 6 comments Download
Misc/ACKS View 1 2 3 4 1 chunk +1 line, -0 lines 0 comments Download
Misc/NEWS View 1 2 3 4 1 chunk +3 lines, -0 lines 0 comments Download

Messages

Total messages: 2
ezio.melotti
http://bugs.python.org/review/9554/diff/12343/Lib/test/test_argparse.py File Lib/test/test_argparse.py (right): http://bugs.python.org/review/9554/diff/12343/Lib/test/test_argparse.py#newcode2852 Lib/test/test_argparse.py:2852: self.assertEqual('' not in ns, True) It should be safe ...
5 years, 6 months ago #1
berkerpeksag
5 years, 6 months ago #2
http://bugs.python.org/review/9554/diff/12343/Lib/test/test_argparse.py
File Lib/test/test_argparse.py (right):

http://bugs.python.org/review/9554/diff/12343/Lib/test/test_argparse.py#newco...
Lib/test/test_argparse.py:2852: self.assertEqual('' not in ns, True)
On 2014/07/05 18:03:04, ezio.melotti wrote:
> It should be safe to convert self.assertEqual('' not in ns, True) to
assertNotIn
> and remove self.assertEqual('' in ns, False) here, since if the first
assertion
> passes, the second one should always pass as well, so having both is
redundant.

Done.

http://bugs.python.org/review/9554/diff/12343/Lib/test/test_argparse.py#newco...
Lib/test/test_argparse.py:4206: with self.assertRaisesRegex(ValueError,
expected_regex):
On 2014/07/05 18:03:04, ezio.melotti wrote:
> I think it would be better to use assertRaises + assertIn for this and the
> previous chunk. 

Done.

http://bugs.python.org/review/9554/diff/12343/Lib/test/test_argparse.py#newco...
Lib/test/test_argparse.py:4384: self.assertEqual('PPP 3.5\n',
cm.exception.stdout)
On 2014/07/05 18:03:04, ezio.melotti wrote:
> This should be assertEqual(actual, expected), i.e.:
>   self.assertEqual(cm.exception.stdout, 'PPP 3.5\n')
> but I think that the same "wrong" style (expected, actual) is used in the rest
> of the file.
> For consistency with the rest it's ok to leave it like this, but it would be
> good to fix it (in a separate issue).

I will open a new issue to fix this, thanks!
Sign in to reply to this message.

RSS Feeds Recent Issues | This issue
This is Rietveld 894c83f36cb7+