unit test strategy

[Aaron Brady]

On Sunday, September 16, 2012 3:01:11 PM UTC-5, Steven D'Aprano wrote:
> On Sun, 16 Sep 2012 11:38:15 -0700, Aaron Brady wrote:
> > Here is an example of some repetitive code.
> > 
> > for view_meth in [ dict.items, dict.keys, dict.values ]:
> > 	dict0= dict( ( k, None ) for k in range( 10 ) ) 
> >       iter0= iter( view_meth( dict0 ) )
> > 	dict.__setitem__( dict0, 0, 1 )
> > 	next( iter0 )
> > 	dict.__setitem__( dict0, 10, 1 )
> > 	self.assertRaises( IterationError, next, iter0 )
> [...]
> 
> First off, if you have any wish for this to be accepted into the standard 
> library, I suggest you stick to PEP 8.

The code in the zip file on the other thread does conform to PEP 8.
 
> Secondly, this is test code. A bit of repetition is not to be concerned 
> about, clarity is far more important than "Don't Repeat Yourself". The 
> aim isn't to write the fastest, or most compact code, but to have good 
> test coverage with tests which are *obviously* correct (rather than test 
> code which has no obvious bugs, which is very different). If a test 
> fails, you should be able to identify quickly what failed without running 
> a debugger to identify what part of the code failed.
> 
> Thirdly, why are you writing dict.__setitem__( dict0, 0, 1 ) instead of 
> dict0[0] = 1 ?
> 
> 
> [...]
> > Specifically my questions are, is the code condensed beyond legibility? 
> 
> Yes.
> 
> 
> > Should 'chain' execute the test directly, or act as a metaprogram and
> > output the test code into a 2nd file, or both?
> 
> None of the above.
> 
> 
> > Should 'chain' create
> > the iterators in a dictionary, or in the function local variables
> > directly with 'exec'?
> 
> Heavens to Murgatroyd, you can't be serious.
> 
> 
> Here's my attempt at this. Note the features:
> 
> - meaningful names (if a bit long, but that's a hazard of tests)
> - distinct methods for each distinct test case
> - comments explaining what the test code does
> - use of subclassing
> 
> 
> # Untested
> class TestDictIteratorModificationDetection(unittest.TestCase):
[snip]
>     def testIteratorFailsAfterSet(self):
>         self.iterator_fails_after_modification(dict.__setitem__, 1, 1)
> 
>     def testIteratorFailsAfterDel(self):
>         self.iterator_fails_after_modification(dict.__delitem__, 1)
> 
>     def testIteratorFailsAfterUpdate(self):
>         self.iterator_fails_after_modification(dict.update, {5: 1})
> 
>     def testIteratorFailsAfterPop(self):
>         self.iterator_fails_after_modification(dict.pop, 4)
[snip]
> 
> I think I've got all the methods which can mutate a dictionary. If I 
> missed any, it's easy enough to add a new test method to the class.
[snip]

Well Mr. D'Aprano, I have some serious disagreements with the script you posted.

You did test all the mutating methods; there are 7; but I don't think it's enough.  You omitted the test case which revealed the bug originally or other pairs of operations; you didn't test on empty sets; you didn't test on "null" modifications; you didn't test on multiple iterators in any form; and you didn't test for memory leaks which is important with a dynamic structure.  A thorough test suite should contain tens if not hundreds of tests for this application.

Your script was very easy to understand.  However in the volume I'm advocating, something more concise would easier to understand, and D-R-Y becomes applicable again.  In a more concise form, the reader could browse what tests are executed, find a certain test or determine if it's omitted.

The "best of both" solution is a metaprogram, which is extremely brief, but does not run tests, and outputs a full test catalog instead, which is then itself scrutinized and run as the real test.  The test script would consequently be two files big, and be run in a specific order.  Not all tests can be condensed in a metaprogram; the script would contain large sections of literal code or it would appear in yet a 3rd file.

> Thirdly, why are you writing dict.__setitem__( dict0, 0, 1 ) instead of 
> dict0[0] = 1 ?

'__setitem__' can be passed to secondary functions whereas square brackets cannot.  The 2nd file, the output of the metaprogram, could contain either or both.  We could pass 'operator.setitem' as an alternative.

I realize I'm introducing yet a 3rd foreign concept with the patch.  If not enough readers approve of it I will have to abandon it, which would be a shame.

OTOH, I appreciate the fact you used my "for view in (dict.items, dict.keys, dict.values):" idea.  Also, what is your argument that an unstarted iterator should be exempt from invalidation when the set/dict is modified?  It is not obvious it should or shouldn't, similar to the behavior of modifying dict values but not keys, and I would side against the exemption.  (Perhaps we should raise that issue on the other thread.)