Julian Gindi added the comment:
Just wanted to see if there was anything else I needed to do to get this patch
rolling :)
--
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue19588
___
Zachary Ware added the comment:
Nope, your patch looks good, I just haven't gotten it committed yet :)
--
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue19588
___
Roundup Robot added the comment:
New changeset c8e138646be1 by Zachary Ware in branch '2.7':
Issue #19588: Fixed tests in test_random that were silently skipped most
http://hg.python.org/cpython/rev/c8e138646be1
New changeset c65882d79c5f by Zachary Ware in branch '3.3':
Issue #19588: Fixed
Zachary Ware added the comment:
Thanks for the patch, Julian!
--
assignee: - zach.ware
resolution: - fixed
stage: patch review - committed/rejected
status: open - closed
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue19588
Serhiy Storchaka added the comment:
Original test was added in issue812202.
--
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue19588
___
___
Zachary Ware added the comment:
The patch looks good to me, Julian. Have you signed a contributor agreement?
If you haven't done so yet and are planning on contributing anything more than
the most trivial of changes, you'll need to do so (see
http://www.python.org/psf/contrib/).
--
Julian Gindi added the comment:
Awesome! I signed the contributor agreement today via E-sign. I look forward
to making more significant contributions soon :)
--
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue19588
Zachary Ware added the comment:
Hmm, actually, I take it back...this is half of the needed patch :). There's
another test method of the same name in MersenneTwister_TestBasicOps, with the
same issue. But this half looks good! I'll leave a comment on Rietveld (which
will send you a 'review'
Tim Peters added the comment:
Yup, the patch is semantically correct. But I'd also swap the order of the
`start =` and `stop =` lines - *everyone* expects `start` to be set first ;-)
--
___
Python tracker rep...@bugs.python.org
Julian Gindi added the comment:
That makes perfect sense :) Here is an updated patch. I also made the change to
the other test of the same name in MersenneTwister_TestBasicOps
--
Added file: http://bugs.python.org/file32747/issue19588_v2.patch
___
Julian Gindi added the comment:
Maybe some documentation could help express the purpose of this test, but I was
able to make the changes mentioned below and the test seems to work better than
it used to. The test no longer returns if a value is 'skipped'. This is my
first attempt at a patch
Changes by Ezio Melotti ezio.melo...@gmail.com:
--
keywords: +easy
nosy: +ezio.melotti
stage: - needs patch
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue19588
___
New submission from Zachary Ware:
See: http://hg.python.org/cpython/file/87099/Lib/test/test_random.py#l241
This test (and its match in MersenneTwister_TestBasicOps) is nearly always
skipped by the 'if stop = start: return' check; in a test with adding
print('skipped', i) before the return
Tim Peters added the comment:
Nice catch! That's insane. `start` and `stop` should indeed be swapped, *and*
the `return` should be `continue`. I didn't write the test, but these things
are obvious to my eyeballs ;-)
--
nosy: +tim.peters
___
14 matches
Mail list logo
