Formalize feature path

The feature path was poorly defined. It could either be a path or
a path with a `classpath:` prefix. This ambiguity allowed room
for various inconsistencies and misinterpretations.

1. Pickle URI's were not actually uniform. Depending on operating
   system different path seperators would be used.

2. Pickle URI's did not identify a resource. The `classpath:`
   prefix would be dropped after loading a feature file. This
   lead to a fairly complex try-catch-try-again-on-the-classpath
   mechanism when loading rerun file.

By properly defining the feature identifier it becomes possible to
parse it in `RuntimeOptions` and propagate a consistent URI to
other parts of cucumber-jvm. This reduces the need for work
arounds in various places.

Features are identified by a URI. This URI can either be absolute:
`scheme:/absolute/path/to.feature`, or relative to the
current working directory:`scheme:relative/path/to.feature`.

In either form, when the scheme is omitted `file` will be
assumed: `path/to.feature`.

On systems that use a `File.separatorChar` other then `/`
`File.separatorChar` can be used as a path separator. When
doing so when the scheme must be omitted: `path\to.feature`.

It is recommended to use `/` as the path separator.

Author: mpkorstanje

core/src/main/java/cucumber/runner/TestCase.java

@@ -8,6 +8,7 @@
 import gherkin.pickles.PickleLocation;
 import gherkin.pickles.PickleTag;
 
+import java.net.URI;
 import java.util.ArrayList;
 import java.util.List;
 
@@ -93,7 +94,7 @@ public List<Integer> getLines() {
     }
 
     private String fileColonLine(PickleLocation location) {
-        return pickleEvent.uri + ":" + location.getLine();
+        return URI.create(pickleEvent.uri).getSchemeSpecificPart() + ":" + location.getLine();
     }
 
     @Override

core/src/main/java/cucumber/runtime/RuntimeOptions.java

@@ -5,8 +5,7 @@
 import cucumber.runtime.io.MultiLoader;
 import cucumber.runtime.io.Resource;
 import cucumber.runtime.io.ResourceLoader;
-import cucumber.runtime.model.PathWithLines;
-import cucumber.util.Encoding;
+import io.cucumber.core.model.FeatureWithLines;
 import cucumber.util.FixJava;
 import cucumber.util.Mapper;
 import gherkin.GherkinDialect;
@@ -22,12 +21,14 @@
 import java.io.IOException;
 import java.io.InputStreamReader;
 import java.io.Reader;
+import java.net.URI;
 import java.util.ArrayList;
-import java.util.Collections;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
 import java.util.ResourceBundle;
+import java.util.Set;
+import java.util.TreeSet;
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
 
