[MERGE] (correct one!) Create fork and reinvoke parallel testing support.

[John Arbash Meinel]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Robert Collins wrote:
> Please ignore the other, 1.4M bundle. It was a mistake :).
> 
> This is the polished parallel test support code I've been aiming at for
> a week or so now. I did a spike on it one evening and showed it to
> Vincent, who prompt fixed a few genuine test suite bugs, and also added
> (though in a different place :)) the code that is now in
> BZRTransformingResult.
> 
> Its not tested :(. I can but its possibly (probably :P) tricky to test
> well & cheaply.
> 
> This feature has a dependency on a specific branch of testtools:
> lp:~lifeless/testtools/ThreadsafeTestForwardingresult is needed to get
> the generic parallel testing support, and the feature also depends on
> subunit to do test remoting.
> 
> To use,
> bzr selftest --parallel=fork, or =subprocess. I have a plugin for ec2
> support - I'm not at all sure we want that in the core.
> 
> I'll obviously add a NEWS entry if the patch is acceptable, but I
> thought it was time to get it reviewed :).
> 

=== modified file 'bzrlib/tests/__init__.py'
- --- bzrlib/tests/__init__.py	2009-03-26 12:15:45 +0000
+++ bzrlib/tests/__init__.py	2009-03-27 09:31:43 +0000

...

                 run += 1
             actionTaken = "Listed"
         else:
- -            test.run(result)
+            try:
+                from testtools import ThreadsafeForwardingResult
+            except ImportError:
+                test.run(result)
+            else:
+                if type(result) == ThreadsafeForwardingResult:
+                    test.run(BZRTransformingResult(result))
+                else:
+                    test.run(result)
             run = result.testsRun

^- This looks like you are trying to do a try/import/except inside an
inner function. (namely, inside the result handler of the test suite.)

This seems a whole lot better factored out to a top level import test.
Going further, it seems this is a genuine case where "isinstance()"
would be better than "type() ==" (not to mention that it should be
"type() is ").

Otherwise, you can't ever subclass ThreadsafeForwardingResult to
customize it somehow.

...

+def local_concurrency():
+    try:
+        content = file('/proc/cpuinfo', 'rb').read()
+        concurrency = cpucount(content)
+    except Exception, e:
+        concurrency = 1
+    return concurrency
+

^- This is pretty ugly, and means that you can't run the test suite in
parallel on any platform other than one that supports /proc/cpuinfo. Why
not just let the user supply -j 2.

Especially since, just because I have 8 cores, maybe I want to keep 1
around so that I can actually have my editor stay responsive.

Certainly Windows doesn't have /proc/cpuinfo, does Mac? Aix? BSD? (They
might, I don't know.)

Autodetect can be nice, but having it be the only way (and perhaps many
times get it wrong), seems to indicate that having a user flag really
would be better.

You also went to all the effort to provide a "subprocess"
implementation, so that systems without "fork" could still have a
parallel test suite. It seems a bit silly to then force concurrency == 1
everywhere.

...

+    def _error_looks_like(self, prefix, err):
+        """Deserialize exception and returns the stringify value."""
+        import subunit
+        value = None
+        typ, exc, _ = err
+        if isinstance(exc, subunit.RemoteException):
+            # stringify the exception gives access to the remote traceback
+            # We search the last line for 'prefix'
+            lines = str(exc).split('\n')
+            if len(lines) > 1:
+                last = lines[-2] # -1 is empty, final \n
+            else:
+                last = lines[-1]
+            if last.startswith(prefix):
+                value = last[len(prefix):]
+        return value
+

^- This is also a bit ugly, though I understand why it is being done. (I
believe the intermediate layers don't understand KnownFailure and
UnavailableFeature, so they just get reported as generic failure and
errors, which then need to get translated again.) Are there plans to
ultimately improve this situation?

BB:comment

(I'm pretty sure Vila has approved, so I'm just adding my observations.)

John
=:->
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (Cygwin)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iEYEARECAAYFAknM6sAACgkQJdeBCYSNAAOKAwCfYey97nrZRYJnOz42iQf2gxUK
vFUAoM4q/0SdXRM4JnmZ6h9hJ1E+nrPR
=FcGl
-----END PGP SIGNATURE-----