Commit 89e7b515 authored by Rémy Coutable's avatar Rémy Coutable

[EE] Detect and keep track of flaky specs

Signed-off-by: default avatarRémy Coutable <remy@rymai.me>
parent 42688020
This diff is collapsed.
...@@ -1047,7 +1047,7 @@ RSpec/BeforeAfterAll: ...@@ -1047,7 +1047,7 @@ RSpec/BeforeAfterAll:
RSpec/DescribeClass: RSpec/DescribeClass:
Enabled: false Enabled: false
# Use `described_class` for tested class / module. # Checks that the second argument to `describe` specifies a method.
RSpec/DescribeMethod: RSpec/DescribeMethod:
Enabled: false Enabled: false
...@@ -1055,8 +1055,7 @@ RSpec/DescribeMethod: ...@@ -1055,8 +1055,7 @@ RSpec/DescribeMethod:
RSpec/DescribeSymbol: RSpec/DescribeSymbol:
Enabled: true Enabled: true
# Checks that the second argument to top level describe is the tested method # Checks that tests use `described_class`.
# name.
RSpec/DescribedClass: RSpec/DescribedClass:
Enabled: true Enabled: true
......
...@@ -95,6 +95,10 @@ possible). ...@@ -95,6 +95,10 @@ possible).
[Poltergeist]: https://github.com/teamcapybara/capybara#poltergeist [Poltergeist]: https://github.com/teamcapybara/capybara#poltergeist
[RackTest]: https://github.com/teamcapybara/capybara#racktest [RackTest]: https://github.com/teamcapybara/capybara#racktest
### EE-specific tests
EE-specific tests follows the same organization, but under the `spec/ee` folder.
#### Best practices #### Best practices
- Create only the necessary records in the database - Create only the necessary records in the database
...@@ -157,8 +161,9 @@ trade-off: ...@@ -157,8 +161,9 @@ trade-off:
- Unit tests are usually cheap, and you should consider them like the basement - Unit tests are usually cheap, and you should consider them like the basement
of your house: you need them to be confident that your code is behaving of your house: you need them to be confident that your code is behaving
correctly. However if you run only unit tests without integration / system tests, you might [miss] the [big] [picture]! correctly. However if you run only unit tests without integration / system
- Integration tests are a bit more expensive, but don't abuse them. A feature test tests, you might [miss] the [big] [picture]!
- Integration tests are a bit more expensive, but don't abuse them. A system test
is often better than an integration test that is stubbing a lot of internals. is often better than an integration test that is stubbing a lot of internals.
- System tests are expensive (compared to unit tests), even more if they require - System tests are expensive (compared to unit tests), even more if they require
a JavaScript driver. Make sure to follow the guidelines in the [Speed](#test-speed) a JavaScript driver. Make sure to follow the guidelines in the [Speed](#test-speed)
...@@ -195,11 +200,27 @@ Please consult the [dedicated "Frontend testing" guide](./fe_guide/testing.md). ...@@ -195,11 +200,27 @@ Please consult the [dedicated "Frontend testing" guide](./fe_guide/testing.md).
- Try to match the ordering of tests to the ordering within the class. - Try to match the ordering of tests to the ordering within the class.
- Try to follow the [Four-Phase Test][four-phase-test] pattern, using newlines - Try to follow the [Four-Phase Test][four-phase-test] pattern, using newlines
to separate phases. to separate phases.
- Try to use `Gitlab.config.gitlab.host` rather than hard coding `'localhost'` - Use `Gitlab.config.gitlab.host` rather than hard coding `'localhost'`
- Don't assert against the absolute value of a sequence-generated attribute (see
[Gotchas](gotchas.md#dont-assert-against-the-absolute-value-of-a-sequence-generated-attribute)).
- Don't supply the `:each` argument to hooks since it's the default.
- On `before` and `after` hooks, prefer it scoped to `:context` over `:all` - On `before` and `after` hooks, prefer it scoped to `:context` over `:all`
[four-phase-test]: https://robots.thoughtbot.com/four-phase-test [four-phase-test]: https://robots.thoughtbot.com/four-phase-test
### Automatic retries and flaky tests detection
On our CI, we use [rspec-retry] to automatically retry a failing example a few
times (see [`spec/spec_helper.rb`] for the precise retries count).
We also use a home-made `RspecFlaky::Listener` listener which records flaky
examples in a JSON report file on `master` (`retrieve-tests-metadata` and `update-tests-metadata` jobs), and warns when a new flaky example
is detected in any other branch (`flaky-examples-check` job). In the future, the
`flaky-examples-check` job will not be allowed to fail.
[rspec-retry]: https://github.com/NoRedInk/rspec-retry
[`spec/spec_helper.rb`]: https://gitlab.com/gitlab-org/gitlab-ce/blob/master/spec/spec_helper.rb
### `let` variables ### `let` variables
GitLab's RSpec suite has made extensive use of `let` variables to reduce GitLab's RSpec suite has made extensive use of `let` variables to reduce
......
module RspecFlaky
# This is a wrapper class for RSpec::Core::Example
class Example
delegate :status, :exception, to: :execution_result
def initialize(rspec_example)
@rspec_example = rspec_example.try(:example) || rspec_example
end
def uid
@uid ||= Digest::MD5.hexdigest("#{description}-#{file}")
end
def example_id
rspec_example.id
end
def file
metadata[:file_path]
end
def line
metadata[:line_number]
end
def description
metadata[:full_description]
end
def attempts
rspec_example.try(:attempts) || 1
end
private
attr_reader :rspec_example
def metadata
rspec_example.metadata
end
def execution_result
rspec_example.execution_result
end
end
end
module RspecFlaky
# 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: 1)
else
super
end
end
def first_flaky_at
self[:first_flaky_at] || Time.now
end
def last_flaky_at
Time.now
end
def last_flaky_job
return unless ENV['CI_PROJECT_URL'] && ENV['CI_JOB_ID']
"#{ENV['CI_PROJECT_URL']}/-/jobs/#{ENV['CI_JOB_ID']}"
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
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
end
def example_passed(notification)
current_example = RspecFlaky::Example.new(notification.example)
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
end
def dump_summary(_)
write_report_file(all_flaky_examples, all_flaky_examples_report_path)
if new_flaky_examples.any?
Rails.logger.warn "\nNew flaky examples detected:\n"
Rails.logger.warn JSON.pretty_generate(to_report(new_flaky_examples))
write_report_file(new_flaky_examples, new_flaky_examples_report_path)
end
end
def to_report(examples)
Hash[examples.map { |k, ex| [k, ex.to_h] }]
end
private
def init_all_flaky_examples
return {} unless File.exist?(all_flaky_examples_report_path)
all_flaky_examples = JSON.parse(File.read(all_flaky_examples_report_path))
Hash[(all_flaky_examples || {}).map { |k, ex| [k, FlakyExample.new(ex)] }]
end
def write_report_file(examples, file_path)
return unless ENV['FLAKY_RSPEC_GENERATE_REPORT'] == 'true'
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(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")
end
def new_flaky_examples_report_path
@new_flaky_examples_report_path ||= ENV['NEW_FLAKY_RSPEC_REPORT_PATH'] ||
Rails.root.join("rspec_flaky/new-report.json")
end
end
end
#!/usr/bin/env ruby
require 'json'
report_file = ARGV.shift
unless report_file
puts 'usage: detect-new-flaky-examples <report-file>'
exit 1
end
puts "Loading #{report_file}..."
report = JSON.parse(File.read(report_file))
if report.any?
puts "New flaky examples were detected!\n"
puts JSON.pretty_generate(report)
exit 1
else
puts "No new flaky examples detected.\n"
exit 0
end
...@@ -4,7 +4,7 @@ require 'json' ...@@ -4,7 +4,7 @@ require 'json'
main_report_file = ARGV.shift main_report_file = ARGV.shift
unless main_report_file unless main_report_file
puts 'usage: merge_reports <main-report> [extra reports...]' puts 'usage: merge-reports <main-report> [extra reports...]'
exit 1 exit 1
end end
......
...@@ -106,12 +106,6 @@ describe Gitlab::HealthChecks::FsShardsCheck do ...@@ -106,12 +106,6 @@ describe Gitlab::HealthChecks::FsShardsCheck do
}.with_indifferent_access }.with_indifferent_access
end end
# Unsolved intermittent failure in CI https://gitlab.com/gitlab-org/gitlab-ce/issues/31128
around do |example| # rubocop:disable RSpec/AroundBlock
times_to_try = ENV['CI'] ? 4 : 1
example.run_with_retry retry: times_to_try
end
it 'provides metrics' do it 'provides metrics' do
metrics = described_class.metrics metrics = described_class.metrics
......
require 'spec_helper'
describe RspecFlaky::Example do
let(: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: 'BOOM!'),
attempts: 1
}
end
let(:rspec_example) { double(example_attrs) }
describe '#initialize' do
shared_examples 'a valid Example instance' do
it 'returns valid attributes' do
example = described_class.new(args)
expect(example.example_id).to eq(example_attrs[:id])
end
end
context 'when given an Rspec::Core::Example that responds to #example' do
let(:args) { double(example: rspec_example) }
it_behaves_like 'a valid Example instance'
end
context 'when given an Rspec::Core::Example that does not respond to #example' do
let(:args) { rspec_example }
it_behaves_like 'a valid Example instance'
end
end
subject { described_class.new(rspec_example) }
describe '#uid' do
it 'returns a hash of the full description' do
expect(subject.uid).to eq(Digest::MD5.hexdigest("#{subject.description}-#{subject.file}"))
end
end
describe '#example_id' do
it 'returns the ID of the RSpec::Core::Example' do
expect(subject.example_id).to eq(rspec_example.id)
end
end
describe '#attempts' do
it 'returns the attempts of the RSpec::Core::Example' do
expect(subject.attempts).to eq(rspec_example.attempts)
end
end
describe '#file' do
it 'returns the metadata[:file_path] of the RSpec::Core::Example' do
expect(subject.file).to eq(rspec_example.metadata[:file_path])
end
end
describe '#line' do
it 'returns the metadata[:line_number] of the RSpec::Core::Example' do
expect(subject.line).to eq(rspec_example.metadata[:line_number])
end
end
describe '#description' do
it 'returns the metadata[:full_description] of the RSpec::Core::Example' do
expect(subject.description).to eq(rspec_example.metadata[:full_description])
end
end
describe '#status' do
it 'returns the execution_result.status of the RSpec::Core::Example' do
expect(subject.status).to eq(rspec_example.execution_result.status)
end
end
describe '#exception' do
it 'returns the execution_result.exception of the RSpec::Core::Example' do
expect(subject.exception).to eq(rspec_example.execution_result.exception)
end
end
end
require 'spec_helper'
describe RspecFlaky::FlakyExample do
let(:flaky_example_attrs) 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_flaky_at: 2345,
last_attempts_count: 2,
flaky_reports: 1
}
end
let(:example_attrs) do
{
uid: 'abc123',
example_id: flaky_example_attrs[:example_id],
file: flaky_example_attrs[:file],
line: flaky_example_attrs[:line],
description: flaky_example_attrs[:description],
status: 'passed',
exception: 'BOOM!',
attempts: flaky_example_attrs[:last_attempts_count]
}
end
let(:example) { double(example_attrs) }
describe '#initialize' do
shared_examples 'a valid FlakyExample instance' do
it 'returns valid attributes' do
flaky_example = described_class.new(args)
expect(flaky_example.uid).to eq(flaky_example_attrs[:uid])
expect(flaky_example.example_id).to eq(flaky_example_attrs[:example_id])
end
end
context 'when given an Rspec::Example' do
let(:args) { example }
it_behaves_like 'a valid FlakyExample instance'
end
context 'when given a hash' do
let(:args) { flaky_example_attrs }
it_behaves_like 'a valid FlakyExample instance'
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
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(last_flaky_at: instance_of(Time), last_flaky_job: nil)
.merge(additional_attrs)
expect(flaky_example.to_h).to match(hash_including(final_hash))
end
end
context 'when given an Rspec::Example' do
let(:args) { example }
context 'when run locally' do
it_behaves_like 'a valid FlakyExample hash' do
let(:additional_attrs) do
{ first_flaky_at: instance_of(Time) }
end
end
end
context 'when run on the CI' do
before do
stub_env('CI_PROJECT_URL', 'https://gitlab.com/gitlab-org/gitlab-ce')
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
end
end
end
context 'when given a hash' do
let(:args) { flaky_example_attrs }
it_behaves_like 'a valid FlakyExample hash'
end
end
end
require 'spec_helper'
describe RspecFlaky::Listener do
let(:flaky_example_report) do
{
'abc123' => {
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,
flaky_reports: 1,
last_flaky_job: nil
}
}
end
let(:example_attrs) do
{
id: 'spec/foo/baz_spec.rb:3',
metadata: {
file_path: 'spec/foo/baz_spec.rb',
line_number: 3,
full_description: 'hello GitLab'
},
execution_result: double(status: 'passed', exception: nil)
}
end
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 Listener instance' do
let(:expected_all_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({})
end
end
context 'when no report file exists' do
it_behaves_like 'a valid Listener instance'
end
context 'when a report file exists and set by ALL_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.rewind
end
end
before do
stub_env('ALL_FLAKY_RSPEC_REPORT_PATH', report_file.path)
end
after do
report_file.close
report_file.unlink
end
it_behaves_like 'a valid Listener instance' do
let(:expected_all_flaky_examples) { flaky_example_report }
end
end
end
describe '#example_passed' do
let(:rspec_example) { double(example_attrs) }
let(:notification) { double(example: rspec_example) }
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 }
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(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(example_attrs.merge(attempts: 2)) }
let(:expected_new_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 }
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))
end
end
end
describe '#dump_summary' do
let(:rspec_example) { double(example_attrs) }
let(:notification) { double(example: rspec_example) }
context 'when a report file path is set by ALL_FLAKY_RSPEC_REPORT_PATH' do
let(:report_file_path) { Rails.root.join('tmp', 'rspec_flaky_report.json') }
before do
stub_env('ALL_FLAKY_RSPEC_REPORT_PATH', report_file_path)
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 FLAKY_RSPEC_GENERATE_REPORT == "false"' do
before do
stub_env('FLAKY_RSPEC_GENERATE_REPORT', 'false')
end
it 'does not write the report file' do
subject.example_passed(notification)
subject.dump_summary(nil)
expect(File.exist?(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
it 'writes the report file' do
subject.example_passed(notification)
subject.dump_summary(nil)
expect(File.exist?(report_file_path)).to be(true)
end
end
end
end
describe '#to_report' do
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))
end
end
end
...@@ -76,6 +76,12 @@ RSpec.configure do |config| ...@@ -76,6 +76,12 @@ RSpec.configure do |config|
config.raise_errors_for_deprecations! config.raise_errors_for_deprecations!
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)
end
config.before(:suite) do config.before(:suite) do
TestEnv.init TestEnv.init
end end
...@@ -111,12 +117,6 @@ RSpec.configure do |config| ...@@ -111,12 +117,6 @@ RSpec.configure do |config|
reset_delivered_emails! reset_delivered_emails!
end end
if ENV['CI']
config.around(:each) do |ex|
ex.run_with_retry retry: 2
end
end
config.around(:each, :use_clean_rails_memory_store_caching) do |example| config.around(:each, :use_clean_rails_memory_store_caching) do |example|
caching_store = Rails.cache caching_store = Rails.cache
Rails.cache = ActiveSupport::Cache::MemoryStore.new Rails.cache = ActiveSupport::Cache::MemoryStore.new
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment