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

On Mon, Sep 8, 2008 at 11:48 AM, Robert Collins
<email address hidden> wrote:
> On Sun, 2008-09-07 at 23:03 +0000, Jonathan Lange wrote:
>> On Mon, Sep 8, 2008 at 8:33 AM, Robert Collins
>> <email address hidden> wrote:
>> > On Sun, 2008-09-07 at 10:42 +0000, Jonathan Lange wrote:
>> >
>> >
>> >> > * Calling finishedWith on a dirty resource now cleans that resource.
>> >> > (Jonathan Lange)
>> >> >
>> >> > It already did that; I'm not sure why this is NEWS :). Specifically
>> >> > though, it actually resets it if its dirty and still in use.
>> >> >
>> >>
>> >> I see what happened here: I conflated two changes that were close
>> >> together in the logs.
>> >>
>> >> I've added this entry to Internals:
>> >> * If calling finishedWith on a TestResource reduces its usage count to
>> >> zero, then the TestResource considers itself clean, i.e. _dirty is set
>> >> to True. (Jonathan Lange)
>> >>
>> >> and have added this to a new bugfixes section:
>> >> * Calling getResource on a dirty resource now triggers a clean and re-make
>> >> of that resource. (Jonathan Lange)
>> >>
>> >> See revisions 56 and 57 for more information.
>> >
>> > This is the pending merge's diff:
>> > - def finishedWith(cls, resource):
>> > - cls._uses -= 1
>> > - if cls._uses == 0:
>> > - cls._cleanResource(resource)
>> > - cls._currentResource = None
>> > - elif cls._dirty:
>> > - cls._cleanResource(resource)
>> > - cls.setResource()
>> > - finishedWith = classmethod(finishedWith)
>> >
>> >
>> > So I don't see what's changed - the prior behaviour was:
>> > if it is no longer in use it is cleaned
>> > otherwise if it is dirty it is reset
>> >
>> > The new behaviour seems identical.
>> >
>>
>> Note that the finishedWith above doesn't change the value of
>> self._dirty when _uses is 0. In the new version, self._dirty is set to
>> False if _uses is 0. That is all.
>
> Ah yes. Ok, that certainly benefits by being documented. (Though the
> dirty flag is irrelevant for an unused resource :)). That should be one
> NEWS item though, not two ?
>

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.

jml