-
Notifications
You must be signed in to change notification settings - Fork 500
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
Cucumber 4 support #762
Cucumber 4 support #762
Conversation
I rebased this PR on top of the other ones. Hopefully CI will pass now. |
You should remove the require |
Done!
…On Wed, Jun 10, 2020 at 7:45 AM David Rodríguez ***@***.***> wrote:
You should remove the require TravisCI check in "Branch settings" now
that we're no longer using it.
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
<#762 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAACYZ7JWYXCVTUMXPOBLTTRV6MA3ANCNFSM4N2IOCMA>
.
|
rescue LoadError | ||
fail_message = <<~ERROR | ||
Grouping by individual cucumber scenarios requires the `cuke_modeler` modeler gem with requirement `~> 3.0`. | ||
Add `gem "cuke_modeler", "~> 3.0"` to your `Gemfile`, run `bundle install` and try again. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you never know what this error is coming from (could be dependency of dependency), so add \nCaused by: #{$!}
or something like that
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I looked into reraising the error, which has the behaviour you want by default when inside a rescue
block, but aborting with a message seemed clear for end users for me. cuke_modeler
has a single dependency, and even if it was the dependency that is missing, the instructions given would fix things.
If you still prefer the more verbose approach, I'm happy to change it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
prefer adding the causing error, not having it can leave users with no backtrace / no idea what's going on when something unexpected happens.
also add "If you are trying to run with a previous version of , try v2.32.0 of parallel_tests."
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ended up reraising. Since rerasing inside a rescue
block, the cause is automatically displayed by ruby. This is how the error looks now:
$ bin/parallel_cucumber --group-by scenarios features/
Traceback (most recent call last):
14: from bin/parallel_cucumber:11:in `<main>'
13: from bin/parallel_cucumber:11:in `load'
12: from /home/deivid/Code/parallel_tests/bin/parallel_cucumber:9:in `<top (required)>'
11: from /home/deivid/Code/parallel_tests/lib/parallel_tests/cli.rb:24:in `run'
10: from /home/deivid/Code/parallel_tests/lib/parallel_tests/cli.rb:82:in `run_tests_in_parallel'
9: from /home/deivid/Code/parallel_tests/lib/parallel_tests/cli.rb:307:in `report_time_taken'
8: from /home/deivid/Code/parallel_tests/lib/parallel_tests.rb:97:in `delta'
7: from /home/deivid/Code/parallel_tests/lib/parallel_tests/cli.rb:307:in `block in report_time_taken'
6: from /home/deivid/Code/parallel_tests/lib/parallel_tests/cli.rb:59:in `block in run_tests_in_parallel'
5: from /home/deivid/Code/parallel_tests/lib/parallel_tests/gherkin/runner.rb:85:in `tests_in_groups'
4: from /home/deivid/Code/parallel_tests/lib/parallel_tests/grouper.rb:10:in `by_scenarios'
3: from /home/deivid/Code/parallel_tests/lib/parallel_tests/grouper.rb:50:in `group_by_scenarios'
2: from /home/deivid/Code/parallel_tests/lib/parallel_tests/grouper.rb:50:in `require'
1: from /home/deivid/Code/parallel_tests/lib/parallel_tests/cucumber/scenarios.rb:9:in `<top (required)>'
/home/deivid/.rbenv/versions/2.6.6/lib/ruby/gems/2.6.0/gems/bundler-2.1.4/lib/bundler/rubygems_integration.rb:346:in `block (2 levels) in replace_gem': cuke_modeler is not part of the bundle. Add it to your Gemfile. (Gem::LoadError)
14: from bin/parallel_cucumber:11:in `<main>'
13: from bin/parallel_cucumber:11:in `load'
12: from /home/deivid/Code/parallel_tests/bin/parallel_cucumber:9:in `<top (required)>'
11: from /home/deivid/Code/parallel_tests/lib/parallel_tests/cli.rb:24:in `run'
10: from /home/deivid/Code/parallel_tests/lib/parallel_tests/cli.rb:82:in `run_tests_in_parallel'
9: from /home/deivid/Code/parallel_tests/lib/parallel_tests/cli.rb:307:in `report_time_taken'
8: from /home/deivid/Code/parallel_tests/lib/parallel_tests.rb:97:in `delta'
7: from /home/deivid/Code/parallel_tests/lib/parallel_tests/cli.rb:307:in `block in report_time_taken'
6: from /home/deivid/Code/parallel_tests/lib/parallel_tests/cli.rb:59:in `block in run_tests_in_parallel'
5: from /home/deivid/Code/parallel_tests/lib/parallel_tests/gherkin/runner.rb:85:in `tests_in_groups'
4: from /home/deivid/Code/parallel_tests/lib/parallel_tests/grouper.rb:10:in `by_scenarios'
3: from /home/deivid/Code/parallel_tests/lib/parallel_tests/grouper.rb:50:in `group_by_scenarios'
2: from /home/deivid/Code/parallel_tests/lib/parallel_tests/grouper.rb:50:in `require'
1: from /home/deivid/Code/parallel_tests/lib/parallel_tests/cucumber/scenarios.rb:8:in `<top (required)>'
/home/deivid/Code/parallel_tests/lib/parallel_tests/cucumber/scenarios.rb:12:in `rescue in <top (required)>': Grouping by individual cucumber scenarios requires the `cuke_modeler` modeler gem with requirement `~> 3.0`. (RuntimeError)
Add `gem "cuke_modeler", "~> 3.0"` to your `Gemfile`, run `bundle install` and try again.
also add "If you are trying to run with a previous version of , try v2.32.0 of parallel_tests."
I assume you mean with a previous version of cucumber? As long as users add cuke_modeler
to their Gemfile
's, this should work for every cucumber version. Since users probably have upgraded parallel_tests
for a reason, I think just the cuke_modeler
recommendation should be fine?
lib/parallel_tests/grouper.rb
Outdated
begin | ||
gem "cuke_modeler", "~> 3.0" | ||
require 'cuke_modeler' | ||
rescue LoadError | ||
fail_message = <<~ERROR | ||
Grouping by number of cucumber steps requires the `cuke_modeler` modeler gem with requirement `~> 3.0`. | ||
Add `gem "cuke_modeler", "~> 3.0"` to your `Gemfile`, run `bundle install` and try again. | ||
ERROR | ||
|
||
fail_message = "\e[31m#{fail_message}\e[0m" if $stdout.tty? | ||
|
||
abort fail_message | ||
end | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
copy-pasted ... maybe maybe a method in parallel_tests.rb
like require_dependency(gem, version)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
... or I guess add it here somewhere since it will be specific to cucumber
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you look at the latest version of the patch and let me know if this is worth refactoring?
this looks sane/good overall! :) |
To not depend on gherkin parsing details.
For consistency.
3.0.0 ... let's see how this goes 🎉 |
Awesome, thanks! |
This PR adds cucumber 4 support.
Only user facing change is that the
cuke_modeler
gem will be needed from now on for splitting by steps or scenarios (--group-by
flag withscenario
andstep
values). In that case, users will get an error telling them to add thecuke_modeler
gem to theirGemfile
.Closes #671.