Is there a way to use the group number?

[stevenproctor]

We are getting errors in our CI builds when we matrix our test runs, with the only-group options enabled.

We set PARALLEL_TEST_PROCESSORS to 6, and do a matrix build where each of the matrix builds specifies an --only-groups option of either 1,2, 3,4, or 5,6.

This allows us to use 3 cheaper servers with 2 processors instead of a bigger server with 6 processors.

Through debugging, I saw that each matrix output is running the different groups, but the TEST_ENV_NUMBER always gets set to '' or '2' that is coming from the number of processes being run, instead of using the test group number for that run, causing the different runs across machines to only use 2 of the 6 database prepared.

Is it a possibility to get the TEST_ENV_NUMBER to refer to the GroupNumber instead of the process number in the group?

Happy to take a shot at this, and make an option --use-group-number that would set the process number to the group number instead of the index of the group number in the only_group array, but not sure if there is a better, preferred, or already exiting, solution to this issue.

[grosser]

having an option for that sounds nice
change needs to go here

parallel_tests/lib/parallel_tests/test/runner.rb

Line 117 in 01218db

def test_env_number(process_number, options = {})

the current behavior makes sense too (users only needed 2 dbs and can run different groups) so --group-as-test-env-number would make sense

a simpler workaround might be to have a "test_env_offset" env var and then do TEST_ENV_NUMBER + offset to pick your db

[stevenproctor]

Looking at this a bit deeper, it looks like the execute_shell_command_in_parallel works this way already:

parallel_tests/lib/parallel_tests/cli.rb

Lines 349 to 363 in 01218db

	runs = if options[:only_group]
	options[:only_group].map { |g| g - 1 }
	else
	(0...num_processes).to_a
	end
	results = if options[:non_parallel]
	ParallelTests.with_pid_file do
	runs.map do |i|
	ParallelTests::Test::Runner.execute_command(command, i, num_processes, options)
	end
	end
	else
	execute_in_parallel(runs, runs.size, options) do |i|
	ParallelTests::Test::Runner.execute_command(command, i, num_processes, options)
	end

where the group number is used as the process number. If the above works, might it make sense to update:

parallel_tests/lib/parallel_tests/cli.rb

Lines 67 to 69 in 01218db

	execute_in_parallel(groups_to_run, groups_to_run.size, options) do |group|
	run_tests(group, groups_to_run.index(group), 1, options)
	end

to possibly look like:

          execute_in_parallel(groups_to_run, groups_to_run.size, options) do |group|
            process_number = options[:group_as_test_env_number] ? group : groups_to_run.index(group)
            run_tests(group, groups_to_run.index(group), 1, options)
          end

The thing that makes me wonder if it would be as straightforward as that would be the num_processes is always set to 1 in the run_tests, even though it is passed as the groups_to_run.size in the execute_in_parallel call.

Also, it doesn't seem to have impact directly in parallel_tests itself if the TEST_ENV_NUMBER is larger than
PARALLEL_TEST_GROUPS, but it might have impact for those that are trying to use the first_process? and last_process? predicates that this gem provides.

parallel_tests/lib/parallel_tests.rb

Lines 66 to 76 in e1bc5c5

	def first_process?
	ENV["TEST_ENV_NUMBER"].to_i <= 1
	end
	def last_process?
	current_process_number = ENV['TEST_ENV_NUMBER']
	total_processes = ENV['PARALLEL_TEST_GROUPS']
	return true if current_process_number.nil? && total_processes.nil?
	current_process_number = '1' if current_process_number.nil?
	current_process_number == total_processes
	end

The last part may be a moot point, as with the num_processes always being set to 1, when the TEST_ENV_NUMBER is set to anything greater than that the last_process? would seem to behave as it does today where it looks to consider the first process to be the last process for a group that has more than one item. I am not sure if that is intended behavior, a bug, a not-covered use case when using only-group, or something else that I might be missing.

Just checking to make sure I am reading the code correctly and built up context on how this is working.

[grosser]

idk why it's set to 1, might have to git-blame a bit to see when it was added
the suggested patch looks about right though
idk how first/last will be impacted by this change, but as long as it's opt-in we can learn and adjust 🤞

[tavlima]

Hey, @stevenproctor, did you get this working, eventually? I'm facing the same issue and was wondering where your investigations led you, as I couldn't find any related PR.