Thread context classloader should be reset after each test

[bacer-esteco]

Overview

Whenever a test case sets the thread context ClassLoader, the test runner thread keeps using that ClassLoader for the following tests. This causes failures when combined with Mockito and mocks of package private classes/interfaces.

Expected Behavior

When the thread is reused to run another test, the context ClassLoader should be reset to the original one.

Steps to Reproduce

To reproduce the problem I created this example repo: https://github.com/bacer-esteco/junit5-bug-report.

Just run the 'test' task.

This problem is reproducible using both Jupiter and Vintage (tried on version 5.4.0), launching the test both from Gradle 5.2.1 and IntelliJ IDEA 2018.3 (and previous).

Related Issues

• #201
• #1688

[sormuras]

Whenever a test case sets the thread context classloader...

...then test case is in charge to teardown the setup it needed to execute.

[sbrannen]

Whenever a test case sets the thread context classloader...

...then test case is in charge to teardown the setup it needed to execute.

I certainly agree that tests should clean up whatever they set up, but I can also understand the desire to have the framework ensure that rogue code doesn't mess up the thread context ClassLoader for the rest of the test plan.

[sbrannen]

Tentatively slated for 5.5 M1 for team discussion

[sbrannen]

I can also understand the desire to have the framework ensure that rogue code doesn't mess up the thread context ClassLoader for the rest of the test plan.

In that regard, this issue is related to #1688.

[sormuras]

What about System.out, System.err, System.properties(), and other "well-known" global resources? Reset them too?

[sbrannen]

What about System.out, System.err, System.properties(), and other "well-known" global resources? Reset them too?

As a framework, we certainly have to draw the line somewhere, but let's not forget that we'll have to set/reset the thread context ClassLoader in conjunction with #201 anyway.

So maybe we should close this issue in favor of a subtask for #201.

[sormuras]

Couldn't resist ... @NewVirtualMachine(Lifecycle.PER_METHOD)

[marcphilipp]

Team Decision: Reset the TCCL to the one present before starting test execution after the test has finished executing.

[Tibor17]

You are closing the door for parallel classes with this solution in the future.

@thekingnothing Do you override Context Class Loader in Thread in JUnit4 Powermock Runner? I do not think so. You create a new CL each time a test method runs and you instantiate new mock objects? Or not either and still the same CL is used but different mock type is registered in default CL?

[sormuras]

You are closing the door for parallel classes with this solution in the future.

I see an open door https://junit.org/junit5/docs/current/user-guide/#writing-tests-parallel-execution

[Tibor17]

Still sounds like a workaround of the problem with Mockito. Perhaps it is Mockito bug and not JUnit5 bug. Regarding #201 this issue #1799 is not a perfect way to customize Thread's CL. The perfect solution is to call an extension API.

[thekingn0thing]

@Tibor17 PowerMock split tests into chunks (a chunk per @PrepareForTest annotation), then creates a new Class Loader for each chuck and load everything with this class loader when starts a new test chuck. PowerMock loads a junit runner class with own classloader, when creates loads a test class itself with own class loader.

Right before running a test PowerMock replaces Context Class Loader and return it back in finally block

			final ClassLoader originalClassLoader = Thread.currentThread().getContextClassLoader();
			Thread.currentThread().setContextClassLoader(key);
			try {
				delegate.run(notifier);
			} finally {
				Thread.currentThread().setContextClassLoader(originalClassLoader);
			}

[marcphilipp]

You are closing the door for parallel classes with this solution in the future.

@Tibor17 Could you please explain why you think that?

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity. Given the limited bandwidth of the team, it will be automatically closed if no further activity occurs. Thank you for your contribution.

[stale[bot]]

This issue has been automatically closed due to inactivity. If you have a good use case for this feature, please feel free to reopen the issue.

[sbrannen]

Reopening in light of #2942 and https://github.com/junit-team/junit5/issues/1799#issuecomment-470903309.