Test the 'bisect' and 'heapq' module Python code in Python 2.6.

[GoogleCodeExporter]

Measure the number of lines executed by the unit tests included with
Python 2.6 for the 'bisect' and 'heapq' modules, using figleaf or
coverage (see MeasuringCodeCoverage).  Add new tests that increase the
code coverage for this module to > 95%.  Submit a context diff (see
SubmittingPatches) and text or HTML files containing both the old and
new test coverage results.

Note that for both bisect and heapq, the C implementation is already
tested thoroughly; for example, see test/test_bisect.py in the Python
2.6 subversion archive.  A good approach would be to modify the bisect
and heapq modules so that you can import the Python code instead of
the C code, and make sure that all of the tests work properly.

Completion:

Attach the results of an 'svn diff' (context diff) against the Python
source tree to this task. Also include text containing the old and new
code coverage numbers and a brief description of the changes you've made.

Relevant wiki pages:

 - RecordingTestCoverage
 - HowToTest
 - RunningPythonTests
 - GettingAndCompilingPython

Task duration: please complete this task within 5 days (120 hours) of claiming 
it.

Original issue reported on code.google.com by the.good...@gmail.com on 26 Nov 2007 at 6:27

[GoogleCodeExporter]

I claim this task.

Original comment by carlosj....@gmail.com on 30 Nov 2007 at 8:10

[GoogleCodeExporter]

Original comment by georg.br...@gmail.com on 30 Nov 2007 at 8:13

• Changed state: Claimed
• Added labels: ClaimedBy-carlosj.hellin

[GoogleCodeExporter]

This task is due Wednesday, December 5, 2007 at 20:10:00 UTC.

Original comment by doug.hel...@gmail.com on 4 Dec 2007 at 5:18

• Added labels: Due-20071205

[GoogleCodeExporter]

Any progress so far?

Original comment by georg.br...@gmail.com on 4 Dec 2007 at 9:33

• Added labels: near-expiration

[GoogleCodeExporter]

Original comment by doug.hel...@gmail.com on 7 Dec 2007 at 1:24

• Added labels: Due-20071205.2010
• Removed labels: Due-20071205

[GoogleCodeExporter]

Okay, per Google rules you have a grace period of three days to at least 
present some
progress.

Original comment by georg.br...@gmail.com on 7 Dec 2007 at 8:58

• Added labels: Due-20071208.2010, expired
• Removed labels: Due-20071205.2010, near-expiration

[GoogleCodeExporter]

Carlos claimed a duplicated in our project, we reopened the ticket again 
(http://
code.google.com/p/google-highly-open-participation-moinmoin/issues/detail?id=7)

Original comment by webmas...@alexanderweb.de on 7 Dec 2007 at 4:59

[GoogleCodeExporter]

Is assumed that this task had expired. I didn't know that I have been given more
time, so I claim another task of another project.
I prefer continue with the task of MoinMoin that I'm going better.

Original comment by carlosj....@gmail.com on 7 Dec 2007 at 7:39

[GoogleCodeExporter]

OK, np.

Original comment by the.good...@gmail.com on 7 Dec 2007 at 7:41

• Changed state: Open
• Removed labels: ClaimedBy-carlosj.hellin, Due-20071208.2010, expired

[GoogleCodeExporter]

I claim this task.

Original comment by jdzolo...@gmail.com on 15 Dec 2007 at 5:53

[GoogleCodeExporter]

You're really a task-crunching machine :)

Due 12-20, 18:00.

Original comment by georg.br...@gmail.com on 15 Dec 2007 at 6:00

• Changed state: Claimed
• Added labels: ClaimedBy-jdzolonga, Due-20071220.1800

[GoogleCodeExporter]

I've simply commented out the parts that include the c implementation and these 
are
the results coverage.py produced when running the current test_suite:

[josip@myhost tests]$ coverage.py -r bisect.py heapq.py
Name     Stmts   Exec  Cover
----------------------------
bisect      34     34   100%
heapq      130    107    82%
----------------------------
TOTAL      164    141    85%

