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 ?


> >> > I don't really like addTestFlat - it doesn't encapsulate the different
> >> > behaviour enough. adsorbSuite did. Also please put Optimise back as the
> >> > spelling. Both are clear, but Optimise is better English.
> >> >
> >
> >> Regarding addTestFlat vs adsorbSuite, I don't see how absorbSuite
> >> better describes the different behaviour. If I did, I probably
> >> wouldn't have changed the name :)
> >
> > I am not enamoured of adsorbSuite; however addTestFlat is also not
> > really good - see the later discussion.
> >
> 
> OK. Let's thrash this one out via voce.


> * We want to support adding other "special" test suites to
>   OptimisingTestSuite. In particular, if we add a test suite that provides
>   services to its tests to an OptimisingTestSuite, adsorbSuite / addTestFlat
>   should not totally flatten the suite, but instead keep the suite, even if it
>   changes the structure of the tests.
> 
>   e.g. addTest maintains the structure:
>     >>> OptimisingTestSuite().addTest(SpecialSuite([a, b]))._tests
>     [SpecialSuite([a, b])]
> 
>   Currently, addTestFlat destroys all suite structure:
>     >>> OptimisingTestSuite().addTestFlat(SpecialSuite([a, b]))._tests
>     [a, b]
> 
>   Instead, it should preserve the suite while changing the structure:
>     >>> OptimisingTestSuite().addTestFlat(SpecialSuite([a, b]))._tests
>     [SpecialSuite(a), SpecialSuite(b)]
> 
>   All of the tests in each of the resulting new SpecialSuites should have
>   identical resource requirements so we can still optimise.
> 
> It's long, but I think it gets the point across.

Yes.

Now for the voice call and we're good I think.

-Rob
-- 
GPG key available at: <http://www.robertcollins.net/keys.txt>.