Parallel execution and @TestInstance(PER_CLASS) result in concurrency issues for instance fields mutated in test and lifecycle methods

[dlanaghen]

@TestInstance annotation seems to cause threading problems with tests in Spring Boot.

<junit5.jupiter.version>5.3.0-M1</junit5.jupiter.version>
<junit5.platform.version>1.2.0</junit5.platform.version>

I was able to replicate this problem by creating a small Spring Boot application using the Spring Initializer (https://start.spring.io/).

The application:

package com.example.demo;

import org.springframework.boot.SpringApplication;
import org.springframework.boot.autoconfigure.SpringBootApplication;

@SpringBootApplication
public class DemoApplication {

	public static void main(String[] args) {
		SpringApplication.run(DemoApplication.class, args);
	}
}

A dummy class for testing:

package com.example.demo;

public class Klass {
    public Klass(String field) {
        this.field = field;
    }

    public String getField() {
        return field;
    }

    public void setField(String field) {
        this.field = field;
    }

    String field;
}

My test class:

package com.example.demo;

import org.junit.Test;
import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.RepeatedTest;
import org.junit.jupiter.api.TestInstance;
import org.junit.jupiter.api.extension.ExtendWith;
import org.junit.runner.RunWith;
import org.springframework.boot.test.context.SpringBootTest;
import org.springframework.test.context.junit.jupiter.SpringExtension;
import org.springframework.test.context.junit4.SpringRunner;

// @TestInstance(TestInstance.Lifecycle.PER_CLASS) // Needed for BeforeAll annotation but causes threading issues. 
@ExtendWith(SpringExtension.class)
@SpringBootTest(classes = {DemoApplication.class})
public class DemoApplicationTests {

	@Test
	public void contextLoads() {
	}

	Klass klass = null;

	@BeforeEach
	public void beforeEach() {
		klass = new Klass(Thread.currentThread().getName());
	}

	@RepeatedTest(8)
	public void parallel() {
		klass.setField("Modified: " + klass.getField());
	}

	@AfterEach
	public void afterEach() {
		System.out.println(String.format("%s - %s", Thread.currentThread().getName(), klass.field ));
	}
}

Output when not using @TestInstance:

ForkJoinPool-1-worker-3 - Modified: ForkJoinPool-1-worker-3
ForkJoinPool-1-worker-0 - Modified: ForkJoinPool-1-worker-0
ForkJoinPool-1-worker-2 - Modified: ForkJoinPool-1-worker-2
ForkJoinPool-1-worker-3 - Modified: ForkJoinPool-1-worker-3
ForkJoinPool-1-worker-4 - Modified: ForkJoinPool-1-worker-4
ForkJoinPool-1-worker-4 - Modified: ForkJoinPool-1-worker-4
ForkJoinPool-1-worker-6 - Modified: ForkJoinPool-1-worker-6
ForkJoinPool-1-worker-2 - Modified: ForkJoinPool-1-worker-2

Output when using @TestInstance:

ForkJoinPool-1-worker-0 - Modified: ForkJoinPool-1-worker-4 <<< oops
ForkJoinPool-1-worker-2 - Modified: ForkJoinPool-1-worker-4 <<<
ForkJoinPool-1-worker-3 - Modified: ForkJoinPool-1-worker-3
ForkJoinPool-1-worker-0 - Modified: ForkJoinPool-1-worker-0
ForkJoinPool-1-worker-4 - Modified: ForkJoinPool-1-worker-0 <<<
ForkJoinPool-1-worker-2 - Modified: ForkJoinPool-1-worker-2
ForkJoinPool-1-worker-3 - Modified: ForkJoinPool-1-worker-3

[sbrannen]

Thanks for raising the issue.

We'll look into it.

[sbrannen]

Out of curiosity, why don't you have junit5.platform.version set to 1.3.0-M1 instead of 1.2.0?

[marcphilipp]

This behavior does not surprise me: You're using the same instance of DemoApplicationTests and call beforeEach(), parallel(), and afterEach() from different threads. When one of the threads prints in afterEach(), another thread is very likely to have already called beforeEach() or parallel() which modifies the test instance.

Since issues like this one are inevitable when using @TestInstance(PER_CLASS), I think we should change the default execution mode to SAME_THREAD in this case.

@junit-team/junit-lambda Thoughts?

[marcphilipp]

@dlanaghen @TestInstance(PER_CLASS) is not "Needed for BeforeAll", only if you want your @BeforeAll methods to be not static.

[dlanaghen]

I realize that, but I had put that notation in a base test class because some of my other classes might need that and I saw no reason not to just include it in my base class.   This is not going to be problem currently for us, but I thought I would report my finding. - Dave

[sbrannen]

This is actually not related to Spring Boot.

I have therefore renamed this issue accordingly.

[sbrannen]

The following simplified example demonstrates the same behavior without Spring Boot in the picture.

@TestInstance(PER_CLASS)
class DemoApplicationTests {

	Klass klass;

	@BeforeEach
	void beforeEach() {
		klass = new Klass(Thread.currentThread().getName());
	}

	@RepeatedTest(8)
	void parallel() {
		klass.setField("Modified: " + klass.getField());
	}

	@AfterEach
	void afterEach() {
		System.out.println(String.format("%s - %s", Thread.currentThread().getName(), klass.getField()));
	}

}

[sbrannen]

Without a change in JUnit itself, you have two options for avoiding the undesired behavior.

Disable Parallel Execution

@TestInstance(PER_CLASS)
@Execution(SAME_THREAD)
class DemoApplicationTests {

	Klass klass;

	@BeforeEach
	void beforeEach() {
		klass = new Klass(Thread.currentThread().getName());
	}

	@RepeatedTest(8)
	void parallel() {
		klass.setField("Modified: " + klass.getField());
	}

	@AfterEach
	void afterEach() {
		System.out.println(String.format("%s - %s", Thread.currentThread().getName(), klass.getField()));
	}

}

Store test instance state in a ThreadLocal

@TestInstance(PER_CLASS)
class DemoApplicationTests {

	ThreadLocal<Klass> klass = new ThreadLocal<>();

	@BeforeEach
	void beforeEach() {
		klass.set(new Klass(Thread.currentThread().getName()));
	}

	@RepeatedTest(8)
	void parallel() {
		klass.get().setField("Modified: " + klass.get().getField());
	}

	@AfterEach
	void afterEach() {
		System.out.println(String.format("%s - %s", Thread.currentThread().getName(), klass.get().getField()));
	}

}

The first option completely disables parallel execution, thereby avoiding the undesired behavior completely but obviously executing each test sequentially.

ForkJoinPool-1-worker-1 - Modified: ForkJoinPool-1-worker-1
ForkJoinPool-1-worker-1 - Modified: ForkJoinPool-1-worker-1
ForkJoinPool-1-worker-1 - Modified: ForkJoinPool-1-worker-1
ForkJoinPool-1-worker-1 - Modified: ForkJoinPool-1-worker-1
ForkJoinPool-1-worker-1 - Modified: ForkJoinPool-1-worker-1
ForkJoinPool-1-worker-1 - Modified: ForkJoinPool-1-worker-1
ForkJoinPool-1-worker-1 - Modified: ForkJoinPool-1-worker-1
ForkJoinPool-1-worker-1 - Modified: ForkJoinPool-1-worker-1

The second option allows the tests to continue to execute in parallel but introduces more work for the author of the test.

ForkJoinPool-1-worker-3 - Modified: ForkJoinPool-1-worker-3
ForkJoinPool-1-worker-4 - Modified: ForkJoinPool-1-worker-4
ForkJoinPool-1-worker-2 - Modified: ForkJoinPool-1-worker-2
ForkJoinPool-1-worker-4 - Modified: ForkJoinPool-1-worker-4
ForkJoinPool-1-worker-2 - Modified: ForkJoinPool-1-worker-2
ForkJoinPool-1-worker-3 - Modified: ForkJoinPool-1-worker-3
ForkJoinPool-1-worker-2 - Modified: ForkJoinPool-1-worker-2
ForkJoinPool-1-worker-2 - Modified: ForkJoinPool-1-worker-2