Skip to content

Commit

Permalink
replaced blacklisting with always wrapped filter which fallbacks to
Browse files Browse the repository at this point in the history 
original filter (#27)
  • Loading branch information
@aaschmid
aaschmid committed Jun 25, 2014
1 parent 0f7892a commit 8526baf
Show file tree
Hide file tree
Showing 5 changed files with 81 additions and 122 deletions.
1 change: 1 addition & 0 deletions README.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -325,6 +325,7 @@ Release notes

### tbd. (???)

* more fault tolerant filtering instead of explicitly maintain a black or white list ([#27](/../../issues/27))
* ...

### v1.7.0 (20-Jun-2014)
Expand Down
40 changes: 19 additions & 21 deletions src/main/java/com/tngtech/java/junit/dataprovider/DataProviderFilter.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,43 +24,41 @@ public class DataProviderFilter extends Filter {
private static final int GROUP_METHOD_IDX = 3;
private static final int GROUP_CLASS = 4;

private final String filterDescription;
private final Matcher filterDescriptionMatcher;
/**
* Original filter which is used if its description (= {@link Filter#describe()}) is not parsable by
* {@link #DESCRIPTION_PATTERN}
* <p>
* This field is package private (= visible) for testing.
* </p>
**/
final Filter filter;

/**
* Creates a new {@link DataProvider} using the textual {@link Filter#describe()} of supplied {@link Filter} to
* determine if a test method should run or not.
*
* @throws IllegalArgumentException if supplied {@link Filter} is {@code null} or
* {@link Description#getDisplayName()} of supplied {@link Description} cannot be parsed
* Creates a new {@link DataProviderFilter} using the textual {@link Filter#describe()} of supplied {@link Filter}
* to determine if a test method should run or not. If given {@code filter} description can not be parsed, request
* for {@link #shouldRun(Description)} are just forwarded to it.
*/
public DataProviderFilter(Filter filter) {
if (filter == null) {
throw new NullPointerException("supplied filter must not be null");
}
filterDescription = filter.describe();
filterDescriptionMatcher = DESCRIPTION_PATTERN.matcher(filterDescription);
if (!filterDescriptionMatcher.find()) {
throw new IllegalArgumentException(String.format("Filter %s with description %s is not supported by %s.",
filter.getClass(), filterDescription, this.getClass().getSimpleName()));
}
this.filter = filter;
}

/**
* @throws IllegalArgumentException if {@link Description#getDisplayName()} of supplied {@link Description} cannot
* be parsed
*/
@Override
public boolean shouldRun(Description description) {
Matcher filterDescriptionMatcher = DESCRIPTION_PATTERN.matcher(filter.describe());
if (!filterDescriptionMatcher.find()) {
return filter.shouldRun(description);
}

if (description.isTest()) {
Matcher descriptionMatcher = DESCRIPTION_PATTERN.matcher(description.getDisplayName());
if (!descriptionMatcher.matches()) {
throw new IllegalArgumentException(String.format("Test method description %s is not suppored by %s.",
description.getDisplayName(), this.getClass().getSimpleName()));
return filter.shouldRun(description);
}
if (!filterDescriptionMatcher.group(GROUP_METHOD_NAME).equals(descriptionMatcher.group(GROUP_METHOD_NAME))
|| !filterDescriptionMatcher.group(GROUP_CLASS).equals(descriptionMatcher.group(GROUP_CLASS))) {

return false;
}
return filterDescriptionMatcher.group(GROUP_METHOD_PARAMS) == null
Expand All @@ -79,6 +77,6 @@ public boolean shouldRun(Description description) {

@Override
public String describe() {
return filterDescription;
return filter.describe();
}
}
42 changes: 7 additions & 35 deletions src/main/java/com/tngtech/java/junit/dataprovider/DataProviderRunner.java
View file
Original file line number Diff line number Diff line change
@@ -1,11 +1,9 @@
package com.tngtech.java.junit.dataprovider;

import java.util.ArrayList;
import java.util.Arrays;
import java.util.List;

import org.junit.Test;
import org.junit.experimental.categories.Categories.CategoryFilter;
import org.junit.runner.manipulation.Filter;
import org.junit.runner.manipulation.NoTestsRemainException;
import org.junit.runners.BlockJUnit4ClassRunner;
Expand All @@ -26,16 +24,6 @@
*/
public class DataProviderRunner extends BlockJUnit4ClassRunner {

/**
* A list of filter packages which must not be wrapped by DataProviderRunner (this is a workaround for some plugins,
* e.g. the maven-surefire-plugin).
*/
// @formatter:off
private static final List<String> BLACKLISTED_FILTER_PACKAGES = Arrays.asList(
"org.apache.maven.surefire"
);
// @formatter:on

/**
* The {@link DataConverter} to be used to convert from supported return types of any data provider to {@link List}
* {@code <}{@link Object}{@code []>} such that data can be further handled.
Expand Down Expand Up @@ -152,17 +140,16 @@ protected List<FrameworkMethod> computeTestMethods() {
* If possible the given {@code filter} is wrapped by {@link DataProviderFilter} to enable filtering of tests using
* a data provider.
*
* @param filter the {@link Filter} to wrap/apply
* @param filter the {@link Filter} to be wrapped or apply, respectively
*/
@Override
public void filter(Filter filter) throws NoTestsRemainException {
Filter useFilter;
if (!(filter instanceof CategoryFilter) && !isFilterBlackListed(filter)) {
useFilter = new DataProviderFilter(filter);
} else {
useFilter = filter;
}
super.filter(useFilter);
// Filter newFilter = filter;
// // TODO not really testable? maybe first create and than test if it is applicable?
// if (DataProviderFilter.canBeWrapped(filter)) {
// newFilter = ;
// }
super.filter(new DataProviderFilter(filter));
}

/**
Expand Down Expand Up @@ -197,21 +184,6 @@ List<FrameworkMethod> generateExplodedTestMethodsFor(List<FrameworkMethod> testM
return result;
}

/**
* <p>
* This method is package private (= visible) for testing.
* </p>
*/
boolean isFilterBlackListed(Filter filter) {
String className = filter.getClass().getName();
for (String blacklistedPackage : BLACKLISTED_FILTER_PACKAGES) {
if (className.startsWith(blacklistedPackage)) {
return true;
}
}
return false;
}

/**
* Returns the data provider method that belongs to the given test method or {@code null} if no such data provider
* exists or the test method is not marked for usage of a data provider
Expand Down
63 changes: 37 additions & 26 deletions src/test/java/com/tngtech/java/junit/dataprovider/DataProviderFilterTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,19 +3,28 @@
import static org.assertj.core.api.Assertions.assertThat;
import static org.mockito.Mockito.doReturn;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.verifyNoMoreInteractions;

import java.util.ArrayList;
import java.util.Arrays;
import java.util.regex.Matcher;

import org.junit.Test;
import org.junit.runner.Description;
import org.junit.runner.RunWith;
import org.junit.runner.manipulation.Filter;
import org.mockito.InjectMocks;
import org.mockito.Mock;
import org.mockito.runners.MockitoJUnitRunner;

@RunWith(MockitoJUnitRunner.class)
public class DataProviderFilterTest extends BaseTest {

@InjectMocks
private DataProviderFilter underTest;

@Mock
private Filter filter;

@edu.umd.cs.findbugs.annotations.SuppressWarnings("DLS_DEAD_LOCAL_STORE")
Expand All @@ -30,32 +39,40 @@ public void testDataProviderFilterShouldThrowNullPointerExceptionWhenFilterIsNul
// Then: expect exception
}

@Test(expected = IllegalArgumentException.class)
public void testDataProviderFilterShouldThrowIllegalArgumentExceptionWhenFilterDescriptionCannotBeParsed() {
@Test
public void testShouldRunShouldCallOriginalFilterShouldRunIfOriginalFilterDescriptionCannotBeParsed() {
// Given:
doReturn("invalid").when(filter).describe();
Description description = setupDescription(true, "test(Clazz)");

// When:
setupDataProviderFilterWith("invalid");
underTest.shouldRun(description);

// Then: expect exception
// Then:
verify(filter).describe();
verify(filter).shouldRun(description);
verifyNoMoreInteractions(filter);
}

@Test(expected = IllegalArgumentException.class)
public void testShouldRunShouldThrowIllegalArgumentExceptionWhenDescriptionCannotBeParsed() {
@Test
public void testShouldRunShouldCallOriginalFilterShouldRunIfIsTestAndGivenDescriptionCannotBeParsed() {
// Given:
setupDataProviderFilterWith("Method testMain[1: ](Clazz)");
doReturn("Method test(Clazz)").when(filter).describe();
Description description = setupDescription(true, "invalid");

// When:
underTest.shouldRun(description);

// Then: expect exception
// Then:
verify(filter).describe();
verify(filter).shouldRun(description);
verifyNoMoreInteractions(filter);
}

@Test
public void testShouldRunShouldReturnFalseWhenDescriptionDoesNotHaveExpectedMethodName() {
// Given:
setupDataProviderFilterWith("Method testMain[1: ](Clazz)");
doReturn("Method testMain[1: ](Clazz)").when(filter).describe();
Description description = setupDescription(true, "testOther[1: ](Clazz)");

// When:
Expand All @@ -68,7 +85,7 @@ public void testShouldRunShouldReturnFalseWhenDescriptionDoesNotHaveExpectedMeth
@Test
public void testShouldRunShouldReturnFalseWhenDescriptionDoesNotHaveExpectedClassName() {
// Given:
setupDataProviderFilterWith("Method testMain[1: ](Clazz)");
doReturn("Method testMain[1: ](Clazz)").when(filter).describe();
Description description = setupDescription(true, "testMain[1: ](ClazzOther)");

// When:
Expand All @@ -81,7 +98,7 @@ public void testShouldRunShouldReturnFalseWhenDescriptionDoesNotHaveExpectedClas
@Test
public void testShouldRunShouldReturnFalseWhenDescriptionHasNoMethodIdx() {
// Given:
setupDataProviderFilterWith("Method testMain[1: ](Clazz)");
doReturn("Method testMain[1: ](Clazz)").when(filter).describe();
Description description = setupDescription(true, "testMain(Clazz)");

// When:
Expand All @@ -94,7 +111,7 @@ public void testShouldRunShouldReturnFalseWhenDescriptionHasNoMethodIdx() {
@Test
public void testShouldRunShouldReturnFalseWhenDescriptionDoesNotHaveExpectedMethodIdx() {
// Given:
setupDataProviderFilterWith("Method testMain[1: ](Clazz)");
doReturn("Method testMain[1: ](Clazz)").when(filter).describe();
Description description = setupDescription(true, "testMain[2: ](Clazz)");

// When:
Expand All @@ -107,7 +124,7 @@ public void testShouldRunShouldReturnFalseWhenDescriptionDoesNotHaveExpectedMeth
@Test
public void testShouldRunShouldReturnTrueWhenDescriptionHaveOnlyMethodNameAndEqualsExactly() {
// Given:
setupDataProviderFilterWith("Method testMain(Clazz)");
doReturn("Method testMain(Clazz)").when(filter).describe();
Description description = setupDescription(true, "testMain(Clazz)");

// When:
Expand All @@ -120,7 +137,7 @@ public void testShouldRunShouldReturnTrueWhenDescriptionHaveOnlyMethodNameAndEqu
@Test
public void testShouldRunShouldReturnTrueWhenDescriptionHaveAdditionalMethodIdxAndEqualsMethodNameAndClass() {
// Given:
setupDataProviderFilterWith("Method testMain(Clazz)");
doReturn("Method testMain(Clazz)").when(filter).describe();
Description description = setupDescription(true, "testMain[1: ](Clazz)");

// When:
Expand All @@ -133,7 +150,7 @@ public void testShouldRunShouldReturnTrueWhenDescriptionHaveAdditionalMethodIdxA
@Test
public void testShouldRunShouldReturnTrueWhenDescriptionHaveAddtionalMethodIdxAndEqualsExcatly() {
// Given:
setupDataProviderFilterWith("Method testMain[1: ](Clazz)");
doReturn("Method testMain[1: ](Clazz)").when(filter).describe();
Description description = setupDescription(true, "testMain[1: ](Clazz)");

// When:
Expand All @@ -146,7 +163,7 @@ public void testShouldRunShouldReturnTrueWhenDescriptionHaveAddtionalMethodIdxAn
@Test
public void testShouldRunShouldReturnTrueWhenDescriptionHaveAdditionalMethodIdxAndMethodParamsAreDifferentButIdxIsEqual() {
// Given:
setupDataProviderFilterWith("Method testMain[1: ](Clazz)");
doReturn("Method testMain[1: ](Clazz)").when(filter).describe();
Description description = setupDescription(true, "testMain[1: test](Clazz)");

// When:
Expand All @@ -159,7 +176,7 @@ public void testShouldRunShouldReturnTrueWhenDescriptionHaveAdditionalMethodIdxA
@Test
public void testShouldRunShouldReturnTrueForMatchingChildDescription() {
// Given:
setupDataProviderFilterWith("Method testMain[1: ](Clazz)");
doReturn("Method testMain[1: ](Clazz)").when(filter).describe();

Description description = setupDescription(false, "", setupDescription(true, "testMain[1: ](Clazz)"));

Expand All @@ -173,7 +190,7 @@ public void testShouldRunShouldReturnTrueForMatchingChildDescription() {
@Test
public void testShouldRunShouldReturnTrueForMultipleChildDescriptionWithLastMatching() {
// Given:
setupDataProviderFilterWith("Method testMain[1: ](Clazz)");
doReturn("Method testMain[1: ](Clazz)").when(filter).describe();

// @formatter:off
Description description = setupDescription(false, "",
Expand All @@ -193,7 +210,7 @@ public void testShouldRunShouldReturnTrueForMultipleChildDescriptionWithLastMatc
@Test
public void testShouldRunShouldReturnFalseForMultipleChildAndFurtherChildDescriptionWithNonMatching() {
// Given:
setupDataProviderFilterWith("Method testMain[1: ](Clazz)");
doReturn("Method testMain[1: ](Clazz)").when(filter).describe();

// @formatter:off
Description description = setupDescription(false, "testMain[2: ](Clazz)",
Expand All @@ -217,7 +234,7 @@ public void testShouldRunShouldReturnFalseForMultipleChildAndFurtherChildDescrip
@Test
public void testDescribeShouldReturnFilterDescripe() {
// Given:
setupDataProviderFilterWith("Method testMain[1: ](Clazz)");
doReturn("Method testMain[1: ](Clazz)").when(filter).describe();

// When:
String result = underTest.describe();
Expand Down Expand Up @@ -326,12 +343,6 @@ public void testDescriptionPatternShouldFindDescriptionWithMethodNameContainingB
assertThatMatcherGroupsAre(matcher, "ain", null, null, "Clazz");
}

private void setupDataProviderFilterWith(String filterDescriptionString) {
filter = mock(Filter.class);
doReturn(filterDescriptionString).when(filter).describe();
underTest = new DataProviderFilter(filter);
}

private Description setupDescription(boolean isTest, String descriptionDisplayName,
Description... childDescriptions) {

Expand Down
Loading

0 comments on commit 8526baf

Please sign in to comment.