[issue20547] Use specific asserts in bigmem tests

[Serhiy Storchaka]

Changes by Serhiy Storchaka :


--
resolution:  -> rejected
stage: patch review -> resolved
status: open -> closed
Added file: http://bugs.python.org/file46778/test_bigmem_asserts_3.patch

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue20547] Use specific asserts in bigmem tests

[Serhiy Storchaka]

Changes by Serhiy Storchaka :


--
pull_requests: +696

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue20547] Use specific asserts in bigmem tests

[Serhiy Storchaka]

Serhiy Storchaka added the comment:

 Additionally, the more specific tests introduce some additional opacity
 that is harmful for knowing what methods and operators are specifically
 used internally in test method.  For end users of Python, they don't have
 to worry much about this, but we as developers of core types really care
 whether self.assertLessThan(x, y) really does x  y, or x.__lt__(y), or
 not y = x, etc.

If the test explicitly designed to test relation or boolean operations, I 
leave it as is. I try to be very careful.

After more careful reviewing this patch, I have found than some tests 
shouldn't be changed, because they produce utterly large error message in case 
of a failure (even if resulted message is truncated, as in case of 
assertEqual, large intermediate strings are created). Here is corrected patch.

--
Added file: http://bugs.python.org/file34079/test_bigmem_asserts_2.patch

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue20547
___diff -r 0f357cf011d3 Lib/test/test_bigmem.py
--- a/Lib/test/test_bigmem.py   Thu Feb 13 10:49:30 2014 +0200
+++ b/Lib/test/test_bigmem.py   Fri Feb 14 13:47:49 2014 +0200
@@ -866,7 +866,7 @@
 h1 = hash(t1)
 del t1
 t2 = (0,) * (size + 1)
-self.assertFalse(h1 == hash(t2))
+self.assertNotEqual(h1, hash(t2))
 
 @bigmemtest(size=_2G + 10, memuse=8)
 def test_index_and_slice(self, size):
@@ -992,8 +992,8 @@
 l = [sys.stdout] * size
 l += l
 self.assertEqual(len(l), size * 2)
