[EE] Fix flaky examples tracking

EE port of gitlab-org/gitlab-ce!14681.
Signed-off-by: Rémy Coutable <remy@rymai.me>

.gitlab-ci.yml

@@ -28,7 +28,7 @@ variables:
   GET_SOURCES_ATTEMPTS: "3"
   KNAPSACK_RSPEC_SUITE_REPORT_PATH: knapsack/${CI_PROJECT_NAME}/rspec_report-master.json
   KNAPSACK_SPINACH_SUITE_REPORT_PATH: knapsack/${CI_PROJECT_NAME}/spinach_report-master.json
-  FLAKY_RSPEC_SUITE_REPORT_PATH: rspec_flaky/${CI_PROJECT_NAME}/report-master.json
+  FLAKY_RSPEC_SUITE_REPORT_PATH: rspec_flaky/report-suite.json
   # This hack is needed to make ES not that memory hungry
   ES_JAVA_OPTS: "-Xms256m -Xmx256m"
 
@@ -93,12 +93,13 @@ stages:
     - export CI_NODE_TOTAL=${JOB_NAME[-1]}
     - export KNAPSACK_REPORT_PATH=knapsack/${CI_PROJECT_NAME}/${JOB_NAME[0]}_node_${CI_NODE_INDEX}_${CI_NODE_TOTAL}_report.json
     - export KNAPSACK_GENERATE_REPORT=true
-    - export ALL_FLAKY_RSPEC_REPORT_PATH=rspec_flaky/${CI_PROJECT_NAME}/all_node_${CI_NODE_INDEX}_${CI_NODE_TOTAL}_report.json
-    - export NEW_FLAKY_RSPEC_REPORT_PATH=rspec_flaky/${CI_PROJECT_NAME}/new_node_${CI_NODE_INDEX}_${CI_NODE_TOTAL}_report.json
+    - export SUITE_FLAKY_RSPEC_REPORT_PATH=${FLAKY_RSPEC_SUITE_REPORT_PATH}
+    - export FLAKY_RSPEC_REPORT_PATH=rspec_flaky/all_${JOB_NAME[0]}_${CI_NODE_INDEX}_${CI_NODE_TOTAL}_report.json
+    - export NEW_FLAKY_RSPEC_REPORT_PATH=rspec_flaky/new_${JOB_NAME[0]}_${CI_NODE_INDEX}_${CI_NODE_TOTAL}_report.json
     - export FLAKY_RSPEC_GENERATE_REPORT=true
     - export CACHE_CLASSES=true
     - cp ${KNAPSACK_RSPEC_SUITE_REPORT_PATH} ${KNAPSACK_REPORT_PATH}
-    - cp ${FLAKY_RSPEC_SUITE_REPORT_PATH} ${ALL_FLAKY_RSPEC_REPORT_PATH}
+    - '[[ -f $FLAKY_RSPEC_REPORT_PATH ]] || echo "{}" > ${FLAKY_RSPEC_REPORT_PATH}'
     - '[[ -f $NEW_FLAKY_RSPEC_REPORT_PATH ]] || echo "{}" > ${NEW_FLAKY_RSPEC_REPORT_PATH}'
     - scripts/gitaly-test-spawn
     - knapsack rspec "--color --format documentation"
@@ -240,7 +241,7 @@ retrieve-tests-metadata:
     - wget -O $KNAPSACK_SPINACH_SUITE_REPORT_PATH http://${TESTS_METADATA_S3_BUCKET}.s3.amazonaws.com/$KNAPSACK_SPINACH_SUITE_REPORT_PATH || rm $KNAPSACK_SPINACH_SUITE_REPORT_PATH
     - '[[ -f $KNAPSACK_RSPEC_SUITE_REPORT_PATH ]] || echo "{}" > ${KNAPSACK_RSPEC_SUITE_REPORT_PATH}'
     - '[[ -f $KNAPSACK_SPINACH_SUITE_REPORT_PATH ]] || echo "{}" > ${KNAPSACK_SPINACH_SUITE_REPORT_PATH}'
-    - mkdir -p rspec_flaky/${CI_PROJECT_NAME}/
+    - mkdir -p rspec_flaky/
     - wget -O $FLAKY_RSPEC_SUITE_REPORT_PATH http://${TESTS_METADATA_S3_BUCKET}.s3.amazonaws.com/$FLAKY_RSPEC_SUITE_REPORT_PATH || rm $FLAKY_RSPEC_SUITE_REPORT_PATH
     - '[[ -f $FLAKY_RSPEC_SUITE_REPORT_PATH ]] || echo "{}" > ${FLAKY_RSPEC_SUITE_REPORT_PATH}'
 
@@ -259,22 +260,21 @@ update-tests-metadata:
     - retry gem install fog-aws mime-types
     - scripts/merge-reports ${KNAPSACK_RSPEC_SUITE_REPORT_PATH} knapsack/${CI_PROJECT_NAME}/rspec-pg_node_*.json
     - scripts/merge-reports ${KNAPSACK_SPINACH_SUITE_REPORT_PATH} knapsack/${CI_PROJECT_NAME}/spinach-pg_node_*.json
-    - scripts/merge-reports ${FLAKY_RSPEC_SUITE_REPORT_PATH} rspec_flaky/${CI_PROJECT_NAME}/all_node_*.json
+    - scripts/merge-reports ${FLAKY_RSPEC_SUITE_REPORT_PATH} rspec_flaky/all_*_*.json
     - '[[ -z ${TESTS_METADATA_S3_BUCKET} ]] || scripts/sync-reports put $TESTS_METADATA_S3_BUCKET $KNAPSACK_RSPEC_SUITE_REPORT_PATH $KNAPSACK_SPINACH_SUITE_REPORT_PATH'
     - '[[ -z ${TESTS_METADATA_S3_BUCKET} ]] || scripts/sync-reports put $TESTS_METADATA_S3_BUCKET $FLAKY_RSPEC_SUITE_REPORT_PATH'
     - rm -f knapsack/${CI_PROJECT_NAME}/*_node_*.json
-    - rm -f rspec_flaky/${CI_PROJECT_NAME}/*_node_*.json
+    - rm -f rspec_flaky/all_*.json rspec_flaky/new_*.json
 
 flaky-examples-check:
   <<: *dedicated-runner
   image: ruby:2.3-alpine
   services: []
   before_script: []
-  cache: {}
   variables:
     SETUP_DB: "false"
     USE_BUNDLE_INSTALL: "false"
-    NEW_FLAKY_SPECS_REPORT: rspec_flaky/${CI_PROJECT_NAME}/new_rspec_flaky_examples.json
+    NEW_FLAKY_SPECS_REPORT: rspec_flaky/report-new.json
   stage: post-test
   allow_failure: yes
   only:
@@ -288,7 +288,7 @@ flaky-examples-check:
       - rspec_flaky/
   script:
     - '[[ -f $NEW_FLAKY_SPECS_REPORT ]] || echo "{}" > ${NEW_FLAKY_SPECS_REPORT}'
-    - scripts/merge-reports $NEW_FLAKY_SPECS_REPORT rspec_flaky/${CI_PROJECT_NAME}/new_node_*.json
+    - scripts/merge-reports ${NEW_FLAKY_SPECS_REPORT} rspec_flaky/new_*_*.json
     - scripts/detect-new-flaky-examples $NEW_FLAKY_SPECS_REPORT
 
 setup-test-env:

lib/rspec_flaky/flaky_example.rb

@@ -9,24 +9,21 @@ module RspecFlaky
           line: example.line,
           description: example.description,
           last_attempts_count: example.attempts,
-          flaky_reports: 1)
+          flaky_reports: 0)
       else
         super
       end
     end
 
-    def first_flaky_at
-      self[:first_flaky_at] || Time.now
-    end
-
-    def last_flaky_at
-      Time.now
-    end
+    def update_flakiness!(last_attempts_count: nil)
+      self.first_flaky_at ||= Time.now
+      self.last_flaky_at = Time.now
+      self.flaky_reports += 1
+      self.last_attempts_count = last_attempts_count if last_attempts_count
 
-    def last_flaky_job
-      return unless ENV['CI_PROJECT_URL'] && ENV['CI_JOB_ID']
-
-      "#{ENV['CI_PROJECT_URL']}/-/jobs/#{ENV['CI_JOB_ID']}"
+      if ENV['CI_PROJECT_URL'] && ENV['CI_JOB_ID']
+        self.last_flaky_job = "#{ENV['CI_PROJECT_URL']}/-/jobs/#{ENV['CI_JOB_ID']}"
+      end
     end
 
     def to_h

lib/rspec_flaky/listener.rb

@@ -2,11 +2,15 @@ require 'json'
 
 module RspecFlaky
   class Listener
-    attr_reader :all_flaky_examples, :new_flaky_examples
-
-    def initialize
-      @new_flaky_examples = {}
-      @all_flaky_examples = init_all_flaky_examples
+    # - suite_flaky_examples: contains all the currently tracked flacky example
+    #   for the whole RSpec suite
+    # - flaky_examples: contains the examples detected as flaky during the
+    #   current RSpec run
+    attr_reader :suite_flaky_examples, :flaky_examples
+
+    def initialize(suite_flaky_examples_json = nil)
+      @flaky_examples = {}
+      @suite_flaky_examples = init_suite_flaky_examples(suite_flaky_examples_json)
     end
 
     def example_passed(notification)
@@ -14,24 +18,16 @@ module RspecFlaky
 
       return unless current_example.attempts > 1
 
-      flaky_example_hash = all_flaky_examples[current_example.uid]
-
-      all_flaky_examples[current_example.uid] =
-        if flaky_example_hash
-          FlakyExample.new(flaky_example_hash).tap do |ex|
-            ex.last_attempts_count = current_example.attempts
-            ex.flaky_reports += 1
-          end
-        else
-          FlakyExample.new(current_example).tap do |ex|
-            new_flaky_examples[current_example.uid] = ex
-          end
-        end
+      flaky_example = suite_flaky_examples.fetch(current_example.uid) { FlakyExample.new(current_example) }
+      flaky_example.update_flakiness!(last_attempts_count: current_example.attempts)
+
+      flaky_examples[current_example.uid] = flaky_example
     end
 
     def dump_summary(_)
-      write_report_file(all_flaky_examples, all_flaky_examples_report_path)
+      write_report_file(flaky_examples, flaky_examples_report_path)
 
+      new_flaky_examples = _new_flaky_examples
       if new_flaky_examples.any?
         Rails.logger.warn "\nNew flaky examples detected:\n"
         Rails.logger.warn JSON.pretty_generate(to_report(new_flaky_examples))
@@ -46,12 +42,24 @@ module RspecFlaky
 
     private
 
-    def init_all_flaky_examples
-      return {} unless File.exist?(all_flaky_examples_report_path)
+    def init_suite_flaky_examples(suite_flaky_examples_json = nil)
+      unless suite_flaky_examples_json
+        return {} unless File.exist?(suite_flaky_examples_report_path)
+
+        suite_flaky_examples_json = File.read(suite_flaky_examples_report_path)
+      end
+
+      suite_flaky_examples = JSON.parse(suite_flaky_examples_json)
 
-      all_flaky_examples = JSON.parse(File.read(all_flaky_examples_report_path))
+      Hash[(suite_flaky_examples || {}).map { |k, ex| [k, FlakyExample.new(ex)] }].freeze
+    end
+
+    def _new_flaky_examples
+      flaky_examples.reject { |uid, _| already_flaky?(uid) }
+    end
 
-      Hash[(all_flaky_examples || {}).map { |k, ex| [k, FlakyExample.new(ex)] }]
+    def already_flaky?(example_uid)
+      suite_flaky_examples.key?(example_uid)
     end
 
     def write_report_file(examples, file_path)
@@ -62,9 +70,14 @@ module RspecFlaky
       File.write(file_path, JSON.pretty_generate(to_report(examples)))
     end
 
-    def all_flaky_examples_report_path
-      @all_flaky_examples_report_path ||= ENV['ALL_FLAKY_RSPEC_REPORT_PATH'] ||
-        Rails.root.join("rspec_flaky/all-report.json")
+    def suite_flaky_examples_report_path
+      @suite_flaky_examples_report_path ||= ENV['SUITE_FLAKY_RSPEC_REPORT_PATH'] ||
+        Rails.root.join("rspec_flaky/suite-report.json")
+    end
+
+    def flaky_examples_report_path
+      @flaky_examples_report_path ||= ENV['FLAKY_RSPEC_REPORT_PATH'] ||
+        Rails.root.join("rspec_flaky/report.json")
     end
 
     def new_flaky_examples_report_path

spec/lib/rspec_flaky/flaky_example_spec.rb

 require 'spec_helper'
 
-describe RspecFlaky::FlakyExample do
+describe RspecFlaky::FlakyExample, :aggregate_failures do
   let(:flaky_example_attrs) do
     {
       example_id: 'spec/foo/bar_spec.rb:2',
@@ -9,6 +9,7 @@ describe RspecFlaky::FlakyExample do
       description: 'hello world',
       first_flaky_at: 1234,
       last_flaky_at: 2345,
+      last_flaky_job: 'https://gitlab.com/gitlab-org/gitlab-ce/-/jobs/12',
       last_attempts_count: 2,
       flaky_reports: 1
     }
@@ -27,57 +28,78 @@ describe RspecFlaky::FlakyExample do
   end
   let(:example) { double(example_attrs) }
 
+  before do
+    # Stub these env variables otherwise specs don't behave the same on the CI
+    stub_env('CI_PROJECT_URL', nil)
+    stub_env('CI_JOB_ID', nil)
+  end
+
   describe '#initialize' do
     shared_examples 'a valid FlakyExample instance' do
-      it 'returns valid attributes' do
-        flaky_example = described_class.new(args)
+      let(:flaky_example) { described_class.new(args) }
 
+      it 'returns valid attributes' do
         expect(flaky_example.uid).to eq(flaky_example_attrs[:uid])
-        expect(flaky_example.example_id).to eq(flaky_example_attrs[:example_id])
+        expect(flaky_example.file).to eq(flaky_example_attrs[:file])
+        expect(flaky_example.line).to eq(flaky_example_attrs[:line])
+        expect(flaky_example.description).to eq(flaky_example_attrs[:description])
+        expect(flaky_example.first_flaky_at).to eq(expected_first_flaky_at)
+        expect(flaky_example.last_flaky_at).to eq(expected_last_flaky_at)
+        expect(flaky_example.last_attempts_count).to eq(flaky_example_attrs[:last_attempts_count])
+        expect(flaky_example.flaky_reports).to eq(expected_flaky_reports)
       end
     end
 
     context 'when given an Rspec::Example' do
-      let(:args) { example }
-
-      it_behaves_like 'a valid FlakyExample instance'
+      it_behaves_like 'a valid FlakyExample instance' do
+        let(:args) { example }
+        let(:expected_first_flaky_at) { nil }
+        let(:expected_last_flaky_at) { nil }
+        let(:expected_flaky_reports) { 0 }
+      end
     end
 
     context 'when given a hash' do
-      let(:args) { flaky_example_attrs }
-
-      it_behaves_like 'a valid FlakyExample instance'
+      it_behaves_like 'a valid FlakyExample instance' do
+        let(:args) { flaky_example_attrs }
+        let(:expected_flaky_reports) { flaky_example_attrs[:flaky_reports] }
+        let(:expected_first_flaky_at) { flaky_example_attrs[:first_flaky_at] }
+        let(:expected_last_flaky_at) { flaky_example_attrs[:last_flaky_at] }
+      end
     end
   end
 
-  describe '#to_h' do
-    before do
-      # Stub these env variables otherwise specs don't behave the same on the CI
-      stub_env('CI_PROJECT_URL', nil)
-      stub_env('CI_JOB_ID', nil)
-    end
+  describe '#update_flakiness!' do
+    shared_examples 'an up-to-date FlakyExample instance' do
+      let(:flaky_example) { described_class.new(args) }
 
-    shared_examples 'a valid FlakyExample hash' do
-      let(:additional_attrs) { {} }
+      it 'updates the first_flaky_at' do
+        now = Time.now
+        expected_first_flaky_at = flaky_example.first_flaky_at ? flaky_example.first_flaky_at : now
+        Timecop.freeze(now) { flaky_example.update_flakiness! }
 
-      it 'returns a valid hash' do
-        flaky_example = described_class.new(args)
-        final_hash = flaky_example_attrs
-          .merge(last_flaky_at: instance_of(Time), last_flaky_job: nil)
-          .merge(additional_attrs)
+        expect(flaky_example.first_flaky_at).to eq(expected_first_flaky_at)
+      end
+
+      it 'updates the last_flaky_at' do
+        now = Time.now
+        Timecop.freeze(now) { flaky_example.update_flakiness! }
 
-        expect(flaky_example.to_h).to match(hash_including(final_hash))
+        expect(flaky_example.last_flaky_at).to eq(now)
       end
-    end
 
-    context 'when given an Rspec::Example' do
-      let(:args) { example }
+      it 'updates the flaky_reports' do
+        expected_flaky_reports = flaky_example.first_flaky_at ? flaky_example.flaky_reports + 1 : 1
+
+        expect { flaky_example.update_flakiness! }.to change { flaky_example.flaky_reports }.by(1)
+        expect(flaky_example.flaky_reports).to eq(expected_flaky_reports)
+      end
+
+      context 'when passed a :last_attempts_count' do
+        it 'updates the last_attempts_count' do
+          flaky_example.update_flakiness!(last_attempts_count: 42)
 
-      context 'when run locally' do
-        it_behaves_like 'a valid FlakyExample hash' do
-          let(:additional_attrs) do
-            { first_flaky_at: instance_of(Time) }
-          end
+          expect(flaky_example.last_attempts_count).to eq(42)
         end
       end
 
@@ -87,10 +109,45 @@ describe RspecFlaky::FlakyExample do
           stub_env('CI_JOB_ID', 42)
         end
 
-        it_behaves_like 'a valid FlakyExample hash' do
-          let(:additional_attrs) do
-            { first_flaky_at: instance_of(Time), last_flaky_job: "https://gitlab.com/gitlab-org/gitlab-ce/-/jobs/42" }
-          end
+        it 'updates the last_flaky_job' do
+          flaky_example.update_flakiness!
+
+          expect(flaky_example.last_flaky_job).to eq('https://gitlab.com/gitlab-org/gitlab-ce/-/jobs/42')
+        end
+      end
+    end
+
+    context 'when given an Rspec::Example' do
+      it_behaves_like 'an up-to-date FlakyExample instance' do
+        let(:args) { example }
+      end
+    end
+
+    context 'when given a hash' do
+      it_behaves_like 'an up-to-date FlakyExample instance' do
+        let(:args) { flaky_example_attrs }
+      end
+    end
+  end
+
+  describe '#to_h' do
+    shared_examples 'a valid FlakyExample hash' do
+      let(:additional_attrs) { {} }
+
+      it 'returns a valid hash' do
+        flaky_example = described_class.new(args)
+        final_hash = flaky_example_attrs.merge(additional_attrs)
+
+        expect(flaky_example.to_h).to eq(final_hash)
+      end
+    end
+
+    context 'when given an Rspec::Example' do
+      let(:args) { example }
+
+      it_behaves_like 'a valid FlakyExample hash' do
+        let(:additional_attrs) do
+          { first_flaky_at: nil, last_flaky_at: nil, last_flaky_job: nil, flaky_reports: 0 }
         end
       end
     end

spec/spec_helper.rb

@@ -88,7 +88,10 @@ RSpec.configure do |config|
   if ENV['CI']
     # This includes the first try, i.e. tests will be run 4 times before failing.
     config.default_retry_count = 4
-    config.reporter.register_listener(RspecFlaky::Listener.new, :example_passed, :dump_summary)
+    config.reporter.register_listener(
+      RspecFlaky::Listener.new,
+      :example_passed,
+      :dump_summary)
   end
 
   config.before(:suite) do