From f133a0cc9cae95dc793912c42905477b5373a9fc Mon Sep 17 00:00:00 2001 From: Sammy Steele Date: Tue, 9 Dec 2025 10:48:36 -0800 Subject: [PATCH 1/4] Enforce a range for supported chunk sizes in CI --- ruby/lib/ci/queue/strategy/suite_bin_packing.rb | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/ruby/lib/ci/queue/strategy/suite_bin_packing.rb b/ruby/lib/ci/queue/strategy/suite_bin_packing.rb index b752518..e22cc79 100644 --- a/ruby/lib/ci/queue/strategy/suite_bin_packing.rb +++ b/ruby/lib/ci/queue/strategy/suite_bin_packing.rb @@ -28,8 +28,9 @@ def initialize(config, redis: nil) else {} end - - @max_duration = config&.suite_max_duration || 120_000 + # Enforce the max chunk duration falls within this range. + @minimum_max_duration = config&.suite_max_duration || 120_000 + @maximum_max_duration = config&.suite_max_duration || 300_000 @fallback_duration = config&.timing_fallback_duration || 100.0 @buffer_percent = config&.suite_buffer_percent || 10 @@ -106,8 +107,8 @@ def calculate_dynamic_max_duration(tests) puts "parallel_job_count: #{parallel_job_count}" - # If no parallel job count, fall back to configured max_duration - return @max_duration unless parallel_job_count && parallel_job_count > 0 + # If no parallel job count, fall back to configured minimum max_duration + return @minimum_max_duration unless parallel_job_count && parallel_job_count > 0 # Calculate total duration of all tests total_duration = tests.sum do |test| @@ -118,11 +119,12 @@ def calculate_dynamic_max_duration(tests) # This gives us the target max chunk time base_max_duration = total_duration.to_f / parallel_job_count - puts "base_max_duration: #{base_max_duration}, @max_duration: #{@max_duration}" + puts "base_max_duration: #{base_max_duration}, @minimum_max_duration: #{@minimum_max_duration}, @maximum_max_duration: #{@maximum_max_duration}" - # Ensure we don't go below a minimum reasonable value + # Ensure we don't go above or below reasonable floor values. # Use configured max_duration as a floor to prevent extremely small chunks - [base_max_duration, @max_duration].max + max_duration = [base_max_duration, @maximum_max_duration].min + [max_duration, @minimum_max_duration].max end def create_chunks_for_suite(suite_name, suite_tests, max_duration) From c7bca9f5c9c8f8ec28b0e438ebe5d0d52f65e00d Mon Sep 17 00:00:00 2001 From: Sammy Steele Date: Tue, 9 Dec 2025 11:48:16 -0800 Subject: [PATCH 2/4] Add env variable into suite bin packing --- .../lib/ci/queue/strategy/suite_bin_packing.rb | 4 ++-- ruby/lib/minitest/queue/runner.rb | 18 ++++++++++++++++++ 2 files changed, 20 insertions(+), 2 deletions(-) diff --git a/ruby/lib/ci/queue/strategy/suite_bin_packing.rb b/ruby/lib/ci/queue/strategy/suite_bin_packing.rb index e22cc79..fe62ce1 100644 --- a/ruby/lib/ci/queue/strategy/suite_bin_packing.rb +++ b/ruby/lib/ci/queue/strategy/suite_bin_packing.rb @@ -29,8 +29,8 @@ def initialize(config, redis: nil) {} end # Enforce the max chunk duration falls within this range. - @minimum_max_duration = config&.suite_max_duration || 120_000 - @maximum_max_duration = config&.suite_max_duration || 300_000 + @minimum_max_duration = config&.suite_minimum_max_chunk_duration || 120_000 + @maximum_max_duration = config&.suite_maximum_max_chunk_duration || 300_000 @fallback_duration = config&.timing_fallback_duration || 100.0 @buffer_percent = config&.suite_buffer_percent || 10 diff --git a/ruby/lib/minitest/queue/runner.rb b/ruby/lib/minitest/queue/runner.rb index c31ca7c..cc7c204 100644 --- a/ruby/lib/minitest/queue/runner.rb +++ b/ruby/lib/minitest/queue/runner.rb @@ -512,6 +512,24 @@ def parser queue_config.max_duration = max end + help = <<~EOS + Defines a lower-bound for the max chunk duration in seconds. + Defaults to 120 seconds. + EOS + opts.separator "" + opts.on('--minimum-max-chunk-duration MILLISECONDS', Integer, help) do |min| + queue_config.minimum_max_chunk_duration = min + end + + help = <<~EOS + Defines an upper-bound for the max chunk duration in seconds. + Defaults to 300 seconds. + EOS + opts.separator "" + opts.on('--maximum-max-chunk-duration MILLISECONDS', Integer, help) do |max| + queue_config.maximum_max_chunk_duration = max + end + help = <<~EOS Defines how many user test tests can be fail. Defaults to none. From 08c13d5ec753b81bcd57a6ed8729f4fcdad10c80 Mon Sep 17 00:00:00 2001 From: Sammy Steele Date: Tue, 9 Dec 2025 12:58:41 -0800 Subject: [PATCH 3/4] Pass environment variables through --- ruby/lib/ci/queue/configuration.rb | 4 ++++ ruby/lib/ci/queue/strategy/suite_bin_packing.rb | 4 ++-- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/ruby/lib/ci/queue/configuration.rb b/ruby/lib/ci/queue/configuration.rb index f79d814..9e9c133 100644 --- a/ruby/lib/ci/queue/configuration.rb +++ b/ruby/lib/ci/queue/configuration.rb @@ -10,6 +10,7 @@ class Configuration attr_accessor :max_test_failed, :redis_ttl attr_accessor :strategy, :timing_file, :timing_fallback_duration, :export_timing_file attr_accessor :suite_max_duration, :suite_buffer_percent + attr_accessor :minimum_max_chunk_duration, :maximum_max_chunk_duration attr_accessor :branch attr_accessor :timing_redis_url attr_accessor :write_duration_averages @@ -61,6 +62,7 @@ def initialize( export_flaky_tests_file: nil, known_flaky_tests: [], strategy: :random, timing_file: nil, timing_fallback_duration: 100.0, export_timing_file: nil, suite_max_duration: 120_000, suite_buffer_percent: 10, + minimum_max_chunk_duration: 120_000, maximum_max_chunk_duration: 300_000, branch: nil, timing_redis_url: nil, heartbeat_grace_period: 30, @@ -76,6 +78,8 @@ def initialize( @max_test_duration = max_test_duration @max_test_duration_percentile = max_test_duration_percentile @max_test_failed = max_test_failed + @minimum_max_chunk_duration = minimum_max_chunk_duration + @maximum_max_chunk_duration = maximum_max_chunk_duration @namespace = namespace @requeue_tolerance = requeue_tolerance @seed = seed diff --git a/ruby/lib/ci/queue/strategy/suite_bin_packing.rb b/ruby/lib/ci/queue/strategy/suite_bin_packing.rb index fe62ce1..0b8deae 100644 --- a/ruby/lib/ci/queue/strategy/suite_bin_packing.rb +++ b/ruby/lib/ci/queue/strategy/suite_bin_packing.rb @@ -29,8 +29,8 @@ def initialize(config, redis: nil) {} end # Enforce the max chunk duration falls within this range. - @minimum_max_duration = config&.suite_minimum_max_chunk_duration || 120_000 - @maximum_max_duration = config&.suite_maximum_max_chunk_duration || 300_000 + @minimum_max_duration = config&.minimum_max_chunk_duration || 120_000 + @maximum_max_duration = config&.maximum_max_chunk_duration || 300_000 @fallback_duration = config&.timing_fallback_duration || 100.0 @buffer_percent = config&.suite_buffer_percent || 10 From 73c65a75df7d05e13226d3a516f859eaedf8932e Mon Sep 17 00:00:00 2001 From: Sammy Steele Date: Tue, 9 Dec 2025 13:17:14 -0800 Subject: [PATCH 4/4] Add additional unit tests --- .../queue/strategy/suite_bin_packing_test.rb | 190 +++++++++++++++++- 1 file changed, 179 insertions(+), 11 deletions(-) diff --git a/ruby/test/ci/queue/strategy/suite_bin_packing_test.rb b/ruby/test/ci/queue/strategy/suite_bin_packing_test.rb index 0eb7e71..b22d427 100644 --- a/ruby/test/ci/queue/strategy/suite_bin_packing_test.rb +++ b/ruby/test/ci/queue/strategy/suite_bin_packing_test.rb @@ -5,7 +5,8 @@ class SuiteBinPackingTest < Minitest::Test def setup @config = CI::Queue::Configuration.new( - suite_max_duration: 120_000, + minimum_max_chunk_duration: 120_000, + maximum_max_chunk_duration: 300_000, suite_buffer_percent: 10, timing_fallback_duration: 100.0 ) @@ -67,7 +68,8 @@ def test_splits_suite_when_over_max_duration end def test_applies_buffer_when_splitting - @config.suite_max_duration = 100_000 + @config.minimum_max_chunk_duration = 100_000 + @config.maximum_max_chunk_duration = 100_000 @config.suite_buffer_percent = 10 tests = create_mock_tests(['TestSuite#test_1', 'TestSuite#test_2']) @@ -186,7 +188,8 @@ def test_chunk_id_format end def test_suite_exactly_at_max_duration_gets_split - @config.suite_max_duration = 100_000 + @config.minimum_max_chunk_duration = 100_000 + @config.maximum_max_chunk_duration = 100_000 @config.suite_buffer_percent = 10 tests = create_mock_tests(['TestSuite#test_1', 'TestSuite#test_2']) @@ -204,7 +207,8 @@ def test_suite_exactly_at_max_duration_gets_split end def test_suite_exactly_at_effective_max_fits_in_one_chunk - @config.suite_max_duration = 100_000 + @config.minimum_max_chunk_duration = 100_000 + @config.maximum_max_chunk_duration = 100_000 @config.suite_buffer_percent = 10 tests = create_mock_tests(['TestSuite#test_1', 'TestSuite#test_2']) @@ -282,7 +286,8 @@ def test_test_count_is_set_correctly end def test_large_suite_creates_multiple_chunks - @config.suite_max_duration = 100_000 + @config.minimum_max_chunk_duration = 100_000 + @config.maximum_max_chunk_duration = 100_000 @config.suite_buffer_percent = 10 # Create a suite that will need many chunks @@ -462,7 +467,7 @@ def test_dynamic_max_duration_calculation # Total duration = 40,000ms # Parallel jobs = 4 # Calculated base_max_duration = 40,000 / 4 = 10,000ms - # However, 10,000ms < configured @max_duration (120,000ms), so floor logic applies + # However, 10,000ms < configured minimum_max_chunk_duration (120,000ms), so floor logic applies # Actual max_duration used = max(10,000, 120,000) = 120,000ms # With 10% buffer, effective_max = 120,000 * 0.9 = 108,000ms # All 4 tests fit in one chunk: 4 * 10,000 = 40,000ms < 108,000ms @@ -482,7 +487,7 @@ def test_dynamic_max_duration_falls_back_to_configured_when_no_env_var chunks = order_with_timing(tests, timing_data) - # Should use configured max_duration (120,000ms default) + # Should use configured minimum_max_chunk_duration (120,000ms default) # So test should fit in one chunk assert_equal 1, chunks.size end @@ -497,7 +502,7 @@ def test_dynamic_max_duration_uses_floor_when_calculated_value_too_small chunks = order_with_timing(tests, timing_data) # Calculated max = 1000 / 100 = 10ms (too small) - # Should use configured max_duration (120,000ms) as floor + # Should use configured minimum_max_chunk_duration (120,000ms) as floor # So test should fit in one chunk assert_equal 1, chunks.size ensure @@ -518,7 +523,7 @@ def test_dynamic_max_duration_with_large_parallelism # Total duration = 20 * 5000 = 100,000ms # Parallel jobs = 10 # Calculated base_max_duration = 100,000 / 10 = 10,000ms - # However, 10,000ms < configured @max_duration (120,000ms), so floor logic applies + # However, 10,000ms < configured minimum_max_chunk_duration (120,000ms), so floor logic applies # Actual max_duration used = max(10,000, 120,000) = 120,000ms # With 10% buffer, effective_max = 120,000 * 0.9 = 108,000ms # All 20 tests fit in one chunk: 20 * 5,000 = 100,000ms < 108,000ms @@ -530,7 +535,7 @@ def test_dynamic_max_duration_with_large_parallelism end def test_dynamic_max_duration_exceeds_default_max_causes_splits - # Set up scenario where computed chunk size > default max_duration + # Set up scenario where computed chunk size > default minimum_max_chunk_duration ENV['BUILDKITE_PARALLEL_JOB_COUNT'] = '2' # Create tests with large total duration @@ -544,7 +549,7 @@ def test_dynamic_max_duration_exceeds_default_max_causes_splits # Total duration = 10 * 30,000 = 300,000ms # Parallel jobs = 2 # Calculated base_max_duration = 300,000 / 2 = 150,000ms - # 150,000ms > configured @max_duration (120,000ms), so use calculated value + # 150,000ms > configured minimum_max_chunk_duration (120,000ms), so use calculated value # Actual max_duration used = max(150,000, 120,000) = 150,000ms # With 10% buffer, effective_max = 150,000 * 0.9 = 135,000ms # Each test is 30,000ms, so we can fit 4 per chunk (4 * 30,000 = 120,000 < 135,000) @@ -560,6 +565,169 @@ def test_dynamic_max_duration_exceeds_default_max_causes_splits ENV.delete('BUILDKITE_PARALLEL_JOB_COUNT') end + def test_dynamic_max_duration_capped_at_maximum + # Set up scenario where computed chunk size would exceed maximum_max_chunk_duration + ENV['BUILDKITE_PARALLEL_JOB_COUNT'] = '1' + + # Create tests with very large total duration + tests = create_mock_tests((1..10).map { |i| "TestSuite#test_#{i}" }) + timing_data = (1..10).each_with_object({}) do |i, hash| + hash["TestSuite#test_#{i}"] = 80_000.0 # Each test is 80 seconds + end + + chunks = order_with_timing(tests, timing_data) + + # Total duration = 10 * 80,000 = 800,000ms + # Parallel jobs = 1 + # Calculated base_max_duration = 800,000 / 1 = 800,000ms + # 800,000ms > configured maximum_max_chunk_duration (300,000ms), so cap it + # Actual max_duration used = min(800,000, 300,000) = 300,000ms + # With 10% buffer, effective_max = 300,000 * 0.9 = 270,000ms + # Each test is 80,000ms, so we can fit 3 per chunk (3 * 80,000 = 240,000 < 270,000) + # With 10 tests, we'll get 4 chunks (3+3+3+1) + test_suite_chunks = chunks.select { |c| c.suite_name == 'TestSuite' } + assert_equal 4, test_suite_chunks.size, 'Should cap at maximum_max_chunk_duration' + + # Verify chunks respect the capped max_duration + test_suite_chunks[0..2].each do |chunk| + assert_equal 3, chunk.test_ids.size, 'First 3 chunks should have 3 tests each' + assert_equal 240_000.0, chunk.estimated_duration + end + assert_equal 1, test_suite_chunks[3].test_ids.size, 'Last chunk should have 1 test' + assert_equal 80_000.0, test_suite_chunks[3].estimated_duration + ensure + ENV.delete('BUILDKITE_PARALLEL_JOB_COUNT') + end + + def test_dynamic_max_duration_within_range + # Set up scenario where computed chunk size falls within the configured range + ENV['BUILDKITE_PARALLEL_JOB_COUNT'] = '5' + + tests = create_mock_tests((1..10).map { |i| "TestSuite#test_#{i}" }) + timing_data = (1..10).each_with_object({}) do |i, hash| + hash["TestSuite#test_#{i}"] = 20_000.0 # Each test is 20 seconds + end + + chunks = order_with_timing(tests, timing_data) + + # Total duration = 10 * 20,000 = 200,000ms + # Parallel jobs = 5 + # Calculated base_max_duration = 200,000 / 5 = 40,000ms + # 40,000ms < minimum_max_chunk_duration (120,000ms), so use minimum + # Actual max_duration used = max(40,000, 120,000) = 120,000ms + # With 10% buffer, effective_max = 120,000 * 0.9 = 108,000ms + # Each test is 20,000ms, so we can fit 5 per chunk (5 * 20,000 = 100,000 < 108,000) + # With 10 tests, we'll get 2 chunks (5+5) + test_suite_chunks = chunks.select { |c| c.suite_name == 'TestSuite' } + assert_equal 2, test_suite_chunks.size, 'Should create 2 chunks with minimum floor applied' + + test_suite_chunks.each do |chunk| + assert_equal 5, chunk.test_ids.size, 'Each chunk should have 5 tests' + assert_equal 100_000.0, chunk.estimated_duration + end + ensure + ENV.delete('BUILDKITE_PARALLEL_JOB_COUNT') + end + + def test_range_based_max_duration_with_custom_values + # Test with custom minimum and maximum values + @config.minimum_max_chunk_duration = 60_000 + @config.maximum_max_chunk_duration = 180_000 + + ENV['BUILDKITE_PARALLEL_JOB_COUNT'] = '2' + + tests = create_mock_tests((1..10).map { |i| "TestSuite#test_#{i}" }) + timing_data = (1..10).each_with_object({}) do |i, hash| + hash["TestSuite#test_#{i}"] = 20_000.0 # Each test is 20 seconds + end + + chunks = order_with_timing(tests, timing_data) + + # Total duration = 10 * 20,000 = 200,000ms + # Parallel jobs = 2 + # Calculated base_max_duration = 200,000 / 2 = 100,000ms + # 100,000ms is within range [60,000, 180,000], so use it + # With 10% buffer, effective_max = 100,000 * 0.9 = 90,000ms + # Each test is 20,000ms, so we can fit 4 per chunk (4 * 20,000 = 80,000 < 90,000) + # With 10 tests, we'll get 3 chunks (4+4+2) + test_suite_chunks = chunks.select { |c| c.suite_name == 'TestSuite' } + assert_equal 3, test_suite_chunks.size, 'Should create 3 chunks with calculated max_duration' + + assert_equal 4, test_suite_chunks[0].test_ids.size, 'First chunk should have 4 tests' + assert_equal 80_000.0, test_suite_chunks[0].estimated_duration + assert_equal 4, test_suite_chunks[1].test_ids.size, 'Second chunk should have 4 tests' + assert_equal 80_000.0, test_suite_chunks[1].estimated_duration + assert_equal 2, test_suite_chunks[2].test_ids.size, 'Third chunk should have 2 tests' + assert_equal 40_000.0, test_suite_chunks[2].estimated_duration + ensure + ENV.delete('BUILDKITE_PARALLEL_JOB_COUNT') + end + + def test_maximum_max_duration_prevents_oversized_chunks + # Test that maximum prevents chunks from being too large + @config.minimum_max_chunk_duration = 50_000 + @config.maximum_max_chunk_duration = 100_000 + + ENV['BUILDKITE_PARALLEL_JOB_COUNT'] = '1' + + tests = create_mock_tests((1..5).map { |i| "TestSuite#test_#{i}" }) + timing_data = (1..5).each_with_object({}) do |i, hash| + hash["TestSuite#test_#{i}"] = 40_000.0 # Each test is 40 seconds + end + + chunks = order_with_timing(tests, timing_data) + + # Total duration = 5 * 40,000 = 200,000ms + # Parallel jobs = 1 + # Calculated base_max_duration = 200,000 / 1 = 200,000ms + # 200,000ms > maximum_max_chunk_duration (100,000ms), so cap it + # Actual max_duration used = min(200,000, 100,000) = 100,000ms + # With 10% buffer, effective_max = 100,000 * 0.9 = 90,000ms + # Each test is 40,000ms, so we can fit 2 per chunk (2 * 40,000 = 80,000 < 90,000) + # With 5 tests, we'll get 3 chunks (2+2+1) + test_suite_chunks = chunks.select { |c| c.suite_name == 'TestSuite' } + assert_equal 3, test_suite_chunks.size, 'Should cap at maximum and create smaller chunks' + + assert_equal 2, test_suite_chunks[0].test_ids.size + assert_equal 2, test_suite_chunks[1].test_ids.size + assert_equal 1, test_suite_chunks[2].test_ids.size + ensure + ENV.delete('BUILDKITE_PARALLEL_JOB_COUNT') + end + + def test_minimum_max_duration_prevents_undersized_chunks + # Test that minimum prevents chunks from being too small + @config.minimum_max_chunk_duration = 200_000 + @config.maximum_max_chunk_duration = 500_000 + + ENV['BUILDKITE_PARALLEL_JOB_COUNT'] = '20' + + tests = create_mock_tests((1..10).map { |i| "TestSuite#test_#{i}" }) + timing_data = (1..10).each_with_object({}) do |i, hash| + hash["TestSuite#test_#{i}"] = 20_000.0 # Each test is 20 seconds + end + + chunks = order_with_timing(tests, timing_data) + + # Total duration = 10 * 20,000 = 200,000ms + # Parallel jobs = 20 + # Calculated base_max_duration = 200,000 / 20 = 10,000ms + # 10,000ms < minimum_max_chunk_duration (200,000ms), so use minimum + # Actual max_duration used = max(10,000, 200,000) = 200,000ms + # With 10% buffer, effective_max = 200,000 * 0.9 = 180,000ms + # Each test is 20,000ms, so we can fit 9 per chunk (9 * 20,000 = 180,000 = 180,000) + # With 10 tests, we'll get 2 chunks (9+1) + test_suite_chunks = chunks.select { |c| c.suite_name == 'TestSuite' } + assert_equal 2, test_suite_chunks.size, 'Should use minimum and create larger chunks' + + assert_equal 9, test_suite_chunks[0].test_ids.size, 'First chunk should have 9 tests' + assert_equal 180_000.0, test_suite_chunks[0].estimated_duration + assert_equal 1, test_suite_chunks[1].test_ids.size, 'Second chunk should have 1 test' + assert_equal 20_000.0, test_suite_chunks[1].estimated_duration + ensure + ENV.delete('BUILDKITE_PARALLEL_JOB_COUNT') + end + private def create_mock_tests(test_ids)