Skip to content

Commit

Permalink
Merge pull request #1806 from mockito/sf2
Browse files Browse the repository at this point in the history
Fixed JUnit5 concurrency bug
  • Loading branch information
mockitoguy committed Oct 31, 2019
2 parents 5292f81 + e43c9ab commit b2b66f4
Show file tree
Hide file tree
Showing 2 changed files with 94 additions and 58 deletions.
Expand Up @@ -5,21 +5,13 @@
package org.mockito.junit.jupiter;


import static org.junit.jupiter.api.extension.ExtensionContext.Namespace.create;
import static org.junit.platform.commons.support.AnnotationSupport.findAnnotation;

import java.lang.reflect.Parameter;
import java.util.LinkedHashSet;
import java.util.Optional;
import java.util.Set;
import org.junit.jupiter.api.extension.AfterEachCallback;
import org.junit.jupiter.api.extension.BeforeEachCallback;
import org.junit.jupiter.api.extension.ExtensionContext;
import org.junit.jupiter.api.extension.ExtensionContext.Namespace;
import org.junit.jupiter.api.extension.ParameterContext;
import org.junit.jupiter.api.extension.ParameterResolutionException;
import org.junit.jupiter.api.extension.ParameterResolver;
import org.junit.jupiter.api.extension.TestInstancePostProcessor;
import org.mockito.Mock;
import org.mockito.Mockito;
import org.mockito.MockitoSession;
Expand All @@ -29,6 +21,13 @@
import org.mockito.junit.MockitoJUnitRunner;
import org.mockito.quality.Strictness;

import java.lang.reflect.Parameter;
import java.util.List;
import java.util.Optional;

import static org.junit.jupiter.api.extension.ExtensionContext.Namespace.create;
import static org.junit.platform.commons.support.AnnotationSupport.findAnnotation;

/**
* Extension that initializes mocks and handles strict stubbings. This extension is the JUnit Jupiter equivalent
* of our JUnit4 {@link MockitoJUnitRunner}.
Expand Down Expand Up @@ -114,12 +113,11 @@
* }
* </code></pre>
*/
public class MockitoExtension implements TestInstancePostProcessor, BeforeEachCallback, AfterEachCallback, ParameterResolver {
public class MockitoExtension implements BeforeEachCallback, AfterEachCallback, ParameterResolver {

private final static Namespace MOCKITO = create("org.mockito");

private final static String SESSION = "session";
private final static String TEST_INSTANCE = "testInstance";

private final Strictness strictness;

Expand All @@ -133,34 +131,14 @@ private MockitoExtension(Strictness strictness) {
this.strictness = strictness;
}

/**
* <p>Callback for post-processing the supplied test instance.
*
* <p><strong>Note</strong>: the {@code ExtensionContext} supplied to a
* {@code TestInstancePostProcessor} will always return an empty
* {@link Optional} value from {@link ExtensionContext#getTestInstance()
* getTestInstance()}. A {@code TestInstancePostProcessor} should therefore
* only attempt to process the supplied {@code testInstance}.
*
* @param testInstance the instance to post-process; never {@code null}
* @param context the current extension context; never {@code null}
*/
@Override
public void postProcessTestInstance(Object testInstance, ExtensionContext context) {
context.getStore(MOCKITO).put(TEST_INSTANCE, testInstance);
}

/**
* Callback that is invoked <em>before</em> each test is invoked.
*
* @param context the current extension context; never {@code null}
*/
@Override
public void beforeEach(final ExtensionContext context) {
Set<Object> testInstances = new LinkedHashSet<>();
testInstances.add(context.getRequiredTestInstance());

this.collectParentTestInstances(context, testInstances);
List<Object> testInstances = context.getRequiredTestInstances().getAllInstances();

Strictness actualStrictness = this.retrieveAnnotationFromTestClasses(context)
.map(MockitoSettings::strictness)
Expand Down Expand Up @@ -192,33 +170,6 @@ private Optional<MockitoSettings> retrieveAnnotationFromTestClasses(final Extens
return annotation;
}

private void collectParentTestInstances(ExtensionContext context, Set<Object> testInstances) {
Optional<ExtensionContext> initialParent = context.getParent();

// Ignoring first parent avoids parallel execution issues
// We can ignore the first parent because it has the test instance that is already in 'testInstances'
// See how 'testInstances' is populated
initialParent.ifPresent(parent -> collectParentTestInstance(parent.getParent(), context, testInstances));
}

@SuppressWarnings("OptionalUsedAsFieldOrParameterType")
private void collectParentTestInstance(
Optional<ExtensionContext> parent, ExtensionContext context,
Set<Object> testInstances) {

parent.ifPresent(currentParent -> {
if (currentParent != context.getRoot()) {
Object testInstance = currentParent.getStore(MOCKITO).remove(TEST_INSTANCE);

if (testInstance != null) {
testInstances.add(testInstance);
}

collectParentTestInstance(currentParent.getParent(), context, testInstances);
}
});
}

/**
* Callback that is invoked <em>after</em> each test has been invoked.
*
Expand Down
@@ -0,0 +1,85 @@
/*
* Copyright (c) 2019 Mockito contributors
* This program is made available under the terms of the MIT License.
*/
package org.mockito;

import org.junit.jupiter.api.Nested;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.ExtendWith;
import org.mockito.junit.jupiter.MockitoExtension;

@ExtendWith(MockitoExtension.class)
class NestedParallelTest {

@Mock
private SomeService someService;

@InjectMocks
private AnotherService anotherService;

@Nested
class NestedTest {

@Test
void test() {
perform();
}

private void perform() {
// when
anotherService.callSomeService();

// then
Mockito.verify(someService).doSomething();
}

@Test
void test2() {
perform();
}

@Test
void test3() {
perform();
}

@Test
void test4() {
perform();
}

@Test
void test5() {
perform();
}

@Test
void test6() {
perform();
}

@Test
void test7() {
perform();
}
}

public static class AnotherService {
private final SomeService someService;

public AnotherService(final SomeService someService) {
this.someService = someService;
}

void callSomeService() {
someService.doSomething();
}
}

static class SomeService {

void doSomething() {
}
}
}

0 comments on commit b2b66f4

Please sign in to comment.