classification
Title: Remove empty tests in test_bytes.FixedStringTest
Type: enhancement Stage: resolved
Components: Tests Versions: Python 3.6, Python 3.5
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: ezio.melotti, georg.brandl, martin.panter, nnorwitz, python-dev, serhiy.storchaka, zach.ware
Priority: normal Keywords: patch

Created on 2013-11-14 17:47 by zach.ware, last changed 2016-02-01 11:43 by martin.panter. This issue is now closed.

Files
File name Uploaded Description Edit
test_bytes.diff zach.ware, 2013-11-14 17:47 Remove non-testing tests review
test_bytes.v2.diff zach.ware, 2013-11-14 18:16 review
test_bytes.v3.diff martin.panter, 2015-12-17 10:31 Also remove redundant tests review
Messages (9)
msg202866 - (view) Author: Zachary Ware (zach.ware) * (Python committer) Date: 2013-11-14 17:47
The attached patch removes test_bytes.FixedStringTest and its subclasses, as they report success when they don't actually test anything at all.
msg202869 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2013-11-14 18:06
ByteArrayAsStringTest and BytesAsStringTest inherit tests from test.string_tests.BaseTest.
msg202871 - (view) Author: Zachary Ware (zach.ware) * (Python committer) Date: 2013-11-14 18:16
So they do, I failed to notice that.  New patch just removes the empty test methods.
msg202874 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2013-11-14 18:39
This LGTM, but the purpose of suppressing these tests is not absolutely clear to me. I suppose they were suppressed in times when bytes and batearray had no the lower() and upper() methods and the __contains__() methods accepted only integers. It will be good if developers which touched this code in past will made a review.
msg203082 - (view) Author: Ezio Melotti (ezio.melotti) * (Python committer) Date: 2013-11-16 21:05
I said this elsewhere but I'll repeat it here.
I think the whole string_tests could/should be reorganized (and possibly documented).  There are lot of classes and mixins on several files, and more than once I found tests that were accidentally not run or had other problems.  This got worse after Python 3, where bytes and strings share even less code.  Reorganizing this should make things easier to follow and will probably solve issues like this.  This of course is a bigger project than simply fixing these methods, so it should probably be addressed in a separate issue.
msg256595 - (view) Author: Martin Panter (martin.panter) * (Python committer) Date: 2015-12-17 10:31
test_contains() does not appear to be overriding anything, so there is no problem removing that.

Unmasking the other three methods will allow six new test methods to run (via ByteArrayAsStringTest and BytesAsStringTest). There are already three equivalent methods defined in buffer_tests.py run via BytearrayPEP3137Test. I suggest to remove those three from buffer_tests. The net result will be the same tests run for bytes(), and three new tests for bytearray(). The buffer_tests file is not used anywhere else.
msg256640 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2015-12-18 06:14
Currently buffer_tests.py is used only for bytearray, and it looks that it duplicates string_tests.py. May be remove all duplicated tests and move the rest of tests (if any) just into test_bytes.py?
msg259319 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2016-02-01 11:08
New changeset 22c2dd04a3d3 by Martin Panter in branch '3.5':
Issue #19587: Remove masked and redundant tests in test_bytes
https://hg.python.org/cpython/rev/22c2dd04a3d3

New changeset 8545a082fcaa by Martin Panter in branch 'default':
Issue #19587: Merge test_bytes cleanup from 3.5
https://hg.python.org/cpython/rev/8545a082fcaa
msg259320 - (view) Author: Martin Panter (martin.panter) * (Python committer) Date: 2016-02-01 11:43
Thanks Serhiy for reviewing. You are right that buffer_tests is only run on bytearray; I must have gotten mixed up. I fixed the comment in the version I committed.

Yes I think I agree with eliminating buffer_tests.py, which is only run for bytearray. I opened Issue 26257 for that.
History
Date User Action Args
2016-02-01 11:43:10martin.pantersetstatus: open -> closed
resolution: fixed
messages: + msg259320

stage: patch review -> resolved
2016-02-01 11:08:51python-devsetnosy: + python-dev
messages: + msg259319
2015-12-18 06:14:08serhiy.storchakasetmessages: + msg256640
2015-12-17 10:31:59martin.pantersetfiles: + test_bytes.v3.diff
versions: + Python 3.5, Python 3.6, - Python 3.3, Python 3.4
nosy: + martin.panter

messages: + msg256595
2013-11-16 21:05:05ezio.melottisetnosy: + ezio.melotti
messages: + msg203082
2013-11-14 18:39:40serhiy.storchakasetnosy: + nnorwitz, georg.brandl

messages: + msg202874
stage: patch review
2013-11-14 18:19:49zach.waresettitle: Remove test_bytes.FixedStringTest -> Remove empty tests in test_bytes.FixedStringTest
2013-11-14 18:16:35zach.waresetfiles: + test_bytes.v2.diff

messages: + msg202871
2013-11-14 18:06:59serhiy.storchakasetmessages: + msg202869
2013-11-14 17:47:22zach.warecreate