Refactor RspecFlaky classes to not use OpenStruct

Signed-off-by: Rémy Coutable <remy@rymai.me>

.rubocop_todo/style/open_struct_use.yml

@@ -47,5 +47,3 @@ Style/OpenStructUse:
   - spec/support/helpers/import_spec_helper.rb
   - spec/support/helpers/login_helpers.rb
   - spec/support/helpers/repo_helpers.rb
-  - spec/tooling/rspec_flaky/flaky_example_spec.rb
-  - tooling/rspec_flaky/flaky_example.rb

spec/tooling/rspec_flaky/flaky_example_spec.rb

 # frozen_string_literal: true
 
-require 'active_support/testing/time_helpers'
 require_relative '../../support/helpers/stub_env'
+require_relative '../../support/time_travel'
 
 require_relative '../../../tooling/rspec_flaky/flaky_example'
 
@@ -36,40 +36,37 @@ RSpec.describe RspecFlaky::FlakyExample, :aggregate_failures do
     }
   end
 
-  let(:example) { OpenStruct.new(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)
+    stub_env('CI_JOB_URL', nil)
   end
 
-  describe '#initialize' do
+  describe '#initialize', :freeze_time do
     shared_examples 'a valid FlakyExample instance' do
       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.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)
+        expect(flaky_example.to_h[:uid]).to eq(flaky_example_attrs[:uid])
+        expect(flaky_example.to_h[:file]).to eq(flaky_example_attrs[:file])
+        expect(flaky_example.to_h[:line]).to eq(flaky_example_attrs[:line])
+        expect(flaky_example.to_h[:description]).to eq(flaky_example_attrs[:description])
+        expect(flaky_example.to_h[:first_flaky_at]).to eq(expected_first_flaky_at)
+        expect(flaky_example.to_h[:last_flaky_at]).to eq(expected_last_flaky_at)
+        expect(flaky_example.to_h[:last_attempts_count]).to eq(flaky_example_attrs[:last_attempts_count])
+        expect(flaky_example.to_h[:flaky_reports]).to eq(expected_flaky_reports)
       end
     end
 
-    context 'when given an Rspec::Example' do
+    context 'when given an Example hash' do
       it_behaves_like 'a valid FlakyExample instance' do
-        let(:args) { example }
-        let(:expected_first_flaky_at) { nil }
-        let(:expected_last_flaky_at) { nil }
+        let(:args) { example_attrs }
+        let(:expected_first_flaky_at) { Time.now }
+        let(:expected_last_flaky_at) { Time.now }
         let(:expected_flaky_reports) { 0 }
       end
     end
 
-    context 'when given a hash' do
+    context 'when given a FlakyExample hash' do
       it_behaves_like 'a valid FlakyExample instance' do
         let(:args) { flaky_example_attrs }
         let(:expected_flaky_reports) { flaky_example_attrs[:flaky_reports] }
@@ -89,17 +86,17 @@ RSpec.describe RspecFlaky::FlakyExample, :aggregate_failures do
         freeze_time do
           flaky_example.update_flakiness!
 
-          expect(flaky_example.first_flaky_at).to eq(Time.now)
+          expect(flaky_example.to_h[:first_flaky_at]).to eq(Time.now)
         end
       end
 
       it 'maintains the first_flaky_at if exists' do
         flaky_example.update_flakiness!
-        expected_first_flaky_at = flaky_example.first_flaky_at
+        expected_first_flaky_at = flaky_example.to_h[:first_flaky_at]
 
         travel_to(Time.now + 42) do
           flaky_example.update_flakiness!
-          expect(flaky_example.first_flaky_at).to eq(expected_first_flaky_at)
+          expect(flaky_example.to_h[:first_flaky_at]).to eq(expected_first_flaky_at)
         end
       end
 
@@ -108,53 +105,54 @@ RSpec.describe RspecFlaky::FlakyExample, :aggregate_failures do
           the_future = Time.now
           flaky_example.update_flakiness!
 
