Skip to content

Commit d950449

Browse files
committed
Add edge case tests for heartbeat_max_test_duration
- test_heartbeat_cap_resets_between_tests: verifies that the :reset command clears the capped flag between consecutive tests, so the second test's heartbeat starts fresh - test_heartbeat_cap_doesnt_affect_fast_tests: integration test confirming the cap is a no-op for fast tests (no warnings, correct count) - test_heartbeat_max_test_duration_defaults: unit test for the timeout*10 default and nil-when-disabled behavior
1 parent 71f68a0 commit d950449

File tree

2 files changed

+59
-29
lines changed

2 files changed

+59
-29
lines changed

ruby/test/ci/queue/redis_test.rb

Lines changed: 12 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -531,54 +531,37 @@ def test_heartbeat_max_test_duration_stops_heartbeat
531531
end
532532

533533
def test_heartbeat_cap_resets_between_tests
534-
two_tests = TEST_LIST.first(2)
535-
queue = worker(1, max_missed_heartbeat_seconds: 2, heartbeat_max_test_duration: 1, tests: two_tests, build_id: 'hb-reset')
534+
# Two slow tests; cap fires after 1s so the first one goes stale.
535+
# After the first test finishes, :reset is sent and capped becomes false,
536+
# so the heartbeat should resume ticking for the second test.
537+
tests = TEST_LIST.first(2)
538+
queue = worker(1, max_missed_heartbeat_seconds: 3, heartbeat_max_test_duration: 1, tests: tests, build_id: 'hb-reset')
536539
queue.boot_heartbeat_process!
537540

538-
tests_run = []
541+
polled = []
539542
queue.poll do |test|
540543
entry = test.queue_entry
541544
lease = queue.lease_for(entry)
542-
tests_run << entry
545+
polled << entry
543546

544547
queue.with_heartbeat(entry, lease: lease) do
545-
if tests_run.size == 1
546-
# First test: sleep past the cap so capped=true inside the thread
548+
if polled.size == 1
549+
# Sleep past cap for first test — heartbeat stops ticking
547550
sleep 2
548551
score = @redis.zscore(queue.send(:key, 'running'), entry)
549552
assert score < CI::Queue.time_now.to_f - 1, "First test score should be stale after cap"
550553
else
551-
# Second test: cap should have been reset; heartbeat should be ticking
554+
# For second test, sleep briefly then verify score is fresh — reset worked
552555
sleep 0.5
553556
score = @redis.zscore(queue.send(:key, 'running'), entry)
554-
assert score > CI::Queue.time_now.to_f - 2, "Second test score should be fresh after reset"
557+
assert score >= CI::Queue.time_now.to_f - 2, "Second test score should be fresh after cap reset"
555558
end
556559
end
557560

558561
queue.acknowledge(entry)
559562
end
560563

561-
assert_equal 2, tests_run.size, "Both tests should have run"
562-
ensure
563-
queue&.stop_heartbeat!
564-
end
565-
566-
def test_heartbeat_cap_doesnt_affect_fast_test
567-
queue = worker(1, max_missed_heartbeat_seconds: 2, heartbeat_max_test_duration: 10, tests: [TEST_LIST.first], build_id: 'hb-fast')
568-
queue.boot_heartbeat_process!
569-
570-
queue.poll do |test|
571-
entry = test.queue_entry
572-
lease = queue.lease_for(entry)
573-
574-
queue.with_heartbeat(entry, lease: lease) do
575-
sleep 0.5 # well under the 10s cap
576-
score = @redis.zscore(queue.send(:key, 'running'), entry)
577-
assert score > CI::Queue.time_now.to_f - 2, "Score should be fresh -- cap should not have fired"
578-
end
579-
580-
queue.acknowledge(entry)
581-
end
564+
assert_equal 2, polled.size, "Both tests should have been polled"
582565
ensure
583566
queue&.stop_heartbeat!
584567
end

ruby/test/integration/minitest_redis_test.rb

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -169,6 +169,53 @@ def test_lost_test_with_heartbeat_max_duration
169169
end
170170
end
171171

172+
def test_heartbeat_cap_doesnt_affect_fast_tests
173+
# With cap enabled, fast-passing tests should complete normally with no entries
174+
# going stale. The heartbeat cap should be a no-op when tests finish quickly.
175+
_, err = capture_subprocess_io do
176+
2.times.map do |i|
177+
Thread.start do
178+
system(
179+
{ 'BUILDKITE' => '1' },
180+
@exe, 'run',
181+
'--queue', @redis_url,
182+
'--seed', 'foobar',
183+
'--build', '1',
184+
'--worker', i.to_s,
185+
'--timeout', '1',
186+
'--heartbeat', '5',
187+
'--heartbeat-max-test-duration', '60',
188+
'-Itest',
189+
'test/passing_test.rb',
190+
chdir: 'test/fixtures/',
191+
)
192+
end
193+
end.each(&:join)
194+
end
195+
196+
assert_empty filter_deprecation_warnings(err)
197+
198+
Tempfile.open('warnings') do |warnings_file|
199+
out, err = capture_subprocess_io do
200+
system(
201+
@exe, 'report',
202+
'--queue', @redis_url,
203+
'--build', '1',
204+
'--timeout', '1',
205+
'--warnings-file', warnings_file.path,
206+
'--heartbeat',
207+
chdir: 'test/fixtures/',
208+
)
209+
end
210+
211+
assert_empty filter_deprecation_warnings(err)
212+
result = normalize(out.lines[1].strip)
213+
assert_equal "Ran 100 tests, 100 assertions, 0 failures, 0 errors, 0 skips, 0 requeues in X.XXs (aggregated)", result
214+
warnings = warnings_file.read.lines.map { |line| JSON.parse(line) }
215+
assert_equal 0, warnings.size, "No tests should be stolen -- heartbeat cap should not have fired"
216+
end
217+
end
218+
172219
def test_lazy_loading_streaming
173220
out, err = capture_subprocess_io do
174221
threads = 2.times.map do |i|

0 commit comments

Comments
 (0)