Merge pull request #42 from figma/eliu/skip-known-flaky-test-retries

flaky_tests: Skip requeuing known flaky tests

Author: eliu-figma

.github/workflows/tests.yml

@@ -31,13 +31,26 @@ jobs:
         uses: ruby/setup-ruby@v1
         with:
           ruby-version: ${{ matrix.ruby }}
+      - name: Add MiniTest compatibility shim
+        run: |
+          cat << 'EOF' > ruby/test/minitest_compat.rb
+          # Compatibility shim for legacy MiniTest constant (pre-5.0)
+          require 'minitest'
+          unless defined?(MiniTest)
+            MiniTest = Minitest
+          end
+          EOF
+          # Add require to files
+          sed -i '5i require_relative "minitest_compat"' ruby/test/test_helper.rb
+          sed -i '5i require_relative "../../test/minitest_compat"' ruby/lib/minitest/queue.rb
       - name: Run Ruby tests
         run: |
           bin/before-install
           bin/test
         env:
           SUITE: ruby
           REDIS_HOST: localhost
+          RUBYOPT: "-W0"
 
   python-tests:
     runs-on: ubuntu-latest

README.md

@@ -37,6 +37,10 @@ Two implementations are provided, please refer to the respective documentations:
   - [Python](python/)
   - [Ruby](ruby/)
 
+## Figma Installation Instructions
+
+Once you have merged your changes into the ci-queue repo, copy the SHAs and pin it in the package.json for integration-test if its an interaction-tests ci-queue update or in the Gemfile in the repo root for sinatra.
+
 ## Redis Requirements
 
 `ci-queue` expects the Redis server to have an [eviction policy](https://redis.io/docs/manual/eviction/#eviction-policies) of `allkeys-lru`.

ruby/Gemfile

@@ -4,3 +4,4 @@ source 'https://rubygems.org'
 gemspec
 
 gem 'activesupport', '~> 5.2.0'
+gem 'rexml'

ruby/lib/ci/queue/configuration.rb

@@ -1,4 +1,6 @@
 # frozen_string_literal: true
+require 'json'
+
 module CI
   module Queue
     class Configuration
@@ -19,6 +21,7 @@ def from_env(env)
             flaky_tests: load_flaky_tests(env['CI_QUEUE_FLAKY_TESTS']),
             statsd_endpoint: env['CI_QUEUE_STATSD_ADDR'],
             redis_ttl: env['CI_QUEUE_REDIS_TTL']&.to_i ||  8 * 60 * 60,
+            known_flaky_tests: load_known_flaky_tests(env['CI_QUEUE_KNOWN_FLAKY_TESTS']),
           )
         end
 
@@ -28,6 +31,18 @@ def load_flaky_tests(path)
         rescue SystemCallError
           []
         end
