classification
Title: duplicate test name in Lib/test/test_heapq.py
Type: behavior Stage: resolved
Components: Tests Versions: Python 3.8, Python 3.7
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: rhettinger Nosy List: berker.peksag, rhettinger, roippi, stutzbach, tim.peters, vajrasky, vstinner, xdegaye
Priority: normal Keywords: patch

Created on 2013-09-28 20:53 by xdegaye, last changed 2019-08-24 05:56 by rhettinger. This issue is now closed.

Files
File name Uploaded Description Edit
duplicate_test_name.patch xdegaye, 2013-09-28 20:53 review
fix_heapq_tests.patch roippi, 2014-09-22 02:49 review
Pull Requests
URL Status Linked Edit
PR 15442 merged rhettinger, 2019-08-24 02:54
PR 15447 merged miss-islington, 2019-08-24 05:31
Messages (13)
msg198550 - (view) Author: Xavier de Gaye (xdegaye) * (Python triager) Date: 2013-09-28 20:53
There are two test_get_only methods. The patch provides a partial
fix, but removes the following two lines from the first method as the
execution of these lines fails:

  for f in (self.module.nlargest, self.module.nsmallest):
      self.assertRaises(TypeError, f, 2, GetOnly())

because heapq.nlargest is stuck in an infinite loop when the sequence
does not have a length. This seems to be a bug in nlargest. See the
following test that runs with 100 % cpu usage.

$ ./python
Python 3.4.0a2+ (default:f6792f734fcc, Sep 28 2013, 17:09:46) 
[GCC 4.3.2] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> class GetOnly:
...     "Dummy sequence class defining __getitem__ but not __len__."
...     def __getitem__(self, ndx):
...         return 10
... 
>>> import heapq
>>> heapq.nlargest(2, GetOnly())
^CTraceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "Lib/heapq.py", line 455, in nlargest
    result = _nlargest(n, it)
  File "<stdin>", line 3, in __getitem__
KeyboardInterrupt
>>>
msg198635 - (view) Author: Tim Peters (tim.peters) * (Python committer) Date: 2013-09-29 19:40
Good catch!  Would like to hear from Raymond what the intent of these tests was - looks like "the real" test_get_only (which hasn't been executing) has multiple failures.
msg198963 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2013-10-04 20:40
@Xavier de Gaye: Nice, you found many similar issues. How did you find them? Would it be possible to automate the detection of duplicated test names?
msg198967 - (view) Author: Xavier de Gaye (xdegaye) * (Python triager) Date: 2013-10-04 21:21
It was proposed, in issue 16056, to enhance `make patchcheck` with the
detection of duplicate code names. This triggered the creation of issue 16079.
The script named duplicate_code_names_2.py, in issue 16079, listed about 20
duplicate names among all the non-nested functions, classes or methods in the
whole repository (default branch), mostly duplicated test names. I have
entered an issue for each one of these cases, they are listed in issue 16079.
Yes, this detection can be automated.
msg199057 - (view) Author: Vajrasky Kok (vajrasky) * Date: 2013-10-06 09:10
I have played around with this test.

The major issue (there are other issues as well but not so difficult) is whether nlargest and nsmallest should support iterator that could be endless iterator or reject it (by checking __len__ attribute) straight away.

Once we decide that, the fix should be not that hard.

For example, I want to get 2 largest numbers from fibonacci number series.

heapq.nlargest(2, fibonacci_iterator) => what is the answer for this?
Exception or let it be stuck forever.
msg227246 - (view) Author: Ben Roberts (roippi) * Date: 2014-09-22 02:07
> The major issue (there are other issues as well but not so difficult) is whether nlargest and nsmallest should support iterator that could be endless iterator or reject it (by checking __len__ attribute) straight away.

Well, failing with an exception isn't right.  I can imagine doing (and probably have done, at some point) nlargest on any number of generators, streams of data, etc.

So unless anyone can solve the halting problem (:-)) the behavior of nlargest is correct as possible; nlargest(2, GetOnly()) behaves just like list(GetOnly()) - they both hang forever.

