Skip to content

Commit

Permalink
Cleanup JavaDoc and Polish
Browse files Browse the repository at this point in the history
Issue: #54
  • Loading branch information
bechte committed Jan 18, 2016
1 parent ab12dc8 commit 9339706
Show file tree
Hide file tree
Showing 17 changed files with 59 additions and 89 deletions.
Expand Up @@ -17,6 +17,5 @@
* @see EngineDiscoveryRequest
*/
public interface DiscoverySelector {

void accept(DiscoverySelectorVisitor visitor);
}
Expand Up @@ -16,6 +16,7 @@

import org.junit.gen5.engine.DiscoveryFilter;
import org.junit.gen5.engine.DiscoverySelector;
import org.junit.gen5.engine.GenericFilter;

/**
* The {@code DiscoveryRequestBuilder} provides a light-weight DSL for
Expand All @@ -26,31 +27,31 @@
* <pre style="code">
* DiscoveryRequestBuilder.request()
* .select(
* packageName("org.junit.gen5"),
* packageName("com.junit.samples"),
* testClass(TestDescriptorTests.class),
* testClassByName("com.junit.samples.SampleTestCase"),
* testMethod("com.junit.samples.SampleTestCase", "test2"),
* testMethod(TestDescriptorTests.class, "test1"),
* testMethod(TestDescriptorTests.class, "test1"),
* testMethod(TestDescriptorTests.class, "testWithParams", ParameterType.class),
* testMethod(TestDescriptorTests.class, testMethod),
* path("/my/local/path1"),
* path("/my/local/path2"),
* uniqueId("unique-id-1"),
* uniqueId("unique-id-2")
* forPackageName("org.junit.gen5"),
* forPackageName("com.junit.samples"),
* forClass(TestDescriptorTests.class),
* forClassName("com.junit.samples.SampleTestCase"),
* forTestMethod("com.junit.samples.SampleTestCase", "test2"),
* forTestMethod(TestDescriptorTests.class, "test1"),
* forTestMethod(TestDescriptorTests.class, "test1"),
* forTestMethod(TestDescriptorTests.class, "testWithParams", ParameterType.class),
* forTestMethod(TestDescriptorTests.class, testMethod),
* forPath("/my/local/path1"),
* forPath("/my/local/path2"),
* forUniqueId("unique-id-1"),
* forUniqueId("unique-id-2")
* )
* .filterBy(engineIds("junit5"))
* .filterBy(classNamePattern("org.junit.gen5.tests"), classNamePattern("org.junit.sample"))
* .filterBy(tagsIncluded("Fast"), tagsExcluded("Slow"))
* .filter(byEngineIds("junit5"))
* .filter(byNamePattern("org.junit.gen5.tests"), byNamePattern("org.junit.sample"))
* .filter(includeTags("Fast"), excludeTags("Slow"))
* ).build();
* </pre>
*/
public final class DiscoveryRequestBuilder {
private List<DiscoverySelector> selectors = new LinkedList<>();
private List<EngineIdFilter> engineIdFilters = new LinkedList<>();
private List<DiscoveryFilter<?>> filters = new LinkedList<>();
private List<PostDiscoveryFilter> postFilters = new LinkedList<>();
private List<DiscoveryFilter<?>> discoveryFilters = new LinkedList<>();
private List<PostDiscoveryFilter> postDiscoveryFilters = new LinkedList<>();

public static DiscoveryRequestBuilder request() {
return new DiscoveryRequestBuilder();
Expand All @@ -70,34 +71,31 @@ public DiscoveryRequestBuilder select(List<DiscoverySelector> elements) {
return this;
}

public DiscoveryRequestBuilder filterBy(EngineIdFilter... filters) {
public DiscoveryRequestBuilder filter(GenericFilter<?>... filters) {

This comment has been minimized.

Copy link
@marcphilipp

marcphilipp Jan 18, 2016

Member

I think the three methods were better than this "generic" one. No need for instanceof and safer (someone might just implement GenericFilter and call filter with it).

This comment has been minimized.

Copy link
@bechte

bechte Jan 19, 2016

Author Contributor

I totally agree, that we have to investigate some more time to come up with a better solution here. But the three methods do not solve the issue. Actually, someone might just implement DiscoveryFilter<?> and call filter with it. So we still have the same issue, that we might have filters, that we will not support internally.

I just don't like, that we force the user to call filter several times, instead of making use of more generic implementation and print a warning if a filter is not accepted (this is the missing part).

Currently, I would state that we do not support "custom filters" but only the ones provided and we need to spend some time to make the filter mechanism more flexible. In my opinion, this need not to be part of alpha1. Let's talk about it later.

if (filters != null) {
this.engineIdFilters.addAll(Arrays.asList(filters));
Arrays.stream(filters).forEach(this::storeFilter);
}
return this;
}

public DiscoveryRequestBuilder filterBy(DiscoveryFilter<?>... filters) {
if (filters != null) {
this.filters.addAll(Arrays.asList(filters));
private void storeFilter(GenericFilter<?> filter) {
if (filter instanceof EngineIdFilter) {
this.engineIdFilters.add((EngineIdFilter) filter);
}
return this;
}

public DiscoveryRequestBuilder filterBy(PostDiscoveryFilter... filters) {
if (filters != null) {
this.postFilters.addAll(Arrays.asList(filters));
else if (filter instanceof PostDiscoveryFilter) {
this.postDiscoveryFilters.add((PostDiscoveryFilter) filter);
}
else if (filter instanceof DiscoveryFilter<?>) {
this.discoveryFilters.add((DiscoveryFilter<?>) filter);
}
return this;
}

public DiscoveryRequest build() {
DiscoveryRequest discoveryRequest = new DiscoveryRequest();
discoveryRequest.addSelectors(this.selectors);
discoveryRequest.addEngineIdFilters(this.engineIdFilters);
discoveryRequest.addFilters(this.filters);
discoveryRequest.addPostFilters(this.postFilters);
discoveryRequest.addFilters(this.discoveryFilters);
discoveryRequest.addPostFilters(this.postDiscoveryFilters);
return discoveryRequest;
}

}
25 changes: 12 additions & 13 deletions junit-launcher/src/main/java/org/junit/gen5/launcher/Launcher.java
Expand Up @@ -13,12 +13,7 @@
import java.util.Map;
import java.util.logging.Logger;

import org.junit.gen5.engine.EngineExecutionListener;
import org.junit.gen5.engine.ExecutionRequest;
import org.junit.gen5.engine.FilterResult;
import org.junit.gen5.engine.TestDescriptor;
import org.junit.gen5.engine.TestEngine;
import org.junit.gen5.engine.TestExecutionResult;
import org.junit.gen5.engine.*;

/**
* Facade for <em>discovering</em> and <em>executing</em> tests using
Expand Down Expand Up @@ -50,20 +45,24 @@
* @see TestExecutionListener
*/
public class Launcher {

private static final Logger LOG = Logger.getLogger(Launcher.class.getName());

private final TestExecutionListenerRegistry listenerRegistry = new TestExecutionListenerRegistry();

private final TestEngineRegistry testEngineRegistry;
private final TestExecutionListenerRegistry testExecutionListenerRegistry;

public Launcher() {
this(new ServiceLoaderTestEngineRegistry());
this(new ServiceLoaderTestEngineRegistry(), new TestExecutionListenerRegistry());
}

// for tests only
// For tests only
Launcher(TestEngineRegistry testEngineRegistry) {
this(testEngineRegistry, new TestExecutionListenerRegistry());
}

// For tests only
Launcher(TestEngineRegistry testEngineRegistry, TestExecutionListenerRegistry testExecutionListenerRegistry) {
this.testEngineRegistry = testEngineRegistry;
this.testExecutionListenerRegistry = testExecutionListenerRegistry;
}

/**
Expand All @@ -72,7 +71,7 @@ public Launcher() {
* @param listeners the listeners to be notified of test execution events
*/
public void registerTestExecutionListeners(TestExecutionListener... listeners) {
listenerRegistry.registerListener(listeners);
testExecutionListenerRegistry.registerListener(listeners);
}

/**
Expand Down Expand Up @@ -123,7 +122,7 @@ private Root discoverRoot(DiscoveryRequest discoveryRequest, String phase) {

private void execute(Root root) {
TestPlan testPlan = TestPlan.from(root);
TestExecutionListener testExecutionListener = listenerRegistry.getCompositeTestExecutionListener();
TestExecutionListener testExecutionListener = testExecutionListenerRegistry.getCompositeTestExecutionListener();
testExecutionListener.testPlanExecutionStarted(testPlan);
ExecutionListenerAdapter engineExecutionListener = new ExecutionListenerAdapter(testPlan,
testExecutionListener);
Expand Down
25 changes: 12 additions & 13 deletions junit-launcher/src/main/java/org/junit/gen5/launcher/Root.java
Expand Up @@ -22,17 +22,20 @@
* @since 5.0
*/
final class Root {

private final Map<TestEngine, TestDescriptor> testEngines = new LinkedHashMap<>();

Iterable<TestEngine> getTestEngines() {
return testEngines.keySet();
}

public void add(TestEngine engine, TestDescriptor testDescriptor) {
testEngines.put(engine, testDescriptor);
}

public Collection<TestDescriptor> getAllTestDescriptors() {
return testEngines.values();
}

Iterable<TestEngine> getTestEngines() {
return testEngines.keySet();
}

TestDescriptor getTestDescriptorFor(TestEngine testEngine) {
return testEngines.get(testEngine);
}
Expand All @@ -47,6 +50,10 @@ void applyFilters(DiscoveryRequest discoveryRequest) {
accept(filteringVisitor);
}

void accept(Visitor visitor) {
testEngines.values().stream().forEach(testEngine -> testEngine.accept(visitor));
}

void prune() {
Visitor pruningVisitor = (descriptor, remove) -> {
if (descriptor.isRoot() || descriptor.hasTests())
Expand All @@ -56,12 +63,4 @@ void prune() {
accept(pruningVisitor);
testEngines.values().removeIf(testEngine -> testEngine.getChildren().isEmpty());
}

void accept(Visitor visitor) {
testEngines.values().stream().forEach(testEngine -> testEngine.accept(visitor));
}

public Collection<TestDescriptor> getAllTestDescriptors() {
return testEngines.values();
}
}
Expand Up @@ -18,7 +18,6 @@
import org.junit.gen5.engine.TestTag;

public class TagFilter {

public static PostDiscoveryFilter includeTags(String... tagNames) {
return includeTags(asList(tagNames));
}
Expand Down
Expand Up @@ -16,7 +16,5 @@
* @since 5.0
*/
interface TestEngineRegistry {

Iterable<TestEngine> getTestEngines();

}
Expand Up @@ -42,7 +42,6 @@
* @see TestIdentifier
*/
public interface TestExecutionListener {

/**
* Called when additional test reporting data has been published for
* the supplied {@link TestIdentifier}. Can be called at all times.
Expand Down Expand Up @@ -146,5 +145,4 @@ default void executionStarted(TestIdentifier testIdentifier) {
*/
default void executionFinished(TestIdentifier testIdentifier, TestExecutionResult testExecutionResult) {
}

}
Expand Up @@ -20,7 +20,6 @@
* @since 5.0
*/
class TestExecutionListenerRegistry {

private final List<TestExecutionListener> testExecutionListeners = new LinkedList<>();

void registerListener(TestExecutionListener... listeners) {
Expand Down
Expand Up @@ -21,7 +21,6 @@
* @since 5.0
*/
public final class TestId implements Serializable {

private static final long serialVersionUID = 1L;

private final String uniqueId;
Expand All @@ -48,5 +47,4 @@ public int hashCode() {
public String toString() {
return this.uniqueId;
}

}
Expand Up @@ -30,17 +30,14 @@
* @see TestPlan
*/
public final class TestIdentifier implements Serializable {

private static final long serialVersionUID = 1L;

private final TestId uniqueId;
private final String displayName;
private final TestSource source;
private final Set<TestTag> tags;

private final boolean test;
private final boolean container;

private final TestId parentId;

static TestIdentifier from(TestDescriptor testDescriptor) {
Expand Down Expand Up @@ -149,5 +146,4 @@ public int hashCode() {
public String toString() {
return getDisplayName() + " [" + uniqueId + "]";
}

}
Expand Up @@ -46,7 +46,6 @@
* @see TestExecutionListener
*/
public final class TestPlan {

private final Set<TestIdentifier> roots = new LinkedHashSet<>();
private final Map<TestId, LinkedHashSet<TestIdentifier>> children = new LinkedHashMap<>();
private final Map<TestId, TestIdentifier> allIdentifiers = new LinkedHashMap<>();
Expand Down
Expand Up @@ -31,7 +31,6 @@
* @see LoggingListener#LoggingListener(BiConsumer)
*/
public class LoggingListener implements TestExecutionListener {

/**
* Create a {@code LoggingListener} which delegates to a
* {@link java.util.logging.Logger} using a log level of
Expand Down Expand Up @@ -107,5 +106,4 @@ private void log(String message, Object... args) {
private void logWithThrowable(String message, Throwable t, Object... args) {
logger.accept(t, () -> String.format(message, args));
}

}
Expand Up @@ -30,9 +30,7 @@
* @see #getSummary()
*/
public class SummaryGeneratingListener implements TestExecutionListener {

private TestPlan testPlan;

private TestExecutionSummary summary;

/**
Expand Down Expand Up @@ -92,5 +90,4 @@ else if (testExecutionResult.getStatus() == FAILED) {
}
testExecutionResult.getThrowable().ifPresent(throwable -> summary.addFailure(testIdentifier, throwable));
}

}
Expand Up @@ -25,9 +25,6 @@
*/
// TODO Design a real interface for TestExecutionSummary and make it threadsafe.
public class TestExecutionSummary {

private final TestPlan testPlan;

final AtomicLong testsStarted = new AtomicLong();
final AtomicLong testsFound = new AtomicLong();
final AtomicLong testsSkipped = new AtomicLong();
Expand All @@ -38,6 +35,7 @@ public class TestExecutionSummary {
long timeStarted;
long timeFinished;

private final TestPlan testPlan;
private String message;
private List<Failure> failures = new ArrayList<>();

Expand Down Expand Up @@ -108,7 +106,6 @@ public void addFailure(TestIdentifier testIdentifier, Throwable throwable) {
}

static class Failure {

private final TestIdentifier testIdentifier;
private final Throwable exception;

Expand All @@ -124,7 +121,5 @@ public TestIdentifier getTestIdentifier() {
public Throwable getException() {
return exception;
}

}

}
Expand Up @@ -213,7 +213,7 @@ void resolvesClasspathSelector() throws Exception {
void resolvesApplyingClassFilters() throws Exception {
File root = getClasspathRoot(PlainJUnit4TestCaseWithSingleTestWhichFails.class);

DiscoveryRequest discoveryRequest = request().select(forPaths(singleton(root))).filterBy(
DiscoveryRequest discoveryRequest = request().select(forPaths(singleton(root))).filter(
ClassFilter.byNamePattern(".*JUnit4.*"), ClassFilter.byNamePattern(".*Plain.*")).build();

TestDescriptor engineDescriptor = engine.discoverTests(discoveryRequest);
Expand Down Expand Up @@ -514,7 +514,7 @@ void doesNotResolveMethodOfClassNotAcceptedByClassFilter() throws Exception {
// @formatter:off
DiscoveryRequest request = request()
.select(MethodSelector.forMethod(testClass, testClass.getMethod("failingTest")))
.filterBy(ClassFilter.byNamePattern("Foo"))
.filter(ClassFilter.byNamePattern("Foo"))
.build();
// @formatter:on

Expand Down

0 comments on commit 9339706

Please sign in to comment.