@@ -58,8 +59,8 @@ public String map(String keyword) {
     private final List<String> glue = new ArrayList<String>();
     private final List<String> tagFilters = new ArrayList<String>();
     private final List<Pattern> nameFilters = new ArrayList<Pattern>();
-    private final Map<String, List<Long>> lineFilters = new HashMap<String, List<Long>>();
-    private final List<String> featurePaths = new ArrayList<String>();
+    private final Map<URI, Set<Integer>> lineFilters = new HashMap<>();
+    private final List<URI> featurePaths = new ArrayList<>();
 
     private final List<String> junitOptions = new ArrayList<String>();
     private final ResourceLoader resourceLoader;
@@ -137,8 +138,8 @@ public RuntimeOptions noSummaryPrinter() {
     private void parse(List<String> args) {
         List<String> parsedTagFilters = new ArrayList<String>();
         List<Pattern> parsedNameFilters = new ArrayList<Pattern>();
-        Map<String, List<Long>> parsedLineFilters = new HashMap<String, List<Long>>();
-        List<String> parsedFeaturePaths = new ArrayList<String>();
+        Map<URI, Set<Integer>> parsedLineFilters = new HashMap<>();
+        List<URI> parsedFeaturePaths = new ArrayList<>();
         List<String> parsedGlue = new ArrayList<String>();
         ParsedPluginData parsedPluginData = new ParsedPluginData();
         List<String> parsedJunitOptions = new ArrayList<String>();
@@ -188,29 +189,21 @@ private void parse(List<String> args) {
             } else if (arg.startsWith("-")) {
                 printUsage();
                 throw new CucumberException("Unknown option: " + arg);
-            } else {
-                List<PathWithLines> paths;
-                if (arg.startsWith("@")) {
-                    paths = loadRerunFile(arg.substring(1));
-                } else {
-                    paths = parsePathWithLines(arg);
-                }
-                for (PathWithLines pathWithLines : paths) {
-                    parsedFeaturePaths.add(pathWithLines.path);
-                    if (!pathWithLines.lines.isEmpty()) {
-                        String key = pathWithLines.path.replace("classpath:", "");
-                        addLineFilters(parsedLineFilters, key, pathWithLines.lines);
-                    }
-                }
+            } else if (arg.startsWith("@")) {
+                FeatureWithLines featureWithLines = parseFeatureWithLines(arg.substring(1));
+                processPathWitheLinesFromRerunFile(parsedLineFilters, parsedFeaturePaths, featureWithLines.uri());
+            } else if (!arg.isEmpty()){
+                FeatureWithLines featureWithLines = parseFeatureWithLines(arg);
+                processFeatureWithLines(parsedLineFilters, parsedFeaturePaths, featureWithLines);
             }
         }
-        if (!parsedTagFilters.isEmpty() || !parsedNameFilters.isEmpty() || !parsedLineFilters.isEmpty() || haveLineFilters(parsedFeaturePaths)) {
+        if (!parsedTagFilters.isEmpty() || !parsedNameFilters.isEmpty() || !parsedLineFilters.isEmpty()) {
             tagFilters.clear();
             tagFilters.addAll(parsedTagFilters);
             nameFilters.clear();
             nameFilters.addAll(parsedNameFilters);
             lineFilters.clear();
-            for (String path : parsedLineFilters.keySet()) {
+            for (URI path : parsedLineFilters.keySet()) {
                 lineFilters.put(path, parsedLineFilters.get(path));
             }
         }
@@ -233,47 +226,50 @@ private void parse(List<String> args) {
         parsedPluginData.updatePluginSummaryPrinterNames(pluginSummaryPrinterNames);
     }
 
-    private void addLineFilters(Map<String, List<Long>> parsedLineFilters, String key, List<Long> lines) {
+    private FeatureWithLines parseFeatureWithLines(String pathWithLines) {
+        return FeatureWithLines.parse(pathWithLines);
+    }
+
+    private void addLineFilters(Map<URI, Set<Integer>> parsedLineFilters, URI key, Set<Integer> lines) {
+        if(lines.isEmpty()){
+            return;
+        }
         if (parsedLineFilters.containsKey(key)) {
             parsedLineFilters.get(key).addAll(lines);
         } else {
-            parsedLineFilters.put(key, lines);
+            parsedLineFilters.put(key, new TreeSet<>(lines));
         }
     }
 
-    private boolean haveLineFilters(List<String> parsedFeaturePaths) {
-        for (String pathName : parsedFeaturePaths) {
-            if (PathWithLines.hasLineFilters(pathName)) {
-                return true;
-            }
+    private void processFeatureWithLines(Map<URI, Set<Integer>> parsedLineFilters, List<URI> parsedFeaturePaths, FeatureWithLines featureWithLines) {
+        parsedFeaturePaths.add(featureWithLines.uri());
+        addLineFilters(parsedLineFilters, featureWithLines.uri(), featureWithLines.lines());
+    }
+
+    private void processPathWitheLinesFromRerunFile(Map<URI, Set<Integer>> parsedLineFilters, List<URI> parsedFeaturePaths, URI rerunPath) {
+        for (FeatureWithLines featureWithLines : loadRerunFile(rerunPath)) {
+            processFeatureWithLines(parsedLineFilters, parsedFeaturePaths, featureWithLines);
         }
-        return false;
     }
 
-    private List<PathWithLines> loadRerunFile(String rerunPath) {
-        List<PathWithLines> featurePaths = new ArrayList<>();
+    private List<FeatureWithLines> loadRerunFile(URI rerunPath) {
+        List<FeatureWithLines> featurePaths = new ArrayList<>();
         Iterable<Resource> resources = resourceLoader.resources(rerunPath, null);
         for (Resource resource : resources) {
             String source = read(resource);
             if (!source.isEmpty()) {
                 Matcher matcher = RERUN_PATH_SPECIFICATION.matcher(source);
                 while (matcher.find()) {
-                    featurePaths.addAll(parsePathWithLines(matcher.group(1)));
+                    featurePaths.add(parseFeatureWithLines(matcher.group(1)));
                 }
             }
         }
         return featurePaths;
     }
 
-    private List<PathWithLines> parsePathWithLines(String pathWithLineFilter) {
-        String normalizedPath = convertFileSeparatorToForwardSlash(pathWithLineFilter);
-        PathWithLines pathWithLines = new PathWithLines(normalizedPath);
-        return Collections.singletonList(pathWithLines);
-    }
-
     private static String read(Resource resource) {
         try {
-            return Encoding.readFile(resource);
+            return FixJava.readReader(new InputStreamReader(resource.getInputStream()));
         } catch (IOException e) {
             throw new CucumberException("Failed to read resource:" + resource.getPath(), e);
         }
@@ -380,7 +376,7 @@ public boolean isWip() {
     }
 
     @Override
-    public List<String> getFeaturePaths() {
+    public List<URI> getFeaturePaths() {
         return featurePaths;
     }
 
@@ -395,7 +391,7 @@ public List<String> getTagFilters() {
     }
 
     @Override
-    public Map<String, List<Long>> getLineFilters() {
+    public Map<URI, Set<Integer>> getLineFilters() {
         return lineFilters;
     }
 

core/src/main/java/cucumber/runtime/RuntimeOptionsFactory.java

@@ -95,7 +95,7 @@ private void addFeatures(CucumberOptions options, List<String> args) {
 
     private void addDefaultFeaturePathIfNoFeaturePathIsSpecified(List<String> args, Class clazz) {
         if (!featuresSpecified) {
-            args.add(MultiLoader.CLASSPATH_SCHEME + packagePath(clazz));
+            args.add(MultiLoader.CLASSPATH_SCHEME_PREFIX + packagePath(clazz));
         }
     }
 
@@ -125,7 +125,7 @@ private void addGlue(CucumberOptions options, List<String> args) {
     private void addDefaultGlueIfNoOverridingGlueIsSpecified(List<String> args, Class clazz) {
         if (!overridingGlueSpecified) {
             args.add("--glue");
-            args.add(MultiLoader.CLASSPATH_SCHEME + packagePath(clazz));
+            args.add(MultiLoader.CLASSPATH_SCHEME_PREFIX + packagePath(clazz));
         }
     }
 

core/src/main/java/cucumber/runtime/filter/Filters.java

@@ -3,7 +3,9 @@
 import gherkin.events.PickleEvent;
 import io.cucumber.core.options.FilterOptions;
 
+import java.net.URI;
 import java.util.ArrayList;
+import java.util.Collection;
 import java.util.List;
 import java.util.Map;
 import java.util.regex.Pattern;
@@ -23,7 +25,7 @@ public Filters(FilterOptions filterOPtions) {
         if (!nameFilters.isEmpty()) {
             this.filters.add(new NamePredicate(nameFilters));
         }
-        Map<String, List<Long>> lineFilters = filterOPtions.getLineFilters();
+        Map<URI, ? extends Collection<Integer>> lineFilters = filterOPtions.getLineFilters();
         if (!lineFilters.isEmpty()) {
             this.filters.add(new LinePredicate(lineFilters));
         }

core/src/main/java/cucumber/runtime/filter/LinePredicate.java

@@ -3,23 +3,24 @@
 import gherkin.events.PickleEvent;
 import gherkin.pickles.PickleLocation;
 
-import java.util.List;
+import java.net.URI;
+import java.util.Collection;
 import java.util.Map;
 
 class LinePredicate implements PicklePredicate {
-    private Map<String, List<Long>> lineFilters;
+    private Map<URI, ? extends Collection<Integer>> lineFilters;
 
-    LinePredicate(Map<String, List<Long>> lineFilters) {
+    LinePredicate(Map<URI, ? extends Collection<Integer>> lineFilters) {
         this.lineFilters = lineFilters;
     }
 
     @Override
     public boolean apply(PickleEvent pickleEvent) {
-        String picklePath = pickleEvent.uri;
+        URI picklePath = URI.create(pickleEvent.uri);
         if (!lineFilters.containsKey(picklePath)) {
             return true;
         }
-        for (Long line : lineFilters.get(picklePath)) {
+        for (Integer line : lineFilters.get(picklePath)) {
             for (PickleLocation location : pickleEvent.pickle.getLocations()) {
                 if (line == location.getLine()) {
                     return true;

core/src/main/java/cucumber/runtime/formatter/PrettyFormatter.java

@@ -28,6 +28,7 @@
 import gherkin.ast.Tag;
 import gherkin.pickles.PickleTag;
 
+import java.net.URI;
 import java.util.List;
 
 final class PrettyFormatter implements EventListener, ColorAware {
@@ -243,7 +244,8 @@ private String getScenarioDefinitionText(ScenarioDefinition definition) {
     }
 
     private String getLocationText(String file, int line) {
-        return getLocationText(file + ":" + line);
+        String path = URI.create(file).getSchemeSpecificPart();
+        return getLocationText(path + ":" + line);
     }
 
     private String getLocationText(String location) {

core/src/main/java/cucumber/runtime/formatter/RerunFormatter.java

@@ -8,11 +8,12 @@
 import cucumber.api.event.TestRunFinished;
 import cucumber.api.formatter.NiceAppendable;
 import cucumber.api.formatter.StrictAware;
+import io.cucumber.core.model.FeatureWithLines;
 
 import java.util.ArrayList;
+import java.util.Collection;
 import java.util.HashMap;
 import java.util.Map;
-import java.util.Set;
 
 
 /**
@@ -21,19 +22,42 @@
  */
 final class RerunFormatter implements EventListener, StrictAware {
     private final NiceAppendable out;
-    private Map<String, ArrayList<Integer>> featureAndFailedLinesMapping = new HashMap<String, ArrayList<Integer>>();
-    private boolean isStrict = false;
+    private final Map<String, Collection<Integer>> featureAndFailedLinesMapping = new HashMap<>();
+    private final EventHandler<TestRunFinished> runFinishHandler = new EventHandler<TestRunFinished>() {
 
-    private EventHandler<TestCaseFinished> testCaseFinishedHandler = new EventHandler<TestCaseFinished>() {
         @Override
-        public void receive(TestCaseFinished event) {
-            handleTestCaseFinished(event);
+        public void receive(TestRunFinished event) {
+            for (Map.Entry<String, Collection<Integer>> entry : featureAndFailedLinesMapping.entrySet()) {
+                FeatureWithLines featureWithLines = FeatureWithLines.parse(entry.getKey(), entry.getValue());
+                out.println(featureWithLines.toString());
+            }
+
+            out.close();
         }
     };
-    private EventHandler<TestRunFinished> runFinishHandler = new EventHandler<TestRunFinished>() {
+    private boolean isStrict = false;
+    private final EventHandler<TestCaseFinished> testCaseFinishedHandler = new EventHandler<TestCaseFinished>() {
+
         @Override
-        public void receive(TestRunFinished event) {
-            handleTestRunFinished();
+        public void receive(TestCaseFinished event) {
+            if (!event.result.isOk(isStrict)) {
+                recordTestFailed(event.testCase);
+            }
+        }
+
+        private void recordTestFailed(TestCase testCase) {
+            String uri = testCase.getUri();
+            Collection<Integer> failedTestCaseLines = getFailedTestCaseLines(uri);
+            failedTestCaseLines.add(testCase.getLine());
+        }
+
+        private Collection<Integer> getFailedTestCaseLines(String uri) {
+            Collection<Integer> failedTestCases = featureAndFailedLinesMapping.get(uri);
+            if (failedTestCases == null) {
+                failedTestCases = new ArrayList<>();
+                featureAndFailedLinesMapping.put(uri, failedTestCases);
+            }
+            return failedTestCases;
         }
     };
 
@@ -52,39 +76,5 @@ public void setEventPublisher(EventPublisher publisher) {
     public void setStrict(boolean strict) {
         isStrict = strict;
     }
-
-    private void handleTestCaseFinished(TestCaseFinished event) {
-        if (!event.result.isOk(isStrict)) {
-            recordTestFailed(event.testCase);
-        }
-    }
-
-    private void handleTestRunFinished() {
-        reportFailedTestCases();
-        out.close();
-    }
-
-    private void recordTestFailed(TestCase testCase) {
-        String path = testCase.getUri();
-        ArrayList<Integer> failedTestCases = this.featureAndFailedLinesMapping.get(path);
-        if (failedTestCases == null) {
-            failedTestCases = new ArrayList<Integer>();
-            this.featureAndFailedLinesMapping.put(path, failedTestCases);
-        }
-
-        failedTestCases.add(testCase.getLine());
-    }
-
-    private void reportFailedTestCases() {
-        Set<Map.Entry<String, ArrayList<Integer>>> entries = featureAndFailedLinesMapping.entrySet();
-        for (Map.Entry<String, ArrayList<Integer>> entry : entries) {
-            if (!entry.getValue().isEmpty()) {
-                out.append(entry.getKey());
-                for (Integer line : entry.getValue()) {
-                    out.append(":").append(line.toString());
-                }
-                out.println();
-            }
-        }
-    }
 }
+