Skip to content

Commit

Permalink
Revise factory APIs in concrete Filters
Browse files Browse the repository at this point in the history
  • Loading branch information
sbrannen committed May 31, 2016
1 parent c8e52a7 commit 7c12038
Show file tree
Hide file tree
Showing 13 changed files with 53 additions and 43 deletions.
6 changes: 3 additions & 3 deletions documentation/src/test/java/example/UsingTheLauncherDemo.java
Expand Up @@ -12,7 +12,7 @@


// tag::imports[] // tag::imports[]


import static org.junit.gen5.engine.discovery.ClassFilter.byNamePattern; import static org.junit.gen5.engine.discovery.ClassFilter.byClassNamePattern;
import static org.junit.gen5.engine.discovery.ClassSelector.selectClass; import static org.junit.gen5.engine.discovery.ClassSelector.selectClass;
import static org.junit.gen5.engine.discovery.PackageSelector.selectPackage; import static org.junit.gen5.engine.discovery.PackageSelector.selectPackage;


Expand Down Expand Up @@ -41,7 +41,7 @@ void discovery() {
selectPackage("com.mycompany.mytests"), selectPackage("com.mycompany.mytests"),
selectClass(MyTestClass.class) selectClass(MyTestClass.class)
) )
.filter(byNamePattern(".*Test")) .filters(byClassNamePattern(".*Test"))
.build(); .build();


TestPlan plan = LauncherFactory.create().discover(specification); TestPlan plan = LauncherFactory.create().discover(specification);
Expand All @@ -58,7 +58,7 @@ void execution() {
selectPackage("com.mycompany.mytests"), selectPackage("com.mycompany.mytests"),
selectClass(MyTestClass.class) selectClass(MyTestClass.class)
) )
.filter(byNamePattern(".*Test")) .filters(byClassNamePattern(".*Test"))
.build(); .build();


Launcher launcher = LauncherFactory.create(); Launcher launcher = LauncherFactory.create();
Expand Down
Expand Up @@ -13,6 +13,9 @@
import static java.util.stream.Collectors.toCollection; import static java.util.stream.Collectors.toCollection;
import static org.junit.gen5.engine.discovery.ClasspathSelector.selectClasspathRoots; import static org.junit.gen5.engine.discovery.ClasspathSelector.selectClasspathRoots;
import static org.junit.gen5.engine.discovery.NameBasedSelector.selectNames; import static org.junit.gen5.engine.discovery.NameBasedSelector.selectNames;
import static org.junit.gen5.launcher.EngineIdFilter.includeEngineId;
import static org.junit.gen5.launcher.TagFilter.excludeTags;
import static org.junit.gen5.launcher.TagFilter.requireTags;
import static org.junit.gen5.launcher.main.TestDiscoveryRequestBuilder.request; import static org.junit.gen5.launcher.main.TestDiscoveryRequestBuilder.request;


import java.io.File; import java.io.File;
Expand All @@ -23,8 +26,6 @@
import org.junit.gen5.commons.util.ReflectionUtils; import org.junit.gen5.commons.util.ReflectionUtils;
import org.junit.gen5.console.options.CommandLineOptions; import org.junit.gen5.console.options.CommandLineOptions;
import org.junit.gen5.engine.discovery.ClassFilter; import org.junit.gen5.engine.discovery.ClassFilter;
import org.junit.gen5.launcher.EngineIdFilter;
import org.junit.gen5.launcher.TagFilter;
import org.junit.gen5.launcher.TestDiscoveryRequest; import org.junit.gen5.launcher.TestDiscoveryRequest;


