Skip to content

Commit

Permalink
#190 Got rid of String-based TestDescriptor.getUniqueId()
Browse files Browse the repository at this point in the history
  • Loading branch information
@jlink
jlink committed Mar 15, 2016
1 parent 189d103 commit 270bf06
Show file tree
Hide file tree
Showing 20 changed files with 53 additions and 75 deletions.
13 changes: 3 additions & 10 deletions junit-engine-api/src/main/java/org/junit/gen5/engine/TestDescriptor.java
View file
Expand Up @@ -30,14 +30,7 @@ public interface TestDescriptor {
* <p>Uniqueness must be guaranteed across an entire test plan,
* regardless of how many engines are used behind the scenes.
*/
String getUniqueId();

/**
* Temporary parallel getter to string-based unique id.
*
* Should be renamed to getUniqueId() as soon as no clients are using the String any more.
*/
UniqueId getUniqueIdObject();
UniqueId getUniqueId();

String getName();

Expand Down Expand Up @@ -88,12 +81,12 @@ default boolean hasTests() {
}

default Optional<? extends TestDescriptor> findByUniqueId(UniqueId uniqueId) {
if (getUniqueIdObject().equals(uniqueId)) {
if (getUniqueId().equals(uniqueId)) {
return Optional.of(this);
}
// else
return getChildren().stream().filter(
testDescriptor -> testDescriptor.getUniqueIdObject().equals(uniqueId)).findFirst();
testDescriptor -> testDescriptor.getUniqueId().equals(uniqueId)).findFirst();
}

interface Visitor {
Expand Down
13 changes: 4 additions & 9 deletions ...ne-api/src/main/java/org/junit/gen5/engine/support/descriptor/AbstractTestDescriptor.java
View file
Expand Up @@ -44,16 +44,11 @@ protected AbstractTestDescriptor(UniqueId uniqueId) {
this.uniqueId = Preconditions.notNull(uniqueId, "uniqueId must not be null");
}

@Override
public final String getUniqueId() {
return getUniqueIdObject().getUniqueString();
}

/**
* Temporary parallel to string-based unique id
*/
@Override
public final UniqueId getUniqueIdObject() {
public final UniqueId getUniqueId() {
return uniqueId;
}

Expand Down Expand Up @@ -83,7 +78,7 @@ protected void removeFromHierarchy() {

@Override
public Optional<? extends TestDescriptor> findByUniqueId(UniqueId uniqueId) {
if (getUniqueIdObject().equals(uniqueId)) {
if (getUniqueId().equals(uniqueId)) {
return Optional.of(this);
}
for (TestDescriptor child : this.children) {
Expand Down Expand Up @@ -137,7 +132,7 @@ public final boolean equals(Object other) {
return false;
}
TestDescriptor otherDescriptor = (TestDescriptor) other;
return this.getUniqueIdObject().equals(otherDescriptor.getUniqueIdObject());
return this.getUniqueId().equals(otherDescriptor.getUniqueId());
}

@Override
Expand All @@ -147,7 +142,7 @@ public final int hashCode() {

@Override
public String toString() {
return getClass().getSimpleName() + ": " + getUniqueIdObject().getUniqueString();
return getClass().getSimpleName() + ": " + getUniqueId().getUniqueString();
}

}
5 changes: 3 additions & 2 deletions junit-launcher/src/main/java/org/junit/gen5/launcher/TestIdentifier.java
View file
Expand Up @@ -47,14 +47,15 @@ public final class TestIdentifier implements Serializable {

public static TestIdentifier from(TestDescriptor testDescriptor) {
// TODO Use Flyweight Pattern for TestId?
String uniqueId = testDescriptor.getUniqueId();
String uniqueId = testDescriptor.getUniqueId().getUniqueString();
String name = testDescriptor.getName();
String displayName = testDescriptor.getDisplayName();
Optional<TestSource> source = testDescriptor.getSource();
Set<TestTag> tags = testDescriptor.getTags();
boolean test = testDescriptor.isTest();
boolean container = !test || !testDescriptor.getChildren().isEmpty();
Optional<String> parentId = testDescriptor.getParent().map(TestDescriptor::getUniqueId);
Optional<String> parentId = testDescriptor.getParent().map(
parentDescriptor -> parentDescriptor.getUniqueId().getUniqueString());
return new TestIdentifier(uniqueId, name, displayName, source, tags, test, container, parentId);
}

Expand Down
2 changes: 1 addition & 1 deletion junit-launcher/src/main/java/org/junit/gen5/launcher/main/ExecutionListenerAdapter.java
View file
Expand Up @@ -56,7 +56,7 @@ public void reportingEntryPublished(TestDescriptor testDescriptor, ReportEntry e
}

private TestIdentifier getTestIdentifier(TestDescriptor testDescriptor) {
return testPlan.getTestIdentifier(testDescriptor.getUniqueId());
return testPlan.getTestIdentifier(testDescriptor.getUniqueId().getUniqueString());
}

}
3 changes: 2 additions & 1 deletion junit-tests/src/test/java/org/junit/gen5/engine/ExecutionEventConditions.java
View file
Expand Up @@ -74,7 +74,8 @@ public static Condition<ExecutionEvent> container() {

public static Condition<ExecutionEvent> uniqueIdSubstring(String uniqueIdSubstring) {
return new Condition<>(
byTestDescriptor(where(TestDescriptor::getUniqueId, uniqueId -> uniqueId.contains(uniqueIdSubstring))),
byTestDescriptor(where(testDescriptor -> testDescriptor.getUniqueId().getUniqueString(),
uniqueId -> uniqueId.contains(uniqueIdSubstring))),
"descriptor with uniqueId substring \"%s\"", uniqueIdSubstring);
}

Expand Down
21 changes: 10 additions & 11 deletions junit-tests/src/test/java/org/junit/gen5/engine/junit4/JUnit4TestEngineDiscoveryTests.java
View file
Expand Up @@ -91,7 +91,7 @@ void resolvesIgnoredJUnit4TestClass() {
assertFalse(runnerDescriptor.isContainer());
assertTrue(runnerDescriptor.isTest());
assertEquals(testClass.getName(), runnerDescriptor.getDisplayName());
assertEquals(uniqueIdForClass(testClass), runnerDescriptor.getUniqueIdObject());
assertEquals(uniqueIdForClass(testClass), runnerDescriptor.getUniqueId());
assertThat(runnerDescriptor.getChildren()).isEmpty();
}

Expand Down Expand Up @@ -175,12 +175,12 @@ void resolvesJUnit4TestCaseWithOverloadedMethod() {

TestDescriptor testMethodDescriptor = testMethodDescriptors.get(0);
assertEquals("theory", testMethodDescriptor.getDisplayName());
assertEquals(uniqueIdForMethod(testClass, "theory", "0"), testMethodDescriptor.getUniqueIdObject());
assertEquals(uniqueIdForMethod(testClass, "theory", "0"), testMethodDescriptor.getUniqueId());
assertClassSource(testClass, testMethodDescriptor);

testMethodDescriptor = testMethodDescriptors.get(1);
assertEquals("theory", testMethodDescriptor.getDisplayName());
assertEquals(uniqueIdForMethod(testClass, "theory", "1"), testMethodDescriptor.getUniqueIdObject());
assertEquals(uniqueIdForMethod(testClass, "theory", "1"), testMethodDescriptor.getUniqueId());
assertClassSource(testClass, testMethodDescriptor);
}

Expand Down Expand Up @@ -522,7 +522,7 @@ void resolvesClassForMethodSelectorForClassWithNonFilterableRunner() throws Exce

TestDescriptor runnerDescriptor = getOnlyElement(engineDescriptor.getChildren());
assertEquals(testClass.getName(), runnerDescriptor.getDisplayName());
assertEquals(uniqueIdForClass(testClass), runnerDescriptor.getUniqueIdObject());
assertEquals(uniqueIdForClass(testClass), runnerDescriptor.getUniqueId());
assertThat(runnerDescriptor.getChildren()).isEmpty();
}

Expand All @@ -539,9 +539,9 @@ void usesCustomUniqueIdsWhenPresent() throws Exception {
TestDescriptor childDescriptor = getOnlyElement(runnerDescriptor.getChildren());

UniqueId prefix = uniqueIdForClass(testClass);
assertThat(childDescriptor.getUniqueIdObject().getUniqueString()).startsWith(prefix.getUniqueString());
assertThat(childDescriptor.getUniqueId().getUniqueString()).startsWith(prefix.getUniqueString());

String customUniqueIdValue = childDescriptor.getUniqueIdObject().getSegments().get(2).getType();
String customUniqueIdValue = childDescriptor.getUniqueId().getSegments().get(2).getType();
assertNotNull(Base64.getDecoder().decode(customUniqueIdValue.getBytes(StandardCharsets.UTF_8)),
"is a valid Base64 encoding scheme");
}
Expand Down Expand Up @@ -580,7 +580,7 @@ private static void assertRunnerTestDescriptor(TestDescriptor runnerDescriptor,
assertFalse(runnerDescriptor.isTest());
assertEquals(testClass.getName(), runnerDescriptor.getDisplayName());
assertEquals(testClass.getName(), runnerDescriptor.getName());
assertEquals(uniqueIdForClass(testClass), runnerDescriptor.getUniqueIdObject());
assertEquals(uniqueIdForClass(testClass), runnerDescriptor.getUniqueId());
assertClassSource(testClass, runnerDescriptor);
}

Expand All @@ -590,8 +590,7 @@ private static void assertTestMethodDescriptor(TestDescriptor testMethodDescript
assertFalse(testMethodDescriptor.isContainer());
assertEquals(methodName, testMethodDescriptor.getDisplayName());
assertEquals(methodName + "(" + testClass.getName() + ")", testMethodDescriptor.getName());
assertEquals(uniqueIdForMethod(uniqueContainerId, testClass, methodName),
testMethodDescriptor.getUniqueIdObject());
assertEquals(uniqueIdForMethod(uniqueContainerId, testClass, methodName), testMethodDescriptor.getUniqueId());
assertThat(testMethodDescriptor.getChildren()).isEmpty();
assertMethodSource(testClass.getMethod(methodName), testMethodDescriptor);
}
Expand All @@ -602,7 +601,7 @@ private static void assertContainerTestDescriptor(TestDescriptor containerDescri
assertFalse(containerDescriptor.isTest());
assertEquals(testClass.getName(), containerDescriptor.getDisplayName());
assertEquals(testClass.getName(), containerDescriptor.getName());
assertEquals(uniqueIdForClasses(suiteClass, testClass), containerDescriptor.getUniqueIdObject());
assertEquals(uniqueIdForClasses(suiteClass, testClass), containerDescriptor.getUniqueId());
assertClassSource(testClass, containerDescriptor);
}

Expand All @@ -611,7 +610,7 @@ private static void assertInitializationError(TestDescriptor testDescriptor, Cla
assertTrue(testDescriptor.isTest());
assertFalse(testDescriptor.isContainer());
assertEquals("initializationError", testDescriptor.getDisplayName());
assertEquals(uniqueIdForErrorInClass(testClass, failingClass), testDescriptor.getUniqueIdObject());
assertEquals(uniqueIdForErrorInClass(testClass, failingClass), testDescriptor.getUniqueId());
assertThat(testDescriptor.getChildren()).isEmpty();
assertClassSource(failingClass, testDescriptor);
}
Expand Down
4 changes: 2 additions & 2 deletions junit-tests/src/test/java/org/junit/gen5/engine/junit5/DiscoveryFilterApplierTests.java
View file
Expand Up @@ -48,7 +48,7 @@ void nonMatchingClassesAreExcluded() {
applier.applyAllFilters(request, engineDescriptor);

List<UniqueId> includedDescriptors = engineDescriptor.allDescendants().stream().map(
TestDescriptor::getUniqueIdObject).collect(Collectors.toList());
TestDescriptor::getUniqueId).collect(Collectors.toList());
Assertions.assertEquals(1, includedDescriptors.size());
Assertions.assertTrue(includedDescriptors.contains(UniqueId.root("class", "matching")));
}
Expand All @@ -69,7 +69,7 @@ void nestedTestClassesAreAlwaysIncludedWhenTheirParentIs() {
applier.applyAllFilters(request, engineDescriptor);

List<UniqueId> includedDescriptors = engineDescriptor.allDescendants().stream().map(
TestDescriptor::getUniqueIdObject).collect(Collectors.toList());
TestDescriptor::getUniqueId).collect(Collectors.toList());
Assertions.assertEquals(2, includedDescriptors.size());
Assertions.assertTrue(includedDescriptors.contains(UniqueId.root("class", "matching")));
Assertions.assertTrue(includedDescriptors.contains(UniqueId.root("nested-class", "nested")));
Expand Down
2 changes: 1 addition & 1 deletion ...ests/src/test/java/org/junit/gen5/engine/junit5/descriptor/JUnit5TestDescriptorTests.java
View file
Expand Up @@ -42,7 +42,7 @@ public void constructFromMethod() throws Exception {
MethodTestDescriptor descriptor = new MethodTestDescriptor(UniqueId.root("method", "a method id"), testClass,
testMethod);

assertEquals(UniqueId.root("method", "a method id"), descriptor.getUniqueIdObject());
assertEquals(UniqueId.root("method", "a method id"), descriptor.getUniqueId());
assertEquals(testMethod, descriptor.getTestMethod());
assertEquals("test", descriptor.getDisplayName(), "display name:");
}
Expand Down
5 changes: 2 additions & 3 deletions .../src/test/java/org/junit/gen5/engine/junit5/discovery/DiscoverySelectorResolverTests.java
View file
Expand Up @@ -357,12 +357,11 @@ public void testNestedTestResolutionFromUniqueIdToMethod() {

private TestDescriptor descriptorByUniqueId(UniqueId uniqueId) {
return engineDescriptor.allDescendants().stream().filter(
d -> d.getUniqueIdObject().equals(uniqueId)).findFirst().get();
d -> d.getUniqueId().equals(uniqueId)).findFirst().get();
}

private List<UniqueId> uniqueIds() {
return engineDescriptor.allDescendants().stream().map(TestDescriptor::getUniqueIdObject).collect(
Collectors.toList());
return engineDescriptor.allDescendants().stream().map(TestDescriptor::getUniqueId).collect(Collectors.toList());
}

}
Expand Down
12 changes: 6 additions & 6 deletions ...ts/src/test/java/org/junit/gen5/engine/support/discovery/AbstractTestDescriptorTests.java
View file
Expand Up @@ -56,13 +56,13 @@ public void visitAllNodes() {
@Test
public void pruneLeaf() {
TestDescriptor.Visitor visitor = (TestDescriptor descriptor, Runnable delete) -> {
if (descriptor.getUniqueIdObject().equals(UniqueId.root("leaf", "leaf1-1")))
if (descriptor.getUniqueId().equals(UniqueId.root("leaf", "leaf1-1")))
delete.run();
};
engineDescriptor.accept(visitor);

List<UniqueId> visited = new ArrayList<>();
engineDescriptor.accept((descriptor, delete) -> visited.add(descriptor.getUniqueIdObject()));
engineDescriptor.accept((descriptor, delete) -> visited.add(descriptor.getUniqueId()));

assertEquals(7, visited.size());
assertTrue(visited.contains(UniqueId.root("group", "group1")));
Expand All @@ -73,7 +73,7 @@ public void pruneLeaf() {
public void pruneGroup() {
final AtomicInteger countVisited = new AtomicInteger();
TestDescriptor.Visitor visitor = (descriptor, delete) -> {
if (descriptor.getUniqueIdObject().equals(UniqueId.root("group", "group1")))
if (descriptor.getUniqueId().equals(UniqueId.root("group", "group1")))
delete.run();
countVisited.incrementAndGet();
};
Expand All @@ -82,7 +82,7 @@ public void pruneGroup() {
assertEquals(4, countVisited.get(), "Children of pruned element are not visited");

List<UniqueId> visited = new ArrayList<>();
engineDescriptor.accept((descriptor, delete) -> visited.add(descriptor.getUniqueIdObject()));
engineDescriptor.accept((descriptor, delete) -> visited.add(descriptor.getUniqueId()));

assertEquals(3, visited.size());
assertFalse(visited.contains(UniqueId.root("group", "group1")));
Expand All @@ -98,7 +98,7 @@ class GroupDescriptor extends AbstractTestDescriptor {

@Override
public String getName() {
return getUniqueIdObject().getUniqueString();
return getUniqueId().getUniqueString();
}

@Override
Expand All @@ -125,7 +125,7 @@ class LeafDescriptor extends AbstractTestDescriptor {

@Override
public String getName() {
return getUniqueIdObject().getUniqueString();
return getUniqueId().getUniqueString();
}

@Override
Expand Down
2 changes: 1 addition & 1 deletion junit-tests/src/test/java/org/junit/gen5/engine/support/hierarchical/DummyTestEngine.java
View file
Expand Up @@ -39,7 +39,7 @@ public DummyEngineDescriptor getEngineDescriptor() {
}

public DummyTestDescriptor addTest(String uniqueName, Runnable runnable) {
UniqueId uniqueId = engineDescriptor.getUniqueIdObject().append("test", uniqueName);
UniqueId uniqueId = engineDescriptor.getUniqueId().append("test", uniqueName);
DummyTestDescriptor child = new DummyTestDescriptor(uniqueId, uniqueName, runnable);
engineDescriptor.addChild(child);
return child;
Expand Down
4 changes: 2 additions & 2 deletions ...c/test/java/org/junit/gen5/engine/support/hierarchical/HierarchicalTestExecutorTests.java
View file
Expand Up @@ -363,7 +363,7 @@ protected MyContainer(UniqueId uniqueId) {

@Override
public String getName() {
return getUniqueIdObject().getUniqueString();
return getUniqueId().getUniqueString();
}

@Override
Expand Down Expand Up @@ -396,7 +396,7 @@ public MyEngineExecutionContext execute(MyEngineExecutionContext context) throws

@Override
public String getName() {
return getUniqueIdObject().getUniqueString();
return getUniqueId().getUniqueString();
}

@Override
Expand Down
10 changes: 5 additions & 5 deletions junit-tests/src/test/java/org/junit/gen5/launcher/main/DefaultLauncherTests.java
View file
Expand Up @@ -87,7 +87,7 @@ void discoverTestPlanForMultipleEngines() {
DefaultLauncher launcher = createLauncher(firstEngine, secondEngine);

TestPlan testPlan = launcher.discover(
request().select(forUniqueId(test1.getUniqueIdObject()), forUniqueId(test2.getUniqueIdObject())).build());
request().select(forUniqueId(test1.getUniqueId()), forUniqueId(test2.getUniqueId())).build());

assertThat(testPlan.getRoots()).hasSize(2);
assertThat(testPlan.getChildren(UniqueId.forEngine("engine1").getUniqueString())).hasSize(1);
Expand All @@ -104,7 +104,7 @@ void launcherWillNotCallEnginesThatAreFilteredByAnEngineIdFilter() {
DefaultLauncher launcher = createLauncher(firstEngine, secondEngine);

TestPlan testPlan = launcher.discover(
request().select(forUniqueId(test1.getUniqueIdObject()), forUniqueId(test2.getUniqueIdObject())).filter(
request().select(forUniqueId(test1.getUniqueId()), forUniqueId(test2.getUniqueId())).filter(
byEngineId("first")).build());

assertThat(testPlan.getRoots()).hasSize(1);
Expand All @@ -122,10 +122,10 @@ void launcherAppliesPostDiscoveryFilters() {
DefaultLauncher launcher = createLauncher(engine);

PostDiscoveryFilter includeWithUniqueIdContainsTest = new PostDiscoveryFilterStub(
descriptor -> FilterResult.includedIf(descriptor.getUniqueIdObject().getUniqueString().contains("test")),
descriptor -> FilterResult.includedIf(descriptor.getUniqueId().getUniqueString().contains("test")),
() -> "filter1");
PostDiscoveryFilter includeWithUniqueIdContains1 = new PostDiscoveryFilterStub(
descriptor -> FilterResult.includedIf(descriptor.getUniqueIdObject().getUniqueString().contains("1")),
descriptor -> FilterResult.includedIf(descriptor.getUniqueId().getUniqueString().contains("1")),
() -> "filter2");

TestPlan testPlan = launcher.discover( //
Expand All @@ -135,7 +135,7 @@ void launcherAppliesPostDiscoveryFilters() {
.build());

assertThat(testPlan.getChildren(UniqueId.forEngine("myEngine").getUniqueString())).hasSize(1);
assertThat(testPlan.getTestIdentifier(test1.getUniqueIdObject().getUniqueString())).isNotNull();
assertThat(testPlan.getTestIdentifier(test1.getUniqueId().getUniqueString())).isNotNull();
}

private static Runnable noOp() {
Expand Down
3 changes: 1 addition & 2 deletions junit-tests/src/test/java/org/junit/gen5/launcher/main/ExecutionListenerAdapterTests.java
View file
Expand Up @@ -33,8 +33,7 @@ void testReportingEntryPublished() {

//cannot mock final classes with mockito
TestPlan testPlan = TestPlan.from(Collections.singleton(testDescriptor));
TestIdentifier testIdentifier = testPlan.getTestIdentifier(
testDescriptor.getUniqueIdObject().getUniqueString());
TestIdentifier testIdentifier = testPlan.getTestIdentifier(testDescriptor.getUniqueId().getUniqueString());

//not yet spyable with mockito? -> https://github.com/mockito/mockito/issues/146
MockTestExecutionListener testExecutionListener = new MockTestExecutionListener();
Expand Down
2 changes: 1 addition & 1 deletion ...t4-engine/src/main/java/org/junit/gen5/engine/junit4/descriptor/JUnit4TestDescriptor.java
View file
Expand Up @@ -53,7 +53,7 @@ public JUnit4TestDescriptor(TestDescriptor parent, String segmentType, String se

JUnit4TestDescriptor(TestDescriptor parent, String segmentType, String segmentValue, Description description,
Optional<? extends TestSource> source) {
super(parent.getUniqueIdObject().append(segmentType, segmentValue));
super(parent.getUniqueId().append(segmentType, segmentValue));

this.description = description;
source.ifPresent(this::setSource);
Expand Down
2 changes: 1 addition & 1 deletion ...ain/java/org/junit/gen5/engine/junit5/descriptor/ClassBasedContainerExtensionContext.java
View file
Expand Up @@ -30,7 +30,7 @@ public ClassBasedContainerExtensionContext(ExtensionContext parent, EngineExecut

@Override
public String getUniqueId() {
return getTestDescriptor().getUniqueIdObject().getUniqueString();
return getTestDescriptor().getUniqueId().getUniqueString();
}

@Override
Expand Down
3 changes: 0 additions & 3 deletions junit5-engine/src/main/java/org/junit/gen5/engine/junit5/descriptor/ClassTestDescriptor.java
View file
Expand Up @@ -54,9 +54,6 @@
/**
* {@link TestDescriptor} for tests based on Java classes.
*
* <p>The pattern of the {@link #getUniqueId unique ID} takes the form of
* <code>{parent unique id}/[class:{fully qualified class name}]</code>.
*
* @since 5.0
*/
@API(Internal)
Expand Down

0 comments on commit 270bf06

Please sign in to comment.