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

    * testresources now depends on pyunit3k. You can still use
testresources
      without using pyunit3k directly. (Jonathan Lange)

Thats not clear. Do you mean 'testresources needs pyunit3k to run its
own test suite'. Or 'if you use testresources you need to have pyunit3k
too.

    * 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 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.

One of the points of SampleTestResource was to demonstrate how to create
a resource for users of the library. We need to reintroduce that
demonstration. This could be a doc file, part of README, whatever.

With the change to instance methods, I think make() and clean() would be
better method names rather than makeResource() and cleanResource().

You've left the copyright munging change in README - it too needs
reverting.

+
+ XXX - that last sentence needs some unpacking, I think. -- jml
+

The sentence that needs unpacking- Its saying that if you add everything
to a single OTS, you get global optimisation. Or you could use several
smaller OTSs.

I think your changes should be (c) You, or alternatively, you can assign
to me. I don't have an opinion as to whats better.

Files you edit, you should assert (c) on by adding a line appropriately.

+* It should be possible to make copies of TestSuites in adsorbSuite,
keeping
+ the containing TestSuites but at a chosen granularity, so that all
tests in
+ the resulting suites have identical resource requirements and
allowing
+ optimisation to still occur.
+ XXX: Clarify with Robert.

Say that you have a similar suite to OptimisingTestSuite, which provides
a service to its contained tests. adsorbSuite should not flatten the
tests but instead keep that suite. (Another reason addTestFlat doesn't
fit that well as a name).

Concrete example

input = SpecialSuite(Test_WithResourceA, TestWithResourceB)

optimisedSuite.METHOD(input)

pp optimisedSuite._tests
[SpecialSuite([Test_WithResourceA]),
 SpecialSuite([Test_WithResourceB])]

The refactoring to 'self.switch' has a bug: an exception raised during
the switch will no longer leave the right resources setup on the OTS
object.

"p" before "r", or a line of VWS to separate stdlib and local imports.

+import random
+import pyunit3k
 import testresources
-import testresources.tests
+from testresources import split_by_resources

There doesn't seem to be a test that the sortTests() method is invoked
anymore.

Why the move of test_suite from the top of
lib/testresources/tests/test_FOO.py to the bottom? I consider it best
practice to have test_suite (or load_tests) at the top of test scripts,
to make them easy to find. Pls discuss.

The previous review was a conceptual one based on your mail, this was
based on the detailed code. With all of these points addressed I'll
merge it immediately.

-Rob