-self.assertTrue(l[0] is l[-1])
-self.assertTrue(l[size - 1] is l[size + 1])
+self.assertIs(l[0], l[-1])
+self.assertIs(l[size - 1], l[size + 1])
 
 @bigmemtest(size=_2G // 2 + 2, memuse=24)
 def test_inplace_concat_small(self, size):
@@ -1092,13 +1092,13 @@
 l = ['']
 l *= size
 self.assertEqual(len(l), size)
-self.assertTrue(l[0] is l[-1])
+self.assertIs(l[0], l[-1])
 del l
 
 l = [''] * size
 l *= 2
 self.assertEqual(len(l), size * 2)
-self.assertTrue(l[size - 1] is l[-1])
+self.assertIs(l[size - 1], l[-1])
 
 @bigmemtest(size=_2G // 2 + 2, memuse=16)
 def test_inplace_repeat_small(self, size):
@@ -1132,8 +1132,8 @@
 l = [object()] * size
 l.append(object())
 self.assertEqual(len(l), size+1)
-self.assertTrue(l[-3] is l[-2])
-self.assertFalse(l[-2] is l[-1])
+self.assertIs(l[-3], l[-2])
+self.assertIsNot(l[-2], l[-1])
 
 @bigmemtest(size=_2G // 5 + 2, memuse=8 * 5)
 def test_count(self, size):
@@ -1145,8 +1145,8 @@
 l = [object] * size
 l.extend(l)
 self.assertEqual(len(l), size * 2)
-self.assertTrue(l[0] is l[-1])
-self.assertTrue(l[size - 1] is l[size + 1])
+self.assertIs(l[0], l[-1])
+self.assertIs(l[size - 1], l[size + 1])
 
 @bigmemtest(size=_2G // 2 + 2, memuse=16)
 def test_extend_small(self, size):
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue20547] Use specific asserts in bigmem tests

[Raymond Hettinger]

Raymond Hettinger added the comment:

 I try to be very careful.

It's great to be careful, but it is a smarter move to not change the test suite 
at all (except in cases where there is a known problem).  

There is almost zero benefit to the patch (i.e. the tests are currently not 
failing and have been stable for a long time).  You risk making a mistake, 
leaving an undetected hole in the tests, and increasing chances of future 
regression during normal maintenance.

Further, this patch churns the code away from what the original test case 
authors were thinking about when they were deeply engaged with the code.  That 
is why Guido wants only holistic refactorings.

Another issue is that if you make the extensive test suite changes, there is no 
case for backporting those changes (especially for Python 2.7).  But then, if 
the versions don't line up, it makes cross-version maintenance more difficult 
if an actual bug arises (i.e. the patches won't apply cleanly).

In short, I recommend that you please don't do this.  It isn't good for the 
project.   Thank you.

--

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue20547
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue20547] Use specific asserts in bigmem tests

[Terry J. Reedy]

Terry J. Reedy added the comment:

Patch2 looks correct, given that the replacement is the right thing to do. The 
assertTrue error message 'False is not true' is definitely not helpful. But I 
see Raymond's point. I think these patches should be discussed on pydev.

--
nosy: +terry.reedy

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue20547
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue20547] Use specific asserts in bigmem tests

[Ezio Melotti]

Ezio Melotti added the comment:

 For end users of Python, they don't have to worry much about this, but
 we as developers of core types really care whether self.assertLessThan(x, y)
 really does x  y, or x.__lt__(y), or not y = x, etc.

FWIW the assert methods should guarantee that the corresponding operator is 
used (e.g.  for assertLess), and I think this is already the case.

 After more careful reviewing this patch, I have found than some tests
 shouldn't be changed, because they produce utterly large error 
 message in case of a failure (even if resulted message is truncated, 
 as in case of assertEqual, large intermediate strings are created).

Some of these cases are already fixed, for others there are still open issues.  
If you find cases that are not tracked you should report them.

--
nosy: +ezio.melotti

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue20547
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue20547] Use specific asserts in bigmem tests

[Raymond Hettinger]

Raymond Hettinger added the comment:

These should be backported.

And it probably shouldn't be done at all unless there is an actual failure with 
an uninformative error message.  Otherwise, you're just destabilizing the test 
suite and creating unnecessary code churn.

In the case of the collections tests, I used test-driven-development for parts 
of it and am very confident in the test as they stand.  If you start  switching 
the test methods, I become less confident in those tests (i.e. I haven't seen 
the new ones fail in the absence of the code they were meant to test).

Additionally, the more specific tests introduce some additional opacity that 
is harmful for knowing what methods and operators are specifically used 
internally in test method.  For end users of Python, they don't have to worry 
much about this, but we as developers of core types really care whether 
self.assertLessThan(x, y) really does x  y, or x.__lt__(y), or not y = x, 
etc.

IOW, I am of the strong opinion that your patches are not a good idea.  The 
more specific tests can be used in new tests or in tests that are failing, 
but going back and making blanket sweeps of the test suite isn't a good 
practice.

Please lookup Guido's comments on holistic refactoring being preferred to 
these kind of sweeps.

--
nosy: +rhettinger

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue20547
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue20547] Use specific asserts in bigmem tests

[Serhiy Storchaka]

New submission from Serhiy Storchaka:

The proposed patch makes bigmem tests use more specific asserts. This will 
provide more useful failure report.

--
components: Tests
files: test_bigmem_asserts.patch
keywords: easy, patch
messages: 210544
nosy: serhiy.storchaka
priority: normal
severity: normal
stage: patch review
status: open
title: Use specific asserts in bigmem tests
type: enhancement
versions: Python 2.7, Python 3.3, Python 3.4
Added file: http://bugs.python.org/file33972/test_bigmem_asserts.patch

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue20547
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com