Simplify worker history retry metadata

Author: yfang-figma

ruby/lib/ci/queue/redis/retry.rb

@@ -3,28 +3,19 @@ module CI
   module Queue
     module Redis
       class Retry < Static
-        attr_reader :history_items, :replayed_tests
-
-        def initialize(tests, config, redis:, history_items: 0, worker_history: false)
+        def initialize(tests, config, redis:)
           @redis = redis
-          @history_items = history_items
-          @replayed_tests = worker_history ? tests.size : 0
-          @worker_history = worker_history
-          @worker_history_complete = false
+          @poll_completed = false
           super(tests, config)
         end
 
-        def worker_history?
-          @worker_history
-        end
-
-        def worker_history_complete?
-          @worker_history_complete
+        def poll_completed?
+          @poll_completed
         end
 
-        def poll(&block)
+        def poll
           super
-          @worker_history_complete = exhausted? if worker_history?
+          @poll_completed = exhausted?
         end
 
         def build

ruby/lib/ci/queue/redis/worker.rb

@@ -283,13 +283,7 @@ def worker_history_retry_queue
             end
           end
 
-          Retry.new(
-            test_ids,
-            config,
-            redis: redis,
-            history_items: reservations.size,
-            worker_history: true
-          )
+          Retry.new(test_ids, config, redis: redis)
         end
 
         # Runs a block while sending periodic heartbeats in a background thread.

ruby/lib/minitest/queue/recovery_reporter.rb

@@ -18,7 +18,7 @@ def initialize(path:, queue:, config:)
       def report
         super
         return unless queue.exhausted?
-        return if worker_history_retry? && !queue.worker_history_complete?
+        return if worker_history_retry? && !queue.poll_completed?
 
         directory = File.dirname(path)
         FileUtils.mkdir_p(directory)
@@ -37,18 +37,16 @@ def report
       attr_reader :config, :path, :queue
 
       def worker_history_retry?
-        queue.respond_to?(:worker_history?) && queue.worker_history?
+        config.retry_mode == :worker_history && queue.respond_to?(:poll_completed?)
       end
 
       def manifest
         {
           schema_version: 1,
           worker_id: config.worker_id.to_s,
           retry_count: config.retry_count,
-          history_items: worker_history_retry? ? queue.history_items : 0,
-          replayed_tests: worker_history_retry? ? queue.replayed_tests : 0,
           resumed_shared_queue: false,
-          replay_completed: !worker_history_retry? || queue.worker_history_complete?
+          replay_completed: !worker_history_retry? || queue.poll_completed?
         }
       end
     end

ruby/lib/minitest/queue/runner.rb

@@ -61,7 +61,7 @@ def run_command
             puts "The retry queue does not contain any failure, we'll process the main queue instead."
           else
             if worker_history_retry?
-              puts "Replaying #{retry_queue.replayed_tests} tests from #{retry_queue.history_items} worker reservations."
+              puts "Replaying this worker's reservation history."
             else
               puts "Retrying failed tests."
             end

ruby/test/ci/queue/configuration_test.rb

@@ -33,13 +33,10 @@ def test_buildkite_defaults
         'BUILDKITE_BUILD_ID' => '9e08ef3c-d6e6-4a86-91dd-577ce5205b8e',
         'BUILDKITE_PARALLEL_JOB' => '12',
         'BUILDKITE_COMMIT' => 'faa647bbb8168a77cf338e7488c3f8445c3e6554',
-        'BUILDKITE_RETRY_COUNT' => '2',
       )
       assert_equal '9e08ef3c-d6e6-4a86-91dd-577ce5205b8e', config.build_id
       assert_equal '12', config.worker_id
       assert_equal 'faa647bbb8168a77cf338e7488c3f8445c3e6554', config.seed
-      assert_equal 2, config.retry_count
-      assert_equal :failures, config.retry_mode
     end
 
     def test_travis_defaults

ruby/test/ci/queue/redis/worker_chunk_test.rb

@@ -243,62 +243,6 @@ def test_populate_with_many_chunks_uses_batching
     end
   end
 
