Allow to run some tests isolated

[pbludov]

We have one test that changes the global state, for example

@Test
void localeTest() {
  Locale.setDefault(Locale.FRENCH);
  assertEquals("Bunjour", getMessage());
}

and hundreds of tests that reads the state:

@Test
void someTest() {
  assertEquals("Hello", getMessage());
}

After enabling parallel test execution our tests start to fail at random locations with messages like that:
[ERROR] message 0 expected:<...Annotation.java:17: [Annotation 'AnnotationAnnotation' have incorrect indentation level 2, expected level should be 0].> but was:<...Annotation.java:17: [Die Annotation 'AnnotationAnnotation' hat eine unerwartete Einrückungstiefe von 2 (erwartet: 0)].>

@Execution(ExecutionMode.SAME_THREAD) for the test localeTest won't help us, since the all other tests are able to run concurrent with every other test and with localeTest too.

This can be solved with @ResourceLock:

@Test
@Execution(ExecutionMode.SAME_THREAD)
@ResourceLock(value="global", mode=READ_WRITE)
void localeTest() {
  Locale.setDefault(Locale.FRENCH);
  assertEquals("Bunjour", getMessage());
}

@Test
@Execution(ExecutionMode.CONCURRENT)
@ResourceLock(value="global", mode=READ)
void test1() {
}

// ...

@Test
@Execution(ExecutionMode.CONCURRENT)
@ResourceLock(value="global", mode=READ)
void test100500() {
}

but this is error prone, since adding a new Test without this annotation will produce a flaky tests which may fail.

It will be much better to mark some tests as "incompatible with parallel execution at all":

@Test
@Execution(ExecutionMode.ALONE) // no other this will be executed in parallel 
void localeTest() {
 Locale.setDefault(Locale.FRENCH);
 assertEquals("Bunjour", getMessage());
}

As a workaround, it is possible to turn such a test into Junit 4 test, which will be executed in other universe, isolated from all other.

[marcphilipp]

Reminds me of the discussion we had in #1634 (comment) and the following comments.

Do you have to set the Locale globally instead of passing it as a parameter to your code under test?

[pbludov]

Do you have to set the Locale globally instead of passing it as a parameter to your code under test?

It is actually stored in a static variable. Yes, the code can be modified to solve this particular issue. But we have many similar cases, for example tests for the code that uses a class loader.

Reminds me of the discussion we had in #1634 (comment) and the following comments.

This is exactly what we are looking for. A way to run some tests that can change the environment separately from all tests that have read-only access to the environment.

[pauldingemans]

@pbludov What setting do you use for parallel execution of your top level classes? Do they run in parallel as well or sequentially? When they run sequentially, you could consider to move the test which alter the system state to a separate top level class in which you execute all methods sequentially.

Another approach to this problem could be that all classes and tests annotated with ExecutionMode.SAME_THREAD are executed and completed before tests marked with ExecutionMode.CONCURRENT will start.

[pbludov]

The config is

          <properties>
            <!-- execute top-level classes in parallel but methods in same thread -->
            <configurationParameters>
              junit.jupiter.execution.parallel.enabled = true
              junit.jupiter.execution.parallel.mode.classes.default = concurrent
              junit.jupiter.execution.parallel.mode.default = same_thread
            </configurationParameters>
          </properties>

The behavior I'm currently observing:

• all tests annotated ExecutionMode.SAME_THREAD are executed sequentially;
• all tests without explicit annotations are executed simultaneously with tests from the ExecutionMode.SAME_THREAD set, but in parallel threads.

Did I understand correctly that if I explicitly mark all tests as either ExecutionMode.SAME_THREAD or ExecutionMode.CONCURRENT, then no other tests will be run simultaneously with the tests from the ExecutionMode.SAME_THREAD set?

[pauldingemans]

Did I understand correctly that if I explicitly mark all tests as either ExecutionMode.SAME_THREAD or ExecutionMode.CONCURRENT, then no other tests will be run simultaneously with the tests from the ExecutionMode.SAME_THREAD set?

Currently not. I didn't make clear enough that this might be a possible way to implement this feature in JUnit instead of using ResourceLocks.

[vlsi]

