Skip to content

Commit

Permalink
Fix using resource locks on test templates
Browse files Browse the repository at this point in the history
Prior to this commit, it was not possible to use `@ResourceLock` on
`@TestTemplate` methods because `TestTemplateInvocationTestDescriptor`
returned a non-empty set of `ExclusiveResources` which is disallowed for
dynamically added `TestDescriptors`. Now, an empty set is returned
instead which works because the parent `TestTemplateTestDescriptor`
returns the resources from the `@ResourceLock` annotation.

Fixes #1697.
  • Loading branch information
marcphilipp committed Dec 15, 2018
1 parent fe1993e commit 60f3125
Show file tree
Hide file tree
Showing 6 changed files with 98 additions and 6 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,9 @@ repository on GitHub.

==== Bug Fixes

* ❓
* `@ResourceLock` can now be declared on test template methods such as `@RepeatedTest` and
`@ParameterizedTest` methods. If `@ResourceLock` is used, the invocations will run in
the same thread, even if parallel execution is enabled.

==== Deprecations and Breaking Changes

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,11 @@

package org.junit.jupiter.engine.descriptor;

import static java.util.Collections.emptySet;
import static org.apiguardian.api.API.Status.INTERNAL;

import java.lang.reflect.Method;
import java.util.Set;

import org.apiguardian.api.API;
import org.junit.jupiter.api.extension.TestTemplateInvocationContext;
Expand All @@ -21,6 +23,7 @@
import org.junit.jupiter.engine.extension.ExtensionRegistry;
import org.junit.platform.engine.TestDescriptor;
import org.junit.platform.engine.UniqueId;
import org.junit.platform.engine.support.hierarchical.ExclusiveResource;

/**
* {@link TestDescriptor} for a {@link org.junit.jupiter.api.TestTemplate @TestTemplate}
Expand All @@ -43,6 +46,12 @@ public class TestTemplateInvocationTestDescriptor extends TestMethodTestDescript
this.index = index;
}

@Override
public Set<ExclusiveResource> getExclusiveResources() {
// @ResourceLock annotations are already collected and returned by the enclosing container
return emptySet();
}

@Override
public String getLegacyReportingName() {
return super.getLegacyReportingName() + "[" + index + "]";
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
/*
* Copyright 2015-2018 the original author or authors.
*
* All rights reserved. This program and the accompanying materials are
* made available under the terms of the Eclipse Public License v2.0 which
* accompanies this distribution and is available at
*
* http://www.eclipse.org/legal/epl-v20.html
*/

package org.junit.jupiter.engine.descriptor;

import static org.assertj.core.api.Assertions.assertThat;
import static org.mockito.ArgumentMatchers.anyInt;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.when;

import java.lang.reflect.Method;

import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.TestTemplate;
import org.junit.jupiter.api.extension.TestTemplateInvocationContext;
import org.junit.jupiter.api.parallel.ResourceLock;
import org.junit.jupiter.engine.config.JupiterConfiguration;
import org.junit.platform.engine.UniqueId;