/** /**
Expand Down Expand Up @@ -67,14 +68,16 @@ private TestDiscoveryRequest buildNameBasedDiscoveryRequest(CommandLineOptions o
} }


private void applyFilters(TestDiscoveryRequest discoveryRequest, CommandLineOptions options) { private void applyFilters(TestDiscoveryRequest discoveryRequest, CommandLineOptions options) {
options.getClassnameFilter().ifPresent(regex -> discoveryRequest.addFilter(ClassFilter.byNamePattern(regex))); options.getClassnameFilter().ifPresent(
regex -> discoveryRequest.addFilter(ClassFilter.byClassNamePattern(regex)));
if (!options.getRequiredTagsFilter().isEmpty()) { if (!options.getRequiredTagsFilter().isEmpty()) {
discoveryRequest.addPostFilter(TagFilter.requireTags(options.getRequiredTagsFilter())); discoveryRequest.addPostFilter(requireTags(options.getRequiredTagsFilter()));
} }
if (!options.getExcludedTagsFilter().isEmpty()) { if (!options.getExcludedTagsFilter().isEmpty()) {
discoveryRequest.addPostFilter(TagFilter.excludeTags(options.getExcludedTagsFilter())); discoveryRequest.addPostFilter(excludeTags(options.getExcludedTagsFilter()));
} }
options.getRequiredEngineFilter().ifPresent( options.getRequiredEngineFilter().ifPresent(
engineId -> discoveryRequest.addEngineIdFilter(EngineIdFilter.from(engineId))); engineId -> discoveryRequest.addEngineIdFilter(includeEngineId(engineId)));
} }

} }
Expand Up @@ -19,7 +19,7 @@
* {@link DiscoveryFilter} that is applied to a {@link Class}. * {@link DiscoveryFilter} that is applied to a {@link Class}.
* *
* @since 5.0 * @since 5.0
* @see #byNamePattern(String) * @see #byClassNamePattern(String)
*/ */
@API(Experimental) @API(Experimental)
public interface ClassFilter extends DiscoveryFilter<Class<?>> { public interface ClassFilter extends DiscoveryFilter<Class<?>> {
Expand All @@ -34,7 +34,7 @@ public interface ClassFilter extends DiscoveryFilter<Class<?>> {
* class names; never {@code null} or empty * class names; never {@code null} or empty
* @see Class#getName() * @see Class#getName()
*/ */
static ClassFilter byNamePattern(String pattern) { static ClassFilter byClassNamePattern(String pattern) {
return new ClassNameFilter(pattern); return new ClassNameFilter(pattern);
} }


Expand Down
Expand Up @@ -23,21 +23,22 @@
* An {@code EngineIdFilter} is applied to engine IDs before * An {@code EngineIdFilter} is applied to engine IDs before
* {@link TestEngine TestEngines} are executed. * {@link TestEngine TestEngines} are executed.
* *
* A {@code TestEngine} with a matching engine ID will be <em>included</em>
* within the test discovery and execution.
*
* @since 5.0 * @since 5.0
* @see TestDiscoveryRequest * @see TestDiscoveryRequest
*/ */
@API(Experimental) @API(Experimental)
public class EngineIdFilter implements Filter<String> { public class EngineIdFilter implements Filter<String> {


/** /**
* Create a new {@code EngineIdFilter} based on the supplied engine ID. * Create a new <em>include</em> {@code EngineIdFilter} based on the
* supplied engine ID.
*
* <p>A {@code TestEngine} with a matching engine ID will be
* <em>included</em> within the test discovery and execution.
* *
* @param engineId the engine ID to match against; never {@code null} or empty * @param engineId the engine ID to match against; never {@code null} or empty
*/ */
public static EngineIdFilter from(String engineId) { public static EngineIdFilter includeEngineId(String engineId) {

This comment has been minimized.

Copy link
@marcphilipp

marcphilipp May 31, 2016

Member

In the light of the current semantics we should call this method requireEngineId.

Edit: see e760992.

This comment has been minimized.

Copy link
@sbrannen

sbrannen May 31, 2016

Author Member

Agreed: I'll make that change and create a follow-up issue to address exclusion of engine IDs.

This comment has been minimized.

Copy link
@marcphilipp

marcphilipp May 31, 2016

Member

Thanks! 👍

Preconditions.notBlank(engineId, "engine ID must not be null or empty"); Preconditions.notBlank(engineId, "engine ID must not be null or empty");
return new EngineIdFilter(engineId.trim()); return new EngineIdFilter(engineId.trim());
} }
Expand Down
Expand Up @@ -49,9 +49,12 @@
* selectUniqueId("unique-id-1"), * selectUniqueId("unique-id-1"),
* selectUniqueId("unique-id-2") * selectUniqueId("unique-id-2")
* ) * )
* .filter(byEngineIds("junit5")) * .filters(
* .filter(byNamePattern("org\.junit\.gen5\.tests.*"), byNamePattern(".*Test[s]?")) * includeEngineId("junit5"),
* .filter(requireTags("fast"), excludeTags("flow")) * byClassNamePattern("org\.junit\.gen5\.tests.*"),
* byClassNamePattern(".*Test[s]?"),
* requireTags("fast"),
* excludeTags("slow"))
* .configurationParameter("key1", "value1") * .configurationParameter("key1", "value1")
* .configurationParameters(configParameterMap) * .configurationParameters(configParameterMap)
* ).build(); * ).build();
Expand Down Expand Up @@ -98,7 +101,7 @@ public TestDiscoveryRequestBuilder selectors(List<DiscoverySelector> selectors)
/** /**
* Add all of the supplied {@code filters} to the request. * Add all of the supplied {@code filters} to the request.
*/ */
public TestDiscoveryRequestBuilder filter(Filter<?>... filters) { public TestDiscoveryRequestBuilder filters(Filter<?>... filters) {
if (filters != null) { if (filters != null) {
Arrays.stream(filters).forEach(this::storeFilter); Arrays.stream(filters).forEach(this::storeFilter);
} }
Expand Down
Expand Up @@ -42,15 +42,15 @@ void composingNoFiltersCreatesFilterThatIncludesEverything() {


@Test @Test
void composingSingleFilterWillReturnTheOriginalOne() { void composingSingleFilterWillReturnTheOriginalOne() {
Filter<Class<?>> singleFilter = ClassFilter.byNamePattern(".*ring.*"); Filter<Class<?>> singleFilter = ClassFilter.byClassNamePattern(".*ring.*");
Filter<Class<?>> composed = Filter.composeFilters(singleFilter); Filter<Class<?>> composed = Filter.composeFilters(singleFilter);
assertSame(singleFilter, composed); assertSame(singleFilter, composed);
} }


@Test @Test
void composingMultipleFiltersIsAConjunctionOfFilters() { void composingMultipleFiltersIsAConjunctionOfFilters() {
Filter<Class<?>> firstFilter = ClassFilter.byNamePattern(".*ring.*"); Filter<Class<?>> firstFilter = ClassFilter.byClassNamePattern(".*ring.*");
Filter<Class<?>> secondFilter = ClassFilter.byNamePattern(".*Join.*"); Filter<Class<?>> secondFilter = ClassFilter.byClassNamePattern(".*Join.*");


Filter<Class<?>> composed = Filter.composeFilters(firstFilter, secondFilter); Filter<Class<?>> composed = Filter.composeFilters(firstFilter, secondFilter);


Expand Down
Expand Up @@ -27,7 +27,7 @@ class ClassFilterTests {
void classNameMatches() { void classNameMatches() {
String regex = "^java\\.lang\\..*"; String regex = "^java\\.lang\\..*";


ClassFilter filter = ClassFilter.byNamePattern(regex); ClassFilter filter = ClassFilter.byClassNamePattern(regex);


assertEquals("Filter class names with regular expression: " + regex, filter.toString()); assertEquals("Filter class names with regular expression: " + regex, filter.toString());
assertTrue(filter.apply(String.class).included()); assertTrue(filter.apply(String.class).included());
Expand Down
Expand Up @@ -224,8 +224,8 @@ void resolvesClasspathSelector() throws Exception {
void resolvesApplyingClassFilters() throws Exception { void resolvesApplyingClassFilters() throws Exception {
File root = getClasspathRoot(PlainJUnit4TestCaseWithSingleTestWhichFails.class); File root = getClasspathRoot(PlainJUnit4TestCaseWithSingleTestWhichFails.class);


TestDiscoveryRequest discoveryRequest = request().selectors(selectClasspathRoots(singleton(root))).filter( TestDiscoveryRequest discoveryRequest = request().selectors(selectClasspathRoots(singleton(root))).filters(
ClassFilter.byNamePattern(".*JUnit4.*"), ClassFilter.byNamePattern(".*Plain.*")).build(); ClassFilter.byClassNamePattern(".*JUnit4.*"), ClassFilter.byClassNamePattern(".*Plain.*")).build();


TestDescriptor engineDescriptor = discoverTests(discoveryRequest); TestDescriptor engineDescriptor = discoverTests(discoveryRequest);


Expand Down Expand Up @@ -514,7 +514,7 @@ void doesNotResolveMethodOfClassNotAcceptedByClassFilter() throws Exception {
// @formatter:off // @formatter:off
TestDiscoveryRequest request = request() TestDiscoveryRequest request = request()
.selectors(MethodSelector.selectMethod(testClass, testClass.getMethod("failingTest"))) .selectors(MethodSelector.selectMethod(testClass, testClass.getMethod("failingTest")))
.filter(ClassFilter.byNamePattern("Foo")) .filters(ClassFilter.byClassNamePattern("Foo"))
.build(); .build();
// @formatter:on // @formatter:on


Expand Down
Expand Up @@ -41,7 +41,7 @@ void logsWarningWhenFilterExcludesClass() {
// @formatter:off // @formatter:off
EngineDiscoveryRequest request = request() EngineDiscoveryRequest request = request()
.selectors(selectClass(Foo.class), selectClass(Bar.class)) .selectors(selectClass(Foo.class), selectClass(Bar.class))
.filter(filter) .filters(filter)
.build(); .build();
// @formatter:on // @formatter:on


Expand Down
Expand Up @@ -38,7 +38,8 @@ class DiscoveryFilterApplierTests {
@Test @Test
void nonMatchingClassesAreExcluded() { void nonMatchingClassesAreExcluded() {


EngineDiscoveryRequest request = request().filter(ClassFilter.byNamePattern(".*\\$MatchingClass")).build(); EngineDiscoveryRequest request = request().filters(
ClassFilter.byClassNamePattern(".*\\$MatchingClass")).build();


// @formatter:off // @formatter:off
TestDescriptor engineDescriptor = engineDescriptor() TestDescriptor engineDescriptor = engineDescriptor()
Expand All @@ -59,7 +60,8 @@ void nonMatchingClassesAreExcluded() {


@Test @Test
void nestedTestClassesAreAlwaysIncludedWhenTheirParentIs() { void nestedTestClassesAreAlwaysIncludedWhenTheirParentIs() {
EngineDiscoveryRequest request = request().filter(ClassFilter.byNamePattern(".*\\$MatchingClass")).build(); EngineDiscoveryRequest request = request().filters(
ClassFilter.byClassNamePattern(".*\\$MatchingClass")).build();


// @formatter:off // @formatter:off
TestDescriptor engineDescriptor = engineDescriptor() TestDescriptor engineDescriptor = engineDescriptor()
Expand Down
Expand Up @@ -15,7 +15,7 @@
import static org.junit.gen5.api.Assertions.assertTrue; import static org.junit.gen5.api.Assertions.assertTrue;
import static org.junit.gen5.api.Assertions.expectThrows; import static org.junit.gen5.api.Assertions.expectThrows;
import static org.junit.gen5.engine.discovery.UniqueIdSelector.selectUniqueId; import static org.junit.gen5.engine.discovery.UniqueIdSelector.selectUniqueId;
import static org.junit.gen5.launcher.EngineIdFilter.from; import static org.junit.gen5.launcher.EngineIdFilter.includeEngineId;
import static org.junit.gen5.launcher.main.LauncherFactoryForTestingPurposesOnly.createLauncher; import static org.junit.gen5.launcher.main.LauncherFactoryForTestingPurposesOnly.createLauncher;
import static org.junit.gen5.launcher.main.TestDiscoveryRequestBuilder.request; import static org.junit.gen5.launcher.main.TestDiscoveryRequestBuilder.request;


Expand Down Expand Up @@ -115,8 +115,8 @@ void launcherWillNotCallEnginesThatAreFilteredByAnEngineIdFilter() {
DefaultLauncher launcher = createLauncher(firstEngine, secondEngine); DefaultLauncher launcher = createLauncher(firstEngine, secondEngine);


TestPlan testPlan = launcher.discover( TestPlan testPlan = launcher.discover(
request().selectors(selectUniqueId(test1.getUniqueId()), selectUniqueId(test2.getUniqueId())).filter( request().selectors(selectUniqueId(test1.getUniqueId()), selectUniqueId(test2.getUniqueId())).filters(
from("first")).build()); includeEngineId("first")).build());


assertThat(testPlan.getRoots()).hasSize(1); assertThat(testPlan.getRoots()).hasSize(1);
TestIdentifier rootIdentifier = testPlan.getRoots().iterator().next(); TestIdentifier rootIdentifier = testPlan.getRoots().iterator().next();
Expand All @@ -141,7 +141,7 @@ void launcherAppliesPostDiscoveryFilters() {
TestPlan testPlan = launcher.discover( // TestPlan testPlan = launcher.discover( //
request() // request() //
.selectors(PackageSelector.selectPackage("any")) // .selectors(PackageSelector.selectPackage("any")) //
.filter(includeWithUniqueIdContainsTest, includeWithUniqueIdContains1) // .filters(includeWithUniqueIdContainsTest, includeWithUniqueIdContains1) //
.build()); .build());


assertThat(testPlan.getChildren(UniqueId.forEngine("myEngine").toString())).hasSize(1); assertThat(testPlan.getChildren(UniqueId.forEngine("myEngine").toString())).hasSize(1);
Expand Down
Expand Up @@ -19,6 +19,7 @@
import static org.junit.gen5.engine.discovery.MethodSelector.selectMethod; import static org.junit.gen5.engine.discovery.MethodSelector.selectMethod;
import static org.junit.gen5.engine.discovery.PackageSelector.selectPackage; import static org.junit.gen5.engine.discovery.PackageSelector.selectPackage;
import static org.junit.gen5.engine.discovery.UniqueIdSelector.selectUniqueId; import static org.junit.gen5.engine.discovery.UniqueIdSelector.selectUniqueId;
import static org.junit.gen5.launcher.EngineIdFilter.includeEngineId;
import static org.junit.gen5.launcher.main.TestDiscoveryRequestBuilder.request; import static org.junit.gen5.launcher.main.TestDiscoveryRequestBuilder.request;


import java.io.File; import java.io.File;
Expand Down Expand Up @@ -181,9 +182,9 @@ class DiscoveryFilterTests {
public void engineFiltersAreStoredInDiscoveryRequest() throws Exception { public void engineFiltersAreStoredInDiscoveryRequest() throws Exception {
// @formatter:off // @formatter:off
TestDiscoveryRequest discoveryRequest = request() TestDiscoveryRequest discoveryRequest = request()
.filter( .filters(
EngineIdFilter.from("engine1"), includeEngineId("engine1"),
EngineIdFilter.from("engine2") includeEngineId("engine2")

This comment has been minimized.

Copy link
@marcphilipp

marcphilipp May 31, 2016

Member

It does not actually make sense to add multiple EngineIdFilters to an request because DefaultLauncher will reject any engine that is excluded by at least one filter... which means that it will not execute any engine.

Edit: see e760992.

This comment has been minimized.

Copy link
@sbrannen

sbrannen May 31, 2016

Author Member

I didn't write that code; I just refactored it. 😉

But I fully agree: we need to overhaul filtering based on Engine IDs.

).build(); ).build();
// @formatter:on // @formatter:on


Expand All @@ -199,7 +200,7 @@ public void discoveryFiltersAreStoredInDiscoveryRequest() throws Exception {


// @formatter:off // @formatter:off
TestDiscoveryRequest discoveryRequest = request() TestDiscoveryRequest discoveryRequest = request()
.filter( .filters(
new DiscoveryFilterStub("filter1"), new DiscoveryFilterStub("filter1"),
new DiscoveryFilterStub("filter2") new DiscoveryFilterStub("filter2")
).build(); ).build();
Expand All @@ -216,7 +217,7 @@ public void postDiscoveryFiltersAreStoredInDiscoveryRequest() throws Exception {


// @formatter:off // @formatter:off
TestDiscoveryRequest discoveryRequest = request() TestDiscoveryRequest discoveryRequest = request()
.filter( .filters(
new PostDiscoveryFilterStub("postFilter1"), new PostDiscoveryFilterStub("postFilter1"),
new PostDiscoveryFilterStub("postFilter2") new PostDiscoveryFilterStub("postFilter2")
).build(); ).build();
Expand All @@ -231,7 +232,7 @@ public void postDiscoveryFiltersAreStoredInDiscoveryRequest() throws Exception {
@Test @Test
public void exceptionForIllegalFilterClass() throws Exception { public void exceptionForIllegalFilterClass() throws Exception {
PreconditionViolationException exception = expectThrows(PreconditionViolationException.class, PreconditionViolationException exception = expectThrows(PreconditionViolationException.class,
() -> request().filter(o -> excluded("reason"))); () -> request().filters(o -> excluded("reason")));


assertEquals("Filter must implement EngineIdFilter, PostDiscoveryFilter, or DiscoveryFilter.", assertEquals("Filter must implement EngineIdFilter, PostDiscoveryFilter, or DiscoveryFilter.",
exception.getMessage()); exception.getMessage());
Expand Down
Expand Up @@ -13,6 +13,7 @@
import static java.util.Arrays.stream; import static java.util.Arrays.stream;
import static java.util.stream.Collectors.toList; import static java.util.stream.Collectors.toList;
import static org.junit.gen5.commons.meta.API.Usage.Maintained; import static org.junit.gen5.commons.meta.API.Usage.Maintained;
import static org.junit.gen5.launcher.EngineIdFilter.includeEngineId;
import static org.junit.gen5.launcher.main.TestDiscoveryRequestBuilder.request; import static org.junit.gen5.launcher.main.TestDiscoveryRequestBuilder.request;


import java.lang.annotation.Annotation; import java.lang.annotation.Annotation;
Expand All @@ -29,7 +30,6 @@
import org.junit.gen5.engine.discovery.ClassSelector; import org.junit.gen5.engine.discovery.ClassSelector;
import org.junit.gen5.engine.discovery.PackageSelector; import org.junit.gen5.engine.discovery.PackageSelector;
import org.junit.gen5.engine.discovery.UniqueIdSelector; import org.junit.gen5.engine.discovery.UniqueIdSelector;
import org.junit.gen5.launcher.EngineIdFilter;
import org.junit.gen5.launcher.Launcher; import org.junit.gen5.launcher.Launcher;
import org.junit.gen5.launcher.PostDiscoveryFilter; import org.junit.gen5.launcher.PostDiscoveryFilter;
import org.junit.gen5.launcher.TagFilter; import org.junit.gen5.launcher.TagFilter;
Expand Down Expand Up @@ -146,7 +146,7 @@ private <T> List<DiscoverySelector> transform(T[] sourceElements, Function<T, Di
private void addClassNameMatchesFilter(TestDiscoveryRequest discoveryRequest) { private void addClassNameMatchesFilter(TestDiscoveryRequest discoveryRequest) {
String regex = getClassNameRegExPattern(); String regex = getClassNameRegExPattern();
if (!regex.isEmpty()) { if (!regex.isEmpty()) {
discoveryRequest.addFilter(ClassFilter.byNamePattern(regex)); discoveryRequest.addFilter(ClassFilter.byClassNamePattern(regex));
} }
} }


Expand All @@ -169,7 +169,7 @@ private void addExcludedTagsFilter(TestDiscoveryRequest discoveryRequest) {
private void addEngineIdFilter(TestDiscoveryRequest discoveryRequest) { private void addEngineIdFilter(TestDiscoveryRequest discoveryRequest) {
String engineId = getExplicitEngineId(); String engineId = getExplicitEngineId();
if (StringUtils.isNotBlank(engineId)) { if (StringUtils.isNotBlank(engineId)) {
discoveryRequest.addEngineIdFilter(EngineIdFilter.from(engineId)); discoveryRequest.addEngineIdFilter(includeEngineId(engineId));
} }
} }


Expand Down

0 comments on commit 7c12038

Please sign in to comment.