Merge pull request #736 from kcooney/filter-factory

Change FilterFactory code to pass in the actual Description

src/main/java/org/junit/runner/FilterFactories.java

@@ -1,24 +1,26 @@
 package org.junit.runner;
 
 import org.junit.internal.Classes;
+import org.junit.runner.FilterFactory.FilterNotCreatedException;
 import org.junit.runner.manipulation.Filter;
 
-import static org.junit.runner.FilterFactory.FilterNotCreatedException;
-
 /**
  * Utility class whose methods create a {@link FilterFactory}.
  */
-public class FilterFactories {
+class FilterFactories {
     /**
      * Creates a {@link Filter}.
      *
      * A filter specification is of the form "package.of.FilterFactory=args-to-filter-factory" or
      * "package.of.FilterFactory".
      *
-     * @param filterSpec The filter specification
+     * @param request the request that will be filtered
+     * @param filterSpec the filter specification
+     * @throws org.junit.runner.FilterFactory.FilterNotCreatedException
      */
-    public static Filter createFilterFromFilterSpec(Description description, String filterSpec)
+    public static Filter createFilterFromFilterSpec(Request request, String filterSpec)
             throws FilterFactory.FilterNotCreatedException {
+        Description topLevelDescription = request.getRunner().getDescription();
         String[] tuple;
 
         if (filterSpec.contains("=")) {
@@ -27,7 +29,7 @@ public static Filter createFilterFromFilterSpec(Description description, String
             tuple = new String[]{ filterSpec, "" };
         }
 
-        return createFilter(tuple[0], new FilterFactoryParams(tuple[1]));
+        return createFilter(tuple[0], new FilterFactoryParams(topLevelDescription, tuple[1]));
     }
 
     /**

src/main/java/org/junit/runner/FilterFactoryParams.java

@@ -1,17 +1,23 @@
 package org.junit.runner;
 
 public final class FilterFactoryParams {
+    private final Description topLevelDescription;
     private final String args;
 
-    public FilterFactoryParams(String args) {
-        if (args == null) {
+    public FilterFactoryParams(Description topLevelDescription, String args) {
+        if (args == null || topLevelDescription == null) {
             throw new NullPointerException();
         }
 
+        this.topLevelDescription = topLevelDescription;
         this.args = args;
     }
 
     public String getArgs() {
         return args;
     }
+
+    public Description getTopLevelDescription() {
+        return topLevelDescription;
+    }
 }

src/main/java/org/junit/runner/JUnitCommandLineParseResult.java

@@ -1,37 +1,36 @@
 package org.junit.runner;
 
 import java.util.ArrayList;
+import java.util.Collections;
 import java.util.List;
 
 import org.junit.internal.Classes;
-import org.junit.internal.runners.ErrorReportingRunner;
+import org.junit.runner.FilterFactory.FilterNotCreatedException;
 import org.junit.runner.manipulation.Filter;
 import org.junit.runners.model.InitializationError;
 
-import static org.junit.runner.Description.createSuiteDescription;
-
 class JUnitCommandLineParseResult {
-    private Filter filter = Filter.ALL;
-    private List<Class<?>> classes = new ArrayList<Class<?>>();
-    private List<Throwable> parserErrors = new ArrayList<Throwable>();
+    private final List<String> filterSpecs = new ArrayList<String>();
+    private final List<Class<?>> classes = new ArrayList<Class<?>>();
+    private final List<Throwable> parserErrors = new ArrayList<Throwable>();
 
     /**
      * Do not use. Testing purposes only.
      */
     JUnitCommandLineParseResult() {}
 
     /**
-     * Returns filters parsed from command line.
+     * Returns filter specs parsed from command line.
      */
-    public Filter getFilter() {
-        return filter;
+    public List<String> getFilterSpecs() {
+        return Collections.unmodifiableList(filterSpecs);
     }
 
     /**
      * Returns test classes parsed from command line.
      */
     public List<Class<?>> getClasses() {
-        return classes;
+        return Collections.unmodifiableList(classes);
     }
 
     /**
@@ -47,44 +46,38 @@ public static JUnitCommandLineParseResult parse(String[] args) {
         return result;
     }
 
-    void parseArgs(String[] args) {
+    private void parseArgs(String[] args) {
         parseParameters(parseOptions(args));
     }
 
-    String[] parseOptions(String[] args) {
+    String[] parseOptions(String... args) {
         for (int i = 0; i != args.length; ++i) {
             String arg = args[i];
 
-            try {
-                if (arg.equals("--")) {
-                    return copyArray(args, i + 1, args.length);
-                } else if (arg.startsWith("--")) {
-                    if (arg.startsWith("--filter=") || arg.equals("--filter")) {
-                        String filterSpec;
-                        if (arg.equals("--filter")) {
-                            ++i;
-
-                            if (i < args.length) {
-                                filterSpec = args[i];
-                            } else {
-                                parserErrors.add(new CommandLineParserError(arg + " value not specified"));
-
-                                break;
-                            }
+            if (arg.equals("--")) {
+                return copyArray(args, i + 1, args.length);
+            } else if (arg.startsWith("--")) {
+                if (arg.startsWith("--filter=") || arg.equals("--filter")) {
+                    String filterSpec;
+                    if (arg.equals("--filter")) {
+                        ++i;
+
+                        if (i < args.length) {
+                            filterSpec = args[i];
                         } else {
-                            filterSpec = arg.substring(arg.indexOf('=') + 1);
+                            parserErrors.add(new CommandLineParserError(arg + " value not specified"));
+                            break;
                         }
-
-                        filter = filter.intersect(FilterFactories.createFilterFromFilterSpec(
-                                createSuiteDescription(arg), filterSpec));
                     } else {
-                        parserErrors.add(new CommandLineParserError("JUnit knows nothing about the " + arg + " option"));
+                        filterSpec = arg.substring(arg.indexOf('=') + 1);
                     }
+
+                    filterSpecs.add(filterSpec);
                 } else {
-                    return copyArray(args, i, args.length);
+                    parserErrors.add(new CommandLineParserError("JUnit knows nothing about the " + arg + " option"));
                 }
-            } catch (FilterFactory.FilterNotCreatedException e) {
-                parserErrors.add(e);
+            } else {
+                return copyArray(args, i, args.length);
             }
         }
 
@@ -111,32 +104,44 @@ void parseParameters(String[] args) {
         }
     }
 
+    private Request errorReport(Throwable cause) {
+        return Request.errorReport(JUnitCommandLineParseResult.class, cause);
+    }
+
     /**
      * Creates a {@link Request}.
      *
      * @param computer {@link Computer} to be used.
      */
     public Request createRequest(Computer computer) {
         if (parserErrors.isEmpty()) {
-            return Request
-                    .classes(computer, classes.toArray(new Class<?>[classes.size()]))
-                    .filterWith(filter);
+            Request request = Request.classes(
+                    computer, classes.toArray(new Class<?>[classes.size()]));
+            return applyFilterSpecs(request);
         } else {
-            return new Request() {
-                @Override
-                public Runner getRunner() {
-                    return new ErrorReportingRunner(
-                            JUnitCommandLineParseResult.class,
-                            new InitializationError(parserErrors));
-                }
-            };
+            return errorReport(new InitializationError(parserErrors));
+        }
+    }
+
+    private Request applyFilterSpecs(Request request) {
+        try {
+            for (String filterSpec : filterSpecs) {
+                Filter filter = FilterFactories.createFilterFromFilterSpec(
+                        request, filterSpec);
+                request = request.filterWith(filter);
+            }
+            return request;
+        } catch (FilterNotCreatedException e) {
+            return errorReport(e);
         }
     }
 
     /**
      * Exception used if there's a problem parsing the command line.
      */
     public static class CommandLineParserError extends Exception {
+        private static final long serialVersionUID= 1L;
+
         public CommandLineParserError(String message) {
             super(message);
         }

src/main/java/org/junit/runner/Request.java

@@ -93,9 +93,9 @@ public static Request classes(Class<?>... classes) {
 
 
     /**
-     * Not used within JUnit.  Clients should simply instantiate ErrorReportingRunner themselves
+     * Creates a {@link Request} that, when processed, will report an error for the given
+     * test class with the given cause.
      */
-    @Deprecated
     public static Request errorReport(Class<?> klass, Throwable cause) {
         return runner(new ErrorReportingRunner(klass, cause));
     }

src/test/java/org/junit/experimental/categories/CategoryFilterFactoryTest.java

@@ -2,6 +2,7 @@
 
 import static org.hamcrest.CoreMatchers.instanceOf;
 import static org.hamcrest.MatcherAssert.assertThat;
+import static org.junit.runner.Description.createSuiteDescription;
 
 import java.util.List;
 
@@ -21,11 +22,12 @@ public class CategoryFilterFactoryTest {
     @Rule
     public TestName testName = new TestName();
 
-    private CategoryFilterFactory categoryFilterFactory = new CategoryFilterFactoryStub();
+    private final CategoryFilterFactory categoryFilterFactory = new CategoryFilterFactoryStub();
 
     @Test
     public void shouldCreateFilter() throws Exception {
         FilterFactoryParams params = new FilterFactoryParams(
+                createSuiteDescription(testName.getMethodName()),
                 CategoryFilterFactoryStub.class.getName());
         Filter filter = categoryFilterFactory.createFilter(params);
 
@@ -35,6 +37,7 @@ public void shouldCreateFilter() throws Exception {
     @Test
     public void shouldThrowException() throws Exception {
         FilterFactoryParams params = new FilterFactoryParams(
+                createSuiteDescription(testName.getMethodName()),
                 "NonExistentFilter");
 
         expectedException.expect(FilterFactory.FilterNotCreatedException.class);

src/test/java/org/junit/runner/FilterFactoriesTest.java

@@ -1,16 +1,18 @@
 package org.junit.runner;
 
+import static org.hamcrest.CoreMatchers.instanceOf;
+import static org.hamcrest.CoreMatchers.is;
+import static org.hamcrest.CoreMatchers.startsWith;
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.junit.Assume.assumeThat;
 import org.junit.Rule;
 import org.junit.Test;
 import org.junit.experimental.categories.ExcludeCategories;
 import org.junit.rules.ExpectedException;
 import org.junit.rules.TestName;
 import org.junit.runner.manipulation.Filter;
-
-import static org.hamcrest.CoreMatchers.instanceOf;
-import static org.hamcrest.CoreMatchers.startsWith;
-import static org.hamcrest.MatcherAssert.assertThat;
-import static org.junit.runner.Description.createSuiteDescription;
+import org.junit.runners.Suite;
+import org.junit.runners.Suite.SuiteClasses;
 
 public class FilterFactoriesTest {
     @Rule
@@ -19,10 +21,14 @@ public class FilterFactoriesTest {
     @Rule
     public TestName testName = new TestName();
 
+    private Request createSuiteRequest() {
+        return Request.aClass(DummySuite.class);
+    }
+
     @Test
     public void shouldCreateFilterWithArguments() throws Exception {
         Filter filter = FilterFactories.createFilterFromFilterSpec(
-                createSuiteDescription(testName.getMethodName()),
+                createSuiteRequest(),
                 ExcludeCategories.class.getName() + "=" + DummyCategory.class.getName());
 
         assertThat(filter.describe(), startsWith("excludes "));
@@ -31,15 +37,32 @@ public void shouldCreateFilterWithArguments() throws Exception {
     @Test
     public void shouldCreateFilterWithNoArguments() throws Exception {
         Filter filter = FilterFactories.createFilterFromFilterSpec(
-                createSuiteDescription(testName.getMethodName()), FilterFactoryStub.class.getName());
+                createSuiteRequest(), FilterFactoryStub.class.getName());
 
         assertThat(filter, instanceOf(DummyFilter.class));
     }
 
+    @Test
+    public void shouldPassOnDescriptionToFilterFactory() throws Exception {
+        Request request = createSuiteRequest();
+        Description description = request.getRunner().getDescription();
+        Filter filter = FilterFactories.createFilterFromFilterSpec(
+                request, FilterFactoryStub.class.getName());
+
+        // This assumption tested in shouldCreateFilterWithNoArguments()
+        assumeThat(filter, instanceOf(DummyFilter.class));
+
+        DummyFilter dummyFilter = (DummyFilter) filter;
+        assertThat(dummyFilter.getTopLevelDescription(), is(description));
+    }
+
     @Test
     public void shouldCreateFilter() throws Exception {
         Filter filter = FilterFactories.createFilter(
-                FilterFactoryStub.class, new FilterFactoryParams(""));
+                FilterFactoryStub.class,
+                new FilterFactoryParams(
+                        Description.createSuiteDescription(testName.getMethodName()),
+                        ""));
 
         assertThat(filter, instanceOf(DummyFilter.class));
     }
@@ -71,12 +94,22 @@ public Filter createFilter(FilterFactoryParams params) throws FilterNotCreatedEx
     }
 
     public static class FilterFactoryStub implements FilterFactory {
-        public Filter createFilter(FilterFactoryParams unused) {
-            return new DummyFilter();
+        public Filter createFilter(FilterFactoryParams params) {
+            return new DummyFilter(params.getTopLevelDescription());
         }
     }
 
     private static class DummyFilter extends Filter {
+        private final Description fTopLevelDescription;
+
+        public DummyFilter(Description topLevelDescription) {
+            fTopLevelDescription = topLevelDescription;
+        }
+
+        public Description getTopLevelDescription() {
+            return fTopLevelDescription;
+        }
+
         @Override
         public boolean shouldRun(Description description) {
             return false;
@@ -90,4 +123,15 @@ public String describe() {
 
     public static class DummyCategory {
     }
+
+    @RunWith(Suite.class)
+    @SuiteClasses(DummyTest.class)
+    public static class DummySuite {
+    }
+
+    public static class DummyTest {
+        @Test
+        public void passes() {
+        }
+    }
 }