testresources

Code review comment for lp:~jml/testresources/tests-meaning-cleanup

  1. tests-meaning-cleanup
  2. Merge into trunk
Revision history for this message
Jonathan Lange (jml) wrote :

On Mon, Sep 8, 2008 at 12:12 PM, Robert Collins
<email address hidden> wrote:
> On Mon, 2008-09-08 at 01:54 +0000, Jonathan Lange wrote:
>>
>> The second NEWS item is that if you do:
>> resource = resource_manager.getResource()
>> resource_manager.dirty(resource)
>> resource = resource_manager.getResource()
>>
>> Then the second invocation of getResource will trigger a clean. This
>> is separate setting self._dirty on unused resources.
>
> Uhm. Something must have changed under the hood - some calling pattern
> is different.
>
> Certainly either what I had was broken, or I'd expect similar things to
> preserve state appropriately as well.
>

I think the current trunk is broken: you'll notice that it doesn't
have any branch for 'if self._dirty'. Certainly, the 'getResource;
dirty; getResource' call pattern doesn't have any tests.

The test I added for this is 'testDirtyingResourceTriggersRemake'.
It's supplemented by testUsingTwiceMakesAndCleansTwice and
testDirtyingWhenUnused. If the diff doesn't make it clear, then we can
discuss it when we are renaming addTestFlat.

jml

tests-meaning-cleanup-rev-57.diff
1=== modified file 'lib/testresources/__init__.py'
2--- lib/testresources/__init__.py 2008-08-17 07:19:40 +0000
3+++ lib/testresources/__init__.py 2008-08-17 07:33:54 +0000
4@@ -190,6 +190,9 @@
5 """
6 if self._uses == 0:
7 self._setResource()
8+ elif self._dirty:
9+ self.cleanResource(self._currentResource)
10+ self._setResource()
11 self._uses += 1
12 return self._currentResource
13
14
15=== modified file 'lib/testresources/tests/test_test_resource.py'
16--- lib/testresources/tests/test_test_resource.py 2008-08-17 07:19:40 +0000
17+++ lib/testresources/tests/test_test_resource.py 2008-08-17 07:33:54 +0000
18@@ -126,6 +126,15 @@
19 resource_manager.finishedWith(resource)
20 self.assertEqual(1, resource_manager.cleans)
21
22+ def testUsingTwiceMakesAndCleansTwice(self):
23+ resource_manager = MockResource()
24+ resource = resource_manager.getResource()
25+ resource_manager.finishedWith(resource)
26+ resource = resource_manager.getResource()
27+ resource_manager.finishedWith(resource)
28+ self.assertEqual(2, resource_manager.makes)
29+ self.assertEqual(2, resource_manager.cleans)
30+
31 def testFinishedWithCallsCleanResourceOnceOnly(self):
32 resource_manager = MockResource()
33 resource = resource_manager.getResource()
34@@ -166,6 +175,25 @@
35 resource_manager.finishedWith(resource1)
36 self.assertEqual(2, resource_manager.cleans)
37
38+ def testDirtyingResourceTriggersRemake(self):
39+ resource_manager = MockResource()
40+ resource = resource_manager.getResource()
41+ self.assertEqual(1, resource_manager.makes)
42+ resource_manager.dirtied(resource)
43+ resource_manager.getResource()
44+ self.assertEqual(1, resource_manager.cleans)
45+ self.assertEqual(2, resource_manager.makes)
46+ self.assertEqual(False, resource_manager._dirty)
47+
48+ def testDirtyingWhenUnused(self):
49+ resource_manager = MockResource()
50+ resource = resource_manager.getResource()
51+ resource_manager.finishedWith(resource)
52+ resource_manager.dirtied(resource)
53+ self.assertEqual(1, resource_manager.makes)
54+ resource = resource_manager.getResource()
55+ self.assertEqual(2, resource_manager.makes)
56+
57
58 class TestSampleResource(pyunit3k.TestCase):
59

« Back to merge proposal