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.

Sign up for GitHub

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

Jump to bottom

[Core] Clear RuntimeOptions.featurePaths if rerun was used #1631

Merged
merged 4 commits into from
May 28, 2019
Merged

[Core] Clear RuntimeOptions.featurePaths if rerun was used #1631

merged 4 commits into from
May 28, 2019

Conversation

tommywo
Copy link
Contributor

@tommywo tommywo commented May 20, 2019 •
edited by mpkorstanje
Loading

Summary

Fixes: #1630

How Has This Been Tested?

Unit tests and a test run on my laptop

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.

@tommywo tommywo requested a review from mpkorstanje May 20, 2019 14:22
@coveralls
Copy link

coveralls commented May 20, 2019 •
edited
Loading

Coverage Status

Coverage increased (+0.02%) to 85.97% when pulling 6eb5d96 on handle-empty-rerun-report-file into 75083d2 on master.

mpkorstanje
mpkorstanje reviewed May 20, 2019
View reviewed changes
Copy link
Contributor

@mpkorstanje mpkorstanje left a comment

Choose a reason for hiding this comment

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

Cheers. Looks good. Just not sure about the scope of isRerun. But I can't come up with a test case to make it break just now.

core/src/main/java/cucumber/runtime/RuntimeOptions.java Outdated
@@ -69,6 +69,7 @@ public String map(String keyword) {
private boolean wip = false;
private SnippetType snippetType = SnippetType.UNDERSCORE;
private int threads = 1;
private boolean isRerun = false;
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this may need to be method scoped.

mpkorstanje
mpkorstanje previously requested changes May 20, 2019
View reviewed changes
Copy link
Contributor

@mpkorstanje mpkorstanje left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Ah. Got a test case.

A cucumber annotation or CLI with a non-empty rerun file. In combination with an environment variable without any features (neither via rerun nor as arguments). This will result in no features found while it should see the features from the rerun file.

Could you add a test case for that?

@tommywo
Copy link
Contributor Author

tommywo commented May 22, 2019

I've added a test case for that and added one more - if the is rerun file passed via cli and a feature it should ignore rerun file from cli -similar like it does to features.

@tommywo tommywo requested a review from mpkorstanje May 22, 2019 09:15
@tommywo tommywo dismissed mpkorstanje’s stale review May 22, 2019 13:19

I've added the test.

@mpkorstanje
Copy link
Contributor

I'm having some trouble finding a test which uses:

CLI: @file:path/rerun.txt
ENVIRONMENT: cucumber.options=--tags @smoke

I think this will result in no feature files being selected to execute.

Could you point it out to me?

mpkorstanje
mpkorstanje reviewed May 23, 2019
View reviewed changes
core/src/main/java/cucumber/runtime/RuntimeOptions.java Outdated
@@ -67,6 +67,7 @@ public String map(String keyword) {
private boolean strict = false;
private boolean monochrome = false;
private boolean wip = false;
private boolean wasRerun = false;
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think this variable is needed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

right!

@tommywo
Copy link
Contributor Author

tommywo commented May 27, 2019

There was no such test with rerun file so i've added one. In that case behaviour is the same like passing features with line to cli - line numbers are ignored and only tags are used.

@tommywo tommywo requested a review from mpkorstanje May 27, 2019 09:51
@mpkorstanje mpkorstanje changed the title clear RuntimeOptions.featurePaths if rerun was used - fixes #1630 [Core] Clear RuntimeOptions.featurePaths if rerun was used May 28, 2019
@mpkorstanje mpkorstanje merged commit 6469d70 into master May 28, 2019
@mpkorstanje mpkorstanje deleted the handle-empty-rerun-report-file branch May 28, 2019 07:03
@mpkorstanje
Copy link
Contributor

All good now. Cheers!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mpkorstanje mpkorstanje Awaiting requested review from mpkorstanje

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

All features are run instead of none
3 participants
@tommywo @coveralls @mpkorstanje