Note that 10 lines from heapq's name == __main__ part are not covered, making 
the
coverage of heapq up to 107/120 = 89.1%. I'm attaching the annotated files, and 
as
you can see the only lines that aren't executed are 207-220. They reason for 
that is
the first part of the condition - hasattr(iterable, '__len__') which is always 
going
to be evaulated to false because of the izip trickness (lines 348-356).

Original comment by jdzolo...@gmail.com on 15 Dec 2007 at 6:04

Attachments:

• bisect.py,cover
• heapq.py,cover

[GoogleCodeExporter]

Hm, I'm attaching them again since google doesn't play well with the commas in 
the
filenames.

Original comment by jdzolo...@gmail.com on 15 Dec 2007 at 6:06

Attachments:

• heapq.cover.py
• bisect.cover.py

[GoogleCodeExporter]

Do you have any suggestions about how to request only the Python implementation 
on
import?

Original comment by the.good...@gmail.com on 15 Dec 2007 at 10:10

[GoogleCodeExporter]

At the moment I can't think of a clever solution. What should I do in order to
complete this task?

Original comment by jdzolo...@gmail.com on 16 Dec 2007 at 12:57

[GoogleCodeExporter]

This is an interesting problem...

One of the reasons to have automated tests is to prevent bitrot: code that is 
rarely
or never executed tends to decay over time. I'm worried about that happening to 
the
code in bisect and heapq.

I played around with sys.modules a bit; here's some code that, suitably 
adapted, can
stop _bisect and _heapq from being imported in test code.  Can you put this 
into the
test_ modules and verify that the Python code is properly executed?

Brett, Georg -- is this sort of thing OK to put into core test code?  It 
shouldn't
have any side effects outside of the specific test, AFAIK, but Brett might know
better, being the import genie.

from tst2 import f
f()

import sys
sys.modules['tst2'] = None

try:
   from tst2 import f
except ImportError:
   print 'cannot import, but thats ok'

del sys.modules['tst2']

from tst2 import f
f()

Original comment by the.good...@gmail.com on 16 Dec 2007 at 8:56

[GoogleCodeExporter]

Oh, and perhaps I don't understand your comment about the nsmallest stuff, but 
the
code doesn't seem untestable, just untested.  (You can always run '_nsmallest'
directly, yes?)

Also note that the import hackery, above, should be used to test *both* the C 
and
Python implementations when possible.

Original comment by the.good...@gmail.com on 16 Dec 2007 at 9:02

[GoogleCodeExporter]

To start, Alexandre for GSoC had to work on this issue; see
http://svn.python.org/view/python/branches/cpy_merge/Lib/test/test_memoryio.py?r
ev=56445&sortby=rev&sortdir=down&view=markup
for his solution.

Second, if you are going to prevent imports, you can do it that way, but do 
realize
that in the case of packages, None has a special meaning when set in 
sys.modules (at
least in 2.x; this should go away in 3.0).  It would be better to use something 
like 42.

Third, you will have to be VERY careful not to leave any junk in sys.modules.  
This
is the kind of thing that leads to really weird errors in other tests or that
prevents a test from being re-entrant (which is needed when doing refleak 
tests).

Fourth, realize that extension modules are typically not designed to be 
reloaded. 
That means if you pull it out you need to store it away and put it back later.  
Don't
just delete it for future imports to reload it.

Scared yet?  =)

But personally I am okay with the approach.  But I would first look at 
Alexandre's as
his doesn't require mucking around in sys.modules.

Original comment by bcannon on 16 Dec 2007 at 9:42

[GoogleCodeExporter]

And in case my last comment was not very clear, I just did a blog post on this 
issue:
http://sayspy.blogspot.com/2007/12/what-is-best-way-to-test-both-python.html .

Original comment by bcannon on 16 Dec 2007 at 10:47

