Skip to content

Commit

Permalink
Merge pull request #242 from Shopify/nikita8/fix-flaky-tests
Browse files Browse the repository at this point in the history
Fix flaky test record for skipped test
  • Loading branch information
ChrisBr authored Oct 13, 2023
2 parents 32ceb1f + 5a7b8f0 commit de56717
Show file tree
Hide file tree
Showing 4 changed files with 10 additions and 6 deletions.
4 changes: 2 additions & 2 deletions ruby/lib/ci/queue/redis/build_record.rb
Original file line number Diff line number Diff line change
Expand Up @@ -47,13 +47,13 @@ def record_error(id, payload, stats: nil)
nil
end

def record_success(id, stats: nil)
def record_success(id, stats: nil, skip_flaky_record: false)
error_reports_deleted_count, requeued_count, _ = redis.pipelined do |pipeline|
pipeline.hdel(key('error-reports'), id.dup.force_encoding(Encoding::BINARY))
pipeline.hget(key('requeues-count'), id.b)
record_stats(stats, pipeline: pipeline)
end
record_flaky(id) if error_reports_deleted_count.to_i > 0 || requeued_count.to_i > 0
record_flaky(id) if !skip_flaky_record && (error_reports_deleted_count.to_i > 0 || requeued_count.to_i > 0)
nil
end

Expand Down
2 changes: 1 addition & 1 deletion ruby/lib/minitest/queue/build_status_recorder.rb
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ def record(test)
if (test.failure || test.error?) && !test.skipped?
build.record_error("#{test.klass}##{test.name}", dump(test), stats: stats)
else
build.record_success("#{test.klass}##{test.name}", stats: stats)
build.record_success("#{test.klass}##{test.name}", stats: stats, skip_flaky_record: test.skipped?)
end
end

Expand Down
1 change: 1 addition & 0 deletions ruby/test/integration/minitest_redis_test.rb
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
# frozen_string_literal: true
require 'test_helper'
require 'tmpdir'
require 'active_support'
require 'active_support/testing/time_helpers'

module Integration
Expand Down
9 changes: 6 additions & 3 deletions ruby/test/minitest/queue/build_status_recorder_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ def setup
def test_aggregation
@reporter.record(result('a', failure: "Something went wrong"))
@reporter.record(result('b', unexpected_error: true))
@reporter.record(result('h', failure: "Something went wrong", requeued: true))

second_queue = worker(2)
second_reporter = BuildStatusRecorder.new(build: second_queue.build)
Expand All @@ -27,13 +28,15 @@ def test_aggregation
second_reporter.record(result('e', skipped: true))
second_reporter.record(result('f', unexpected_error: true))
second_reporter.record(result('g', requeued: true))
second_reporter.record(result('h', skipped: true, requeued: true))

assert_equal 7, summary.assertions
assert_equal 2, summary.failures
assert_equal 9, summary.assertions
assert_equal 3, summary.failures
assert_equal 3, summary.errors
assert_equal 1, summary.skips
assert_equal 2, summary.skips
assert_equal 1, summary.requeues
assert_equal 5, summary.error_reports.size
assert_equal 0, summary.flaky_reports.size
end

def test_retrying_test
Expand Down

0 comments on commit de56717

Please sign in to comment.