Support parallel test execution in the JUnit5 runner

[fmeum]

Before this change, BazelJUnitOutputListener wasn't thread-safe, resulting in exceptions such as the following:

java.util.ConcurrentModificationException
	at java.base/java.util.LinkedList$LLSpliterator.tryAdvance(LinkedList.java:1258)
	at java.base/java.util.stream.Streams$ConcatSpliterator.tryAdvance(Streams.java:720)
	at java.base/java.util.stream.ReferencePipeline.forEachWithCancel(ReferencePipeline.java:129)
	at java.base/java.util.stream.AbstractPipeline.copyIntoWithCancel(AbstractPipeline.java:527)
	at java.base/java.util.stream.AbstractPipeline.copyInto(AbstractPipeline.java:513)
	at java.base/java.util.stream.AbstractPipeline.wrapAndCopyInto(AbstractPipeline.java:499)
	at java.base/java.util.stream.FindOps$FindOp.evaluateSequential(FindOps.java:150)
	at java.base/java.util.stream.AbstractPipeline.evaluate(AbstractPipeline.java:234)
	at java.base/java.util.stream.ReferencePipeline.findFirst(ReferencePipeline.java:647)
	at com.github.bazel_contrib.contrib_rules_jvm.junit5.TestSuiteResult.get(TestSuiteResult.java:95)
	at com.github.bazel_contrib.contrib_rules_jvm.junit5.TestSuiteResult.lambda$get$3(TestSuiteResult.java:92)
        ...

[fmeum]

@shs96c

[shs96c]

I'm surprised that the streams should throw an exception, but if the test passes, I'm going to assume the solution works.

The exception given in the PR description suggests that we could make a copy of the results and nestedSuites collections in TestSuiteResult before iterating over them. Did you consider making TestSuiteResult.get synchronized instead?

[fmeum]

The exception given in the PR description suggests that we could make a copy of the results and nestedSuites collections in TestSuiteResult before iterating over them. Did you consider making TestSuiteResult.get synchronized instead?

I don't think that would work as making a copy would also require iterating over the lists while addDynamicTest may concurrently modify them.

We could make TestSuiteResult#get and TestSuiteResult#add synchronized instead, which gets the test to pass. It wouldn't be as obviously correct as the approach in this PR, but may be more efficient. What do you prefer?

[shs96c]

Working code is what I prefer. We can always reorganise things later if we find that this introduces a bottleneck or reduces the performance of people's tests.

[shs96c]

Hmm... thinking about this, would a ReadWriteLock around access to the list of tests give us the best of both worlds?

[fmeum]

Hmm... thinking about this, would a ReadWriteLock around access to the list of tests give us the best of both worlds?

That's a good idea, did that instead.

[shs96c]

LGTM. Thank you for the last change