-          expect(flaky_example.last_flaky_at).to eq(the_future)
+          expect(flaky_example.to_h[:last_flaky_at]).to eq(the_future)
         end
       end
 
       it 'updates the flaky_reports' do
-        expected_flaky_reports = flaky_example.first_flaky_at ? flaky_example.flaky_reports + 1 : 1
+        expected_flaky_reports = flaky_example.to_h[:first_flaky_at] ? flaky_example.to_h[: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)
+        expect { flaky_example.update_flakiness! }.to change { flaky_example.to_h[:flaky_reports] }.by(1)
+        expect(flaky_example.to_h[: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)
 
-          expect(flaky_example.last_attempts_count).to eq(42)
+          expect(flaky_example.to_h[:last_attempts_count]).to eq(42)
         end
       end
 
       context 'when run on the CI' do
+        let(:job_url) { 'https://gitlab.com/gitlab-org/gitlab-foss/-/jobs/42' }
+
         before do
-          stub_env('CI_PROJECT_URL', 'https://gitlab.com/gitlab-org/gitlab-foss')
-          stub_env('CI_JOB_ID', 42)
+          stub_env('CI_JOB_URL', job_url)
         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-foss/-/jobs/42')
+          expect(flaky_example.to_h[:last_flaky_job]).to eq(job_url)
         end
       end
     end
 
-    context 'when given an Rspec::Example' do
+    context 'when given an Example hash' do
       it_behaves_like 'an up-to-date FlakyExample instance' do
-        let(:args) { example }
+        let(:args) { example_attrs }
       end
     end
 
-    context 'when given a hash' do
+    context 'when given a FlakyExample hash' do
       it_behaves_like 'an up-to-date FlakyExample instance' do
         let(:args) { flaky_example_attrs }
       end
     end
   end
 
-  describe '#to_h' do
+  describe '#to_h', :freeze_time do
     shared_examples 'a valid FlakyExample hash' do
       let(:additional_attrs) { {} }
 
@@ -166,17 +164,17 @@ RSpec.describe RspecFlaky::FlakyExample, :aggregate_failures do
       end
     end
 
-    context 'when given an Rspec::Example' do
-      let(:args) { example }
+    context 'when given an Example hash' do
+      let(:args) { example_attrs }
 
       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 }
+          { first_flaky_at: Time.now, last_flaky_at: Time.now, last_flaky_job: nil, flaky_reports: 0 }
         end
       end
     end
 
-    context 'when given a hash' do
+    context 'when given a FlakyExample hash' do
       let(:args) { flaky_example_attrs }
 
       it_behaves_like 'a valid FlakyExample hash'

spec/tooling/rspec_flaky/flaky_examples_collection_spec.rb

 # frozen_string_literal: true
 
+require_relative '../../support/time_travel'
+
 require_relative '../../../tooling/rspec_flaky/flaky_examples_collection'
 
