Navigation Menu

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

Cucumber 4 support #762

Merged
merged 9 commits into from Jun 10, 2020
Merged

Cucumber 4 support #762

merged 9 commits into from Jun 10, 2020

Conversation

deivid-rodriguez
Copy link
Contributor

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 with scenario and step values). In that case, users will get an error telling them to add the cuke_modeler gem to their Gemfile.

Closes #671.

@deivid-rodriguez
Copy link
Contributor Author

I rebased this PR on top of the other ones. Hopefully CI will pass now.

@deivid-rodriguez
Copy link
Contributor Author

You should remove the require TravisCI check in "Branch settings" now that we're no longer using it.

@grosser
Copy link
Owner

grosser commented Jun 10, 2020 via email

Gemfile.lock Show resolved Hide resolved
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.
Copy link
Owner

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

Copy link
Contributor Author

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.

Copy link
Owner

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."

Copy link
Contributor Author

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?

Comment on lines 45 to 58
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

Copy link
Owner

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)

Copy link
Owner

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

Copy link
Contributor Author

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?

@grosser
Copy link
Owner

grosser commented Jun 10, 2020

this looks sane/good overall! :)

@grosser grosser merged commit d9af8f6 into grosser:master Jun 10, 2020
@deivid-rodriguez deivid-rodriguez deleted the cucumber_4_support branch June 10, 2020 16:51
@grosser
Copy link
Owner

grosser commented Jun 10, 2020

3.0.0

... let's see how this goes 🎉

@deivid-rodriguez
Copy link
Contributor Author

Awesome, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Cucumber 4 support
2 participants