diff options
author | Rémy Coutable <remy@rymai.me> | 2017-10-04 14:01:48 +0300 |
---|---|---|
committer | Rémy Coutable <remy@rymai.me> | 2017-10-04 16:14:38 +0300 |
commit | ebfb5a50757b65530e9649815619ec206513c65e (patch) | |
tree | c61b31e1f84989a6ef48f17cb428ee9b32a93956 /spec/lib/rspec_flaky | |
parent | 4a0f720a502ac02423cb9db20727ba386de3e1f1 (diff) |
Ensure RSpecFlaky doesn't automatically update flaky examples
Previously, instantiating a RspecFlaky::FlakyExample object would
automatically update its first_flaky_at, last_flaky_at and
last_flaky_job.
That was wrong because we would overwrite every time the suite report
with this false data.
We now:
- Get the suite report and only read from it
- Write only the currently detected flaky examples in the report, so
that the final report is only updated with flaky examples that were
actually detected in each job. Before, job1 could overwrite the legit
report from job2!
- Write the newly detected flaky examples by rejecting the already
tracked flaky specs instead of using another hash.
Signed-off-by: Rémy Coutable <remy@rymai.me>
Diffstat (limited to 'spec/lib/rspec_flaky')
-rw-r--r-- | spec/lib/rspec_flaky/flaky_example_spec.rb | 129 | ||||
-rw-r--r-- | spec/lib/rspec_flaky/listener_spec.rb | 173 |
2 files changed, 220 insertions, 82 deletions
diff --git a/spec/lib/rspec_flaky/flaky_example_spec.rb b/spec/lib/rspec_flaky/flaky_example_spec.rb index cbfc1e538ab..d19c34bebb3 100644 --- a/spec/lib/rspec_flaky/flaky_example_spec.rb +++ b/spec/lib/rspec_flaky/flaky_example_spec.rb @@ -1,6 +1,6 @@ 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 diff --git a/spec/lib/rspec_flaky/listener_spec.rb b/spec/lib/rspec_flaky/listener_spec.rb index 0e193bf408b..5d04d43c66f 100644 --- a/spec/lib/rspec_flaky/listener_spec.rb +++ b/spec/lib/rspec_flaky/listener_spec.rb @@ -1,22 +1,35 @@ require 'spec_helper' -describe RspecFlaky::Listener do - let(:flaky_example_report) do +describe RspecFlaky::Listener, :aggregate_failures do + let(:already_flaky_example_uid) { '6e869794f4cfd2badd93eb68719371d1' } + let(:suite_flaky_example_report) do { - 'abc123' => { + already_flaky_example_uid => { 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: instance_of(Time), - last_attempts_count: 2, + last_flaky_at: 4321, + last_attempts_count: 3, flaky_reports: 1, last_flaky_job: nil } } end - let(:example_attrs) do + let(:already_flaky_example_attrs) do + { + id: 'spec/foo/bar_spec.rb:2', + metadata: { + file_path: 'spec/foo/bar_spec.rb', + line_number: 2, + full_description: 'hello world' + }, + execution_result: double(status: 'passed', exception: nil) + } + end + let(:already_flaky_example) { RspecFlaky::FlakyExample.new(suite_flaky_example_report[already_flaky_example_uid]) } + let(:new_example_attrs) do { id: 'spec/foo/baz_spec.rb:3', metadata: { @@ -36,14 +49,15 @@ describe RspecFlaky::Listener do describe '#initialize' do shared_examples 'a valid Listener instance' do - let(:expected_all_flaky_examples) { {} } + let(:expected_suite_flaky_examples) { {} } it 'returns a valid Listener instance' do listener = described_class.new - expect(listener.to_report(listener.all_flaky_examples)) - .to match(hash_including(expected_all_flaky_examples)) - expect(listener.new_flaky_examples).to eq({}) + expect(listener.to_report(listener.suite_flaky_examples)) + .to eq(expected_suite_flaky_examples) + expect(listener.__send__(:_new_flaky_examples)).to eq({}) + expect(listener.flaky_examples).to eq({}) end end @@ -51,16 +65,16 @@ describe RspecFlaky::Listener do it_behaves_like 'a valid Listener instance' end - context 'when a report file exists and set by ALL_FLAKY_RSPEC_REPORT_PATH' do + 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(flaky_example_report)) + f.write(JSON.pretty_generate(suite_flaky_example_report)) f.rewind end end before do - stub_env('ALL_FLAKY_RSPEC_REPORT_PATH', report_file.path) + stub_env('SUITE_FLAKY_RSPEC_REPORT_PATH', report_file.path) end after do @@ -69,74 +83,122 @@ describe RspecFlaky::Listener do end it_behaves_like 'a valid Listener instance' do - let(:expected_all_flaky_examples) { flaky_example_report } + let(:expected_suite_flaky_examples) { suite_flaky_example_report } end end end describe '#example_passed' do - let(:rspec_example) { double(example_attrs) } + let(:rspec_example) { double(new_example_attrs) } let(:notification) { double(example: rspec_example) } + let(:listener) { described_class.new(suite_flaky_example_report.to_json) } shared_examples 'a non-flaky example' do it 'does not change the flaky examples hash' do - expect { subject.example_passed(notification) } - .not_to change { subject.all_flaky_examples } + expect { listener.example_passed(notification) } + .not_to change { listener.flaky_examples } end end - describe 'when the RSpec example does not respond to attempts' do - it_behaves_like 'a non-flaky example' - end + shared_examples 'an existing flaky example' do + let(:expected_flaky_example) do + { + example_id: 'spec/foo/bar_spec.rb:2', + file: 'spec/foo/bar_spec.rb', + line: 2, + description: 'hello world', + first_flaky_at: 1234, + last_attempts_count: 2, + flaky_reports: 2, + last_flaky_job: nil + } + end - describe 'when the RSpec example has 1 attempt' do - let(:rspec_example) { double(example_attrs.merge(attempts: 1)) } + it 'changes the flaky examples hash' do + new_example = RspecFlaky::Example.new(rspec_example) - it_behaves_like 'a non-flaky example' + now = Time.now + Timecop.freeze(now) do + expect { listener.example_passed(notification) } + .to change { listener.flaky_examples[new_example.uid].to_h } + end + + expect(listener.flaky_examples[new_example.uid].to_h) + .to eq(expected_flaky_example.merge(last_flaky_at: now)) + end end - describe 'when the RSpec example has 2 attempts' do - let(:rspec_example) { double(example_attrs.merge(attempts: 2)) } - let(:expected_new_flaky_example) do + shared_examples 'a new flaky example' do + let(:expected_flaky_example) do { example_id: 'spec/foo/baz_spec.rb:3', file: 'spec/foo/baz_spec.rb', line: 3, description: 'hello GitLab', - first_flaky_at: instance_of(Time), - last_flaky_at: instance_of(Time), last_attempts_count: 2, flaky_reports: 1, last_flaky_job: nil } end - it 'does not change the flaky examples hash' do - expect { subject.example_passed(notification) } - .to change { subject.all_flaky_examples } - + it 'changes the all flaky examples hash' do new_example = RspecFlaky::Example.new(rspec_example) - expect(subject.all_flaky_examples[new_example.uid].to_h) - .to match(hash_including(expected_new_flaky_example)) + now = Time.now + Timecop.freeze(now) do + expect { listener.example_passed(notification) } + .to change { listener.flaky_examples[new_example.uid].to_h } + end + + expect(listener.flaky_examples[new_example.uid].to_h) + .to eq(expected_flaky_example.merge(first_flaky_at: now, last_flaky_at: now)) + end + end + + describe 'when the RSpec example does not respond to attempts' do + it_behaves_like 'a non-flaky example' + end + + describe 'when the RSpec example has 1 attempt' do + let(:rspec_example) { double(new_example_attrs.merge(attempts: 1)) } + + it_behaves_like 'a non-flaky example' + end + + describe 'when the RSpec example has 2 attempts' do + let(:rspec_example) { double(new_example_attrs.merge(attempts: 2)) } + + it_behaves_like 'a new flaky example' + + context 'with an existing flaky example' do + let(:rspec_example) { double(already_flaky_example_attrs.merge(attempts: 2)) } + + it_behaves_like 'an existing flaky example' end end end describe '#dump_summary' do - let(:rspec_example) { double(example_attrs) } - let(:notification) { double(example: rspec_example) } + let(:listener) { described_class.new(suite_flaky_example_report.to_json) } + let(:new_flaky_rspec_example) { double(new_example_attrs.merge(attempts: 2)) } + let(:already_flaky_rspec_example) { double(already_flaky_example_attrs.merge(attempts: 2)) } + let(:notification_new_flaky_rspec_example) { double(example: new_flaky_rspec_example) } + let(:notification_already_flaky_rspec_example) { double(example: already_flaky_rspec_example) } - context 'when a report file path is set by ALL_FLAKY_RSPEC_REPORT_PATH' do + 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') } before do - stub_env('ALL_FLAKY_RSPEC_REPORT_PATH', report_file_path) + 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) 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 @@ -144,12 +206,13 @@ describe RspecFlaky::Listener do stub_env('FLAKY_RSPEC_GENERATE_REPORT', 'false') end - it 'does not write the report file' do - subject.example_passed(notification) + it 'does not write any report file' do + listener.example_passed(notification_new_flaky_rspec_example) - subject.dump_summary(nil) + 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 @@ -158,21 +221,39 @@ describe RspecFlaky::Listener do stub_env('FLAKY_RSPEC_GENERATE_REPORT', 'true') end - it 'writes the report file' do - subject.example_passed(notification) + 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) - subject.dump_summary(nil) + 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(subject.to_report('abc123' => RspecFlaky::FlakyExample.new(flaky_example_report['abc123']))) - .to match(hash_including(flaky_example_report)) + expect(listener.to_report(already_flaky_example_uid => already_flaky_example)) + .to match(hash_including(suite_flaky_example_report)) end end end |