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

adding parser to extract files from cucumber profile #576

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

vveliev
Copy link

@vveliev vveliev commented Jul 31, 2017

Adding ability to execute cucumber profile with parallel test, using file list provided in cucumber profile

@@ -174,6 +174,7 @@ def parse_options!(argv)
opts.on("--only-group INT[, INT]", Array) { |groups| options[:only_group] = groups.map(&:to_i) }

opts.on("-e", "--exec [COMMAND]", "execute this code parallel and with ENV['TEST_ENV_NUMBER']") { |path| options[:execute] = path }
opts.on("--cucumber_profile [PROFILE]", "execute cucumber profile") {|profile| options[:cucumber_profile] = profile}
Copy link
Owner

Choose a reason for hiding this comment

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

use - in options -> cucumber-profile

@@ -211,6 +212,7 @@ def parse_options!(argv)

files, remaining = extract_file_paths(argv)
unless options[:execute]
files = extract_files_from_cucumber_profile(options[:cucumber_profile]) if (!files.any?)&&options.key?(:cucumber_profile)
Copy link
Owner

Choose a reason for hiding this comment

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

if files.empty? && cucumber_profile = options[:cucumber_profile]
  files = extract_files_from_cucumber_profile(cucumber_profile)
end

Copy link
Owner

Choose a reason for hiding this comment

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

... also add here raise "only supported for cucumber" unless @runner.name == 'cucumber' instead of in extract_files_from_cucumber_profile

@@ -28,6 +29,12 @@ def summarize_results(results)
output.join("\n\n")
end


Copy link
Owner

Choose a reason for hiding this comment

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

remove extra line


def files_from_profile(name)
profile = ::Cucumber::Cli::ProfileLoader.new.args_from(name)
profile.delete_if{|x| !x.match(self.test_suffix)}
Copy link
Owner

Choose a reason for hiding this comment

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

-> profile.grep(test_suffix)

@grosser
Copy link
Owner

grosser commented Jul 31, 2017

that looks pretty good ... how about a test so I don't break it on the next release ? :D

@grosser
Copy link
Owner

grosser commented Aug 1, 2017

parsing --test-options seems a bit hacky ... and I'd guess it blows up on unknown options ... I liked the previous --cucumber-profile option more ... or did that not work out ?

@@ -28,6 +29,11 @@ def summarize_results(results)
output.join("\n\n")
end


def files_from_profile(name)
::Cucumber::Cli::ProfileLoader.new.args_from(name).grep(self.test_suffix)
Copy link
Owner

Choose a reason for hiding this comment

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

don't need the self.

@vveliev
Copy link
Author

vveliev commented Aug 1, 2017

--cucumber-profile, it's missing logic , as I did not passed cucumber profile option to the cucumber itself ....

parallel_cucumber -o '--profile smoke --tag @setup'
parallel_cucumber -o '--tag @setup' --cucumber-profile smoke
parallel_cucumber -o '--profile smoke --tag @setup' --cucumber-profile smoke

So I think there is potential problem when profile coming from stand alone option... not sure what's the best solution.

@grosser
Copy link
Owner

grosser commented Aug 1, 2017 via email

@vveliev
Copy link
Author

vveliev commented Aug 1, 2017

Yes it could , but then It should be more like a merge ....or raise an error if profile exist in options

@grosser
Copy link
Owner

grosser commented Aug 1, 2017 via email

@teegalasv
Copy link

@grosser Any update on this pull request. i think this feature will help me too. If you know any other way to achieve this and already exists can you please point me to it.

@grosser
Copy link
Owner

grosser commented Jan 6, 2020

idk, I'm not working on this ... you could fork and use this as a base for a new PR

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.

4 participants