GeezerVison is showing github.com — list duplicate test names with patchcheck · Issue #60283 · python/cpython
list duplicate test names with patchcheck #60283Description
| BPO |
16079 |
| Nosy |
@rhettinger, @gpshead, @vstinner, @rbtcollins, @ned-deily, @ezio-melotti, @merwok, @cjerdonek, @xdegaye, @ambv, @miss-islington |
| PRs |
bpo-16079: fix duplicate test method name in test_gzip. #12827[3.7] bpo-16079: fix duplicate test method name in test_gzip. (GH-12827) #12828bpo-16079: Add the duplicate_meth_defs.py tool as a pre-commit check on Travis #12886[2.7] bpo-16079: Add the duplicate_meth_defs.py tool (GH-12886) #12940[3.7] bpo-16079: Add the duplicate_meth_defs.py tool as a pre-commit check on Travis (GH-12886) #12950 |
| Files |
duplicate_test_names.patchduplicate_code_names.pystd_lib_duplicates.txtduplicate_code_names_2.pyignored_duplicatesduplicate_code_names_3.py |
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 = None
created_at = <Date 2012-09-28.10:15:37.662>
labels = ['easy', 'tests', 'type-bug', 'library', '3.9']
title = 'list duplicate test names with patchcheck'
updated_at = <Date 2019-11-27.18:45:45.910>
user = 'https://github.com/xdegaye' bugs.python.org fields:
activity = <Date 2019-11-27.18:45:45.910>
actor = 'brett.cannon'
assignee = 'none'
closed = False
closed_date = None
closer = None
components = ['Library (Lib)', 'Tests']
creation = <Date 2012-09-28.10:15:37.662>
creator = 'xdegaye'
dependencies = []
files = ['27326', '27375', '27376', '31891', '31892', '48265']
hgrepos = []
issue_num = 16079
keywords = ['patch', 'easy']
message_count = 31.0
messages = ['171428', '171499', '171505', '171509', '171511', '171512', '171513', '171519', '171536', '171544', '171548', '171558', '171559', '171576', '171734', '198525', '198586', '229625', '340168', '340219', '340221', '340252', '340286', '340295', '340323', '340584', '340585', '340687', '340785', '340786', '351668']
nosy_count = 11.0
nosy_names = ['rhettinger', 'gregory.p.smith', 'vstinner', 'rbcollins', 'ned.deily', 'ezio.melotti', 'eric.araujo', 'chris.jerdonek', 'xdegaye', 'lukasz.langa', 'miss-islington']
pr_nums = ['12827', '12828', '12886', '12940', '12950']
priority = 'normal'
resolution = None
stage = 'patch review'
status = 'open'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue16079'
versions = ['Python 3.9'] Linked PRs
Activityxdegaye commented on Sep 28, 2012 MannequinAuthor More actions See also bpo-16056 for the current list of duplicate test names in
the std lib.
The attached patch improves patchcheck.py to list duplicate test
names when running 'make patchcheck'. This patch to the default
branch can also be applied asis to the 2.7 branch.
An example of patchcheck output with the patch applied:
==================
$ make patchcheck
./python ./Tools/scripts/patchcheck.py
Getting the list of files that have been added/changed ... 1 file
Fixing whitespace ... 0 files
Fixing C file whitespace ... 0 files
Fixing docs whitespace ... 0 files
Duplicate test names ... 1 test:
TestErrorHandling.test_get_only in file Lib/test/test_heapq.py
Docs modified ... NO
Misc/ACKS updated ... NO
Misc/NEWS updated ... NO
configure regenerated ... not needed
pyconfig.h.in regenerated ... not needed Did you run the test suite?
================== merwok commented on Sep 28, 2012 Nice feature to do without adding a dependency on a lint tool! cjerdonek commented on Sep 28, 2012 I would like to see this written in a way that would let one run it globally or on a single file independent of a patch (e.g. an independent script from which patchcheck could import certain functions). Or is that what you explicitly didn't want Éric? :)
This would let one do a report or global check as was done for bpo-16056. It would also make it a bit easier to check manually that the script is checking for duplicates correctly.
Also, some suggestions:
+def testmethod_names(code, name=[]):
It might be clearer to use the name=None form.
+ test_files = [fn for fn in python_files if
+ fn.startswith(os.path.join('Lib', 'test'))]
Are you getting the test files in test/ subdirectories of subpackages? I think checking that the file name starts with "test_" might be sufficient to get all test files.
+ if name[-1].startswith('test_'):
I believe 'test' is the prefix that unittest uses. I'm pretty sure we have some tests that don't start with 'test_'. ezio-melotti commented on Sep 28, 2012
I would like to see this written in a way that would let one
run it globally or on a single file independent of a patch
+1
It can be added to Tools/scripts and imported by patchcheck.
I'm pretty sure we have some tests that don't start with 'test_'.
IIRC those are just test helpers that are not executed directly.
OTOH I don't see why looking for test_*, every py file might contain duplicate names so they should all be checked. cjerdonek commented on Sep 28, 2012 merwok commented on Sep 28, 2012 sqlite3 tests use CheckThing style (urgh). Reacted by Erlend E. Aasland ezio-melotti commented on Sep 28, 2012
Here are a couple examples of test method names that don't begin with "test_":
I thought you were talking about test files. I still don't see why looking for test_* methods, every class might contain duplicate method names, so they should all be checked. cjerdonek commented on Sep 28, 2012
I thought you were talking about test files.
Oh, I see why you said that then. To find the test files themselves, this logic was used in the patch:
+ fn.startswith(os.path.join('Lib', 'test'))]
Regarding your question for the general case, I'm not sure if there is ever a use case for duplicate method names. Is there? xdegaye commented on Sep 28, 2012 MannequinAuthor More actions Note that using the module code object to find duplicates does not
allow for selecting among the different code types: function, nested
function, method or class.
Duplicates are extensively used within the std lib:
Running find_duplicate_test_names.py, the initial script from issue
16056, on the whole std lib instead of just Lib/test, after updating
the script to list all the duplicates (except <lambda>, <genexp>,
...) with:
if not name[-1].startswith('<'):
yield '.'.join(name)prints 347 (on a total of 1368 std lib .py files) duplicate
functions, methods or classes.
To eliminate module level functions (but not nested functions), the
script is run now with the following change:
if len(name) > 2 and not name[-1].startswith('<'):
yield '.'.join(name)and lists 188 duplicate nested functions, methods or classes. In
this list there are 131 duplicates in .py files located in the
subdirectory of a "test" directory. xdegaye commented on Sep 28, 2012 MannequinAuthor More actions Using the python class browser (pyclbr.py) in conjunction with the
search for duplicates in the module code object would allow to
restrict the listing of duplicates to functions and methods or even
just to methods (depending on the feature requirements), without
listing the duplicate classes and duplicate nested functions. With
an associated performance cost. ezio-melotti commented on Sep 28, 2012 It doesn't necessary have to be limited to methods, anything duplicate might turn out to be a bug. If the script doesn't mix scopes there shouldn't be too many false positives, and if they are it shouldn't be a big deal if they are reported on the changed file by make patchcheck.
I'm not sure if there is ever a use case for duplicate
method names. Is there?
Nothing that can't be done in a more elegant way afaict.
It might make sense for variables though, where you have e.g.:
foo = do_something(x)
foo = do_something_more(foo) xdegaye commented on Sep 29, 2012 MannequinAuthor More actions
I'm not sure if there is ever a use case for duplicate method
names. Is there?
property getter, setter, and deleter methods do have the same name. xdegaye commented on Sep 29, 2012 MannequinAuthor More actions
Here are a couple examples of test method names that don't begin
with "test_":
def testLoadTk(self):
def testLoadTkFailure(self):
Also Lib/test/test_smtplib.py test method names start with 'test'
instead of 'test_' although the 'Regression tests package for Python'
documentation states: "The test methods in the test module should
start with test_". erlend-aasland commented on Jun 27, 2023 merwok commented on Jun 27, 2023 Isn’t that repo for tooling and meta issues?
IMO this should stay in the CPython tracker for visibility. erlend-aasland commented on Jun 28, 2023
Isn’t that repo for tooling and meta issues?
Yes, and CI issues. Most relevant comments in this thread are about running these checks in CI. So it belongs in the core-workflow repo. merwok commented on Jun 28, 2023 Oh, in my opinion the tool / config should be available for devs to run locally first, then also run from CI!
That’s why I see this as a normal CPython ticket. erlend-aasland commented on Jun 28, 2023
MetadataMetadataDevelopmentNo branches or pull requestsIssue actions
|