class TestTemplateInvocationTestDescriptorTests {

@Test
void invocationsDoNotDeclareExclusiveResources() throws Exception {
Class<MyTestCase> testClass = MyTestCase.class;
Method testTemplateMethod = testClass.getDeclaredMethod("testTemplate");
JupiterConfiguration configuration = mock(JupiterConfiguration.class);
TestTemplateTestDescriptor parent = new TestTemplateTestDescriptor(UniqueId.root("segment", "template"),
testClass, testTemplateMethod, configuration);
TestTemplateInvocationContext invocationContext = mock(TestTemplateInvocationContext.class);
when(invocationContext.getDisplayName(anyInt())).thenReturn("invocation");

TestTemplateInvocationTestDescriptor testDescriptor = new TestTemplateInvocationTestDescriptor(
parent.getUniqueId().append(TestTemplateInvocationTestDescriptor.SEGMENT_TYPE, "1"), testClass,
testTemplateMethod, invocationContext, 1, configuration);

assertThat(parent.getExclusiveResources()).hasSize(1);
assertThat(testDescriptor.getExclusiveResources()).isEmpty();
}

static class MyTestCase {
@TestTemplate
@ResourceLock("a")
void testTemplate() {
}
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -22,19 +22,27 @@
*/
class NodeExecutionAdvisor {

private final Map<TestDescriptor, ExecutionMode> forcedExecutionModeByTestDescriptor = new HashMap<>();
private final Map<TestDescriptor, ExecutionMode> forcedDescendantExecutionModeByTestDescriptor = new HashMap<>();
private final Map<TestDescriptor, ResourceLock> resourceLocksByTestDescriptor = new HashMap<>();

void forceExecutionMode(TestDescriptor testDescriptor, ExecutionMode executionMode) {
forcedExecutionModeByTestDescriptor.put(testDescriptor, executionMode);
void forceDescendantExecutionMode(TestDescriptor testDescriptor, ExecutionMode executionMode) {
forcedDescendantExecutionModeByTestDescriptor.put(testDescriptor, executionMode);
}

void useResourceLock(TestDescriptor testDescriptor, ResourceLock resourceLock) {
resourceLocksByTestDescriptor.put(testDescriptor, resourceLock);
}

Optional<ExecutionMode> getForcedExecutionMode(TestDescriptor testDescriptor) {
return Optional.ofNullable(forcedExecutionModeByTestDescriptor.get(testDescriptor));
return testDescriptor.getParent().flatMap(this::lookupExecutionModeForcedByAncestor);
}

private Optional<ExecutionMode> lookupExecutionModeForcedByAncestor(TestDescriptor testDescriptor) {
ExecutionMode value = forcedDescendantExecutionModeByTestDescriptor.get(testDescriptor);
if (value != null) {
return Optional.of(value);
}
return testDescriptor.getParent().flatMap(this::lookupExecutionModeForcedByAncestor);
}

ResourceLock getResourceLock(TestDescriptor testDescriptor) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,9 +38,10 @@ private void walk(TestDescriptor testDescriptor, NodeExecutionAdvisor advisor) {
}
else {
Set<ExclusiveResource> allResources = new HashSet<>(exclusiveResources);
advisor.forceDescendantExecutionMode(testDescriptor, SAME_THREAD);
doForChildrenRecursively(testDescriptor, child -> {
allResources.addAll(getExclusiveResources(child));
advisor.forceExecutionMode(child, SAME_THREAD);
advisor.forceDescendantExecutionMode(child, SAME_THREAD);
});
advisor.useResourceLock(testDescriptor, lockManager.getLockForResources(allResources));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -171,6 +171,14 @@ void threadInterruptedByUserCode() {
assertThat(events.stream().filter(event(test(), finishedSuccessfully())::matches)).size().isEqualTo(4);
}

@Test
void executesTestTemplatesWithResourceLocksInSameThread() {
List<Event> events = execute(2, ConcurrentTemplateTestCase.class);

assertThat(events.stream().filter(event(test(), finishedSuccessfully())::matches)).size().isEqualTo(10);
assertThat(ThreadReporter.getThreadNames(events)).hasSize(1);
}

private List<Instant> getTimestampsFor(List<Event> events, Condition<Event> condition) {
// @formatter:off
return events.stream()
Expand Down Expand Up @@ -496,6 +504,16 @@ void test4() throws InterruptedException {

}

@Execution(CONCURRENT)
@ExtendWith(ThreadReporter.class)
static class ConcurrentTemplateTestCase {
@RepeatedTest(10)
@ResourceLock("a")
void repeatedTest() throws Exception {
Thread.sleep(100);
}
}

private static void incrementBlockAndCheck(AtomicInteger sharedResource, CountDownLatch countDownLatch)
throws InterruptedException {
int value = incrementAndBlock(sharedResource, countDownLatch);
Expand Down

0 comments on commit 60f3125

Please sign in to comment.