Skip to content

Commit

Permalink
Check that children of engine descriptors are runner descriptors
Browse files Browse the repository at this point in the history 
Fixes #337.
  • Loading branch information
@marcphilipp
marcphilipp committed Jul 25, 2016
1 parent b496fe4 commit 42d9dab
Show file tree
Hide file tree
Showing 2 changed files with 48 additions and 34 deletions.
29 changes: 16 additions & 13 deletions ...age-engine/src/main/java/org/junit/vintage/engine/discovery/UniqueIdSelectorResolver.java
View file
Expand Up @@ -11,14 +11,16 @@
package org.junit.vintage.engine.discovery; package org.junit.vintage.engine.discovery;


import static java.lang.String.format; import static java.lang.String.format;
import static java.util.function.Predicate.isEqual;
import static org.junit.vintage.engine.descriptor.VintageTestDescriptor.ENGINE_ID; import static org.junit.vintage.engine.descriptor.VintageTestDescriptor.ENGINE_ID;
import static org.junit.vintage.engine.descriptor.VintageTestDescriptor.SEGMENT_TYPE_RUNNER;


import java.util.List;
import java.util.Optional; import java.util.Optional;
import java.util.logging.Logger; import java.util.logging.Logger;


import org.junit.platform.commons.util.ReflectionUtils; import org.junit.platform.commons.util.ReflectionUtils;
import org.junit.platform.engine.UniqueId; import org.junit.platform.engine.UniqueId;
import org.junit.platform.engine.UniqueId.Segment;
import org.junit.platform.engine.discovery.UniqueIdSelector; import org.junit.platform.engine.discovery.UniqueIdSelector;


