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

Option to output feature files to texts grouped by process. #637

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Timbyhider
Copy link

@Timbyhider Timbyhider commented Jun 6, 2018

@grosser

First of all. Thank you for your work on this gem! It has vastly increased the speed of our scripts.

My changes were made in an effort to solve a few issues we were having running test scripts in parallel with our Ruby/Cucumber/Gherkin test suite. I am open to suggestions and happy to add whatever is necessary to get this included and will do my best to support the functionality. These changes give the user the ability to add the --features_by_process option to their job. Once the tests are parsed and divided up they will be stored in .txt files in the project directory as 'features_from_process_<process_number>.txt'

This solves two issues we were having. By outputting the features to text files we are quickly able to rerun subsets of features should one process fail. Additionally, this can help users with large test suites. We have a large test suite and were unable to run the entirety of the suite on less than 8 cores/processes as the commands generated exceeded the character limit of windows commands. The commands generated will run the tests via the @features_from_process_<process_number>.txt format rather than by listing out the path and names of each feature.

else
(0...num_processes).to_a
end
options[:only_group].map{|g| g - 1}
Copy link
Owner

Choose a reason for hiding this comment

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

2-space indent plz

run_tests(group, groups.index(group), num_processes, options)
end
end
groups_to_run = options[:only_group].collect{|i| groups[i - 1]}.compact
Copy link
Owner

Choose a reason for hiding this comment

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

2-space indent

report_number_of_tests(groups)
if options[:features_from_process]
ran = []
for x in 0...num_processes
Copy link
Owner

Choose a reason for hiding this comment

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

num_processes.times do |process_number| ?

ran = []
for x in 0...num_processes
ran << File.expand_path(Dir.pwd) + "/features_from_process_#{x}.txt"
File.open(File.expand_path(Dir.pwd) + "/features_from_process_#{x}.txt", 'w+') { |file| groups[x].each{|feature| file.write("\n#{feature}")}}
Copy link
Owner

Choose a reason for hiding this comment

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

prefer File.write file, content

ran = []
for x in 0...num_processes
ran << File.expand_path(Dir.pwd) + "/features_from_process_#{x}.txt"
File.open(File.expand_path(Dir.pwd) + "/features_from_process_#{x}.txt", 'w+') { |file| groups[x].each{|feature| file.write("\n#{feature}")}}
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 build the path twice

if options[:features_from_process]
ran = []
for x in 0...num_processes
ran << File.expand_path(Dir.pwd) + "/features_from_process_#{x}.txt"
Copy link
Owner

Choose a reason for hiding this comment

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

idk why the expand_path is needed if it is added to pwd ...
prefer not dumping tempfiles into local directory it messes with git ...
can dump into tmp/ maybe even use Tempfile to get a truly unique file

@@ -216,6 +224,7 @@ def parse_options!(argv)
opts.on("--unknown-runtime [FLOAT]", Float, "Use given number as unknown runtime (otherwise use average time)") { |time| options[:unknown_runtime] = time }
opts.on("--first-is-1", "Use \"1\" as TEST_ENV_NUMBER to not reuse the default test environment") { options[:first_is_1] = true }
opts.on("--verbose", "Print more output") { options[:verbose] = true }
opts.on("--features_from_process", "Write features to text file") { options[:features_from_process] = true }
Copy link
Owner

Choose a reason for hiding this comment

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

features-from-process

Copy link
Owner

Choose a reason for hiding this comment

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

the name does not really convey a purpose/feature to me and neither does the description ...

cucumber_opts(options[:test_options]),
"@features_from_process_#{process_number}.txt"
].compact.reject(&:empty?).join(' ')
execute_command(cmd, process_number, num_processes, options)
Copy link
Owner

Choose a reason for hiding this comment

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

try to reuse the execute_command above maybe with a shared method

@grosser
Copy link
Owner

grosser commented Jun 7, 2018

yeah I heard about that too many files problem before :(

ideally it would auto-detect the need for this hackery somehow (on windows + >xyz chars)

any idea if this "read from file" logic would work for other test runners too ?

needs at least a minimal cli test so I don't break it later on :)

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.

None yet

2 participants