diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index 2249115e82a..3a97777e99f 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -368,6 +368,7 @@ update-tests-metadata: - 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/all_*_*.json + - scripts/prune-old-flaky-specs ${FLAKY_RSPEC_SUITE_REPORT_PATH} - '[[ -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 diff --git a/lib/rspec_flaky/config.rb b/lib/rspec_flaky/config.rb index a17ae55910e..06e96f969f1 100644 --- a/lib/rspec_flaky/config.rb +++ b/lib/rspec_flaky/config.rb @@ -1,9 +1,7 @@ -require 'json' - module RspecFlaky class Config def self.generate_report? - ENV['FLAKY_RSPEC_GENERATE_REPORT'] == 'true' + !!(ENV['FLAKY_RSPEC_GENERATE_REPORT'] =~ /1|true/) end def self.suite_flaky_examples_report_path diff --git a/lib/rspec_flaky/flaky_examples_collection.rb b/lib/rspec_flaky/flaky_examples_collection.rb index 973c95b0212..dea23c325be 100644 --- a/lib/rspec_flaky/flaky_examples_collection.rb +++ b/lib/rspec_flaky/flaky_examples_collection.rb @@ -1,11 +1,9 @@ -require 'json' +require 'active_support/hash_with_indifferent_access' + +require_relative 'flaky_example' module RspecFlaky class FlakyExamplesCollection < SimpleDelegator - def self.from_json(json) - new(JSON.parse(json)) - end - def initialize(collection = {}) unless collection.is_a?(Hash) raise ArgumentError, "`collection` must be a Hash, #{collection.class} given!" @@ -22,7 +20,7 @@ module RspecFlaky super(Hash[collection_of_flaky_examples]) end - def to_report + def to_h Hash[map { |uid, example| [uid, example.to_h] }].deep_symbolize_keys end diff --git a/lib/rspec_flaky/listener.rb b/lib/rspec_flaky/listener.rb index 4a5bfec9967..5b5e4f7c7de 100644 --- a/lib/rspec_flaky/listener.rb +++ b/lib/rspec_flaky/listener.rb @@ -1,5 +1,11 @@ require 'json' +require_relative 'config' +require_relative 'example' +require_relative 'flaky_example' +require_relative 'flaky_examples_collection' +require_relative 'report' + module RspecFlaky class Listener # - suite_flaky_examples: contains all the currently tracked flacky example @@ -9,7 +15,7 @@ module RspecFlaky attr_reader :suite_flaky_examples, :flaky_examples def initialize(suite_flaky_examples_json = nil) - @flaky_examples = FlakyExamplesCollection.new + @flaky_examples = RspecFlaky::FlakyExamplesCollection.new @suite_flaky_examples = init_suite_flaky_examples(suite_flaky_examples_json) end @@ -18,47 +24,36 @@ module RspecFlaky return unless current_example.attempts > 1 - flaky_example = suite_flaky_examples.fetch(current_example.uid) { FlakyExample.new(current_example) } + flaky_example = suite_flaky_examples.fetch(current_example.uid) { RspecFlaky::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(flaky_examples, RspecFlaky::Config.flaky_examples_report_path) + 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_report) + Rails.logger.warn JSON.pretty_generate(new_flaky_examples.to_h) - write_report_file(new_flaky_examples, RspecFlaky::Config.new_flaky_examples_report_path) + RspecFlaky::Report.new(new_flaky_examples).write(RspecFlaky::Config.new_flaky_examples_report_path) + # write_report_file(new_flaky_examples, RspecFlaky::Config.new_flaky_examples_report_path) end end - def to_report(examples) - Hash[examples.map { |k, ex| [k, ex.to_h] }] - end - private def init_suite_flaky_examples(suite_flaky_examples_json = nil) - unless suite_flaky_examples_json + if suite_flaky_examples_json + RspecFlaky::Report.load_json(suite_flaky_examples_json).flaky_examples + else return {} unless File.exist?(RspecFlaky::Config.suite_flaky_examples_report_path) - suite_flaky_examples_json = File.read(RspecFlaky::Config.suite_flaky_examples_report_path) + RspecFlaky::Report.load(RspecFlaky::Config.suite_flaky_examples_report_path).flaky_examples end - - FlakyExamplesCollection.from_json(suite_flaky_examples_json) - end - - def write_report_file(examples_collection, file_path) - return unless RspecFlaky::Config.generate_report? - - report_path_dir = File.dirname(file_path) - FileUtils.mkdir_p(report_path_dir) unless Dir.exist?(report_path_dir) - - File.write(file_path, JSON.pretty_generate(examples_collection.to_report)) end end end diff --git a/lib/rspec_flaky/report.rb b/lib/rspec_flaky/report.rb new file mode 100644 index 00000000000..a8730d3b7c7 --- /dev/null +++ b/lib/rspec_flaky/report.rb @@ -0,0 +1,54 @@ +require 'json' +require 'time' + +require_relative 'config' +require_relative 'flaky_examples_collection' + +module RspecFlaky + # This class is responsible for loading/saving JSON reports, and pruning + # outdated examples. + class Report < SimpleDelegator + OUTDATED_DAYS_THRESHOLD = 90 + + attr_reader :flaky_examples + + def self.load(file_path) + load_json(File.read(file_path)) + end + + def self.load_json(json) + new(RspecFlaky::FlakyExamplesCollection.new(JSON.parse(json))) + end + + def initialize(flaky_examples) + unless flaky_examples.is_a?(RspecFlaky::FlakyExamplesCollection) + raise ArgumentError, "`flaky_examples` must be a RspecFlaky::FlakyExamplesCollection, #{flaky_examples.class} given!" + end + + @flaky_examples = flaky_examples + super(flaky_examples) + end + + def write(file_path) + unless RspecFlaky::Config.generate_report? + puts "! Generating reports is disabled. To enable it, please set the `FLAKY_RSPEC_GENERATE_REPORT=1` !" # rubocop:disable Rails/Output + return + end + + report_path_dir = File.dirname(file_path) + FileUtils.mkdir_p(report_path_dir) unless Dir.exist?(report_path_dir) + + File.write(file_path, JSON.pretty_generate(flaky_examples.to_h)) + end + + 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 + end + + self.class.new(RspecFlaky::FlakyExamplesCollection.new(updated_hash)) + end + end +end diff --git a/scripts/prune-old-flaky-specs b/scripts/prune-old-flaky-specs new file mode 100755 index 00000000000..ab03a0230cc --- /dev/null +++ b/scripts/prune-old-flaky-specs @@ -0,0 +1,19 @@ +#!/usr/bin/env ruby + +require_relative '../lib/rspec_flaky/report' + +report_file = ARGV.shift +unless report_file + puts 'usage: prune-old-flaky-specs ' + exit 1 +end + +new_report_file = ARGV.shift || report_file +report = RspecFlaky::Report.load(report_file) +puts "Loading #{report_file}..." +puts "Current report has #{report.size} entries." + +new_report = report.prune_outdated + +puts "New report has #{new_report.size} entries: #{report.size - new_report.size} entries older than 90 days were removed." +puts "Saved #{new_report_file}." if new_report.write(new_report_file) diff --git a/spec/lib/rspec_flaky/config_spec.rb b/spec/lib/rspec_flaky/config_spec.rb index 83556787e85..4a71b1feebd 100644 --- a/spec/lib/rspec_flaky/config_spec.rb +++ b/spec/lib/rspec_flaky/config_spec.rb @@ -16,23 +16,25 @@ describe RspecFlaky::Config, :aggregate_failures do end end - context "when ENV['FLAKY_RSPEC_GENERATE_REPORT'] is set to 'false'" do - before do - stub_env('FLAKY_RSPEC_GENERATE_REPORT', 'false') + context "when ENV['FLAKY_RSPEC_GENERATE_REPORT'] is set" do + using RSpec::Parameterized::TableSyntax + + where(:env_value, :result) do + '1' | true + 'true' | true + 'foo' | false + '0' | false + 'false' | false end - it 'returns false' do - expect(described_class).not_to be_generate_report - end - end + with_them do + before do + stub_env('FLAKY_RSPEC_GENERATE_REPORT', env_value) + end - context "when ENV['FLAKY_RSPEC_GENERATE_REPORT'] is set to 'true'" do - before do - stub_env('FLAKY_RSPEC_GENERATE_REPORT', 'true') - end - - it 'returns true' do - expect(described_class).to be_generate_report + it 'returns false' do + expect(described_class.generate_report?).to be(result) + end end end end diff --git a/spec/lib/rspec_flaky/flaky_examples_collection_spec.rb b/spec/lib/rspec_flaky/flaky_examples_collection_spec.rb index 06a8ba0d02e..6731a27ed17 100644 --- a/spec/lib/rspec_flaky/flaky_examples_collection_spec.rb +++ b/spec/lib/rspec_flaky/flaky_examples_collection_spec.rb @@ -24,14 +24,6 @@ describe RspecFlaky::FlakyExamplesCollection, :aggregate_failures do } end - describe '.from_json' do - it 'accepts a JSON' do - collection = described_class.from_json(JSON.pretty_generate(collection_hash)) - - expect(collection.to_report).to eq(described_class.new(collection_hash).to_report) - end - end - describe '#initialize' do it 'accepts no argument' do expect { described_class.new }.not_to raise_error @@ -46,11 +38,11 @@ describe RspecFlaky::FlakyExamplesCollection, :aggregate_failures do end end - describe '#to_report' do + describe '#to_h' do it 'calls #to_h on the values' do collection = described_class.new(collection_hash) - expect(collection.to_report).to eq(collection_report) + expect(collection.to_h).to eq(collection_report) end end @@ -61,7 +53,7 @@ describe RspecFlaky::FlakyExamplesCollection, :aggregate_failures do a: { example_id: 'spec/foo/bar_spec.rb:2' }, c: { example_id: 'spec/bar/baz_spec.rb:4' }) - expect((collection2 - collection1).to_report).to eq( + expect((collection2 - collection1).to_h).to eq( c: { example_id: 'spec/bar/baz_spec.rb:4', first_flaky_at: nil, diff --git a/spec/lib/rspec_flaky/listener_spec.rb b/spec/lib/rspec_flaky/listener_spec.rb index bfb7648b486..ef085445081 100644 --- a/spec/lib/rspec_flaky/listener_spec.rb +++ b/spec/lib/rspec_flaky/listener_spec.rb @@ -4,7 +4,7 @@ describe RspecFlaky::Listener, :aggregate_failures do let(:already_flaky_example_uid) { '6e869794f4cfd2badd93eb68719371d1' } let(:suite_flaky_example_report) do { - already_flaky_example_uid => { + "#{already_flaky_example_uid}": { example_id: 'spec/foo/bar_spec.rb:2', file: 'spec/foo/bar_spec.rb', line: 2, @@ -55,8 +55,7 @@ describe RspecFlaky::Listener, :aggregate_failures do it 'returns a valid Listener instance' do listener = described_class.new - expect(listener.to_report(listener.suite_flaky_examples)) - .to eq(expected_suite_flaky_examples) + expect(listener.suite_flaky_examples.to_h).to eq(expected_suite_flaky_examples) expect(listener.flaky_examples).to eq({}) end end @@ -65,25 +64,35 @@ describe RspecFlaky::Listener, :aggregate_failures do it_behaves_like 'a valid Listener instance' end - context 'when a report file exists and set by SUITE_FLAKY_RSPEC_REPORT_PATH' do - let(:report_file) do - Tempfile.new(%w[rspec_flaky_report .json]).tap do |f| - f.write(JSON.pretty_generate(suite_flaky_example_report)) - f.rewind + context 'when SUITE_FLAKY_RSPEC_REPORT_PATH is set' do + let(:report_file_path) { 'foo/report.json' } + + before do + stub_env('SUITE_FLAKY_RSPEC_REPORT_PATH', report_file_path) + end + + context 'and report file exists' do + before do + expect(File).to receive(:exist?).with(report_file_path).and_return(true) + end + + it 'delegates the load to RspecFlaky::Report' do + report = RspecFlaky::Report.new(RspecFlaky::FlakyExamplesCollection.new(suite_flaky_example_report)) + + expect(RspecFlaky::Report).to receive(:load).with(report_file_path).and_return(report) + expect(described_class.new.suite_flaky_examples.to_h).to eq(report.flaky_examples.to_h) end end - before do - stub_env('SUITE_FLAKY_RSPEC_REPORT_PATH', report_file.path) - end + context 'and report file does not exist' do + before do + expect(File).to receive(:exist?).with(report_file_path).and_return(false) + end - after do - report_file.close - report_file.unlink - end - - it_behaves_like 'a valid Listener instance' do - let(:expected_suite_flaky_examples) { suite_flaky_example_report } + it 'return an empty hash' do + expect(RspecFlaky::Report).not_to receive(:load) + expect(described_class.new.suite_flaky_examples.to_h).to eq({}) + end end end end @@ -186,74 +195,21 @@ describe RspecFlaky::Listener, :aggregate_failures do let(:notification_already_flaky_rspec_example) { double(example: already_flaky_rspec_example) } context 'when a report file path is set by FLAKY_RSPEC_REPORT_PATH' do - let(:report_file_path) { Rails.root.join('tmp', 'rspec_flaky_report.json') } - let(:new_report_file_path) { Rails.root.join('tmp', 'rspec_flaky_new_report.json') } + it 'delegates the writes to RspecFlaky::Report' do + listener.example_passed(notification_new_flaky_rspec_example) + listener.example_passed(notification_already_flaky_rspec_example) - before do - stub_env('FLAKY_RSPEC_REPORT_PATH', report_file_path) - stub_env('NEW_FLAKY_RSPEC_REPORT_PATH', new_report_file_path) - FileUtils.rm(report_file_path) if File.exist?(report_file_path) - FileUtils.rm(new_report_file_path) if File.exist?(new_report_file_path) + report1 = double + report2 = double + + 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(report2).to receive(:write).with(RspecFlaky::Config.new_flaky_examples_report_path) + + listener.dump_summary(nil) end - - after do - FileUtils.rm(report_file_path) if File.exist?(report_file_path) - FileUtils.rm(new_report_file_path) if File.exist?(new_report_file_path) - end - - context 'when FLAKY_RSPEC_GENERATE_REPORT == "false"' do - before do - stub_env('FLAKY_RSPEC_GENERATE_REPORT', 'false') - end - - it 'does not write any report file' do - listener.example_passed(notification_new_flaky_rspec_example) - - listener.dump_summary(nil) - - expect(File.exist?(report_file_path)).to be(false) - expect(File.exist?(new_report_file_path)).to be(false) - end - end - - context 'when FLAKY_RSPEC_GENERATE_REPORT == "true"' do - before do - stub_env('FLAKY_RSPEC_GENERATE_REPORT', 'true') - end - - around do |example| - Timecop.freeze { example.run } - end - - it 'writes the report files' do - listener.example_passed(notification_new_flaky_rspec_example) - listener.example_passed(notification_already_flaky_rspec_example) - - listener.dump_summary(nil) - - expect(File.exist?(report_file_path)).to be(true) - expect(File.exist?(new_report_file_path)).to be(true) - - expect(File.read(report_file_path)) - .to eq(JSON.pretty_generate(listener.to_report(listener.flaky_examples))) - - new_example = RspecFlaky::Example.new(notification_new_flaky_rspec_example) - new_flaky_example = RspecFlaky::FlakyExample.new(new_example) - new_flaky_example.update_flakiness! - - expect(File.read(new_report_file_path)) - .to eq(JSON.pretty_generate(listener.to_report(new_example.uid => new_flaky_example))) - end - end - end - end - - describe '#to_report' do - let(:listener) { described_class.new(suite_flaky_example_report.to_json) } - - it 'transforms the internal hash to a JSON-ready hash' do - expect(listener.to_report(already_flaky_example_uid => already_flaky_example)) - .to match(hash_including(suite_flaky_example_report)) end end end diff --git a/spec/lib/rspec_flaky/report_spec.rb b/spec/lib/rspec_flaky/report_spec.rb new file mode 100644 index 00000000000..7d57d99f7e5 --- /dev/null +++ b/spec/lib/rspec_flaky/report_spec.rb @@ -0,0 +1,125 @@ +require 'spec_helper' + +describe RspecFlaky::Report, :aggregate_failures do + let(:a_hundred_days) { 3600 * 24 * 100 } + let(:collection_hash) do + { + a: { example_id: 'spec/foo/bar_spec.rb:2' }, + b: { example_id: 'spec/foo/baz_spec.rb:3', first_flaky_at: (Time.now - a_hundred_days).to_s, last_flaky_at: (Time.now - a_hundred_days).to_s } + } + end + let(:suite_flaky_example_report) do + { + '6e869794f4cfd2badd93eb68719371d1': { + example_id: 'spec/foo/bar_spec.rb:2', + file: 'spec/foo/bar_spec.rb', + line: 2, + description: 'hello world', + first_flaky_at: 1234, + last_flaky_at: 4321, + last_attempts_count: 3, + flaky_reports: 1, + last_flaky_job: nil + } + } + end + let(:flaky_examples) { RspecFlaky::FlakyExamplesCollection.new(collection_hash) } + let(:report) { described_class.new(flaky_examples) } + + describe '.load' do + let!(:report_file) do + Tempfile.new(%w[rspec_flaky_report .json]).tap do |f| + f.write(JSON.pretty_generate(suite_flaky_example_report)) + f.rewind + end + end + + after do + report_file.close + report_file.unlink + end + + it 'loads the report file' do + expect(described_class.load(report_file.path).flaky_examples.to_h).to eq(suite_flaky_example_report) + end + end + + describe '.load_json' do + let(:report_json) do + JSON.pretty_generate(suite_flaky_example_report) + end + + it 'loads the report file' do + expect(described_class.load_json(report_json).flaky_examples.to_h).to eq(suite_flaky_example_report) + end + end + + describe '#initialize' do + it 'accepts a RspecFlaky::FlakyExamplesCollection' do + expect { report }.not_to raise_error + end + + it 'does not accept anything else' do + expect { described_class.new([1, 2, 3]) }.to raise_error(ArgumentError, "`flaky_examples` must be a RspecFlaky::FlakyExamplesCollection, Array given!") + end + end + + it 'delegates to #flaky_examples using SimpleDelegator' do + expect(report.__getobj__).to eq(flaky_examples) + end + + describe '#write' do + let(:report_file_path) { Rails.root.join('tmp', 'rspec_flaky_report.json') } + + before do + FileUtils.rm(report_file_path) if File.exist?(report_file_path) + end + + after do + FileUtils.rm(report_file_path) if File.exist?(report_file_path) + end + + context 'when RspecFlaky::Config.generate_report? is false' do + before do + allow(RspecFlaky::Config).to receive(:generate_report?).and_return(false) + end + + it 'does not write any report file' do + report.write(report_file_path) + + expect(File.exist?(report_file_path)).to be(false) + end + end + + context 'when RspecFlaky::Config.generate_report? is true' do + before do + allow(RspecFlaky::Config).to receive(:generate_report?).and_return(true) + end + + it 'delegates the writes to RspecFlaky::Report' do + report.write(report_file_path) + + expect(File.exist?(report_file_path)).to be(true) + expect(File.read(report_file_path)) + .to eq(JSON.pretty_generate(report.flaky_examples.to_h)) + end + end + end + + describe '#prune_outdated' do + it 'returns a new collection without the examples older than 90 days by default' do + new_report = flaky_examples.to_h.dup.tap { |r| r.delete(:b) } + new_flaky_examples = report.prune_outdated + + expect(new_flaky_examples).to be_a(described_class) + expect(new_flaky_examples.to_h).to eq(new_report) + expect(flaky_examples).to have_key(:b) + end + + it 'accepts a given number of days' do + new_flaky_examples = report.prune_outdated(days: 200) + + expect(new_flaky_examples.to_h).to eq(report.to_h) + end + end +end