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
Remove empty tests in test_bytes.FixedStringTest #63786
Comments
The attached patch removes test_bytes.FixedStringTest and its subclasses, as they report success when they don't actually test anything at all. |
ByteArrayAsStringTest and BytesAsStringTest inherit tests from test.string_tests.BaseTest. |
So they do, I failed to notice that. New patch just removes the empty test methods. |
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. |
I said this elsewhere but I'll repeat it here. |
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. |
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? |
New changeset 22c2dd04a3d3 by Martin Panter in branch '3.5': New changeset 8545a082fcaa by Martin Panter in branch 'default': |
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 bpo-26257 for that. |
Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.
Show more details
GitHub fields:
bugs.python.org fields:
The text was updated successfully, but these errors were encountered: