diff --git a/qa/Gemfile b/qa/Gemfile index f5fc06c2d76f4e7943c5cd7d7638c11be6c30f33..9b26b6b21618a924f28bc7f63dce633a9a4a4892 100644 --- a/qa/Gemfile +++ b/qa/Gemfile @@ -14,7 +14,6 @@ gem 'rspec', '~> 3.13' gem 'selenium-webdriver', '= 4.19.0' gem 'airborne', '~> 0.3.7', require: false # airborne is messing with rspec sandboxed mode so not requiring by default gem 'rest-client', '~> 2.1.0' -gem 'rspec-retry', '~> 0.6.2', require: 'rspec/retry' gem 'rspec_junit_formatter', '~> 0.6.0' gem 'faker', '~> 3.3', '>= 3.3.1' gem 'knapsack', '~> 4.0' diff --git a/qa/Gemfile.lock b/qa/Gemfile.lock index 4e06b0c15140956dafce272d9af5bbf4fca3dcb1..9727f5fdfd194211b251b270ed360929ff1d47f4 100644 --- a/qa/Gemfile.lock +++ b/qa/Gemfile.lock @@ -289,8 +289,6 @@ GEM rspec-parameterized-table_syntax (1.0.0) binding_of_caller rspec-parameterized-core (< 2) - rspec-retry (0.6.2) - rspec-core (> 3.3) rspec-support (3.13.0) rspec_junit_formatter (0.6.0) rspec-core (>= 2, < 4, != 2.12.0) @@ -376,7 +374,6 @@ DEPENDENCIES rotp (~> 6.3.0) rspec (~> 3.13) rspec-parameterized (~> 1.0.0) - rspec-retry (~> 0.6.2) rspec_junit_formatter (~> 0.6.0) ruby-debug-ide (~> 0.7.3) selenium-webdriver (= 4.19.0) diff --git a/qa/qa/runtime/env.rb b/qa/qa/runtime/env.rb index f541a534c776a89415cd445316c4961dc4ffcc4b..39f77f3945b8e0cda9a5ea06922037f61aae6c0c 100644 --- a/qa/qa/runtime/env.rb +++ b/qa/qa/runtime/env.rb @@ -521,10 +521,6 @@ def runtime_scenario_attributes ENV['QA_RUNTIME_SCENARIO_ATTRIBUTES'] end - def disable_rspec_retry? - enabled?(ENV['QA_DISABLE_RSPEC_RETRY'], default: false) - end - def simulate_slow_connection? enabled?(ENV['QA_SIMULATE_SLOW_CONNECTION'], default: false) end diff --git a/qa/qa/specs/spec_helper.rb b/qa/qa/specs/spec_helper.rb index 96c5dbfaefebd1c442de8f58d0c227b841dfe1c2..2a2754133e677b127fe3a27d0588df5df899d027 100644 --- a/qa/qa/specs/spec_helper.rb +++ b/qa/qa/specs/spec_helper.rb @@ -121,26 +121,6 @@ # This option allows to use shorthand aliases for adding :focus metadata - fit, fdescribe and fcontext config.filter_run_when_matching :focus - - if ENV['CI'] && !QA::Runtime::Env.disable_rspec_retry? - # show retry status in spec process - config.verbose_retry = true - - # show exception that triggers a retry if verbose_retry is set to true - config.display_try_failure_messages = true - - non_quarantine_retries = QA::Runtime::Env.ci_project_name.match?(/staging|canary|production/) ? 3 : 2 - config.around do |example| - quarantine = example.metadata[:quarantine] - different_quarantine_context = QA::Specs::Helpers::Quarantine.quarantined_different_context?(quarantine) - focused_quarantine = QA::Specs::Helpers::Quarantine.filters.key?(:quarantine) - - # Do not disable retry when spec is quarantined but on different environment - next example.run_with_retry(retry: non_quarantine_retries) if different_quarantine_context && !focused_quarantine - - example.run_with_retry(retry: quarantine ? 1 : non_quarantine_retries) - end - end end Dir[::File.join(__dir__, "features/shared_examples/**/*.rb")].each { |f| require f } diff --git a/qa/qa/support/formatters/test_metrics_formatter.rb b/qa/qa/support/formatters/test_metrics_formatter.rb index 13b075042c5fbf3eaf5d30a94e107ef341ecffbd..7f6a0a987f2772374fb55de1981c2dd5eefce81b 100644 --- a/qa/qa/support/formatters/test_metrics_formatter.rb +++ b/qa/qa/support/formatters/test_metrics_formatter.rb @@ -33,8 +33,9 @@ def stop(notification) :ci_job_url, :ci_job_name, :rspec_retried?, - :disable_rspec_retry?, - to: "QA::Runtime::Env" + to: QA::Runtime::Env + + delegate :retry_failed_specs?, to: ::Gitlab::QA::Runtime::Env # Save execution data for the run # @@ -80,7 +81,9 @@ def push_fabrication_metrics def save_test_metrics return log(:debug, "Saving test metrics json not enabled, skipping") unless save_metrics_json? - file = "tmp/test-metrics-#{env('CI_JOB_NAME_SLUG') || 'local'}-retry-#{rspec_retried?}.json" + file = "tmp/test-metrics-#{env('CI_JOB_NAME_SLUG') || 'local'}" \ + "#{retry_failed_specs? ? "-retry-#{rspec_retried?}" : ''}.json" + File.write(file, execution_data.to_json) && log(:debug, "Saved test metrics to #{file}") rescue StandardError => e log(:error, "Failed to save test execution metrics, error: #{e}") @@ -93,7 +96,7 @@ def save_test_metrics # @return [Hash] def test_stats(example) # do not save failures from initial non retry run, as they will be retried and become flaky or failed - return if disable_rspec_retry? && (!rspec_retried? && example.execution_result.status == :failed) + return if retry_failed_specs? && (!rspec_retried? && example.execution_result.status == :failed) { name: 'test-stats', @@ -228,18 +231,8 @@ def quarantined(example) def status(example) rspec_status = example.execution_result.status return rspec_status if [:pending, :failed].include?(rspec_status) - # if rspec-retry gem is disabled, check for rspec retry process - return rspec_retried? && rspec_status == :passed ? :flaky : :passed if disable_rspec_retry? - - retry_attempts(example.metadata) > 0 ? :flaky : :passed - end - # Retry attempts - # - # @param [Hash] metadata - # @return [Integer] - def retry_attempts(metadata) - metadata[:retry_attempts] || 0 + rspec_retried? && rspec_status == :passed ? :flaky : :passed end # Additional custom metrics tags diff --git a/qa/spec/support/formatters/test_metrics_formatter_spec.rb b/qa/spec/support/formatters/test_metrics_formatter_spec.rb index 7739c8adae35dadbf8b7afa72d6411c0be20cfa3..aa6dc66977d93cc134a868984f47f95badc9fc7f 100644 --- a/qa/spec/support/formatters/test_metrics_formatter_spec.rb +++ b/qa/spec/support/formatters/test_metrics_formatter_spec.rb @@ -31,6 +31,7 @@ let(:fabrication_resources) { {} } let(:testcase) { 'https://gitlab.com/gitlab-org/gitlab/-/quality/test_cases/1234' } let(:status) { :passed } + let(:retry_failed_specs) { false } let(:influx_client_args) do { @@ -96,9 +97,10 @@ def run_spec(passed: true, &spec) end before do + allow(::Gitlab::QA::Runtime::Env).to receive(:retry_failed_specs?).and_return(retry_failed_specs) allow(InfluxDB2::Client).to receive(:new).with(url, token, **influx_client_args) { influx_client } allow(QA::Tools::TestResourceDataProcessor).to receive(:resources) { fabrication_resources } - allow_any_instance_of(RSpec::Core::Example::ExecutionResult).to receive(:run_time).and_return(0) # rubocop:disable RSpec/AnyInstanceOf + allow_any_instance_of(RSpec::Core::Example::ExecutionResult).to receive(:run_time).and_return(0) # rubocop:disable RSpec/AnyInstanceOf -- simplifies mocking runtime end context 'without influxdb variables configured' do @@ -260,9 +262,7 @@ def run_spec(passed: true, &spec) end context 'with retry in separate process' do - before do - stub_env('QA_DISABLE_RSPEC_RETRY', 'true') - end + let(:retry_failed_specs) { true } context 'with initial run' do it 'skips failed spec' do @@ -436,10 +436,25 @@ def run_spec(passed: true, &spec) allow(File).to receive(:write) end - it 'saves test metrics as json files' do - run_spec + context 'without retry enabled' do + let(:file) { 'tmp/test-metrics-test-job.json' } + + it 'saves test metrics as json files' do + run_spec + + expect(File).to have_received(:write).with(file, [data].to_json) + end + end + + context 'with retry enabled' do + let(:retry_failed_specs) { true } + let(:file) { 'tmp/test-metrics-test-job-retry-false.json' } - expect(File).to have_received(:write).with("tmp/test-metrics-test-job-retry-false.json", [data].to_json) + it 'saves test metrics as json files' do + run_spec + + expect(File).to have_received(:write).with(file, [data].to_json) + end end end end