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

Update runner.rb - A suggested fix for 'The command line is too long.' in Windows when the number of tests are huge #981

Closed
wants to merge 5 commits into from
Closed
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
54 changes: 53 additions & 1 deletion lib/parallel_tests/test/runner.rb
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,41 @@ def tests_with_size(tests, options)
tests
end

def process_in_batches(cmd, os_cmd_length_limit, tests)
# Filter elements not starting with value in tests to retain in each batch
split_elements = cmd.partition { |s| s.start_with?(tests) }

# elements that needs to be checked for length and sliced into batches
non_retained_elements = split_elements.first

# common parameters for each batch
retained_elements = split_elements.last

batches = []
index = 0
while index < non_retained_elements.length
batch = retained_elements.dup
total_length = batch.map(&:length).sum
total_length += batch.size # account for spaces between elements

while index < non_retained_elements.length
current_element_length = non_retained_elements[index].length
current_element_length += 1 # account for spaces between elements

# Check if the current element can be added without exceeding the os cmd length limit
break unless total_length + current_element_length <= os_cmd_length_limit

batch << non_retained_elements[index]
total_length += current_element_length
index += 1
end

batches << batch
end

batches
end

def execute_command(cmd, process_number, num_processes, options)
number = test_env_number(process_number, options).to_s
env = (options[:env] || {}).merge(
Expand All @@ -99,7 +134,24 @@ def execute_command(cmd, process_number, num_processes, options)

print_command(cmd, env) if report_process_command?(options) && !options[:serialize_stdout]

execute_command_and_capture_output(env, cmd, options)
result = []
process_in_batches(cmd, 8191, options[:files].first).map do |subcmd|
Copy link
Owner

Choose a reason for hiding this comment

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

I don't get the options[:files].first) part, why is the first file the suffix for all other files ?
(tested with start_with? in process_in_batches)

Copy link
Owner

Choose a reason for hiding this comment

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

... I'd split the commands at the first file

Copy link
Author

Choose a reason for hiding this comment

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

the common parameters can appear after the files as well .. hence i was doing a starts_with ..

Copy link
Owner

Choose a reason for hiding this comment

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

thx, updated in my draft, plz see if that looks alright

result << execute_command_and_capture_output(env, subcmd, options)
end

# combine the output of result array into a single Hash
combined_result = {}
result.each do |res|
if combined_result.empty?
combined_result = res
else
combined_result[:stdout] = combined_result[:stdout].to_s + res[:stdout].to_s
combined_result[:exit_status] = res[:exit_status] > combined_result[:exit_status] ? res[:exit_status] : combined_result[:exit_status] # keep the max
combined_result[:command] = combined_result[:command] | res[:command]
rajaramaniyer marked this conversation as resolved.
Show resolved Hide resolved
end
end

combined_result
end

def print_command(command, env)
Expand Down