Suggestions for changes to resource interface

I'm not sure I follow the difference. self._dirty is a memo, but wouldn't it be trivial to make that a property for some resources?

In terms of making it nicer for reuse we could have a query method to be called, which can ask the resource.

The current TestResource code treats self._dirty as an attribute it can set, so turning it into a property is not much of a solution at present.

Furthermore, it only really addresses the "easier to check for dirtiness after the fact" resources rather than the "easier to reset than check if dirty" resources.

I guess trying to prototype this design might be the best next step to see what it looks like code wise.

The attached branch (lp:~jamesh/testresources/bug-308869) implements this API, while maintaining support for the existing dirtied() API.

Using the new API should be as simple as implementing make(), reset() and clean(), where reset() is called between tests that both use the resource.

So this seems to make all resources get make() called on them on every use. Which completely defeats the caching aspect as far I can tell.

I'm also a little confused by the difference between dirty and needsreset, because they seem the same to me.

I like the idea of documenting and clarifying the interface for resource implementors.

Currently its just make() and clean(). Can I accurately summarise this bug as
"need a way to reset rather than doing clean+make"

?

make() does not get called on every test: there is a test in the branch that shows the sequence is "make, reset, reset, ..., clean". There is a default implementation of reset() that provides the old behaviour: "call clean+make if the dirty flag is set".

The difference between dirty and needsreset is basically as follows:

_needsReset: will always be set to True when the resource is used by a test. The getResource() method will call reset() on such a resource.

_dirty: This one is controlled by the test or resource code as before. The default reset() implementation will do clean() followed by make() if this flag is set for compatibility with existing resources.

In retrospect, it might have been a bit cleaner to split this out into a subclass.

Well that makes the patch clearer. But it seems to me to boil down to 'use reset' rather than 'clean+make' when getResource needs to clean the resource.

Or perhaps I'm missing some additional semantics?

The additional semantic is that resources always need to be reset if they have been used: it isn't something the running test has a say in, as is the case for the dirty flag.

With that as a base, the old "clean on dirty" semantics can easily be built on top.

I've linked a follow on branch from yours that doesn't implement needsReset which I am still struggling with. It does however make reset() a public method.

The needsReset method I don't like because it doesn't feel different enough to dirtied(), and looks liable to misbehave. Specifically it seemed identical to it with the sole exception that ResourcedTestCase was going to call it, and having to call a function to make sure reset happens, while no different to dirtied(), means that there are two methods resource instances, or clients, need to call to make sure things work properly. I'd like to keep that problematic part of the interface as small as possible.

What I'm considering instead is to have an optional parameter to getResource which OptimisingTestResource could pass in, which allows a 'faster to reset than check' resource to:
 - override isDirty() as lamba: True
 - define reset()

and have reset called between test cases, and not otherwise.

All outstanding branches have landed. I know need to know (or be reminded) of what things are missing to meet your needs.

Closing as changes were made, and James hasn't listed anything not delivered.