/** /**
Expand All @@ -41,12 +43,9 @@ void resolve(UniqueIdSelector selector, TestClassCollector collector) {
() -> format("Unresolvable Unique ID (%s): Cannot resolve the engine's unique ID", uniqueId)); () -> format("Unresolvable Unique ID (%s): Cannot resolve the engine's unique ID", uniqueId));
} }
else { else {
uniqueId.getEngineId().ifPresent(engineId -> { uniqueId.getEngineId().filter(isEqual(ENGINE_ID)).ifPresent(
if (engineId.equals(ENGINE_ID)) { engineId -> determineTestClassName(uniqueId).ifPresent(
String testClassName = determineTestClassName(uniqueId); testClassName -> resolveIntoFilteredTestClass(testClassName, uniqueId, collector)));
resolveIntoFilteredTestClass(testClassName, uniqueId, collector);
}
});
} }
} }


Expand All @@ -60,12 +59,16 @@ private void resolveIntoFilteredTestClass(String testClassName, UniqueId uniqueI
} }
} }


private String determineTestClassName(UniqueId uniqueId) { private Optional<String> determineTestClassName(UniqueId uniqueId) {
List<UniqueId.Segment> segments = uniqueId.getSegments(); Segment runnerSegment = uniqueId.getSegments().get(1); // skip engine node
segments.remove(0); // drop engine node if (SEGMENT_TYPE_RUNNER.equals(runnerSegment.getType())) {
UniqueId.Segment runnerSegment = segments.remove(0); return Optional.of(runnerSegment.getValue());
// TODO [#337] Check that it really is a runner segment }
return runnerSegment.getValue(); logger.warning(
() -> format("Unresolvable Unique ID (%s): Unique ID segment after engine segment must be of type \""
+ SEGMENT_TYPE_RUNNER + "\"",
uniqueId));

This comment has been minimized.

Sign in to view

Copy link
@sbrannen

sbrannen Jul 25, 2016

Member

Ummmmm.... why did you use String.format() combined with String concatenation? 😜

This comment has been minimized.

Sign in to view

Copy link
@marcphilipp

marcphilipp Jul 25, 2016

Author Member

Because Java will only do the String concatenation once since it's a constant.

This comment has been minimized.

Sign in to view

Copy link
@sbrannen

sbrannen Jul 30, 2016

Member

Sure.

I was merely asking on the grounds of style, since it looks a bit odd. 😉
return Optional.empty();
} }


} }
53 changes: 32 additions & 21 deletions ...ngine/src/test/java/org/junit/vintage/engine/discovery/UniqueIdSelectorResolverTests.java
View file
Expand Up @@ -15,6 +15,7 @@
import static org.junit.platform.commons.util.CollectionUtils.getOnlyElement; import static org.junit.platform.commons.util.CollectionUtils.getOnlyElement;
import static org.junit.platform.engine.discovery.DiscoverySelectors.selectUniqueId; import static org.junit.platform.engine.discovery.DiscoverySelectors.selectUniqueId;
import static org.junit.vintage.engine.VintageUniqueIdBuilder.engineId; import static org.junit.vintage.engine.VintageUniqueIdBuilder.engineId;
import static org.junit.vintage.engine.descriptor.VintageTestDescriptor.ENGINE_ID;


import java.util.Set; import java.util.Set;
import java.util.logging.Level; import java.util.logging.Level;
Expand All @@ -31,54 +32,64 @@
*/ */
class UniqueIdSelectorResolverTests { class UniqueIdSelectorResolverTests {


private RecordCollectingLogger logger = new RecordCollectingLogger();
private TestClassCollector collector = new TestClassCollector();

@Test @Test
void logsWarningOnUnloadableTestClass() { void logsWarningOnUnloadableTestClass() {
UniqueId uniqueId = VintageUniqueIdBuilder.uniqueIdForClass("foo.bar.UnknownClass"); UniqueId uniqueId = VintageUniqueIdBuilder.uniqueIdForClass("foo.bar.UnknownClass");
RecordCollectingLogger logger = new RecordCollectingLogger();
UniqueIdSelector selector = selectUniqueId(uniqueId); UniqueIdSelector selector = selectUniqueId(uniqueId);
TestClassCollector collector = new TestClassCollector();


new UniqueIdSelectorResolver(logger).resolve(selector, collector); new UniqueIdSelectorResolver(logger).resolve(selector, collector);


Set<TestClassRequest> requests = collector.toRequests(c -> true); assertNoRequests();
assertThat(requests).isEmpty(); assertLoggedWarning("Unresolvable Unique ID (" + uniqueId + "): Unknown class foo.bar.UnknownClass");
assertThat(logger.getLogRecords()).hasSize(1);
LogRecord logRecord = getOnlyElement(logger.getLogRecords());
assertEquals(Level.WARNING, logRecord.getLevel());
assertEquals("Unresolvable Unique ID (" + uniqueId + "): Unknown class foo.bar.UnknownClass",
logRecord.getMessage());
} }


@Test @Test
void logsWarningForEngineUniqueId() { void logsWarningForEngineUniqueId() {
String uniqueId = engineId().toString(); String uniqueId = engineId().toString();
RecordCollectingLogger logger = new RecordCollectingLogger();
UniqueIdSelector selector = selectUniqueId(uniqueId); UniqueIdSelector selector = selectUniqueId(uniqueId);
TestClassCollector collector = new TestClassCollector();


new UniqueIdSelectorResolver(logger).resolve(selector, collector); new UniqueIdSelectorResolver(logger).resolve(selector, collector);


Set<TestClassRequest> requests = collector.toRequests(c -> true); assertNoRequests();
assertThat(requests).isEmpty(); assertLoggedWarning("Unresolvable Unique ID (" + engineId() + "): Cannot resolve the engine's unique ID");
assertThat(logger.getLogRecords()).hasSize(1);
LogRecord logRecord = getOnlyElement(logger.getLogRecords());
assertEquals(Level.WARNING, logRecord.getLevel());
assertEquals("Unresolvable Unique ID (" + engineId() + "): Cannot resolve the engine's unique ID",
logRecord.getMessage());
} }


@Test @Test
void ignoresUniqueIdsOfOtherEngines() { void ignoresUniqueIdsOfOtherEngines() {
UniqueId uniqueId = UniqueId.forEngine("someEngine"); UniqueId uniqueId = UniqueId.forEngine("someEngine");
RecordCollectingLogger logger = new RecordCollectingLogger();
UniqueIdSelector selector = selectUniqueId(uniqueId); UniqueIdSelector selector = selectUniqueId(uniqueId);
TestClassCollector collector = new TestClassCollector();


new UniqueIdSelectorResolver(logger).resolve(selector, collector); new UniqueIdSelectorResolver(logger).resolve(selector, collector);


assertNoRequests();
assertThat(logger.getLogRecords()).isEmpty();
}

@Test
void logsWarningOnUnexpectedTestDescriptor() {
UniqueId uniqueId = UniqueId.forEngine(ENGINE_ID).append("wrong-type", "foo:bar");
UniqueIdSelector selector = selectUniqueId(uniqueId);

new UniqueIdSelectorResolver(logger).resolve(selector, collector);

assertNoRequests();
assertLoggedWarning("Unresolvable Unique ID (" + uniqueId
+ "): Unique ID segment after engine segment must be of type \"runner\"");
}

private void assertLoggedWarning(String expectedMessage) {
assertThat(logger.getLogRecords()).hasSize(1);
LogRecord logRecord = getOnlyElement(logger.getLogRecords());
assertEquals(Level.WARNING, logRecord.getLevel());
assertEquals(expectedMessage, logRecord.getMessage());
}

private void assertNoRequests() {
Set<TestClassRequest> requests = collector.toRequests(c -> true); Set<TestClassRequest> requests = collector.toRequests(c -> true);
assertThat(requests).isEmpty(); assertThat(requests).isEmpty();
assertThat(logger.getLogRecords()).isEmpty();
} }


} }

0 comments on commit 42d9dab

Please sign in to comment.