-RSpec.describe RspecFlaky::FlakyExamplesCollection, :aggregate_failures do
+RSpec.describe RspecFlaky::FlakyExamplesCollection, :aggregate_failures, :freeze_time do
   let(:collection_hash) do
     {
       a: { example_id: 'spec/foo/bar_spec.rb:2' },
@@ -14,15 +16,19 @@ RSpec.describe RspecFlaky::FlakyExamplesCollection, :aggregate_failures do
     {
       a: {
         example_id: 'spec/foo/bar_spec.rb:2',
-        first_flaky_at: nil,
-        last_flaky_at: nil,
-        last_flaky_job: nil
+        first_flaky_at: Time.now,
+        last_flaky_at: Time.now,
+        last_flaky_job: nil,
+        flaky_reports: 0,
+        last_attempts_count: nil
       },
       b: {
         example_id: 'spec/foo/baz_spec.rb:3',
-        first_flaky_at: nil,
-        last_flaky_at: nil,
-        last_flaky_job: nil
+        first_flaky_at: Time.now,
+        last_flaky_at: Time.now,
+        last_flaky_job: nil,
+        flaky_reports: 0,
+        last_attempts_count: nil
       }
     }
   end
@@ -59,9 +65,11 @@ RSpec.describe RspecFlaky::FlakyExamplesCollection, :aggregate_failures do
       expect((collection2 - collection1).to_h).to eq(
         c: {
           example_id: 'spec/bar/baz_spec.rb:4',
-          first_flaky_at: nil,
-          last_flaky_at: nil,
-          last_flaky_job: nil
+          first_flaky_at: Time.now,
+          last_flaky_at: Time.now,
+          last_flaky_job: nil,
+          flaky_reports: 0,
+          last_attempts_count: nil
         })
     end
 

spec/tooling/rspec_flaky/listener_spec.rb

 # frozen_string_literal: true
 
-require 'active_support/testing/time_helpers'
 require_relative '../../support/helpers/stub_env'
+require_relative '../../support/time_travel'
 
 require_relative '../../../tooling/rspec_flaky/listener'
 
@@ -53,8 +53,7 @@ RSpec.describe RspecFlaky::Listener, :aggregate_failures 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)
+    stub_env('CI_JOB_URL', nil)
     stub_env('SUITE_FLAKY_RSPEC_REPORT_PATH', nil)
   end
 
@@ -217,7 +216,7 @@ RSpec.describe RspecFlaky::Listener, :aggregate_failures do
         expect(RspecFlaky::Report).to receive(:new).with(listener.flaky_examples).and_return(report1)
         expect(report1).to receive(:write).with(RspecFlaky::Config.flaky_examples_report_path)
 
-        expect(RspecFlaky::Report).to receive(:new).with(listener.flaky_examples - listener.suite_flaky_examples).and_return(report2)
+        expect(RspecFlaky::Report).to receive(:new).with(listener.__send__(:new_flaky_examples)).and_return(report2)
         expect(report2).to receive(:write).with(RspecFlaky::Config.new_flaky_examples_report_path)
 
         listener.dump_summary(nil)

spec/tooling/rspec_flaky/report_spec.rb

@@ -2,9 +2,11 @@
 
 require 'tempfile'
 
+require_relative '../../support/time_travel'
+
 require_relative '../../../tooling/rspec_flaky/report'
 
-RSpec.describe RspecFlaky::Report, :aggregate_failures do
+RSpec.describe RspecFlaky::Report, :aggregate_failures, :freeze_time do
   let(:thirty_one_days) { 3600 * 24 * 31 }
   let(:collection_hash) do
     {

tooling/rspec_flaky/example.rb

@@ -38,6 +38,16 @@ module RspecFlaky
       rspec_example.respond_to?(:attempts) ? rspec_example.attempts : 1
     end
 
+    def to_h
+      {
+        example_id: example_id,
+        file: file,
+        line: line,
+        description: description,
+        last_attempts_count: attempts
+      }
+    end
+
     private
 
     attr_reader :rspec_example

tooling/rspec_flaky/flaky_example.rb

@@ -3,38 +3,47 @@
 require 'ostruct'
 
 module RspecFlaky
+  ALLOWED_ATTRIBUTES = %i[
+    example_id
+    file
+    line
+    description
+    first_flaky_at
+    last_flaky_at
+    last_flaky_job
+    last_attempts_count
+    flaky_reports
+  ].freeze
+
   # This represents a flaky RSpec example and is mainly meant to be saved in a JSON file
-  class FlakyExample < OpenStruct
-    def initialize(example)
-      if example.respond_to?(:example_id)
-        super(
-          example_id: example.example_id,
-          file: example.file,
-          line: example.line,
-          description: example.description,
-          last_attempts_count: example.attempts,
-          flaky_reports: 0)
-      else
-        super
+  class FlakyExample
+    attr_reader :attributes
+    alias_method :to_h, :attributes
+
+    def initialize(example_hash)
+      example_hash[:last_attempts_count] ||= example_hash[:attempts]
+      @attributes = {
+        first_flaky_at: Time.now,
+        last_flaky_at: Time.now,
+        last_flaky_job: nil,
+        last_attempts_count: example_hash[:attempts],
+        flaky_reports: 0
+      }.merge(example_hash.slice(*ALLOWED_ATTRIBUTES))
+
+      %i[first_flaky_at last_flaky_at].each do |attr|
+        attributes[attr] = Time.parse(attributes[attr]) if attributes[attr].is_a?(String)
       end
     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
+      attributes[:first_flaky_at] ||= Time.now
+      attributes[:last_flaky_at] = Time.now
+      attributes[:flaky_reports] += 1
+      attributes[:last_attempts_count] = last_attempts_count if last_attempts_count
 
-      if ENV['CI_PROJECT_URL'] && ENV['CI_JOB_ID']
-        self.last_flaky_job = "#{ENV['CI_PROJECT_URL']}/-/jobs/#{ENV['CI_JOB_ID']}"
+      if ENV['CI_JOB_URL']
+        attributes[:last_flaky_job] = "#{ENV['CI_JOB_URL']}"
       end
     end
-
-    def to_h
-      super.merge(
-        first_flaky_at: first_flaky_at,
-        last_flaky_at: last_flaky_at,
-        last_flaky_job: last_flaky_job)
-    end
   end
 end

tooling/rspec_flaky/flaky_examples_collection.rb

@@ -16,7 +16,7 @@ module RspecFlaky
         collection.map do |uid, example|
           [
             uid,
-            example.is_a?(RspecFlaky::FlakyExample) ? example : RspecFlaky::FlakyExample.new(example)
+            RspecFlaky::FlakyExample.new(example.to_h.symbolize_keys)
           ]
         end
 

tooling/rspec_flaky/listener.rb

@@ -26,7 +26,7 @@ module RspecFlaky
 
       return unless current_example.attempts > 1
 
-      flaky_example = suite_flaky_examples.fetch(current_example.uid) { RspecFlaky::FlakyExample.new(current_example) }
+      flaky_example = suite_flaky_examples.fetch(current_example.uid) { RspecFlaky::FlakyExample.new(current_example.to_h) }
       flaky_example.update_flakiness!(last_attempts_count: current_example.attempts)
 
       flaky_examples[current_example.uid] = flaky_example
@@ -36,7 +36,6 @@ module RspecFlaky
       RspecFlaky::Report.new(flaky_examples).write(RspecFlaky::Config.flaky_examples_report_path)
       # write_report_file(flaky_examples, RspecFlaky::Config.flaky_examples_report_path)
 
-      new_flaky_examples = flaky_examples - suite_flaky_examples
       if new_flaky_examples.any?
         rails_logger_warn("\nNew flaky examples detected:\n")
         rails_logger_warn(JSON.pretty_generate(new_flaky_examples.to_h)) # rubocop:disable Gitlab/Json
@@ -48,6 +47,10 @@ module RspecFlaky
 
     private
 
+    def new_flaky_examples
+      @new_flaky_examples ||= flaky_examples - suite_flaky_examples
+    end
+
     def init_suite_flaky_examples(suite_flaky_examples_json = nil)
       if suite_flaky_examples_json
         RspecFlaky::Report.load_json(suite_flaky_examples_json).flaky_examples

tooling/rspec_flaky/report.rb

@@ -46,8 +46,9 @@ module RspecFlaky
     def prune_outdated(days: OUTDATED_DAYS_THRESHOLD)
       outdated_date_threshold = Time.now - (3600 * 24 * days)
       updated_hash = flaky_examples.dup
-        .delete_if do |uid, hash|
-          hash[:last_flaky_at] && Time.parse(hash[:last_flaky_at]).to_i < outdated_date_threshold.to_i
+        .delete_if do |_uid, flaky_example|
+          last_flaky_at = flaky_example.to_h[:last_flaky_at]
+          last_flaky_at && last_flaky_at.to_i < outdated_date_threshold.to_i
         end
 
       self.class.new(RspecFlaky::FlakyExamplesCollection.new(updated_hash))