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.

Sign up for GitHub

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

Jump to bottom

Duplicate and unused code in tests #90356

Closed
sobolevn opened this issue Dec 30, 2021 · 11 comments
Closed

Duplicate and unused code in tests #90356

sobolevn opened this issue Dec 30, 2021 · 11 comments
Labels
3.9 only security fixes 3.10 only security fixes 3.11 only security fixes tests Tests in the Lib/test dir type-bug An unexpected behavior, bug, or error

Comments

@sobolevn
Copy link
Member

BPO 46198
Nosy @merwok, @JelleZijlstra, @sobolevn, @AlexWaygood
PRs
  • bpo-46198: rename duplicate tests and remove unused code #30297
  • [3.10] bpo-46198: rename duplicate tests and remove unused code (GH-30297) #31796
  • [3.9] bpo-46198: rename duplicate tests and remove unused code (GH-30297) #31797
  • bpo-46198: Fix test_asyncio.test_sslproto #31801
    		•
    Files
  • duplicate-test-methods.diff
    		•

    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:

    assignee = None
closed_at = <Date 2022-03-10.23:18:46.194>
created_at = <Date 2021-12-30.00:23:42.583>
labels = ['type-bug', 'tests', '3.9', '3.10', '3.11']
title = 'Duplicate and unused code in tests'
updated_at = <Date 2022-03-10.23:18:46.193>
user = 'https://github.com/sobolevn'

    bugs.python.org fields:

    activity = <Date 2022-03-10.23:18:46.193>
actor = 'AlexWaygood'
assignee = 'none'
closed = True
closed_date = <Date 2022-03-10.23:18:46.194>
closer = 'AlexWaygood'
components = ['Tests']
creation = <Date 2021-12-30.00:23:42.583>
creator = 'sobolevn'
dependencies = []
files = ['50534']
hgrepos = []
issue_num = 46198
keywords = ['patch']
message_count = 11.0
messages = ['409338', '409430', '409439', '409444', '409451', '409471', '409477', '414855', '414879', '414880', '414881']
nosy_count = 4.0
nosy_names = ['eric.araujo', 'JelleZijlstra', 'sobolevn', 'AlexWaygood']
pr_nums = ['30297', '31796', '31797', '31801']
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue46198'
versions = ['Python 3.9', 'Python 3.10', 'Python 3.11']
    @sobolevn
    Copy link
    Member Author

    There are two tests with the same name in a same test class in [Lib/test/test_email/test__header_value_parser.py](https://github.com/python/cpython/blob/main/Lib/test/test_email/test__header_value_parser.py): test_get_unstructured_invalid_ew

    1. cpython/Lib/test/test_email/test__header_value_parser.py

      Line 304 in 8e11237

      def test_get_unstructured_invalid_ew(self):

    2. cpython/Lib/test/test_email/test__header_value_parser.py

      Line 398 in 8e11237

      def test_get_unstructured_invalid_ew(self):

    So, because of this bad naming - the first test is always shadowed by the second one and is silently skipped. With my patch: 1660 tests, without: 1659 tests.

    PR to rename the second test is on its way.

    @sobolevn sobolevn added 3.9 only security fixes 3.10 only security fixes 3.11 only security fixes tests Tests in the Lib/test dir type-bug An unexpected behavior, bug, or error labels Dec 30, 2021
    @merwok
    Copy link
    Member

    merwok commented Dec 31, 2021

    Also fund a duplicate TestHelpers class in test_enum.

    A warning for duplicate method is a false positive, because the two methods are used on different python versions. But the first method uses self.Grades.B wrongly (should be Grades.B) so it will break when python 3.12 is started!

    @sobolevn
    Copy link
    Member Author

    Eric, should I patch this as well in this issue?
    Or should I open a new one? Would be happy to fix it :)

    @merwok
    Copy link
    Member

    merwok commented Jan 1, 2022

    Sometimes people fix small things in multiple modules, sometimes they create separate tickets to have one patch per module with reviews from different maintainers and so on. For this case, I check pyflakes and found a handful of true positives, so I would retitle the ticket to be about duplicate methods in tests.

    @sobolevn
    Copy link
    Member Author

    sobolevn commented Jan 1, 2022

    Thanks! I will also attach all flake8's output if someone is going to need it:

    » flake8 --select=F811 Lib/test
Lib/test/test_descr.py:1165:13: F811 redefinition of unused 'C' from line 1158
Lib/test/test_descr.py:1172:13: F811 redefinition of unused 'C' from line 1165
Lib/test/test_descr.py:1179:13: F811 redefinition of unused 'C' from line 1172
Lib/test/test_descr.py:1186:13: F811 redefinition of unused 'C' from line 1179
Lib/test/test_descr.py:1192:9: F811 redefinition of unused 'C' from line 1186
Lib/test/test_descr.py:1198:9: F811 redefinition of unused 'C' from line 1192
Lib/test/test_descr.py:1488:13: F811 redefinition of unused 'C' from line 1480
Lib/test/test_descr.py:1498:13: F811 redefinition of unused 'C' from line 1488
Lib/test/test_descr.py:1506:13: F811 redefinition of unused 'C' from line 1498
Lib/test/test_descr.py:1514:13: F811 redefinition of unused 'C' from line 1506
Lib/test/test_descr.py:4076:13: F811 redefinition of unused 'X' from line 4073
Lib/test/test_descr.py:4079:13: F811 redefinition of unused 'X' from line 4076
Lib/test/test_descr.py:4084:13: F811 redefinition of unused 'X' from line 4079
Lib/test/test_descr.py:4087:13: F811 redefinition of unused 'X' from line 4084
Lib/test/test_descr.py:4090:9: F811 redefinition of unused 'X' from line 4087
Lib/test/test_buffer.py:759:5: F811 redefinition of unused 'genslices' from line 694
Lib/test/test_buffer.py:760:5: F811 redefinition of unused 'genslices_ndim' from line 698
Lib/test/test_buffer.py:761:5: F811 redefinition of unused 'permutations' from line 20
Lib/test/test_syntax.py:1555:5: F811 redefinition of unused 'test_break_outside_loop' from line 1525
Lib/test/test_typing.py:108:13: F811 redefinition of unused 'A' from line 105
Lib/test/test_typing.py:148:13: F811 redefinition of unused 'A' from line 145
Lib/test/test_typing.py:332:13: F811 redefinition of unused 'C' from line 329
Lib/test/test_typing.py:335:13: F811 redefinition of unused 'C' from line 332
Lib/test/test_typing.py:880:13: F811 redefinition of unused 'P' from line 877
Lib/test/test_typing.py:883:13: F811 redefinition of unused 'P' from line 880
Lib/test/test_typing.py:1385:13: F811 redefinition of unused 'P' from line 1383
Lib/test/test_typing.py:1387:13: F811 redefinition of unused 'P' from line 1385
Lib/test/test_typing.py:1389:13: F811 redefinition of unused 'P' from line 1387
Lib/test/test_typing.py:1665:13: F811 redefinition of unused 'MyGeneric' from line 1663
Lib/test/test_typing.py:2295:13: F811 redefinition of unused 'Subclass' from line 2292
Lib/test/test_typing.py:2478:13: F811 redefinition of unused 'C' from line 2475
Lib/test/test_typing.py:2522:13: F811 redefinition of unused 'C' from line 2519
Lib/test/test_typing.py:4532:5: F811 redefinition of unused 'test_hash_eq' from line 4475
Lib/test/test_typing.py:4652:13: F811 redefinition of unused 'C' from line 4648
Lib/test/test_typing.py:4844:13: F811 redefinition of unused 'C' from line 4841
Lib/test/datetimetester.py:1867:9: F811 redefinition of unused 'io' from line 5
Lib/test/datetimetester.py:3991:9: F811 redefinition of unused 'io' from line 5
Lib/test/test_genericclass.py:101:13: F811 redefinition of unused 'D' from line 95
Lib/test/test_genericclass.py:114:13: F811 redefinition of unused 'D' from line 108
Lib/test/test_subclassinit.py:235:13: F811 redefinition of unused 'MyClass' from line 221
Lib/test/test_subclassinit.py:246:9: F811 redefinition of unused 'MyClass' from line 235
Lib/test/test_subclassinit.py:268:9: F811 redefinition of unused 'MyClass' from line 259
Lib/test/test_tabnanny.py:7:1: F811 redefinition of unused 'mock' from line 6
Lib/test/test_compile.py:1178:5: F811 redefinition of unused 'test_func_args' from line 1171
Lib/test/test_dataclasses.py:517:13: F811 redefinition of unused 'A' from line 512
Lib/test/test_dataclasses.py:527:13: F811 redefinition of unused 'A' from line 517
Lib/test/test_dataclasses.py:681:21: F811 redefinition of unused 'Point' from line 672
Lib/test/test_dataclasses.py:692:21: F811 redefinition of unused 'Point' from line 681
Lib/test/test_dataclasses.py:702:17: F811 redefinition of unused 'C' from line 697
Lib/test/test_dataclasses.py:1840:9: F811 redefinition of unused 'C' from line 1834
Lib/test/test_dataclasses.py:2387:13: F811 redefinition of unused 'C' from line 2378
Lib/test/test_dataclasses.py:2396:13: F811 redefinition of unused 'C' from line 2387
Lib/test/test_dataclasses.py:2405:13: F811 redefinition of unused 'C' from line 2396
Lib/test/test_dataclasses.py:2778:13: F811 redefinition of unused 'C' from line 2770
Lib/test/test_dataclasses.py:2784:9: F811 redefinition of unused 'C' from line 2778
Lib/test/test_dataclasses.py:3678:13: F811 redefinition of unused 'A' from line 3673
Lib/test/test_dataclasses.py:3683:13: F811 redefinition of unused 'A' from line 3678
Lib/test/test_dataclasses.py:3825:13: F811 redefinition of unused 'A' from line 3816
Lib/test/test_dataclasses.py:3834:13: F811 redefinition of unused 'A' from line 3825
Lib/test/test_dataclasses.py:3843:9: F811 redefinition of unused 'A' from line 3834
Lib/test/test_dataclasses.py:3851:9: F811 redefinition of unused 'A' from line 3843
Lib/test/test_dataclasses.py:3870:13: F811 redefinition of unused 'B' from line 3857
Lib/test/test_functools.py:566:13: F811 redefinition of unused 'B' from line 563
Lib/test/test_functools.py:569:13: F811 redefinition of unused 'B' from line 566
Lib/test/test_yield_from.py:921:9: F811 redefinition of unused 'two' from line 890
Lib/test/time_hashlib.py:60:5: F811 redefinition of unused 'creatorFunc' from line 9
Lib/test/test_keywordonlyarg.py:173:13: F811 redefinition of unused 'f' from line 169
Lib/test/test_codecs.py:35:5: F811 redefinition of unused 'UINT' from line 35
Lib/test/test_dict.py:1441:5: F811 redefinition of unused 'test_dict_items_result_gc' from line 1429
Lib/test/test_pkg.py:200:9: F811 redefinition of unused 't5' from line 193
Lib/test/test_pty.py:19:1: F811 redefinition of unused 'tty' from line 10
Lib/test/test_enum.py:625:13: F811 redefinition of unused 'Wrong' from line 622
Lib/test/test_enum.py:628:13: F811 redefinition of unused 'Wrong' from line 625
Lib/test/test_enum.py:631:13: F811 redefinition of unused 'Wrong' from line 628
Lib/test/test_enum.py:634:13: F811 redefinition of unused 'Wrong' from line 631
Lib/test/test_enum.py:746:13: F811 redefinition of unused 'Color' from line 739
Lib/test/test_enum.py:754:13: F811 redefinition of unused 'Color' from line 746
Lib/test/test_enum.py:758:17: F811 redefinition of unused 'red' from line 756
Lib/test/test_enum.py:873:5: F811 redefinition of unused 'test_mixin_format_warning' from line 860
Lib/test/test_enum.py:1022:13: F811 redefinition of unused 'Huh' from line 1012
Lib/test/test_enum.py:2106:13: F811 redefinition of unused 'Color' from line 2101
Lib/test/test_enum.py:2603:13: F811 redefinition of unused 'ThirdFailedStrEnum' from line 2599
Lib/test/test_enum.py:2607:13: F811 redefinition of unused 'ThirdFailedStrEnum' from line 2603
Lib/test/test_enum.py:2667:13: F811 redefinition of unused 'ThirdFailedStrEnum' from line 2663
Lib/test/test_enum.py:2671:13: F811 redefinition of unused 'ThirdFailedStrEnum' from line 2667
Lib/test/test_enum.py:2729:13: F811 redefinition of unused 'ThirdFailedStrEnum' from line 2725
Lib/test/test_enum.py:2733:13: F811 redefinition of unused 'ThirdFailedStrEnum' from line 2729
Lib/test/test_enum.py:4168:1: F811 redefinition of unused 'TestHelpers' from line 107
Lib/test/test_import/__init__.py:277:13: F811 redefinition of unused 'x' from line 272
Lib/test/test_zoneinfo/test_zoneinfo.py:1096:9: F811 redefinition of unused '_add' from line 1068
Lib/test/test_zoneinfo/test_zoneinfo.py:1120:9: F811 redefinition of unused '_add' from line 1096
Lib/test/test_zoneinfo/test_zoneinfo.py:1145:9: F811 redefinition of unused '_add' from line 1120
Lib/test/test_zoneinfo/test_zoneinfo.py:1169:9: F811 redefinition of unused '_add' from line 1145
Lib/test/test_zoneinfo/test_zoneinfo.py:1182:9: F811 redefinition of unused '_add' from line 1169
Lib/test/test_zoneinfo/test_zoneinfo.py:1195:9: F811 redefinition of unused '_add' from line 1182
Lib/test/test_zoneinfo/test_zoneinfo.py:1216:9: F811 redefinition of unused '_add' from line 1195
Lib/test/test_zoneinfo/test_zoneinfo.py:1249:9: F811 redefinition of unused '_add' from line 1216
Lib/test/test_zoneinfo/test_zoneinfo.py:1272:9: F811 redefinition of unused '_add' from line 1249
Lib/test/support/__init__.py:1392:9: F811 redefinition of unused '_platform_specific' from line 1388
Lib/test/test_email/test__header_value_parser.py:398:5: F811 redefinition of unused 'test_get_unstructured_invalid_ew' from line 304
Lib/test/test_asyncio/test_sslproto.py:18:1: F811 redefinition of unused 'support' from line 5

    I will refactor some more in my existing PR.
    Thanks a lot, Éric!

    @sobolevn sobolevn changed the title Duplicated test name test_get_unstructured_invalid_ew in test__header_value_parser.py Duplicate and unused code in tests Jan 1, 2022
    @sobolevn sobolevn changed the title Duplicated test name test_get_unstructured_invalid_ew in test__header_value_parser.py Duplicate and unused code in tests Jan 1, 2022
    @merwok
    Copy link
    Member

    merwok commented Jan 1, 2022

    I think most of these are false positives (it’s fine if 10 different tests define a function f), so should not be changed just to appease a lint tool. Others are genuine!

    @sobolevn
    Copy link
    Member Author

    sobolevn commented Jan 1, 2022

    I think most of these are false positives

    Yes, they are! Please, check out my PR:
    #30297

    сб, 1 янв. 2022 г. в 21:52, Éric Araujo <report@bugs.python.org>:

    Éric Araujo merwok@netwok.org added the comment:

    I think most of these are false positives (it’s fine if 10 different tests
    define a function f), so should not be changed just to appease a lint
    tool. Others are genuine!

    ----------

    Python tracker <report@bugs.python.org>
    <https://bugs.python.org/issue46198\>

    @JelleZijlstra
    Copy link
    Member

    New changeset 6c83c8e by Nikita Sobolev in branch 'main':
    bpo-46198: rename duplicate tests and remove unused code (GH-30297)
    6c83c8e

    @JelleZijlstra
    Copy link
    Member

    New changeset f7f7838 by Jelle Zijlstra in branch '3.9':
    [3.9] bpo-46198: rename duplicate tests and remove unused code (GH-30297) (GH-31797)
    f7f7838

    @JelleZijlstra
    Copy link
    Member

    New changeset 4052dd2 by Alex Waygood in branch 'main':
    bpo-46198: Fix test_asyncio.test_sslproto (GH-31801)
    4052dd2

    @JelleZijlstra
    Copy link
    Member

    New changeset 4199b7f by Jelle Zijlstra in branch '3.10':
    [3.10] bpo-46198: rename duplicate tests and remove unused code (GH-30297) (GH-31796)
    4199b7f

    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Assignees
    No one assigned
    Labels
    3.9 only security fixes 3.10 only security fixes 3.11 only security fixes tests Tests in the Lib/test dir type-bug An unexpected behavior, bug, or error
    Projects
    None yet
    Milestone
    No milestone
    Development

    No branches or pull requests
    4 participants
    @merwok @JelleZijlstra @sobolevn @AlexWaygood