[GoogleCodeExporter]

Brett, thanks for the advice. I'll send the diff tomorrow since I'm too tired 
now.

Original comment by jdzolo...@gmail.com on 16 Dec 2007 at 10:51

[GoogleCodeExporter]

I can't get it running, it ends up with some errors. Can someone please tell me 
what
I'm doing wrong? The diff attached is generated under /trunk

Original comment by jdzolo...@gmail.com on 17 Dec 2007 at 6:32

Attachments:

• heapq.diff

[GoogleCodeExporter]

Yeah, having itertools.tee get set to None is really weird.  If you hold on to 
a reference to the original instance of 
heapq then the failures you are getting look like they are bugs in the Python 
code.

I will keep looking.

Original comment by bcannon on 17 Dec 2007 at 7:34

[GoogleCodeExporter]

Found the problem. You have a typo in your globals statement; add the missing 
'l' in 'smallest'.  The problem 
was that since you were not resetting the function you were still using the old 
function whose globals were 
cleared when the module was deleted (which includes setting all globals in the 
module to None).

Original comment by bcannon on 17 Dec 2007 at 8:11

[GoogleCodeExporter]

Thanks bcannon, so after all it was a small typo :( I'm attaching the new diff.

Original comment by jdzolo...@gmail.com on 17 Dec 2007 at 10:29

Attachments:

• bisectheapq.diff

[GoogleCodeExporter]

I guess there's no way to get rid of the SyntaxWarnings, eh?  Ah well, that's 
fine.
Thanks for sticking with; this may seem like a small brick in the wall, but 
getting
good test coverage is a minimum for good testing!  Thanks!

Original comment by the.good...@gmail.com on 19 Dec 2007 at 6:13

• Changed state: Completed
• Added labels: NeedsCommit

[GoogleCodeExporter]

My apologies if I'm not reading the conversation correctly.  Instead of 
changing the module to optionally allow/disallow importing 
replacement functions from the C extension, what about giving all of the 
replaced Python functions a leading "_" prefix as well as the 
normal name.  The C extensions replaces the normal name but the prefixed 
version will also be present.  Then modify the test code so it 
knows how to test either form of the name.

For example, the testing code could do

    class FakeModuleForTesting(object):
        bisect = staticmethod(bisect._bisect)
        insort_left = staticmethod(bisect._insort_left)

and pass around either bisect (the module) or this FakeModuleForTesting to each 
of the testing functions.

Original comment by andrewda...@gmail.com on 19 Dec 2007 at 11:14

[GoogleCodeExporter]

I didn't feel comfortable specifying modifications to the core Lib/ code, which 
can
have unforeseen side effects.  I agree that this is a good solution but it is 
easier
to change tests than it is to change Lib code ;).

Original comment by the.good...@gmail.com on 19 Dec 2007 at 11:24

[GoogleCodeExporter]

The evil solution (meaning I'm not serious, I think), is to replace the system 
__import__ with something which 
intercepts the "import _bisect", grabs the already defined Python functions and 
stores them somewhere safe, 
then lets the existing import go to completion.  This won't work if any other 
test causes an import of bisect 
before the __import__ replacement, so no, I'm not serious.

Another solution is to punt - write the patch with changes and submit it to the 
tracker, and let others help 
with the decision.

Plus, the idea of unit tests is to help let people change the API like this.

Original comment by andrewda...@gmail.com on 20 Dec 2007 at 3:27

[GoogleCodeExporter]

That's what I tried first (the __import__ hack), but it turned out to be very
troublesome :)

Original comment by jdzolo...@gmail.com on 20 Dec 2007 at 5:44

[GoogleCodeExporter]

Committed r61021 -- it's a bit different from your solution, avoiding the global
statement, but the sys.modules hackery is still there :)

Original comment by georg.br...@gmail.com on 23 Feb 2008 at 10:37

• Removed labels: ClaimedBy-jdzolonga, NeedsCommit