Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Core] Use cucumber-ruby rerun file specification #1187

Merged
merged 2 commits into from
Jul 25, 2017

Conversation

mpkorstanje
Copy link
Contributor

@mpkorstanje mpkorstanje commented Jul 24, 2017

Implemented the rerun file specification from cucumber ruby[1]. It
handles spaces, new lines as a separator as well as no separator at all.

References:
[1] https://github.com/cucumber/cucumber-ruby/blame/master/lib/cucumber/cli/rerun_file.rb

Types of changes

  • Bug fix (non-breaking change which fixes an issue).
  • New feature (non-breaking change which adds functionality).
  • Breaking change (fix or feature that would cause existing functionality to not work as expected).

Checklist:

  • I've added tests for my code.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.

Implemented the rerun file specification from cucumber ruby[1]. It
handles spaces, new lines as a separator as well as no separator at all.

References:
 [1] https://github.com/cucumber/cucumber-ruby/blame/master/lib/cucumber/cli/rerun_file.rb

public class CucumberFeature implements Serializable {
private static final long serialVersionUID = 1L;
private final String uri;
private String language;
private GherkinDocument gherkinDocument;
private String gherkinSource;
public static final Pattern RERUN_PATH_SPECIFICATION = Pattern.compile("(?m:^| |)(.*?\\.feature(?:(?::\\d+)*))");
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

?m: for multi line or this input:

/some/path/foo.feature
[space]/some/path/foo.feature

Will yield: ["/some/path/foo.feature", "/some/path/bar.feature"].

Not sure if this actually a bad thing though.

@@ -71,7 +66,10 @@ private static void loadFromRerunFile(FeatureBuilder builder, ResourceLoader res
for (Resource resource : resources) {
String source = read(resource);
if (!source.isEmpty()) {
featurePaths.addAll(Arrays.asList(source.split(" ")));
Copy link
Contributor Author

@mpkorstanje mpkorstanje Jul 24, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure how I missed this the first time round. Nasty duplication.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With Gherkin v2 the filters were passed to the parser and the filtering was done there, so it was straightforward to add the line filters from the rerun file to the filters passed through the load method. Now with Gherkin v4 the filtering is instead done after the compiling of the features to pickles, so we are actually reading the rerun file twice, once to find the feature files to be parsed, and once to get the line filters to the pickle filter chain. This duplication was probably caused by that those two used was fixed at different times in the work or upgrading to Gherkin v4 (and the extract method refactoring missed).

@brasmusson brasmusson merged commit e52fc4f into master Jul 25, 2017
brasmusson added a commit that referenced this pull request Jul 25, 2017
@brasmusson brasmusson deleted the use-ruby-rerun-regex branch July 25, 2017 12:49
@lock
Copy link

lock bot commented Oct 24, 2018

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@lock lock bot locked as resolved and limited conversation to collaborators Oct 24, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants