From 55f1d104303740be6513e5e07e5422218c573129 Mon Sep 17 00:00:00 2001 From: Lin Jen-Shin Date: Fri, 23 Feb 2024 19:33:49 +0800 Subject: [PATCH 1/2] Only use rspec-retry for features (system) specs --- spec/spec_helper.rb | 18 ++++-------------- 1 file changed, 4 insertions(+), 14 deletions(-) diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 5b0b0087d7dca6..b48e366fee3aa6 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -213,22 +213,12 @@ include StubMember if ENV['CI'] || ENV['RETRIES'] - # This includes the first try, i.e. tests will be run 2 times before failing. - config.default_retry_count = ENV.fetch('RETRIES', 1).to_i + 1 - # Gradually stop using rspec-retry # See https://gitlab.com/gitlab-org/gitlab/-/issues/438388 - %i[ - bin channels commands components config contracts controller db - dependencies dot_gitlab_ci experiments finders - graphql haml_lint helpers initializers keeps lib mailers metrics_server - migrations models policies presenters rack_servers requests - routing rubocop scripts serializers services sidekiq sidekiq_cluster - spam support_specs tasks tooling uploaders validators views workers - ].each do |type| - config.prepend_before(:each, type: type) do |example| - example.metadata[:retry] = 1 - end + config.default_retry_count = 1 + config.prepend_before(:each, type: :feature) do |example| + # This includes the first try, i.e. tests will be run 2 times before failing. + example.metadata[:retry] = ENV.fetch('RETRIES', 1).to_i + 1 end config.exceptions_to_hard_fail = [DeprecationToolkitEnv::DeprecationBehaviors::SelectiveRaise::RaiseDisallowedDeprecation] -- GitLab From e4b61bdc3f31f421410b4912521f9d1f31ed2190 Mon Sep 17 00:00:00 2001 From: Lin Jen-Shin Date: Wed, 6 Mar 2024 01:02:48 +0800 Subject: [PATCH 2/2] Quarantine tests which cannot pass without rspec-retry --- .../api/graphql/product_analytics/dashboards_spec.rb | 2 +- spec/helpers/users_helper_spec.rb | 4 ++-- spec/initializers/lograge_spec.rb | 4 ++-- .../user_max_access_level_in_projects_preloader_spec.rb | 8 ++++---- .../mutations/merge_requests/set_assignees_spec.rb | 9 +++++---- spec/requests/search_controller_spec.rb | 4 ++-- 6 files changed, 16 insertions(+), 15 deletions(-) diff --git a/ee/spec/requests/api/graphql/product_analytics/dashboards_spec.rb b/ee/spec/requests/api/graphql/product_analytics/dashboards_spec.rb index 07f9f17f01cb1b..42bba577d669a0 100644 --- a/ee/spec/requests/api/graphql/product_analytics/dashboards_spec.rb +++ b/ee/spec/requests/api/graphql/product_analytics/dashboards_spec.rb @@ -77,7 +77,7 @@ resource_parent.add_developer(user) end - it 'returns all dashboards' do + it 'returns all dashboards', quarantine: 'https://gitlab.com/gitlab-org/gitlab/-/issues/446187' do post_graphql(query, current_user: user) expect(graphql_data_at(resource_parent_type, :customizable_dashboards, :nodes).pluck('title')) diff --git a/spec/helpers/users_helper_spec.rb b/spec/helpers/users_helper_spec.rb index a3d77d76591773..3ae63ffc2dea53 100644 --- a/spec/helpers/users_helper_spec.rb +++ b/spec/helpers/users_helper_spec.rb @@ -613,7 +613,7 @@ def stub_profile_permission_allowed(allowed, current_user = nil) allow(helper).to receive(:current_user).and_return(nil) end - it 'executes no queries' do + it 'executes no queries', quarantine: 'https://gitlab.com/gitlab-org/gitlab/-/issues/444713' do sample = ActiveRecord::QueryRecorder.new do helper.load_max_project_member_accesses(projects) end @@ -627,7 +627,7 @@ def stub_profile_permission_allowed(allowed, current_user = nil) allow(helper).to receive(:current_user).and_return(user) end - it 'preloads ProjectPolicy#lookup_access_level! and UsersHelper#max_member_project_member_access for current_user in two queries', :aggregate_failures do + it 'preloads ProjectPolicy#lookup_access_level! and UsersHelper#max_member_project_member_access for current_user in two queries', :aggregate_failures, quarantine: 'https://gitlab.com/gitlab-org/gitlab/-/issues/446111' do preload_queries = ActiveRecord::QueryRecorder.new do helper.load_max_project_member_accesses(projects) end diff --git a/spec/initializers/lograge_spec.rb b/spec/initializers/lograge_spec.rb index c423c144dc2cbf..2c24428d542530 100644 --- a/spec/initializers/lograge_spec.rb +++ b/spec/initializers/lograge_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe 'lograge', type: :request do +RSpec.describe 'lograge', type: :request, feature_category: :logging do let(:headers) { { 'X-Request-ID' => 'new-correlation-id' } } let(:large_params) do @@ -150,7 +150,7 @@ event.payload[:exception_object] = exception end - it 'adds exception data to log' do + it 'adds exception data to log', quarantine: 'https://gitlab.com/gitlab-org/gitlab/-/issues/446202' do subscriber.process_action(event) expect(log_data['exception.class']).to eq('RuntimeError') diff --git a/spec/models/preloaders/user_max_access_level_in_projects_preloader_spec.rb b/spec/models/preloaders/user_max_access_level_in_projects_preloader_spec.rb index 2070d6d167dd38..97c0771e42462d 100644 --- a/spec/models/preloaders/user_max_access_level_in_projects_preloader_spec.rb +++ b/spec/models/preloaders/user_max_access_level_in_projects_preloader_spec.rb @@ -19,7 +19,7 @@ end context 'without preloader' do - it 'runs some queries' do + it 'runs some queries', quarantine: 'https://gitlab.com/gitlab-org/gitlab/-/issues/444712' do # we have an existing N+1, one for each project for which user is not a member # in this spec, project_3, project_4, project_5 # https://gitlab.com/gitlab-org/gitlab/-/issues/362890 @@ -35,14 +35,14 @@ described_class.new(projects_arg, user).execute end - it 'avoids N+1 queries' do + it 'avoids N+1 queries', quarantine: 'https://gitlab.com/gitlab-org/gitlab/-/issues/446114' do expect { query }.not_to make_queries end context 'when projects is an array of IDs' do let(:projects_arg) { projects.map(&:id) } - it 'avoids N+1 queries' do + it 'avoids N+1 queries', quarantine: 'https://gitlab.com/gitlab-org/gitlab/-/issues/446114' do expect { query }.not_to make_queries end end @@ -53,7 +53,7 @@ Project.where(id: ProjectAuthorization.where(project_id: projects).select(:project_id)) end - it 'avoids N+1 queries' do + it 'avoids N+1 queries', quarantine: 'https://gitlab.com/gitlab-org/gitlab/-/issues/446114' do expect { query }.not_to make_queries end end diff --git a/spec/requests/api/graphql/mutations/merge_requests/set_assignees_spec.rb b/spec/requests/api/graphql/mutations/merge_requests/set_assignees_spec.rb index bbac48bccb28d8..c0ce8572368008 100644 --- a/spec/requests/api/graphql/mutations/merge_requests/set_assignees_spec.rb +++ b/spec/requests/api/graphql/mutations/merge_requests/set_assignees_spec.rb @@ -89,7 +89,7 @@ def run_mutation! merge_request.save! end - it 'replaces the assignee' do + it 'replaces the assignee', quarantine: 'https://gitlab.com/gitlab-org/gitlab/-/issues/444646' do run_mutation! expect(response).to have_gitlab_http_status(:success) @@ -116,7 +116,7 @@ def run_mutation! merge_request.save! end - it 'removes assignee' do + it 'removes assignee', quarantine: 'https://gitlab.com/gitlab-org/gitlab/-/issues/446115' do run_mutation! expect(response).to have_gitlab_http_status(:success) @@ -138,7 +138,7 @@ def run_mutation! merge_request.save! end - it 'does not replace the assignee in CE' do + it 'does not replace the assignee in CE', quarantine: 'https://gitlab.com/gitlab-org/gitlab/-/issues/446115' do run_mutation! expect(response).to have_gitlab_http_status(:success) @@ -157,7 +157,8 @@ def run_mutation! merge_request.save! end - it 'removes the users in the list, while adding none' do + it 'removes the users in the list, while adding none', + quarantine: 'https://gitlab.com/gitlab-org/gitlab/-/issues/446115' do run_mutation! expect(response).to have_gitlab_http_status(:success) diff --git a/spec/requests/search_controller_spec.rb b/spec/requests/search_controller_spec.rb index eaf11653256af9..0112cf52c655a0 100644 --- a/spec/requests/search_controller_spec.rb +++ b/spec/requests/search_controller_spec.rb @@ -17,7 +17,7 @@ def send_search_request(params) end shared_examples 'an efficient database result' do - it 'avoids N+1 database queries' do + it 'avoids N+1 database queries', quarantine: 'https://gitlab.com/gitlab-org/gitlab/-/issues/446130' do create(object, *creation_traits, creation_args) control = ActiveRecord::QueryRecorder.new(skip_cached: false) { send_search_request(params) } @@ -121,7 +121,7 @@ def send_search_request(params) let(:params_for_one) { { search: 'test', project_id: project.id, scope: 'commits', per_page: 1 } } let(:params_for_many) { { search: 'test', project_id: project.id, scope: 'commits', per_page: 5 } } - it 'avoids N+1 database queries' do + it 'avoids N+1 database queries', quarantine: 'https://gitlab.com/gitlab-org/gitlab/-/issues/444710' do control = ActiveRecord::QueryRecorder.new { send_search_request(params_for_one) } expect(response.body).to include('search-results') # Confirm search results to prevent false positives -- GitLab