It looks like we need this in Apache JMeter as well.
One of the tests alters TimeZone.setDefault.
Note: the nature of the test is to ensure that the test would use default timezone for date calculations, so we can't really pass it as a parameter.

Is there a way to understand the effective locks used for the test?

For instance, what if I add an interface like the following

@ResourceLock(value = Resources.LOCALE, mode = ResourceAccessMode.READ)
@ResourceLock(value = Resources.TIME_ZONE, mode = ResourceAccessMode.READ)
interface CommonReadLocks {
}

then I make the sensitive tests to implement that interface

and then add the relevant @ResourceLock(value = Resources.TIME_ZONE, mode = ResourceAccessMode.READ_WRITE) annotations to the test methods that do modify time zone.

Would that work?

I would expect that method-level annotations would override "inherited ones". So the engine should probably be able to schedule methods as appropriate.

The sad thing is it is not clear how to cross-check if annotations work as expected.

[marcphilipp]

Is there a way to understand the effective locks used for the test?

We could log them or send report entries for them. Would that help you?

I would expect that method-level annotations would override "inherited ones". So the engine should probably be able to schedule methods as appropriate.

In order to avoid deadlocks, locks are currently pulled up to the first node in the test tree that declares a lock. In your example that would mean that all implementations of CommonReadLocks would be able to run in parallel unless they contain a method that uses mode READ_WRITE in which case the whole class would use READ_WRITE. There's a related open issue to make that smarter: #2038.

[nipafx]

Over at JUnitPioneer (@vlsi, @pbludov: you may want to check out our @DefaultTimeZone and @DefaultLocale annotations 😊), we ran into a problem where we configure a stricter security manager for one of our tests and it leads to random problems in other tests. In this comment I wrote:

1. There seems to be no simple way to express that, despite all the parallelism that was enabled for the other tests, the security-manager tests should be run on their own. The only solution seems to be resource locks that need to be applied to all tests.

2. The security manager tests behave similarly to the many extensions we have that change global state (e.g. default locale) and arguably, the same problem can arise there: While we can use resource locks to keep tests using our extensions from treading on one another, we can't prevent "unextended" tests from running concurrently to extended ones. The user would have to apply respective annotations. Which sucks.

Right now it feels like this is a serious problem for all extensions that touch global state that unextended tests may also use: There is no way to run these concurrently without adding annotations to tests that are seemingly unrelated to the extended ones. Worse, it may be very hard to even find the ones that need to be forced into single threads.

I wonder whether an ExecutionMode like SingleThreaded would make sense.

While we arrived there independently of one another, our problem and analyses are identical and @pbludov suggested the same solution. I think this is worth looking into.

[marcphilipp]

@leonard84 had a similar use case recently, right?

[leonard84]

Yes, I'm currently adding parallel support to Spock and we have an extension @ConfineMetaClassChanges for tests that manipulate one or more meta classes in groovy, this is by its nature a global change potentially affecting all other tests. The extension automatically sets the READ_WRITE lock for the MetaClassRegistry, but every other test would also need to acquire a READ lock for the same resource for it to be effective.

So a way to mark a test/class to run in a global exclusive mode would be a nice solution for this kind of problems.

A possible implementation would be that each test acquires either a READ or READ_WRITE lock to a special ___global___ resource, this is always done as the first lock. If nothing else is set only a READ lock is acquired, for exclusive tests you switch it to READ_WRITE mode. We would need to change the deadlock avoidance algorithm a bit to treat the global lock in a special way so as not to cause all tests to run in SAME_THREAD mode.

[nipafx]

I'd really like to see an official solution, so maybe I'm shooting myself in the foot, but there's a fairly straightforward way for us to solve this without onboard support:

• create our own @Test annotation and search/replace all imports
• meta-annotate with READ access to custom resource lock, e.g. "SecurityManager", "MetaClassChanges ", etc.
• request READ_WRITE access to that lock in respective tests

[marcphilipp]

@leonard84 and I started working on an implementation today. We added an @Isolated annotation to Jupiter which causes the outermost test class to be run in isolation even if the annotation is only used on a test method. That was necessary in order to avoid deadlocks since upgrading from a read lock to a write is not supported by the underlying ReentrantReadWriteLock implementation.