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

Trim paths from @rerun files #660

Merged
merged 1 commit into from
Nov 23, 2016
Merged

Trim paths from @rerun files #660

merged 1 commit into from
Nov 23, 2016

Conversation

CodyRay
Copy link
Contributor

@CodyRay CodyRay commented Oct 23, 2016

Fixes #657. This prevents all the tests from running when there is a trailing newline in the rerun file.

When I run cucumber.js with `-f json @rerun.txt`
Then the exit status should be 0

Scenario: empty rerun file
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is a change in behavior. Previously an empty rerun file also ran all the feature files in the current directory

Copy link
Member

@charlierudolph charlierudolph left a comment

Choose a reason for hiding this comment

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

This is looking good. Thanks @haroldhues

When I run cucumber.js with `-f json @rerun.txt`
Then the exit status should be 0

Scenario: empty rerun file
Copy link
Member

Choose a reason for hiding this comment

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

I agree with this change in functionality.


"""
When I run cucumber.js with `-f json @rerun.txt`
Then the exit status should be 0
Copy link
Member

Choose a reason for hiding this comment

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

Please also check which scenarios ran.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added

Scenario: empty rerun file
Given a file named "@rerun.txt" with:
"""
"""
Copy link
Member

Choose a reason for hiding this comment

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

Please create a new step (or use an existing step if it exists) so this can read something like:

Given an empty file named "@rerun.txt"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added

"""
"""
When I run cucumber.js with `-f json @rerun.txt`
Then the exit status should be 0
Copy link
Member

Choose a reason for hiding this comment

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

Please also check that no scenarios run.

@@ -11,7 +11,8 @@ function Configuration(options, args) {
var filename = path.basename(arg);
if (filename[0] === '@') {
var content = fs.readFileSync(arg, 'utf8');
unexpandedFeaturePaths = unexpandedFeaturePaths.concat(content.split('\n'));
var paths = content.split('\n').map(_.trim).filter(_.identity);
Copy link
Member

Choose a reason for hiding this comment

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

I'd prefer we use _.compact instead of filter(_.identify). Could also use lodash's _.chain in order to keep it as readable as it if now.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Nice catch on _.compact!

Fixes #657. This prevents all the tests from running when there is a trailing
newline in the @rerun file.
@charlierudolph charlierudolph merged commit ecca6a2 into cucumber:master Nov 23, 2016
@charlierudolph
Copy link
Member

Thanks @haroldhues! Sorry it took me so long to get back to this

@charlierudolph
Copy link
Member

Just a heads up @haroldhues I ended up reverting the empty rerun file behavior to run all features instead of nothing.

@CodyRay
Copy link
Contributor Author

CodyRay commented Nov 29, 2016

@charlierudolph Just curious, why revert the change in behavior for the empty file?

@charlierudolph
Copy link
Member

Mainly thought it was better to run all tests then no tests.

@lock
Copy link

lock bot commented Oct 25, 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 25, 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.

None yet

2 participants