diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
index c4469ea98a303ee094330f4dfb882dd39f077d51..9b9b217126b13c292e7957bd424c0ac224c914ee 100644
--- a/.gitlab-ci.yml
+++ b/.gitlab-ci.yml
@@ -165,7 +165,7 @@ variables:
RSPEC_PACKED_TESTS_MAPPING_PATH: crystalball/packed-mapping.json
RSPEC_PROFILING_FOLDER_PATH: rspec/profiling
RSPEC_TESTS_MAPPING_PATH: crystalball/mapping.json
- RSPEC_FAST_QUARANTINE_PATH: rspec/fast_quarantine-gitlab.txt
+ RSPEC_FAST_QUARANTINE_LOCAL_PATH: rspec/fast_quarantine-gitlab.txt
TMP_TEST_FOLDER: "${CI_PROJECT_DIR}/tmp/tests"
TMP_TEST_GITLAB_WORKHORSE_PATH: "${TMP_TEST_FOLDER}/${GITLAB_WORKHORSE_FOLDER}"
diff --git a/.gitlab/ci/rails.gitlab-ci.yml b/.gitlab/ci/rails.gitlab-ci.yml
index 17fd8ae38f914a83d71deec1015372abf3e18f08..a4041f771d9b98911fcaaf0a859b873a143e8ff2 100644
--- a/.gitlab/ci/rails.gitlab-ci.yml
+++ b/.gitlab/ci/rails.gitlab-ci.yml
@@ -346,7 +346,7 @@ rspec:flaky-tests-report:
# so we use `dependencies` here.
dependencies: !reference ["rspec:coverage", "dependencies"]
variables:
- SKIPPED_FLAKY_TESTS_REPORT_PATH: rspec/flaky/skipped_flaky_tests_report.txt
+ SKIPPED_TESTS_REPORT_PATH: rspec/skipped_tests_report.txt
RETRIED_TESTS_REPORT_PATH: rspec/flaky/retried_tests_report.txt
before_script:
- source scripts/utils.sh
diff --git a/doc/development/testing_guide/flaky_tests.md b/doc/development/testing_guide/flaky_tests.md
index d8bdc56f26556808ec9047c80e9e545ba2bcdfdb..40be7a7d0944acee9accbeb5f027553228def3d6 100644
--- a/doc/development/testing_guide/flaky_tests.md
+++ b/doc/development/testing_guide/flaky_tests.md
@@ -193,6 +193,8 @@ This means it is skipped unless run with `--tag quarantine`:
bin/rspec --tag quarantine
```
+After the long-term quarantining MR has reached production, you should revert the fast-quarantine MR you created earlier.
+
### Jest
For Jest specs, you can use the `.skip` method along with the `eslint-disable-next-line` comment to disable the `jest/no-disabled-tests` ESLint rule and include the issue URL. Here's an example:
diff --git a/scripts/rspec_helpers.sh b/scripts/rspec_helpers.sh
index b64e6ed6f58b53307438bbd4c7bbf4d09bee946a..4e73bf48021c560108917d2c98cef75c9063ac6d 100644
--- a/scripts/rspec_helpers.sh
+++ b/scripts/rspec_helpers.sh
@@ -13,9 +13,9 @@ function retrieve_tests_metadata() {
echo "{}" > "${FLAKY_RSPEC_SUITE_REPORT_PATH}"
fi
- if [[ ! -f "${RSPEC_FAST_QUARANTINE_PATH}" ]]; then
- curl --location -o "${RSPEC_FAST_QUARANTINE_PATH}" "https://gitlab-org.gitlab.io/quality/engineering-productivity/fast-quarantine/${RSPEC_FAST_QUARANTINE_PATH}" ||
- echo "" > "${RSPEC_FAST_QUARANTINE_PATH}"
+ if [[ ! -f "${RSPEC_FAST_QUARANTINE_LOCAL_PATH}" ]]; then
+ curl --location -o "${RSPEC_FAST_QUARANTINE_LOCAL_PATH}" "https://gitlab-org.gitlab.io/quality/engineering-productivity/fast-quarantine/${RSPEC_FAST_QUARANTINE_LOCAL_PATH}" ||
+ echo "" > "${RSPEC_FAST_QUARANTINE_LOCAL_PATH}"
fi
}
@@ -139,7 +139,7 @@ function debug_rspec_variables() {
echoinfo "FLAKY_RSPEC_SUITE_REPORT_PATH: ${FLAKY_RSPEC_SUITE_REPORT_PATH:-}"
echoinfo "FLAKY_RSPEC_REPORT_PATH: ${FLAKY_RSPEC_REPORT_PATH:-}"
echoinfo "NEW_FLAKY_RSPEC_REPORT_PATH: ${NEW_FLAKY_RSPEC_REPORT_PATH:-}"
- echoinfo "SKIPPED_FLAKY_TESTS_REPORT_PATH: ${SKIPPED_FLAKY_TESTS_REPORT_PATH:-}"
+ echoinfo "SKIPPED_TESTS_REPORT_PATH: ${SKIPPED_TESTS_REPORT_PATH:-}"
echoinfo "CRYSTALBALL: ${CRYSTALBALL:-}"
@@ -205,7 +205,7 @@ function rspec_paralellized_job() {
export KNAPSACK_TEST_FILE_PATTERN=$(ruby -r./tooling/quality/test_level.rb -e "puts Quality::TestLevel.new(${spec_folder_prefixes}).pattern(:${test_level})")
export FLAKY_RSPEC_REPORT_PATH="${rspec_flaky_folder_path}all_${report_name}_report.json"
export NEW_FLAKY_RSPEC_REPORT_PATH="${rspec_flaky_folder_path}new_${report_name}_report.json"
- export SKIPPED_FLAKY_TESTS_REPORT_PATH="${rspec_flaky_folder_path}skipped_flaky_tests_${report_name}_report.txt"
+ export SKIPPED_TESTS_REPORT_PATH="rspec/skipped_tests_${report_name}.txt"
if [[ -d "ee/" ]]; then
export KNAPSACK_GENERATE_REPORT="true"
@@ -408,7 +408,7 @@ function generate_flaky_tests_reports() {
mkdir -p ${rspec_flaky_folder_path}
- find ${rspec_flaky_folder_path} -type f -name 'skipped_flaky_tests_*_report.txt' -exec cat {} + >> "${SKIPPED_FLAKY_TESTS_REPORT_PATH}"
+ find ${rspec_flaky_folder_path} -type f -name 'skipped_tests_*.txt' -exec cat {} + >> "${SKIPPED_TESTS_REPORT_PATH}"
find ${rspec_flaky_folder_path} -type f -name 'retried_tests_*_report.txt' -exec cat {} + >> "${RETRIED_TESTS_REPORT_PATH}"
cleanup_individual_job_reports
diff --git a/spec/support/fast_quarantine.rb b/spec/support/fast_quarantine.rb
index dd805f6ec285356c3acc4458cae0b29319f52820..18d4df887a3fa5d50031b9f4fde81de7fb70b976 100644
--- a/spec/support/fast_quarantine.rb
+++ b/spec/support/fast_quarantine.rb
@@ -1,50 +1,36 @@
# frozen_string_literal: true
-return unless ENV['CI']
return if ENV['FAST_QUARANTINE'] == "false"
return if ENV['CI_MERGE_REQUEST_LABELS'].to_s.include?('pipeline:run-flaky-tests')
-require_relative '../../tooling/rspec_flaky/config'
+require_relative '../../tooling/lib/tooling/fast_quarantine'
-# rubocop:disable Style/GlobalVars
RSpec.configure do |config|
- $fast_quarantined_entity_identifiers = begin
- raise "#{ENV['RSPEC_FAST_QUARANTINE_PATH']} doesn't exist" unless File.exist?(ENV['RSPEC_FAST_QUARANTINE_PATH'])
-
- quarantined_entity_identifiers = File.read(ENV['RSPEC_FAST_QUARANTINE_PATH']).lines
- quarantined_entity_identifiers.compact!
- quarantined_entity_identifiers.map! do |quarantined_entity_identifier|
- quarantined_entity_identifier.delete_prefix('./').strip
- end
- rescue => e # rubocop:disable Style/RescueStandardError
- puts e
- []
- end
- $skipped_tests = []
+ fast_quarantine_local_path = ENV.fetch('RSPEC_FAST_QUARANTINE_LOCAL_PATH', 'rspec/fast_quarantine-gitlab.txt')
+ fast_quarantine_path = ENV.fetch(
+ 'RSPEC_FAST_QUARANTINE_PATH',
+ File.expand_path("../../#{fast_quarantine_local_path}", __dir__)
+ )
+ fast_quarantine = Tooling::FastQuarantine.new(fast_quarantine_path: fast_quarantine_path)
+ skipped_examples = []
config.around do |example|
- fast_quarantined_entity_identifier = $fast_quarantined_entity_identifiers.find do |quarantined_entity_identifier|
- case quarantined_entity_identifier
- when /^.+_spec\.rb\[[\d:]+\]$/ # example id, e.g. spec/tasks/gitlab/usage_data_rake_spec.rb[1:5:2:1]
- example.id == "./#{quarantined_entity_identifier}"
- else # whole file, e.g. ee/spec/features/boards/swimlanes/epics_swimlanes_sidebar_spec.rb
- example.metadata[:rerun_file_path] == "./#{quarantined_entity_identifier}"
- end
- end
-
- if fast_quarantined_entity_identifier
- puts "Skipping #{example.id} '#{example.full_description}' because it's been fast-quarantined with '#{fast_quarantined_entity_identifier}'."
- $skipped_tests << example.id
+ if fast_quarantine.skip_example?(example)
+ skipped_examples << example.id
+ skip "Skipping #{example.id} because it's been fast-quarantined."
else
example.run
end
end
config.after(:suite) do
- next unless RspecFlaky::Config.skipped_flaky_tests_report_path
- next if $skipped_tests.empty?
+ next if skipped_examples.empty?
+
+ skipped_tests_report_path = ENV.fetch(
+ 'SKIPPED_TESTS_REPORT_PATH',
+ File.expand_path("../../rspec/flaky/skipped_tests.txt", __dir__)
+ )
- File.write(RspecFlaky::Config.skipped_flaky_tests_report_path, "#{ENV['CI_JOB_URL']}\n#{$skipped_tests.join("\n")}\n\n")
+ File.write(skipped_tests_report_path, "#{ENV.fetch('CI_JOB_URL', 'local-run')}\n#{skipped_examples.join("\n")}\n\n")
end
end
-# rubocop:enable Style/GlobalVars
diff --git a/spec/tooling/lib/tooling/fast_quarantine_spec.rb b/spec/tooling/lib/tooling/fast_quarantine_spec.rb
new file mode 100644
index 0000000000000000000000000000000000000000..bb60a335ce239643ec3662cfa1d797589b63ef7b
--- /dev/null
+++ b/spec/tooling/lib/tooling/fast_quarantine_spec.rb
@@ -0,0 +1,193 @@
+# frozen_string_literal: true
+
+require_relative '../../../../tooling/lib/tooling/fast_quarantine'
+require 'tempfile'
+
+RSpec.describe Tooling::FastQuarantine, feature_category: :tooling do
+ attr_accessor :fast_quarantine_file
+
+ around do |example|
+ self.fast_quarantine_file = Tempfile.new('fast_quarantine_file')
+
+ # See https://ruby-doc.org/stdlib-1.9.3/libdoc/tempfile/rdoc/
+ # Tempfile.html#class-Tempfile-label-Explicit+close
+ begin
+ example.run
+ ensure
+ fast_quarantine_file.close
+ fast_quarantine_file.unlink
+ end
+ end
+
+ let(:fast_quarantine_path) { fast_quarantine_file.path }
+ let(:fast_quarantine_file_content) { '' }
+ let(:instance) do
+ described_class.new(fast_quarantine_path: fast_quarantine_path)
+ end
+
+ before do
+ File.write(fast_quarantine_path, fast_quarantine_file_content)
+ end
+
+ describe '#initialize' do
+ context 'when fast_quarantine_path does not exist' do
+ it 'prints a warning' do
+ allow(File).to receive(:exist?).and_return(false)
+
+ expect { instance }.to output("#{fast_quarantine_path} doesn't exist!\n").to_stderr
+ end
+ end
+
+ context 'when fast_quarantine_path exists' do
+ it 'does not raise an error' do
+ expect { instance }.not_to raise_error
+ end
+ end
+ end
+
+ describe '#identifiers' do
+ before do
+ allow(File).to receive(:read).and_call_original
+ end
+
+ context 'when the fast quarantine file is empty' do
+ let(:fast_quarantine_file_content) { '' }
+
+ it 'returns []' do
+ expect(instance.identifiers).to eq([])
+ end
+ end
+
+ context 'when the fast quarantine file is not empty' do
+ let(:fast_quarantine_file_content) { "./spec/foo_spec.rb\nspec/foo_spec.rb:42\n./spec/baz_spec.rb[1:2:3]" }
+
+ it 'returns parsed and sanitized lines' do
+ expect(instance.identifiers).to eq(%w[
+ spec/foo_spec.rb
+ spec/foo_spec.rb:42
+ spec/baz_spec.rb[1:2:3]
+ ])
+ end
+
+ context 'when reading the file raises an error' do
+ before do
+ allow(File).to receive(:read).with(fast_quarantine_path).and_raise('')
+ end
+
+ it 'returns []' do
+ expect(instance.identifiers).to eq([])
+ end
+ end
+
+ describe 'memoization' do
+ it 'memoizes the identifiers list' do
+ expect(File).to receive(:read).with(fast_quarantine_path).once.and_call_original
+
+ instance.identifiers
+
+ # calling #identifiers again doesn't call File.read
+ instance.identifiers
+ end
+ end
+ end
+ end
+
+ describe '#skip_example?' do
+ let(:fast_quarantine_file_content) { "./spec/foo_spec.rb\nspec/bar_spec.rb:42\n./spec/baz_spec.rb[1:2:3]" }
+ let(:example_id) { './spec/foo_spec.rb[1:2:3]' }
+ let(:example_metadata) { {} }
+ let(:example) { instance_double(RSpec::Core::Example, id: example_id, metadata: example_metadata) }
+
+ describe 'skipping example by id' do
+ let(:example_id) { './spec/baz_spec.rb[1:2:3]' }
+
+ it 'skips example by id' do
+ expect(instance.skip_example?(example)).to be_truthy
+ end
+ end
+
+ describe 'skipping example by line' do
+ context 'when example location matches' do
+ let(:example_metadata) do
+ { location: './spec/bar_spec.rb:42' }
+ end
+
+ it 'skips example by line' do
+ expect(instance.skip_example?(example)).to be_truthy
+ end
+ end
+
+ context 'when example group location matches' do
+ let(:example_metadata) do
+ {
+ example_group: { location: './spec/bar_spec.rb:42' }
+ }
+ end
+
+ it 'skips example by line' do
+ expect(instance.skip_example?(example)).to be_truthy
+ end
+ end
+
+ context 'when nested parent example group location matches' do
+ let(:example_metadata) do
+ {
+ example_group: {
+ parent_example_group: {
+ parent_example_group: {
+ parent_example_group: { location: './spec/bar_spec.rb:42' }
+ }
+ }
+ }
+ }
+ end
+
+ it 'skips example by line' do
+ expect(instance.skip_example?(example)).to be_truthy
+ end
+ end
+ end
+
+ describe 'skipping example by file' do
+ context 'when example file_path matches' do
+ let(:example_metadata) do
+ { file_path: './spec/foo_spec.rb' }
+ end
+
+ it 'skips example by file' do
+ expect(instance.skip_example?(example)).to be_truthy
+ end
+ end
+
+ context 'when example group file_path matches' do
+ let(:example_metadata) do
+ {
+ example_group: { file_path: './spec/foo_spec.rb' }
+ }
+ end
+
+ it 'skips example by file' do
+ expect(instance.skip_example?(example)).to be_truthy
+ end
+ end
+
+ context 'when nested parent example group file_path matches' do
+ let(:example_metadata) do
+ {
+ example_group: {
+ parent_example_group: {
+ parent_example_group: {
+ parent_example_group: { file_path: './spec/foo_spec.rb' }
+ }
+ }
+ }
+ }
+ end
+
+ it 'skips example by file' do
+ expect(instance.skip_example?(example)).to be_truthy
+ end
+ end
+ end
+ end
+end
diff --git a/spec/tooling/rspec_flaky/config_spec.rb b/spec/tooling/rspec_flaky/config_spec.rb
index c95e5475d66a81297f226eb61b297709d3543a6c..63f42d7c6cc62e3e8dc260a860aa9c0cbc1df8a9 100644
--- a/spec/tooling/rspec_flaky/config_spec.rb
+++ b/spec/tooling/rspec_flaky/config_spec.rb
@@ -14,7 +14,6 @@
stub_env('FLAKY_RSPEC_SUITE_REPORT_PATH', nil)
stub_env('FLAKY_RSPEC_REPORT_PATH', nil)
stub_env('NEW_FLAKY_RSPEC_REPORT_PATH', nil)
- stub_env('SKIPPED_FLAKY_TESTS_REPORT_PATH', nil)
# Ensure the behavior is the same locally and on CI (where Rails is defined since we run this test as part of the whole suite), i.e. Rails isn't defined
allow(described_class).to receive(:rails_path).and_wrap_original do |method, path|
path
@@ -104,22 +103,4 @@
end
end
end
-
- describe '.skipped_flaky_tests_report_path' do
- context "when ENV['SKIPPED_FLAKY_TESTS_REPORT_PATH'] is not set" do
- it 'returns the default path' do
- expect(described_class.skipped_flaky_tests_report_path).to eq('rspec/flaky/skipped_flaky_tests_report.txt')
- end
- end
-
- context "when ENV['SKIPPED_FLAKY_TESTS_REPORT_PATH'] is set" do
- before do
- stub_env('SKIPPED_FLAKY_TESTS_REPORT_PATH', 'foo/skipped_flaky_tests_report.txt')
- end
-
- it 'returns the value of the env variable' do
- expect(described_class.skipped_flaky_tests_report_path).to eq('foo/skipped_flaky_tests_report.txt')
- end
- end
- end
end
diff --git a/tooling/lib/tooling/fast_quarantine.rb b/tooling/lib/tooling/fast_quarantine.rb
new file mode 100644
index 0000000000000000000000000000000000000000..a0dc8bc460ba0d4ca5c80c5d5a6813e821e02265
--- /dev/null
+++ b/tooling/lib/tooling/fast_quarantine.rb
@@ -0,0 +1,57 @@
+# frozen_string_literal: true
+
+module Tooling
+ class FastQuarantine
+ def initialize(fast_quarantine_path:)
+ warn "#{fast_quarantine_path} doesn't exist!" unless File.exist?(fast_quarantine_path.to_s)
+
+ @fast_quarantine_path = fast_quarantine_path
+ end
+
+ def identifiers
+ @identifiers ||= begin
+ quarantined_entity_identifiers = File.read(fast_quarantine_path).lines
+ quarantined_entity_identifiers.compact!
+ quarantined_entity_identifiers.map! do |quarantined_entity_identifier|
+ quarantined_entity_identifier.delete_prefix('./').strip
+ end
+ rescue => e # rubocop:disable Style/RescueStandardError
+ $stdout.puts e
+ []
+ end
+ end
+
+ def skip_example?(example)
+ identifiers.find do |quarantined_entity_identifier|
+ case quarantined_entity_identifier
+ when /^.+_spec\.rb\[[\d:]+\]$/ # example id, e.g. spec/tasks/gitlab/usage_data_rake_spec.rb[1:5:2:1]
+ example.id == "./#{quarantined_entity_identifier}"
+ when /^.+_spec\.rb:\d+$/ # file + line, e.g. spec/tasks/gitlab/usage_data_rake_spec.rb:42
+ fetch_metadata_from_ancestors(example, :location)
+ .any?("./#{quarantined_entity_identifier}")
+ when /^.+_spec\.rb$/ # whole file, e.g. ee/spec/features/boards/swimlanes/epics_swimlanes_sidebar_spec.rb
+ fetch_metadata_from_ancestors(example, :file_path)
+ .any?("./#{quarantined_entity_identifier}")
+ end
+ end
+ end
+
+ private
+
+ attr_reader :fast_quarantine_path
+
+ def fetch_metadata_from_ancestors(example, attribute)
+ metadata = [example.metadata[attribute]]
+ example_group = example.metadata[:example_group]
+
+ loop do
+ break if example_group.nil?
+
+ metadata << example_group[attribute]
+ example_group = example_group[:parent_example_group]
+ end
+
+ metadata
+ end
+ end
+end
diff --git a/tooling/rspec_flaky/config.rb b/tooling/rspec_flaky/config.rb
index 36e356715876995f4a677cbe11b6e3a44a4b8f46..0e36e985aad59ebb9b6907642b6092c8134c7ede 100644
--- a/tooling/rspec_flaky/config.rb
+++ b/tooling/rspec_flaky/config.rb
@@ -18,10 +18,6 @@ def self.new_flaky_examples_report_path
ENV['NEW_FLAKY_RSPEC_REPORT_PATH'] || rails_path("rspec/flaky/new-report.json")
end
- def self.skipped_flaky_tests_report_path
- ENV['SKIPPED_FLAKY_TESTS_REPORT_PATH'] || rails_path("rspec/flaky/skipped_flaky_tests_report.txt")
- end
-
def self.rails_path(path)
return path unless defined?(Rails)