The proper action here is just a fix in the tests: make GetOnly raise IndexError at some point so that it is a proper sequence.  (also rename the second test to get_cmp_only)
msg227248 - (view) Author: Ben Roberts (roippi) * Date: 2014-09-22 02:49
Attached patch fixes the tests.
msg228372 - (view) Author: Ben Roberts (roippi) * Date: 2014-10-03 20:37
Any thoughts/reviews on the patch?
msg228400 - (view) Author: Berker Peksag (berker.peksag) * (Python committer) Date: 2014-10-03 22:24
In the 2.7 branch, the test was removed in issue 7871 (changeset https://hg.python.org/cpython/rev/0130e574f5be).
msg228407 - (view) Author: Ben Roberts (roippi) * Date: 2014-10-03 22:51
That is one approach, of course, but imo pretty incomplete.  A test that is named "test_get_only" presumably would be expected to test __getitem__, not __cmp__.  Confusingly there is still a GetOnly item declared in the module, but it is not used.  Compare to test_len_only, which has a paired LenOnly class and test name.

Either GetOnly should be completely excised from the module and the test renamed to test_cmp_only (alternately test_cmp_err), or the GetOnly class should be fixed as in my patch.
msg350349 - (view) Author: Raymond Hettinger (rhettinger) * (Python committer) Date: 2019-08-24 05:31
New changeset 4101181fd87c2fab6456663d3c8cc99377cf0463 by Raymond Hettinger in branch 'master':
bpo-19119: Remove invalid test and rename a misnamed test (GH-15442)
https://github.com/python/cpython/commit/4101181fd87c2fab6456663d3c8cc99377cf0463
msg350350 - (view) Author: Raymond Hettinger (rhettinger) * (Python committer) Date: 2019-08-24 05:54
New changeset ef3ccd737020a0bb49ea0bfe48069f89bb5370a1 by Raymond Hettinger (Miss Islington (bot)) in branch '3.8':
bpo-19119: Remove invalid test and rename a misnamed test (GH-15442) (GH-15447)
https://github.com/python/cpython/commit/ef3ccd737020a0bb49ea0bfe48069f89bb5370a1
msg350351 - (view) Author: Raymond Hettinger (rhettinger) * (Python committer) Date: 2019-08-24 05:56
Removed the bogus test -- it was a cut-and-paste error from a scratch file.
History
Date User Action Args
2019-08-24 05:56:23rhettingersetstatus: open -> closed
resolution: fixed
messages: + msg350351

stage: patch review -> resolved
2019-08-24 05:54:10rhettingersetmessages: + msg350350
2019-08-24 05:31:39miss-islingtonsetpull_requests: + pull_request15139
2019-08-24 05:31:25rhettingersetmessages: + msg350349
2019-08-24 02:54:55rhettingersetpull_requests: + pull_request15135
2019-04-25 14:56:28xdegayesetversions: + Python 3.7
2019-04-24 15:26:27xdegayesetversions: + Python 3.8, - Python 3.4, Python 3.5
2016-04-24 20:31:24berker.peksaglinkissue26840 superseder
2014-10-03 22:51:02roippisetmessages: + msg228407
2014-10-03 22:24:56berker.peksagsetnosy: + berker.peksag
messages: + msg228400
components: + Tests, - Library (Lib)
2014-10-03 20:37:51roippisetmessages: + msg228372
2014-09-22 02:49:28roippisetfiles: + fix_heapq_tests.patch

messages: + msg227248
2014-09-22 02:07:04roippisetnosy: + roippi
messages: + msg227246
2014-09-21 08:05:04berker.peksagsetstage: patch review
versions: + Python 3.5, - Python 3.3
2013-10-06 09:10:07vajraskysetnosy: + vajrasky
messages: + msg199057
2013-10-04 21:21:34xdegayesetmessages: + msg198967
2013-10-04 20:40:54vstinnersetnosy: + vstinner
messages: + msg198963
2013-09-30 11:41:03vstinnersetversions: + Python 3.3
2013-09-30 03:28:12rhettingersetassignee: rhettinger
2013-09-29 19:40:55tim.peterssetnosy: + tim.peters
messages: + msg198635
2013-09-28 20:53:22xdegayecreate