+
+        def load_known_flaky_tests(path)
+          if path == nil
+            return []
+          end
+          json_data = JSON.parse(::File.read(path))
+          known_flaky_test_ids = json_data.map { |test| "#{test['testSuite']}##{test['testName']}" }
+          puts "ci-queue: Loaded #{known_flaky_test_ids.size} known flaky tests. These will be skipped from requeueing"
+          known_flaky_test_ids.to_set
+        rescue SystemCallError, JSON::ParserError, TypeError
+          []
+        end
       end
 
       def initialize(
@@ -36,12 +51,13 @@ def initialize(
         grind_count: nil, max_duration: nil, failure_file: nil, max_test_duration: nil,
         max_test_duration_percentile: 0.5, track_test_duration: false, max_test_failed: nil,
         queue_init_timeout: nil, redis_ttl: 8 * 60 * 60, report_timeout: nil, inactive_workers_timeout: nil,
-        export_flaky_tests_file: nil
+        export_flaky_tests_file: nil, known_flaky_tests: []
       )
         @build_id = build_id
         @circuit_breakers = [CircuitBreaker::Disabled]
         @failure_file = failure_file
         @flaky_tests = flaky_tests
+        @known_flaky_tests = known_flaky_tests
         @grind_count = grind_count
         @max_requeues = max_requeues
         @max_test_duration = max_test_duration
@@ -91,6 +107,10 @@ def flaky?(test)
         @flaky_tests.include?(test.id)
       end
 
+      def known_flaky?(id)
+        @known_flaky_tests.include?(id)
+      end
+
       def seed
         @seed || build_id
       end

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

@@ -110,7 +110,7 @@ def requeue(test, offset: Redis.requeue_offset)
           raise_on_mismatching_test(test_key)
           global_max_requeues = config.global_max_requeues(total)
 
-          requeued = config.max_requeues > 0 && global_max_requeues > 0 && eval_script(
+          requeued = config.max_requeues > 0 && global_max_requeues > 0 && !config.known_flaky?(test_key) && eval_script(
             :requeue,
             keys: [
               key('processed'),

ruby/lib/ci/queue/static.rb

@@ -106,7 +106,7 @@ def requeue(test)
       attr_reader :index
 
       def should_requeue?(key)
-        requeues[key] < config.max_requeues && requeues.values.inject(0, :+) < config.global_max_requeues(total)
+        requeues[key] < config.max_requeues && requeues.values.inject(0, :+) < config.global_max_requeues(total) && !config.known_flaky?(key)
       end
 
       def requeues

ruby/lib/minitest/queue.rb

@@ -241,7 +241,7 @@ def run_from_queue(reporter, *)
         end
 
         requeued = false
-        if failed && CI::Queue.requeueable?(result) && queue.requeue(example)
+        if failed && CI::Queue.requeueable?(result) && !queue.config.known_flaky?(example.id) && queue.requeue(example)
           requeued = true
           result.requeue!
           reporter.record(result)

ruby/test/ci/queue/configuration_test.rb

@@ -1,5 +1,8 @@
 # frozen_string_literal: true
 require 'test_helper'
+require 'tempfile'
+require 'ostruct'
+require 'json'
 
 module CI::Queue
   class ConfigurationTest < Minitest::Test
@@ -128,5 +131,66 @@ def test_report_timeout_set
       assert_equal 45, config.report_timeout
     end
 
+    def test_load_known_flaky_tests_with_valid_file
+      Tempfile.open('known_flaky_tests.json') do |file|
+        test_data = [
+          { 'testSuite' => 'TestClass1', 'testName' => 'test_method1' },
+          { 'testSuite' => 'TestClass2', 'testName' => 'test_method2' }
+        ]
+        file.write(test_data.to_json)
+        file.close
+
+        known_flaky_tests = Configuration.load_known_flaky_tests(file.path)
+        assert_includes known_flaky_tests, 'TestClass1#test_method1'
+        assert_includes known_flaky_tests, 'TestClass2#test_method2'
+        assert_equal 2, known_flaky_tests.size
+      end
+    end
+
+    def test_load_known_flaky_tests_with_missing_file
+      known_flaky_tests = Configuration.load_known_flaky_tests('/tmp/does-not-exist.json')
+      assert_empty known_flaky_tests
+
+      known_flaky_tests = Configuration.load_known_flaky_tests(nil)
+      assert_empty known_flaky_tests
+    end
+
+    def test_load_known_flaky_tests_with_invalid_json
+      Tempfile.open('invalid.json') do |file|
+        file.write('{ invalid json }')
+        file.close
+
+        known_flaky_tests = Configuration.load_known_flaky_tests(file.path)
+        assert_empty known_flaky_tests
+      end
+    end
+
+    def test_known_flaky_method
+      known_flaky_tests = Set.new(['TestClass1#test_method1', 'TestClass2#test_method2'])
+      config = Configuration.new(known_flaky_tests: known_flaky_tests)
+      
+      assert config.known_flaky?('TestClass1#test_method1')
+      assert config.known_flaky?('TestClass2#test_method2')
+      refute config.known_flaky?('TestClass3#test_method3')
+    end
+
+    def test_from_env_with_known_flaky_tests
+      Tempfile.open('known_flaky_tests.json') do |file|
+        test_data = [
+          { 'testSuite' => 'TestClass1', 'testName' => 'test_method1' },
+          { 'testSuite' => 'TestClass2', 'testName' => 'test_method2' }
+        ]
+        file.write(test_data.to_json)
+        file.close
+
+        env = { 'CI_QUEUE_KNOWN_FLAKY_TESTS' => file.path }
+        config = Configuration.from_env(env)
+        
+        assert config.known_flaky?('TestClass1#test_method1')
+        assert config.known_flaky?('TestClass2#test_method2')
+        refute config.known_flaky?('TestClass3#test_method3')
+      end
+    end
+
   end
 end