-  def test_worker_history_retry_expands_chunks_and_deduplicates_tests
-    tests = create_mock_tests(['TestA#test_1', 'TestA#test_2', 'TestB#test_1'])
-    chunk = CI::Queue::TestChunk.new(
-      'TestA:chunk_0',
-      'TestA',
-      ['TestA#test_1', 'TestA#test_2'],
-      2000.0
-    )
-
-    @worker.stub(:reorder_tests, [chunk, tests.last]) do
-      @worker.populate(tests)
-    end
-
-    history_key = 'build:42:worker:1:queue'
-    @redis.del(history_key)
-    [chunk.id, tests.last.id, chunk.id, tests[1].id].each do |id|
-      @redis.lpush(history_key, id)
-    end
-
-    retry_queue = @worker.retry_queue(scope: :worker_history)
-    retry_queue.populate(tests)
-
-    assert_equal ['TestA#test_1', 'TestA#test_2', 'TestB#test_1'], retry_queue.to_a.map(&:id)
-    assert_equal 4, retry_queue.history_items
-    assert_equal 3, retry_queue.replayed_tests
-    assert retry_queue.worker_history?
-  end
-
-  def test_worker_history_retry_requires_reservations
-    error = assert_raises(CI::Queue::Redis::WorkerHistoryError) do
-      @worker.retry_queue(scope: :worker_history)
-    end
-
-    assert_equal 'Reservation history is missing for worker 1', error.message
-  end
-
-  def test_worker_history_retry_requires_chunk_metadata
-    @redis.lpush('build:42:worker:1:queue', 'TestA:chunk_0')
-
-    error = assert_raises(CI::Queue::Redis::WorkerHistoryError) do
-      @worker.retry_queue(scope: :worker_history)
-    end
-
-    assert_equal 'Chunk metadata is missing for TestA:chunk_0', error.message
-  end
-
-  def test_worker_history_retry_is_incomplete_when_a_test_cannot_be_resolved
-    @redis.lpush('build:42:worker:1:queue', 'MissingTest#test_1')
-    retry_queue = @worker.retry_queue(scope: :worker_history)
-    retry_queue.populate([])
-
-    assert_raises(KeyError) { retry_queue.poll { |_test| } }
-    assert retry_queue.exhausted?
-    refute retry_queue.worker_history_complete?
-  end
-
   private
 
   def create_mock_tests(test_ids)

ruby/test/ci/queue/static_test.rb

@@ -4,18 +4,6 @@
 class CI::Queue::StaticTest < Minitest::Test
   include SharedQueueAssertions
 
-  def test_shutdown_stops_polling
-    order = []
-
-    @queue.poll do |test|
-      order << test
-      @queue.shutdown!
-    end
-
-    assert_equal 1, order.size
-    refute @queue.exhausted?
-  end
-
   private
 
   def build_queue

ruby/test/integration/minitest_redis_test.rb

@@ -271,20 +271,16 @@ def test_worker_history_retry_writes_recovery_manifest
 
         first_manifest = JSON.parse(File.read(manifest_path))
         assert_equal 0, first_manifest['retry_count']
-        assert_equal 0, first_manifest['history_items']
-        assert_equal 0, first_manifest['replayed_tests']
         refute first_manifest['resumed_shared_queue']
         assert first_manifest['replay_completed']
 
         out, = run_worker_history_worker(manifest_path, retry_count: 1)
 
-        assert_includes out, 'Replaying 100 tests from 100 worker reservations.'
+        assert_includes out, "Replaying this worker's reservation history."
         retry_manifest = JSON.parse(File.read(manifest_path))
         assert_equal 1, retry_manifest['schema_version']
         assert_equal '1', retry_manifest['worker_id']
         assert_equal 1, retry_manifest['retry_count']
-        assert_equal 100, retry_manifest['history_items']
-        assert_equal 100, retry_manifest['replayed_tests']
         refute retry_manifest['resumed_shared_queue']
         assert retry_manifest['replay_completed']
       end