Another smaller issue is that gotest needs to forward the flag.
-rob
Many tests won't work if run in parallel.
(Many will, too.) The presence of a --parallel flag means scripts and so on will want to turn it on, and tests must be able to protect themselves from erroneous runs. That mechanism is missing here.
Another smaller issue is that gotest needs to forward the flag.
>
> Many tests won't work if run in parallel.
> That is true, and it's the reason parallel is false by default.
That's not good enough. Tests designed without it may incorrectly fail if -parallel is on.
>
> (Many will, too.) The presence of a --parallel flag means scripts and so on will want to turn it on, and tests must be able to protect themselves from erroneous runs. That mechanism is missing here.
> Do you mean something as grouping tests together to be run in serial?
> A simpler option is to add testing.IsParallel and tests can "bail out" if it's true.
The tests should run if the flag is set, but need a way to say "not in parallel". Depending on the granularity, it could be as simple to use as a global of some form in the test that turns off parallelism for that package's tests. Having a test just bail will not run it at all.
> Another smaller issue is that gotest needs to forward the flag.
>
> If you mean pass it to testing flags, the patch implement that in
> + &flagSpec{name: "parallel", isBool: true, passToTest: true},
>
Oh yes, there it is. Good.
I'm not a fan of this change. It puts unexpected constraints on the individual tests and we need to think hard about the implications and controls.
-rob
A different approach is to have a global variable in the testing package,
var Parallelize bool
and then if a test file knows that it is safe to have test cases run
in parallel it can write
func init() { testing.Parallelize = true }
and then gotest can just do the right thing without needing a flag.
This, then, has the right behaviour for existing tests.
Dave.
I've been thinking about putting this in gocheck for a while too, and
how to solve that problem.
I suggest whitelisting individual tests that are knowingly able to run
in parallel, and the easiest way to do this is to name them
differently. A nice way to do this would be to have a different prefix
for tests that can be parallelized (e.g. GoTest rather than Test). We
also don't need command line parameters, IMO. If a test can run in
parallel, the runner decides when to do it. If it can't, it shouldn't
ever be run.
--
Gustavo Niemeyer
http://niemeyer.net
http://niemeyer.net/plus
http://niemeyer.net/twitter
http://niemeyer.net/blog
-- I never filed a patent.
> I prefer something like this. I have many tests that will fail if parallel
> is turned on (because they muck with global state). It is hard to imagine
> having a knob that will not be fiddled with by others in a larger
> organization.
My suggestion covers that. The variable's value would not be persisted
between invocations of gotest, so a test (or collection of test files
in one package) need only be concerned by its own parallelizability.
Dave.
func init() { testing.Parallelize = true }
Too complicated and fragile.
-rob
+1 (that was my thought too).
except that i couldn't think of an appropriate name, and i don't
like GoTestXXX.
we're trying to indicate that the test is side-effect-free,
but i don't think something like PureTestXXX works.
PTestXXX ? (P standing for either "pure" or "parallel")
As an exception, this can be accommodated having a
t.Parallel()
method that a test can call to declare that it is okay
to run in parallel with other tests.
The implementation would be to run each test in sequence,
and then the t.Parallel method would let the main loop
continue on to run other tests while this one ran.
It is unclear to me whether you want t.Parallel to mean
'this can run in parallel with any other test' or
'this can run in parallel with any other test that called t.Parallel'.
Either would be straightforward to implement.
Russ
that sounds good to me.
i'm not sure the second option would be easy to implement though,
because if a test marked parallel is running, you'd have
no way of knowing if another test would be marked parallel
until you run it, but by then it's too late.
therefore i vote for the first - a test that calls t.Parallel should
be ok to run concurrently with any other test.
It is easy. You leave that goroutine
hanging during the call to t.Parallel,
run all the other tests (hanging any
others that call t.Parallel too) and then
once all the non-parallel stuff is done
you start activating the delayed parallel ones.
Russ
i still think that on balance i'd vote for the first option.
then marking a test as parallel essentially means "don't
hold up the testing process because of this test", not
"run this in parallel with other tests marked parallel".
it makes it useful even if there's only one test marked
parallel.
only if it doesn't break all the other tests.
simple rule:
if it might break other tests, then don't mark
it as parallel.
we guarantee that all tests will be called in sequence,
so if there's some shared state that needs to be set up,
then it can be done before calling t.Parallel.
these kind of tests are fairly rare though. perhaps a method could be defined
to signify this kind of test. t.Sequential, or t.Exclusive maybe?
On 15 September 2011 17:17, Russ Cox <r...@golang.org> wrote:
if two tests are slow, then you can t.Parallel them if it bothers you.
if one test is slow, well, one test is slow.
russ
individually the non-parallel tests might be fast, but all the non-parallel
tests added together might amount to something.
i don't mind too much though.
I'd prefer to have a different prefix like GoTest for the parallel
tests. This enables the runner to more easily decide how to run the
parallel tests before actually running them, and it's also pretty easy
to identify externally when a test has been marked as parallel.
I am strongly opposed to this. It breaks the simple rule
about how you find tests, and it sets a bad precedent
for future extensions.
Russ
I'm not strongly in favor of it, so won't argue too much. :-)
It just feels like a unique and relevant characteristic to the success
of the test that is worth the little cost of the || in the test
collection logic. I can't foresee many other reasons to add a prefix,
and the precedent has already been settled by benchmarks.
That's the end of my argument, though.
A benchmark is not a kind of test.
<bikeshed comment here>
sure
on the other hand (thinking a bit more) there are tests that
really want to be run without other concurrent tests - i'm thinking of
time sensitive tests where another busy process might interfere.
However if we run tests in parallel, we might have a race condition with GOMAXPROCS.
I see 3 solutions:
1. Have t.Parallel check if len(cpuList) > 1 and exit with error
2. Have t.Parallel check if len(cpuList) > 1 and if so warn and run the test in serial
3. Have "for _, procs := range cpuList {" be the outer loop. This means tests will run in parallel only in the same GOMAXPROCS.
My vote goes to #3
In favour of t.Parallel(),
- You get the fidelity of selecting specific test functions for
being run in parallel.
- It's always safe to add a test function that must be run serially.
In favour of testing.Parallelize,
- Your test code doesn't need a per-function notation.
- Adding a test later will pick up the existing parallelism.
- It's much easier to understand the behaviour.
I see the CL out for t.Parallel() already, but I wonder whether we've
settled the decision yet.
Dave.
Russ