diff options
49 files changed, 461 insertions, 365 deletions
diff --git a/.rubocop_todo/capybara/testid_finders.yml b/.rubocop_todo/capybara/testid_finders.yml index d34b645c5d8..0238bfbc86a 100644 --- a/.rubocop_todo/capybara/testid_finders.yml +++ b/.rubocop_todo/capybara/testid_finders.yml @@ -1,40 +1,6 @@ --- Capybara/TestidFinders: Exclude: - - 'ee/spec/features/protected_branches_spec.rb' - - 'ee/spec/features/registrations/combined_registration_spec.rb' - - 'ee/spec/features/registrations/identity_verification_spec.rb' - - 'ee/spec/features/remote_development/workspaces_dropdown_group_spec.rb' - - 'ee/spec/features/remote_development/workspaces_spec.rb' - - 'ee/spec/features/search/elastic/group_search_spec.rb' - - 'ee/spec/features/search/zoekt/search_spec.rb' - - 'ee/spec/features/tanuki_bot_chat_spec.rb' - - 'ee/spec/features/trials/saas/creation_with_multiple_existing_namespace_flow_spec.rb' - - 'ee/spec/features/trials/show_trial_banner_spec.rb' - - 'spec/features/admin/admin_deploy_keys_spec.rb' - - 'spec/features/admin/admin_dev_ops_reports_spec.rb' - - 'spec/features/admin/admin_groups_spec.rb' - - 'spec/features/admin/admin_projects_spec.rb' - - 'spec/features/admin/admin_runners_spec.rb' - - 'spec/features/admin/admin_settings_spec.rb' - - 'spec/features/admin/admin_uses_repository_checks_spec.rb' - - 'spec/features/admin/broadcast_messages_spec.rb' - - 'spec/features/admin/users/user_spec.rb' - - 'spec/features/admin/users/users_spec.rb' - - 'spec/features/alert_management/alert_details_spec.rb' - - 'spec/features/boards/board_filters_spec.rb' - - 'spec/features/boards/boards_spec.rb' - - 'spec/features/boards/issue_ordering_spec.rb' - - 'spec/features/boards/new_issue_spec.rb' - - 'spec/features/boards/sidebar_assignee_spec.rb' - - 'spec/features/broadcast_messages_spec.rb' - - 'spec/features/callouts/registration_enabled_spec.rb' - - 'spec/features/clusters/create_agent_spec.rb' - - 'spec/features/commits_spec.rb' - - 'spec/features/dashboard/group_spec.rb' - - 'spec/features/dashboard/issues_spec.rb' - - 'spec/features/dashboard/merge_requests_spec.rb' - - 'spec/features/dashboard/milestones_spec.rb' - 'spec/features/dashboard/projects_spec.rb' - 'spec/features/dashboard/todos/todos_spec.rb' - 'spec/features/groups/board_sidebar_spec.rb' diff --git a/app/helpers/sidebars_helper.rb b/app/helpers/sidebars_helper.rb index 33ca5ad584e..f983812ad22 100644 --- a/app/helpers/sidebars_helper.rb +++ b/app/helpers/sidebars_helper.rb @@ -358,7 +358,9 @@ module SidebarsHelper def context_switcher_links links = [ ({ title: s_('Navigation|Your work'), link: root_path, icon: 'work' } if current_user), - { title: s_('Navigation|Explore'), link: explore_root_path, icon: 'compass' } + { title: s_('Navigation|Explore'), link: explore_root_path, icon: 'compass' }, + ({ title: s_('Navigation|Profile'), link: profile_path, icon: 'profile' } if current_user), + ({ title: s_('Navigation|Preferences'), link: profile_preferences_path, icon: 'preferences' } if current_user) ] # Usually, using current_user.admin? is discouraged because it does not diff --git a/app/models/service_desk/custom_email_credential.rb b/app/models/service_desk/custom_email_credential.rb index 5986ac8a43f..82bda673491 100644 --- a/app/models/service_desk/custom_email_credential.rb +++ b/app/models/service_desk/custom_email_credential.rb @@ -2,6 +2,14 @@ module ServiceDesk class CustomEmailCredential < ApplicationRecord + # Used to explicitly set the SMTP AUTH method. + # If nil Net::SMTP will choose one of methods listed by the SMTP server. + enum smtp_authentication: { + plain: 0, + login: 1, + cram_md5: 2 + } + attr_encrypted :smtp_username, mode: :per_attribute_iv, algorithm: 'aes-256-gcm', @@ -44,7 +52,8 @@ module ServiceDesk password: smtp_password, address: smtp_address, domain: Mail::Address.new(service_desk_setting.custom_email).domain, - port: smtp_port || 587 + port: smtp_port || 587, + authentication: smtp_authentication } end diff --git a/app/workers/all_queues.yml b/app/workers/all_queues.yml index 716cd494810..965053cf5f8 100644 --- a/app/workers/all_queues.yml +++ b/app/workers/all_queues.yml @@ -2557,7 +2557,7 @@ :urgency: :low :resource_boundary: :unknown :weight: 1 - :idempotent: false + :idempotent: true :tags: [] - :name: bulk_imports_entity :worker_name: BulkImports::EntityWorker @@ -2611,7 +2611,7 @@ :urgency: :low :resource_boundary: :memory :weight: 1 - :idempotent: false + :idempotent: true :tags: [] - :name: bulk_imports_pipeline_batch :worker_name: BulkImports::PipelineBatchWorker @@ -2620,7 +2620,7 @@ :urgency: :low :resource_boundary: :memory :weight: 1 - :idempotent: false + :idempotent: true :tags: [] - :name: bulk_imports_relation_batch_export :worker_name: BulkImports::RelationBatchExportWorker diff --git a/app/workers/bulk_import_worker.rb b/app/workers/bulk_import_worker.rb index 5b9b46081cc..588e936e5e3 100644 --- a/app/workers/bulk_import_worker.rb +++ b/app/workers/bulk_import_worker.rb @@ -1,11 +1,12 @@ # frozen_string_literal: true -class BulkImportWorker # rubocop:disable Scalability/IdempotentWorker +class BulkImportWorker include ApplicationWorker data_consistency :always feature_category :importers sidekiq_options retry: false, dead: false + idempotent! def perform(bulk_import_id) bulk_import = BulkImport.find_by_id(bulk_import_id) diff --git a/app/workers/bulk_imports/pipeline_batch_worker.rb b/app/workers/bulk_imports/pipeline_batch_worker.rb index 142c85f3d6e..f5e2efcff97 100644 --- a/app/workers/bulk_imports/pipeline_batch_worker.rb +++ b/app/workers/bulk_imports/pipeline_batch_worker.rb @@ -1,21 +1,29 @@ # frozen_string_literal: true module BulkImports - class PipelineBatchWorker # rubocop:disable Scalability/IdempotentWorker + class PipelineBatchWorker include ApplicationWorker include ExclusiveLeaseGuard data_consistency :always # rubocop:disable SidekiqLoadBalancing/WorkerDataConsistency feature_category :importers - sidekiq_options retry: false, dead: false + sidekiq_options dead: false, retry: 3 worker_has_external_dependencies! worker_resource_boundary :memory + idempotent! + + sidekiq_retries_exhausted do |msg, exception| + new.perform_failure(msg['args'].first, exception) + end def perform(batch_id) @batch = ::BulkImports::BatchTracker.find(batch_id) + @tracker = @batch.tracker @pending_retry = false + return unless process_batch? + log_extra_metadata_on_done(:pipeline_class, @tracker.pipeline_name) try_obtain_lease { run } @@ -23,6 +31,13 @@ module BulkImports ::BulkImports::FinishBatchedPipelineWorker.perform_async(tracker.id) unless pending_retry end + def perform_failure(batch_id, exception) + @batch = ::BulkImports::BatchTracker.find(batch_id) + @tracker = @batch.tracker + + fail_batch(exception) + end + private attr_reader :batch, :tracker, :pending_retry @@ -36,8 +51,6 @@ module BulkImports rescue BulkImports::RetryPipelineError => e @pending_retry = true retry_batch(e) - rescue StandardError => e - fail_batch(e) end def fail_batch(exception) @@ -59,6 +72,8 @@ module BulkImports exception_message: exception.message, correlation_id_value: Labkit::Correlation::CorrelationId.current_or_new_id ) + + ::BulkImports::FinishBatchedPipelineWorker.perform_async(tracker.id) end def context @@ -84,5 +99,9 @@ module BulkImports self.class.perform_in(delay, batch.id) end + + def process_batch? + batch.created? || batch.started? + end end end diff --git a/app/workers/bulk_imports/pipeline_worker.rb b/app/workers/bulk_imports/pipeline_worker.rb index 2d54cb58f62..7573ce9c3ac 100644 --- a/app/workers/bulk_imports/pipeline_worker.rb +++ b/app/workers/bulk_imports/pipeline_worker.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true module BulkImports - class PipelineWorker # rubocop:disable Scalability/IdempotentWorker + class PipelineWorker include ApplicationWorker include ExclusiveLeaseGuard @@ -9,13 +9,18 @@ module BulkImports data_consistency :always feature_category :importers - sidekiq_options retry: false, dead: false + sidekiq_options dead: false, retry: 3 worker_has_external_dependencies! deduplicate :until_executing worker_resource_boundary :memory + idempotent! version 2 + sidekiq_retries_exhausted do |msg, exception| + new.perform_failure(msg['args'][0], msg['args'][2], exception) + end + # Keep _stage parameter for backwards compatibility. def perform(pipeline_tracker_id, _stage, entity_id) @entity = ::BulkImports::Entity.find(entity_id) @@ -24,20 +29,21 @@ module BulkImports log_extra_metadata_on_done(:pipeline_class, @pipeline_tracker.pipeline_name) try_obtain_lease do - if pipeline_tracker.enqueued? + if pipeline_tracker.enqueued? || pipeline_tracker.started? logger.info(log_attributes(message: 'Pipeline starting')) run - else - message = "Pipeline in #{pipeline_tracker.human_status_name} state instead of expected enqueued state" - - logger.error(log_attributes(message: message)) - - fail_tracker(StandardError.new(message)) unless pipeline_tracker.finished? || pipeline_tracker.skipped? end end end + def perform_failure(pipeline_tracker_id, entity_id, exception) + @entity = ::BulkImports::Entity.find(entity_id) + @pipeline_tracker = ::BulkImports::Tracker.find(pipeline_tracker_id) + + fail_tracker(exception) + end + private attr_reader :pipeline_tracker, :entity @@ -67,8 +73,6 @@ module BulkImports end rescue BulkImports::RetryPipelineError => e retry_tracker(e) - rescue StandardError => e - fail_tracker(e) end def source_version diff --git a/db/migrate/20231031141439_add_smtp_authentication_to_service_desk_custom_email_credentials.rb b/db/migrate/20231031141439_add_smtp_authentication_to_service_desk_custom_email_credentials.rb new file mode 100644 index 00000000000..e15e500af90 --- /dev/null +++ b/db/migrate/20231031141439_add_smtp_authentication_to_service_desk_custom_email_credentials.rb @@ -0,0 +1,10 @@ +# frozen_string_literal: true + +class AddSmtpAuthenticationToServiceDeskCustomEmailCredentials < Gitlab::Database::Migration[2.2] + milestone '16.6' + + def change + add_column :service_desk_custom_email_credentials, :smtp_authentication, :integer, + limit: 2, null: true, default: nil + end +end diff --git a/db/post_migrate/20231023164908_async_drop_index_users_on_accepted_term_id.rb b/db/post_migrate/20231023164908_async_drop_index_users_on_accepted_term_id.rb new file mode 100644 index 00000000000..a5688a9b196 --- /dev/null +++ b/db/post_migrate/20231023164908_async_drop_index_users_on_accepted_term_id.rb @@ -0,0 +1,19 @@ +# frozen_string_literal: true + +class AsyncDropIndexUsersOnAcceptedTermId < Gitlab::Database::Migration[2.2] + milestone '16.6' + disable_ddl_transaction! + + TABLE_NAME = 'users' + INDEX_NAME = 'index_users_on_accepted_term_id' + COLUMN = 'accepted_term_id' + + # TODO: Index to be destroyed synchronously in https://gitlab.com/gitlab-org/gitlab/-/merge_requests/135293 + def up + prepare_async_index_removal TABLE_NAME, COLUMN, name: INDEX_NAME + end + + def down + prepare_async_index_removal TABLE_NAME, COLUMN, name: INDEX_NAME + end +end diff --git a/db/post_migrate/20231027013210_remove_partial_index_deployments_for_legacy_successful_deployments.rb b/db/post_migrate/20231027013210_remove_partial_index_deployments_for_legacy_successful_deployments.rb new file mode 100644 index 00000000000..2bd52fdc10a --- /dev/null +++ b/db/post_migrate/20231027013210_remove_partial_index_deployments_for_legacy_successful_deployments.rb @@ -0,0 +1,20 @@ +# frozen_string_literal: true + +class RemovePartialIndexDeploymentsForLegacySuccessfulDeployments < Gitlab::Database::Migration[2.2] + INDEX_NAME = 'partial_index_deployments_for_legacy_successful_deployments' + + milestone '16.6' + + disable_ddl_transaction! + + def up + remove_concurrent_index_by_name :deployments, name: INDEX_NAME + end + + def down + # This is based on the following `CREATE INDEX` command in db/init_structure.sql: + # CREATE INDEX partial_index_deployments_for_legacy_successful_deployments ON deployments + # USING btree (id) WHERE ((finished_at IS NULL) AND (status = 2)); + add_concurrent_index :deployments, :id, name: INDEX_NAME, where: '((finished_at IS NULL) AND (status = 2))' + end +end diff --git a/db/schema_migrations/20231023164908 b/db/schema_migrations/20231023164908 new file mode 100644 index 00000000000..f94a5e457bf --- /dev/null +++ b/db/schema_migrations/20231023164908 @@ -0,0 +1 @@ +4d742e6f54307710370453fdd72313c0a0f6928bdf2e4812bc5c16ec1043dd3f
\ No newline at end of file diff --git a/db/schema_migrations/20231027013210 b/db/schema_migrations/20231027013210 new file mode 100644 index 00000000000..fdf26416bdc --- /dev/null +++ b/db/schema_migrations/20231027013210 @@ -0,0 +1 @@ +3ee898fd7593c7a300bdfc0dc6f041e2fb65f3600b85788521175449d250590f
\ No newline at end of file diff --git a/db/schema_migrations/20231031141439 b/db/schema_migrations/20231031141439 new file mode 100644 index 00000000000..bbdae989385 --- /dev/null +++ b/db/schema_migrations/20231031141439 @@ -0,0 +1 @@ +568e7a227911f23e4285e1bbcc9dd516ecbd2013501a2add13e99e98880effc8
\ No newline at end of file diff --git a/db/structure.sql b/db/structure.sql index 3381764ee2e..0da8e0b7ce1 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -23217,6 +23217,7 @@ CREATE TABLE service_desk_custom_email_credentials ( encrypted_smtp_username_iv bytea, encrypted_smtp_password bytea, encrypted_smtp_password_iv bytea, + smtp_authentication smallint, CONSTRAINT check_6dd11e956a CHECK ((char_length(smtp_address) <= 255)) ); @@ -35020,8 +35021,6 @@ CREATE UNIQUE INDEX partial_index_bulk_import_exports_on_project_id_and_relation CREATE INDEX partial_index_ci_builds_on_scheduled_at_with_scheduled_jobs ON ci_builds USING btree (scheduled_at) WHERE ((scheduled_at IS NOT NULL) AND ((type)::text = 'Ci::Build'::text) AND ((status)::text = 'scheduled'::text)); -CREATE INDEX partial_index_deployments_for_legacy_successful_deployments ON deployments USING btree (id) WHERE ((finished_at IS NULL) AND (status = 2)); - CREATE INDEX partial_index_slack_integrations_with_bot_user_id ON slack_integrations USING btree (id) WHERE (bot_user_id IS NOT NULL); CREATE UNIQUE INDEX partial_index_sop_configs_on_namespace_id ON security_orchestration_policy_configurations USING btree (namespace_id) WHERE (namespace_id IS NOT NULL); diff --git a/doc/development/database/loose_foreign_keys.md b/doc/development/database/loose_foreign_keys.md index fd380bee385..08d618a26ae 100644 --- a/doc/development/database/loose_foreign_keys.md +++ b/doc/development/database/loose_foreign_keys.md @@ -251,8 +251,12 @@ When the loose foreign key definition is no longer needed (parent table is remov we need to remove the definition from the YAML file and ensure that we don't leave pending deleted records in the database. -1. Remove the deletion tracking trigger from the parent table (if the parent table is still there). 1. Remove the loose foreign key definition from the configuration (`config/gitlab_loose_foreign_keys.yml`). + +The deletion tracking trigger needs to be removed only when the parent table no longer uses loose foreign keys. +If the model still has at least one `loose_foreign_key` definition remaining, then these steps can be skipped: + +1. Remove the trigger from the parent table (if the parent table is still there). 1. Remove leftover deleted records from the `loose_foreign_keys_deleted_records` table. Migration for removing the trigger: diff --git a/doc/development/database/understanding_explain_plans.md b/doc/development/database/understanding_explain_plans.md index 92688eb01dc..3e8978e1046 100644 --- a/doc/development/database/understanding_explain_plans.md +++ b/doc/development/database/understanding_explain_plans.md @@ -352,7 +352,6 @@ Indexes: "index_users_on_static_object_token" UNIQUE, btree (static_object_token) "index_users_on_unlock_token" UNIQUE, btree (unlock_token) "index_on_users_name_lower" btree (lower(name::text)) - "index_users_on_accepted_term_id" btree (accepted_term_id) "index_users_on_admin" btree (admin) "index_users_on_created_at" btree (created_at) "index_users_on_email_trigram" gin (email gin_trgm_ops) diff --git a/doc/user/clusters/agent/user_access.md b/doc/user/clusters/agent/user_access.md index a1e98a2db76..b3735770a97 100644 --- a/doc/user/clusters/agent/user_access.md +++ b/doc/user/clusters/agent/user_access.md @@ -202,7 +202,7 @@ glab cluster agent update-kubeconfig --agent '<agent-id>' --kubeconfig ~/gitlab. ### Configure local access manually using a personal access token -You can configure access to a Kubernetes cluster using a long-lived personal access token following these steps: +You can configure access to a Kubernetes cluster using a long-lived personal access token: 1. On the left sidebar, select **Search or go to** and find your project. 1. Select **Operate > Kubernetes clusters** and retrieve the numerical ID of the agent you want to access. You need the ID to construct the full API token. diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 7e222c286b2..9dc87be9af6 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -31344,9 +31344,15 @@ msgstr "" msgid "Navigation|Plan" msgstr "" +msgid "Navigation|Preferences" +msgstr "" + msgid "Navigation|Primary navigation" msgstr "" +msgid "Navigation|Profile" +msgstr "" + msgid "Navigation|Projects you visit often will appear here." msgstr "" diff --git a/spec/features/admin/admin_deploy_keys_spec.rb b/spec/features/admin/admin_deploy_keys_spec.rb index f59b4db5cc2..f9510ef296a 100644 --- a/spec/features/admin/admin_deploy_keys_spec.rb +++ b/spec/features/admin/admin_deploy_keys_spec.rb @@ -18,7 +18,7 @@ RSpec.describe 'admin deploy keys', :js, feature_category: :system_access do it 'show all public deploy keys' do visit admin_deploy_keys_path - page.within(find('[data-testid="deploy-keys-list"]', match: :first)) do + within_testid('deploy-keys-list', match: :first) do expect(page).to have_content(deploy_key.title) expect(page).to have_content(another_deploy_key.title) end @@ -29,7 +29,7 @@ RSpec.describe 'admin deploy keys', :js, feature_category: :system_access do visit admin_deploy_keys_path - page.within(find('[data-testid="deploy-keys-list"]', match: :first)) do + within_testid('deploy-keys-list', match: :first) do expect(page).to have_content(write_key.project.full_name) end end @@ -49,7 +49,7 @@ RSpec.describe 'admin deploy keys', :js, feature_category: :system_access do expect(page).to have_current_path admin_deploy_keys_path, ignore_query: true - page.within(find('[data-testid="deploy-keys-list"]', match: :first)) do + within_testid('deploy-keys-list', match: :first) do expect(page).to have_content('laptop') end end @@ -69,7 +69,7 @@ RSpec.describe 'admin deploy keys', :js, feature_category: :system_access do expect(page).to have_current_path admin_deploy_keys_path, ignore_query: true - page.within(find('[data-testid="deploy-keys-list"]', match: :first)) do + within_testid('deploy-keys-list', match: :first) do expect(page).to have_content('new-title') end end @@ -88,7 +88,7 @@ RSpec.describe 'admin deploy keys', :js, feature_category: :system_access do end expect(page).to have_current_path admin_deploy_keys_path, ignore_query: true - page.within(find('[data-testid="deploy-keys-list"]', match: :first)) do + within_testid('deploy-keys-list', match: :first) do expect(page).not_to have_content(deploy_key.title) end end diff --git a/spec/features/admin/admin_dev_ops_reports_spec.rb b/spec/features/admin/admin_dev_ops_reports_spec.rb index f290464b043..99d43e6b0da 100644 --- a/spec/features/admin/admin_dev_ops_reports_spec.rb +++ b/spec/features/admin/admin_dev_ops_reports_spec.rb @@ -19,8 +19,8 @@ RSpec.describe 'DevOps Report page', :js, feature_category: :devops_reports do expect(page).to have_content 'Introducing Your DevOps Report' - page.within(find('[data-testid="devops-score-container"]')) do - find('[data-testid="close-icon"]').click + within_testid('devops-score-container') do + find_by_testid('close-icon').click end expect(page).not_to have_content 'Introducing Your DevOps Report' diff --git a/spec/features/admin/admin_groups_spec.rb b/spec/features/admin/admin_groups_spec.rb index 1e3dbd7fea4..f071da1835a 100644 --- a/spec/features/admin/admin_groups_spec.rb +++ b/spec/features/admin/admin_groups_spec.rb @@ -138,7 +138,7 @@ RSpec.describe 'Admin Groups', feature_category: :groups_and_projects do it 'shows access requests with link to manage access' do visit admin_group_path(group) - page.within '[data-testid="access-requests"]' do + within_testid('access-requests') do expect(page).to have_content access_request.user.name expect(page).to have_link 'Manage access', href: group_group_members_path(group, tab: 'access_requests') end diff --git a/spec/features/admin/admin_projects_spec.rb b/spec/features/admin/admin_projects_spec.rb index 3454b7af962..b793299e253 100644 --- a/spec/features/admin/admin_projects_spec.rb +++ b/spec/features/admin/admin_projects_spec.rb @@ -95,7 +95,7 @@ RSpec.describe "Admin::Projects", feature_category: :groups_and_projects do context 'when project has open access requests' do it 'shows access requests with link to manage access' do - page.within '[data-testid="access-requests"]' do + within_testid('access-requests') do expect(page).to have_content access_request.user.name expect(page).to have_link 'Manage access', href: project_project_members_path(project, tab: 'access_requests') end diff --git a/spec/features/admin/admin_runners_spec.rb b/spec/features/admin/admin_runners_spec.rb index 676e5e7ad0e..1416124497a 100644 --- a/spec/features/admin/admin_runners_spec.rb +++ b/spec/features/admin/admin_runners_spec.rb @@ -100,7 +100,7 @@ RSpec.describe "Admin Runners", feature_category: :runner_fleet do visit admin_runners_path within_runner_row(runner.id) do - expect(find("[data-testid='job-count']")).to have_content '2' + expect(find_by_testid('job-count')).to have_content '2' end end @@ -116,8 +116,8 @@ RSpec.describe "Admin Runners", feature_category: :runner_fleet do expect(current_url).to match(admin_runner_path(runner)) - expect(find("[data-testid='td-status']")).to have_content "Running" - expect(find("[data-testid='td-job']")).to have_content "##{job.id}" + expect(find_by_testid('td-status')).to have_content "Running" + expect(find_by_testid('td-job')).to have_content "##{job.id}" end describe 'search' do @@ -291,7 +291,7 @@ RSpec.describe "Admin Runners", feature_category: :runner_fleet do expect(page).to have_link('Group 1') expect(page).to have_link('Project 1') - page.within('[data-testid="runner-type-tabs"]') do + within_testid('runner-type-tabs') do expect(page).to have_link('All', class: 'active') end end @@ -302,7 +302,7 @@ RSpec.describe "Admin Runners", feature_category: :runner_fleet do expect(page).to have_content 'runner-project' expect(page).to have_content 'runner-group' - page.within('[data-testid="runner-type-tabs"]') do + within_testid('runner-type-tabs') do click_on('Project') expect(page).to have_link('Project', class: 'active') @@ -315,7 +315,7 @@ RSpec.describe "Admin Runners", feature_category: :runner_fleet do it 'show the same counts after selecting another tab' do visit admin_runners_path - page.within('[data-testid="runner-type-tabs"]') do + within_testid('runner-type-tabs') do click_on('Project') expect(page).to have_link('All 2') @@ -329,7 +329,7 @@ RSpec.describe "Admin Runners", feature_category: :runner_fleet do visit admin_runners_path - page.within('[data-testid="runner-type-tabs"]') do + within_testid('runner-type-tabs') do click_on 'Project' end @@ -355,7 +355,7 @@ RSpec.describe "Admin Runners", feature_category: :runner_fleet do expect(page).to have_content 'runner-group' expect(page).not_to have_content 'runner-paused-project' - page.within('[data-testid="runner-type-tabs"]') do + within_testid('runner-type-tabs') do click_on 'Project' end @@ -367,7 +367,7 @@ RSpec.describe "Admin Runners", feature_category: :runner_fleet do context 'when type does not match' do before do visit admin_runners_path - page.within('[data-testid="runner-type-tabs"]') do + within_testid('runner-type-tabs') do click_on 'Instance' end end @@ -440,24 +440,28 @@ RSpec.describe "Admin Runners", feature_category: :runner_fleet do visit admin_runners_path - within '[data-testid="runner-list"] tbody tr:nth-child(1)' do - expect(page).to have_content 'runner-2' - end + within_testid('runner-list') do + within('tbody tr:nth-child(1)') do + expect(page).to have_content 'runner-2' + end - within '[data-testid="runner-list"] tbody tr:nth-child(2)' do - expect(page).to have_content 'runner-1' + within('tbody tr:nth-child(2)') do + expect(page).to have_content 'runner-1' + end end click_on 'Created date' # Open "sort by" dropdown click_on 'Last contact' click_on 'Sort direction: Descending' - within '[data-testid="runner-list"] tbody tr:nth-child(1)' do - expect(page).to have_content 'runner-1' - end + within_testid('runner-list') do + within('tbody tr:nth-child(1)') do + expect(page).to have_content 'runner-1' + end - within '[data-testid="runner-list"] tbody tr:nth-child(2)' do - expect(page).to have_content 'runner-2' + within('tbody tr:nth-child(2)') do + expect(page).to have_content 'runner-2' + end end end end @@ -522,8 +526,8 @@ RSpec.describe "Admin Runners", feature_category: :runner_fleet do describe 'runner show page breadcrumbs' do it 'contains the current runner id and token' do - page.within '[data-testid="breadcrumb-links"]' do - expect(page.find('[data-testid="breadcrumb-current-link"]')).to have_link( + within_testid('breadcrumb-links') do + expect(find_by_testid('breadcrumb-current-link')).to have_link( "##{runner.id} (#{runner.short_sha})" ) end @@ -555,7 +559,7 @@ RSpec.describe "Admin Runners", feature_category: :runner_fleet do end it 'deletes runner and redirects to runner list' do - expect(page.find('[data-testid="alert-success"]')).to have_content('deleted') + expect(find_by_testid('alert-success')).to have_content('deleted') expect(current_url).to match(admin_runners_path) end end @@ -581,9 +585,9 @@ RSpec.describe "Admin Runners", feature_category: :runner_fleet do describe 'breadcrumbs' do it 'contains the current runner id and token' do - page.within '[data-testid="breadcrumb-links"]' do + within_testid('breadcrumb-links') do expect(page).to have_link("##{project_runner.id} (#{project_runner.short_sha})") - expect(page.find('[data-testid="breadcrumb-current-link"]')).to have_content("Edit") + expect(find_by_testid('breadcrumb-current-link')).to have_content("Edit") end end end @@ -604,7 +608,7 @@ RSpec.describe "Admin Runners", feature_category: :runner_fleet do it 'show success alert and redirects to runner page' do expect(current_url).to match(admin_runner_path(project_runner)) - expect(page.find('[data-testid="alert-success"]')).to have_content('saved') + expect(find_by_testid('alert-success')).to have_content('saved') end end @@ -631,11 +635,13 @@ RSpec.describe "Admin Runners", feature_category: :runner_fleet do describe 'enable/create' do shared_examples 'assignable runner' do it 'enables a runner for a project' do - within find('[data-testid="unassigned-projects"] tr', text: project2.full_name) do - click_on 'Enable' + within_testid('unassigned-projects') do + within('tr', text: project2.full_name) do + click_on 'Enable' + end end - assigned_project = page.find('[data-testid="assigned-projects"]') + assigned_project = find_by_testid('assigned-projects') expect(page).to have_content('Runner assigned to project.') expect(assigned_project).to have_content(project2.name) @@ -671,11 +677,11 @@ RSpec.describe "Admin Runners", feature_category: :runner_fleet do end it 'removed project runner from project' do - within '[data-testid="assigned-projects"]' do + within_testid('assigned-projects') do click_on 'Disable' end - new_runner_project = page.find('[data-testid="unassigned-projects"]') + new_runner_project = find_by_testid('unassigned-projects') expect(page).to have_content('Runner unassigned from project.') expect(new_runner_project).to have_content(project1.name) diff --git a/spec/features/admin/admin_settings_spec.rb b/spec/features/admin/admin_settings_spec.rb index dbdc0a03404..4e0198b1f2b 100644 --- a/spec/features/admin/admin_settings_spec.rb +++ b/spec/features/admin/admin_settings_spec.rb @@ -22,7 +22,7 @@ RSpec.describe 'Admin updates settings', feature_category: :shared do end it 'change visibility settings' do - page.within('[data-testid="admin-visibility-access-settings"]') do + within_testid('admin-visibility-access-settings') do choose "application_setting_default_project_visibility_20" click_button 'Save changes' end @@ -31,19 +31,19 @@ RSpec.describe 'Admin updates settings', feature_category: :shared do end it 'uncheck all restricted visibility levels' do - page.within('[data-testid="restricted-visibility-levels"]') do + within_testid('restricted-visibility-levels') do uncheck s_('VisibilityLevel|Public') uncheck s_('VisibilityLevel|Internal') uncheck s_('VisibilityLevel|Private') end - page.within('[data-testid="admin-visibility-access-settings"]') do + within_testid('admin-visibility-access-settings') do click_button 'Save changes' end expect(page).to have_content "Application settings saved successfully" - page.within('[data-testid="restricted-visibility-levels"]') do + within_testid('restricted-visibility-levels') do expect(find_field(s_('VisibilityLevel|Public'))).not_to be_checked expect(find_field(s_('VisibilityLevel|Internal'))).not_to be_checked expect(find_field(s_('VisibilityLevel|Private'))).not_to be_checked @@ -53,7 +53,7 @@ RSpec.describe 'Admin updates settings', feature_category: :shared do it 'modify import sources' do expect(current_settings.import_sources).to be_empty - page.within('[data-testid="admin-import-export-settings"]') do + within_testid('admin-import-export-settings') do check "Repository by URL" click_button 'Save changes' end @@ -63,12 +63,12 @@ RSpec.describe 'Admin updates settings', feature_category: :shared do end it 'change Visibility and Access Controls' do - page.within('[data-testid="admin-import-export-settings"]') do - page.within('[data-testid="project-export"]') do + within_testid('admin-import-export-settings') do + within_testid('project-export') do uncheck 'Enabled' end - page.within('[data-testid="bulk-import"]') do + within_testid('bulk-import') do check 'Enabled' end @@ -81,7 +81,7 @@ RSpec.describe 'Admin updates settings', feature_category: :shared do end it 'change Keys settings' do - page.within('[data-testid="admin-visibility-access-settings"]') do + within_testid('admin-visibility-access-settings') do select 'Are forbidden', from: 'RSA SSH keys' select 'Are allowed', from: 'DSA SSH keys' select 'Must be at least 384 bits', from: 'ECDSA SSH keys' @@ -103,7 +103,7 @@ RSpec.describe 'Admin updates settings', feature_category: :shared do end it 'change Account and Limit Settings' do - page.within(find('[data-testid="account-and-limit-settings-content"]')) do + within_testid('account-and-limit-settings-content') do uncheck 'Gravatar enabled' click_button 'Save changes' end @@ -113,7 +113,7 @@ RSpec.describe 'Admin updates settings', feature_category: :shared do end it 'change Maximum export size' do - page.within(find('[data-testid="admin-import-export-settings"]')) do + within_testid('admin-import-export-settings') do fill_in 'Maximum export size (MiB)', with: 25 click_button 'Save changes' end @@ -123,7 +123,7 @@ RSpec.describe 'Admin updates settings', feature_category: :shared do end it 'change Maximum import size' do - page.within(find('[data-testid="admin-import-export-settings"]')) do + within_testid('admin-import-export-settings') do fill_in 'Maximum import size (MiB)', with: 15 click_button 'Save changes' end @@ -169,7 +169,7 @@ RSpec.describe 'Admin updates settings', feature_category: :shared do expect(page).to have_unchecked_field(_('Deactivate dormant users after a period of inactivity')) expect(current_settings.deactivate_dormant_users).to be_falsey - page.within(find('[data-testid="account-and-limit-settings-content"]')) do + within_testid('account-and-limit-settings-content') do check _('Deactivate dormant users after a period of inactivity') click_button _('Save changes') end @@ -185,7 +185,7 @@ RSpec.describe 'Admin updates settings', feature_category: :shared do it 'change dormant users period', :js do expect(page).to have_field(_('Days of inactivity before deactivation'), disabled: true) - page.within(find('[data-testid="account-and-limit-settings-content"]')) do + within_testid('account-and-limit-settings-content') do check _('Deactivate dormant users after a period of inactivity') fill_in _('Days of inactivity before deactivation'), with: '180' click_button _('Save changes') @@ -202,7 +202,7 @@ RSpec.describe 'Admin updates settings', feature_category: :shared do selector = '#application_setting_deactivate_dormant_users_period_error' expect(page).not_to have_selector(selector, visible: :visible) - page.within(find('[data-testid="account-and-limit-settings-content"]')) do + within_testid('account-and-limit-settings-content') do check 'application_setting_deactivate_dormant_users' fill_in _('application_setting_deactivate_dormant_users_period'), with: '30' click_button 'Save changes' @@ -818,7 +818,7 @@ RSpec.describe 'Admin updates settings', feature_category: :shared do it 'changes gitlab shell operation limits settings' do visit network_admin_application_settings_path - page.within('[data-testid="gitlab-shell-operation-limits"]') do + within_testid('gitlab-shell-operation-limits') do fill_in 'Maximum number of Git operations per minute', with: 100 click_button 'Save changes' end diff --git a/spec/features/admin/admin_uses_repository_checks_spec.rb b/spec/features/admin/admin_uses_repository_checks_spec.rb index d9d36ec3bae..05232de35e5 100644 --- a/spec/features/admin/admin_uses_repository_checks_spec.rb +++ b/spec/features/admin/admin_uses_repository_checks_spec.rb @@ -47,7 +47,7 @@ RSpec.describe 'Admin uses repository checks', :request_store, feature_category: ) visit_admin_project_page(project) - page.within('[data-testid="last-repository-check-failed-alert"]') do + within_testid('last-repository-check-failed-alert') do expect(page.text).to match(/Last repository check \(just now\) failed/) end end diff --git a/spec/features/admin/broadcast_messages_spec.rb b/spec/features/admin/broadcast_messages_spec.rb index b89ebc34d6a..e4a2e31ee1c 100644 --- a/spec/features/admin/broadcast_messages_spec.rb +++ b/spec/features/admin/broadcast_messages_spec.rb @@ -36,12 +36,12 @@ RSpec.describe 'Admin Broadcast Messages', :js, feature_category: :onboarding do # edit page.within(first_message_container) do - find('[data-testid="edit-message"]').click + find_by_testid('edit-message').click end wait_for_requests - expect(find('[data-testid="message-input"]').value).to eq('test message') + expect(find_by_testid('message-input').value).to eq('test message') fill_in 'Message', with: 'changed test message' @@ -61,11 +61,11 @@ RSpec.describe 'Admin Broadcast Messages', :js, feature_category: :onboarding do end def preview_container - find('[data-testid="preview-broadcast-message"]') + find_by_testid('preview-broadcast-message') end def first_message_container - find('[data-testid="message-row"]', match: :first) + find_by_testid('message-row', match: :first) end end end diff --git a/spec/features/admin/users/user_spec.rb b/spec/features/admin/users/user_spec.rb index fbf11063f7c..b8dc725c17f 100644 --- a/spec/features/admin/users/user_spec.rb +++ b/spec/features/admin/users/user_spec.rb @@ -156,7 +156,7 @@ RSpec.describe 'Admin::Users::User', feature_category: :user_management do it 'disables impersonate button' do subject - impersonate_btn = find('[data-testid="impersonate-user-link"]') + impersonate_btn = find_by_testid('impersonate-user-link') expect(impersonate_btn).not_to be_nil expect(impersonate_btn['disabled']).not_to be_nil @@ -174,7 +174,7 @@ RSpec.describe 'Admin::Users::User', feature_category: :user_management do subject expect(page).to have_content('Impersonate') - impersonate_btn = find('[data-testid="impersonate-user-link"]') + impersonate_btn = find_by_testid('impersonate-user-link') expect(impersonate_btn['disabled']).to be_nil end end diff --git a/spec/features/admin/users/users_spec.rb b/spec/features/admin/users/users_spec.rb index 8ee30c50a7d..4e988674858 100644 --- a/spec/features/admin/users/users_spec.rb +++ b/spec/features/admin/users/users_spec.rb @@ -43,7 +43,7 @@ RSpec.describe 'Admin::Users', feature_category: :user_management do end it 'clicking edit user takes us to edit page', :aggregate_failures do - page.within("[data-testid='user-actions-#{user.id}']") do + within_testid("user-actions-#{user.id}") do click_link 'Edit' end @@ -71,7 +71,7 @@ RSpec.describe 'Admin::Users', feature_category: :user_management do it 'displays count of users projects' do visit admin_users_path - expect(page.find("[data-testid='user-project-count-#{current_user.id}']").text).to eq("1") + expect(find_by_testid("user-project-count-#{current_user.id}").text).to eq("1") end end @@ -321,7 +321,7 @@ RSpec.describe 'Admin::Users', feature_category: :user_management do click_user_dropdown_toggle(user.id) - find('[data-testid="approve"]').click + find_by_testid('approve').click expect(page).to have_content("Approve user #{user.name}?") @@ -378,7 +378,7 @@ RSpec.describe 'Admin::Users', feature_category: :user_management do wait_for_requests - expect(page.find("[data-testid='user-group-count-#{current_user.id}']").text).to eq("2") + expect(find_by_testid("user-group-count-#{current_user.id}").text).to eq("2") end end end @@ -542,7 +542,7 @@ RSpec.describe 'Admin::Users', feature_category: :user_management do it 'allows group membership to be revoked', :js do page.within(first('.group_member')) do - find('.btn[data-testid="remove-user"]').click + find_by_testid('remove-user').click end accept_gl_confirm(button_text: 'Remove') @@ -587,7 +587,7 @@ RSpec.describe 'Admin::Users', feature_category: :user_management do end def check_breadcrumb(content) - expect(find('[data-testid="breadcrumb-current-link"]')).to have_content(content) + expect(find_by_testid('breadcrumb-current-link')).to have_content(content) end end diff --git a/spec/features/alert_management/alert_details_spec.rb b/spec/features/alert_management/alert_details_spec.rb index 7fa9bccbece..58ce9e68468 100644 --- a/spec/features/alert_management/alert_details_spec.rb +++ b/spec/features/alert_management/alert_details_spec.rb @@ -27,7 +27,7 @@ RSpec.describe 'Alert details', :js, feature_category: :incident_management do it 'shows the alert tabs' do page.within('.alert-management-details') do - alert_tabs = find('[data-testid="alertDetailsTabs"]') + alert_tabs = find_by_testid('alertDetailsTabs') expect(alert_tabs).to have_content('Alert details') expect(alert_tabs).to have_content('Metrics') @@ -47,10 +47,10 @@ RSpec.describe 'Alert details', :js, feature_category: :incident_management do it 'updates the alert todo button from the right sidebar' do expect(page).to have_selector('[data-testid="alert-todo-button"]') - todo_button = find('[data-testid="alert-todo-button"]') + todo_button = find_by_testid('alert-todo-button') expect(todo_button).to have_content('Add a to do') - find('[data-testid="alert-todo-button"]').click + find_by_testid('alert-todo-button').click wait_for_requests expect(todo_button).to have_content('Mark as done') @@ -58,7 +58,7 @@ RSpec.describe 'Alert details', :js, feature_category: :incident_management do it 'updates the alert status from the right sidebar' do page.within('.alert-status') do - alert_status = find('[data-testid="status"]') + alert_status = find_by_testid('status') expect(alert_status).to have_content('Triggered') @@ -77,7 +77,7 @@ RSpec.describe 'Alert details', :js, feature_category: :incident_management do expect(alert_assignee).to have_content('None - assign yourself') - find('[data-testid="unassigned-users"]').click + find_by_testid('unassigned-users').click wait_for_requests diff --git a/spec/features/boards/board_filters_spec.rb b/spec/features/boards/board_filters_spec.rb index 1ee02de9a66..386bb196504 100644 --- a/spec/features/boards/board_filters_spec.rb +++ b/spec/features/boards/board_filters_spec.rb @@ -15,7 +15,7 @@ RSpec.describe 'Issue board filters', :js, feature_category: :team_planning do let_it_be(:issue_2) { create(:labeled_issue, project: project, milestone: milestone_2, assignees: [user], labels: [project_label], confidential: true) } let_it_be(:award_emoji1) { create(:award_emoji, name: 'thumbsup', user: user, awardable: issue_1) } - let(:filtered_search) { find('[data-testid="issue-board-filtered-search"]') } + let(:filtered_search) { find_by_testid('issue-board-filtered-search') } let(:filter_input) { find('.gl-filtered-search-term-input') } let(:filter_dropdown) { find('.gl-filtered-search-suggestion-list') } let(:filter_first_suggestion) { find('.gl-filtered-search-suggestion-list').first('.gl-filtered-search-suggestion') } diff --git a/spec/features/boards/boards_spec.rb b/spec/features/boards/boards_spec.rb index 85e54c0f451..69019563e73 100644 --- a/spec/features/boards/boards_spec.rb +++ b/spec/features/boards/boards_spec.rb @@ -28,7 +28,7 @@ RSpec.describe 'Project issue boards', :js, feature_category: :team_planning do let_it_be(:user, reload: true) { create(:user) } let_it_be(:user2, reload: true) { create(:user) } - let(:filtered_search) { find('[data-testid="issue-board-filtered-search"]') } + let(:filtered_search) { find_by_testid('issue-board-filtered-search') } let(:filter_input) { find('.gl-filtered-search-term-input') } let(:filter_submit) { find('.gl-search-box-by-click-search-button') } @@ -296,7 +296,7 @@ RSpec.describe 'Project issue boards', :js, feature_category: :team_planning do it 'shows issue count on the list' do page.within(find(".board:nth-child(2)")) do - expect(page.find('[data-testid="board-items-count"]')).to have_text(total_planning_issues) + expect(find_by_testid('board-items-count')).to have_text(total_planning_issues) expect(page).not_to have_selector('.max-issue-size') end end @@ -389,7 +389,7 @@ RSpec.describe 'Project issue boards', :js, feature_category: :team_planning do wait_for_board_cards(2, 1) - find('[data-testid="filtered-search-clear-button"]').click + find_by_testid('filtered-search-clear-button').click filter_submit.click end diff --git a/spec/features/boards/issue_ordering_spec.rb b/spec/features/boards/issue_ordering_spec.rb index 35e387c9d8a..dd63fd8b80e 100644 --- a/spec/features/boards/issue_ordering_spec.rb +++ b/spec/features/boards/issue_ordering_spec.rb @@ -131,7 +131,7 @@ RSpec.describe 'Issue Boards', :js, feature_category: :team_planning do end context 'ordering in list using move to position' do - let(:move_to_position) { find('[data-testid="board-move-to-position"]') } + let(:move_to_position) { find_by_testid('board-move-to-position') } before do visit project_board_path(project, board) diff --git a/spec/features/boards/new_issue_spec.rb b/spec/features/boards/new_issue_spec.rb index 682ccca38bd..8f9c197e6ba 100644 --- a/spec/features/boards/new_issue_spec.rb +++ b/spec/features/boards/new_issue_spec.rb @@ -11,7 +11,7 @@ RSpec.describe 'Issue Boards new issue', :js, feature_category: :team_planning d let_it_be(:existing_issue) { create(:issue, project: project, title: 'other issue', relative_position: 50) } let(:board_list_header) { first('[data-testid="board-list-header"]') } - let(:project_select_dropdown) { find('[data-testid="project-select-dropdown"]') } + let(:project_select_dropdown) { find_by_testid('project-select-dropdown') } before do stub_feature_flags(apollo_boards: false) @@ -100,7 +100,7 @@ RSpec.describe 'Issue Boards new issue', :js, feature_category: :team_planning d wait_for_requests - page.within('[data-testid="sidebar-labels"]') do + within_testid('sidebar-labels') do click_button 'Edit' wait_for_requests diff --git a/spec/features/boards/sidebar_assignee_spec.rb b/spec/features/boards/sidebar_assignee_spec.rb index 899ab5863e1..93e45b3e3f8 100644 --- a/spec/features/boards/sidebar_assignee_spec.rb +++ b/spec/features/boards/sidebar_assignee_spec.rb @@ -65,7 +65,7 @@ RSpec.describe 'Project issue boards sidebar assignee', :js, wait_for_requests page.within('.dropdown-menu-user') do - find('[data-testid="unassign"]').click + find_by_testid('unassign').click end expect(page).to have_content('None') diff --git a/spec/features/broadcast_messages_spec.rb b/spec/features/broadcast_messages_spec.rb index 98f87face15..f887242384c 100644 --- a/spec/features/broadcast_messages_spec.rb +++ b/spec/features/broadcast_messages_spec.rb @@ -125,8 +125,8 @@ RSpec.describe 'Broadcast Messages', feature_category: :onboarding do visit admin_broadcast_messages_path - page.within('[data-testid="message-row"]', match: :first) do - find("[data-testid='delete-message-#{message.id}']").click + within_testid('message-row', match: :first) do + find_by_testid("delete-message-#{message.id}").click end accept_gl_confirm(button_text: 'Delete message') @@ -145,7 +145,7 @@ RSpec.describe 'Broadcast Messages', feature_category: :onboarding do end def expect_broadcast_message(text) - page.within('[data-testid="banner-broadcast-message"]') do + within_testid('banner-broadcast-message') do expect(page).to have_content text end end @@ -157,7 +157,7 @@ RSpec.describe 'Broadcast Messages', feature_category: :onboarding do end def expect_to_be_on_explore_projects_page - page.within('[data-testid="explore-projects-title"]') do + within_testid('explore-projects-title') do expect(page).to have_content 'Explore projects' end end diff --git a/spec/features/callouts/registration_enabled_spec.rb b/spec/features/callouts/registration_enabled_spec.rb index 3282a40854d..4cd5cb9a857 100644 --- a/spec/features/callouts/registration_enabled_spec.rb +++ b/spec/features/callouts/registration_enabled_spec.rb @@ -43,7 +43,7 @@ RSpec.describe 'Registration enabled callout', feature_category: :system_access before do visit admin_root_path - find('[data-testid="close-registration-enabled-callout"]').click + find_by_testid('close-registration-enabled-callout').click wait_for_requests diff --git a/spec/features/clusters/create_agent_spec.rb b/spec/features/clusters/create_agent_spec.rb index d90c43f452c..79eaecdf582 100644 --- a/spec/features/clusters/create_agent_spec.rb +++ b/spec/features/clusters/create_agent_spec.rb @@ -27,7 +27,7 @@ RSpec.describe 'Cluster agent registration', :js, feature_category: :deployment_ end it 'allows the user to select an agent to install, and displays the resulting agent token' do - find('[data-testid="clusters-default-action-button"]').click + find_by_testid('clusters-default-action-button').click expect(page).to have_content('Register') diff --git a/spec/features/commits_spec.rb b/spec/features/commits_spec.rb index 1df1b7373d9..71af021eded 100644 --- a/spec/features/commits_spec.rb +++ b/spec/features/commits_spec.rb @@ -41,13 +41,13 @@ RSpec.describe 'Commits', feature_category: :source_code_management do end it 'contains commit short id' do - page.within('[data-testid="pipeline-details-header"]') do + within_testid('pipeline-details-header') do expect(page).to have_content pipeline.sha[0..7] end end it 'contains generic commit status build' do - page.within('[data-testid="jobs-tab-table"]') do + within_testid('jobs-tab-table') do expect(page).to have_content "##{status.id}" # build id expect(page).to have_content 'generic' # build name end @@ -122,7 +122,7 @@ RSpec.describe 'Commits', feature_category: :source_code_management do describe 'Cancel build' do it 'cancels build', :js, :sidekiq_might_not_need_inline do visit pipeline_path(pipeline) - find('[data-testid="cancel-pipeline"]').click + find_by_testid('cancel-pipeline').click expect(page).to have_content 'Canceled' end end diff --git a/spec/features/dashboard/group_spec.rb b/spec/features/dashboard/group_spec.rb index ea600758607..7510a92e19b 100644 --- a/spec/features/dashboard/group_spec.rb +++ b/spec/features/dashboard/group_spec.rb @@ -15,7 +15,7 @@ RSpec.describe 'Dashboard Group', feature_category: :groups_and_projects do it 'creates new group', :js do visit dashboard_groups_path - find('[data-testid="new-group-button"]').click + find_by_testid('new-group-button').click click_link 'Create group' new_name = 'Samurai' diff --git a/spec/features/dashboard/issues_spec.rb b/spec/features/dashboard/issues_spec.rb index 39f934ba766..7008f702622 100644 --- a/spec/features/dashboard/issues_spec.rb +++ b/spec/features/dashboard/issues_spec.rb @@ -62,9 +62,11 @@ RSpec.describe 'Dashboard Issues', :js, feature_category: :team_planning do it 'shows projects only with issues feature enabled' do click_button _('Select project to create issue') - page.within('[data-testid="new-resource-dropdown"] [role="menu"]') do - expect(page).to have_content(project.full_name) - expect(page).not_to have_content(project_with_issues_disabled.full_name) + within_testid('new-resource-dropdown') do + within('[role="menu"]') do + expect(page).to have_content(project.full_name) + expect(page).not_to have_content(project_with_issues_disabled.full_name) + end end end @@ -75,7 +77,7 @@ RSpec.describe 'Dashboard Issues', :js, feature_category: :team_planning do project_path = "/#{project.full_path}" - page.within('[data-testid="new-resource-dropdown"]') do + within_testid('new-resource-dropdown') do find_button(project.full_name).click end diff --git a/spec/features/dashboard/merge_requests_spec.rb b/spec/features/dashboard/merge_requests_spec.rb index 005ed73afc0..8a7652858b8 100644 --- a/spec/features/dashboard/merge_requests_spec.rb +++ b/spec/features/dashboard/merge_requests_spec.rb @@ -53,7 +53,7 @@ RSpec.describe 'Dashboard Merge Requests', :js, feature_category: :code_review_w click_button 'Select project to create merge request' wait_for_requests - page.within('[data-testid="new-resource-dropdown"]') do + within_testid('new-resource-dropdown') do expect(page).to have_content(project.full_name) expect(page).not_to have_content(project_with_disabled_merge_requests.full_name) diff --git a/spec/features/dashboard/milestones_spec.rb b/spec/features/dashboard/milestones_spec.rb index 91719c56b84..915626de33c 100644 --- a/spec/features/dashboard/milestones_spec.rb +++ b/spec/features/dashboard/milestones_spec.rb @@ -39,7 +39,7 @@ RSpec.describe 'Dashboard > Milestones', :js, feature_category: :team_planning d it 'takes user to a new milestone page' do click_button 'Select project to create milestone' - page.within('[data-testid="new-resource-dropdown"]') do + within_testid('new-resource-dropdown') do click_button group.name click_link "New milestone in #{group.name}" end diff --git a/spec/helpers/sidebars_helper_spec.rb b/spec/helpers/sidebars_helper_spec.rb index 3e5ee714b32..d24e09e81ac 100644 --- a/spec/helpers/sidebars_helper_spec.rb +++ b/spec/helpers/sidebars_helper_spec.rb @@ -403,15 +403,15 @@ RSpec.describe SidebarsHelper, feature_category: :navigation do describe 'context switcher persistent links' do let_it_be(:public_link) do - [ - { title: s_('Navigation|Explore'), link: '/explore', icon: 'compass' } - ] + { title: s_('Navigation|Explore'), link: '/explore', icon: 'compass' } end let_it_be(:public_links_for_user) do [ { title: s_('Navigation|Your work'), link: '/', icon: 'work' }, - *public_link + public_link, + { title: s_('Navigation|Profile'), link: '/-/profile', icon: 'profile' }, + { title: s_('Navigation|Preferences'), link: '/-/profile/preferences', icon: 'preferences' } ] end @@ -436,7 +436,7 @@ RSpec.describe SidebarsHelper, feature_category: :navigation do let(:user) { nil } it 'returns only the public links for an anonymous user' do - expect(subject[:context_switcher_links]).to eq(public_link) + expect(subject[:context_switcher_links]).to eq([public_link]) end end diff --git a/spec/models/service_desk/custom_email_credential_spec.rb b/spec/models/service_desk/custom_email_credential_spec.rb index a990b77128e..dbf47a8f6a7 100644 --- a/spec/models/service_desk/custom_email_credential_spec.rb +++ b/spec/models/service_desk/custom_email_credential_spec.rb @@ -55,6 +55,39 @@ RSpec.describe ServiceDesk::CustomEmailCredential, feature_category: :service_de end end + describe '#delivery_options' do + let(:expected_attributes) do + { + address: 'smtp.example.com', + domain: 'example.com', + user_name: 'user@example.com', + port: 587, + password: 'supersecret', + authentication: nil + } + end + + let(:setting) { build_stubbed(:service_desk_setting, project: project, custom_email: 'user@example.com') } + + subject { credential.delivery_options } + + before do + # credential.service_desk_setting is delegated to project and we only use build_stubbed + project.service_desk_setting = setting + end + + it { is_expected.to include(expected_attributes) } + + context 'when authentication is set' do + before do + credential.smtp_authentication = 'login' + expected_attributes[:authentication] = 'login' + end + + it { is_expected.to include(expected_attributes) } + end + end + describe 'associations' do it { is_expected.to belong_to(:project) } diff --git a/spec/support/helpers/email_helpers.rb b/spec/support/helpers/email_helpers.rb index 57386233775..9dffe035b2a 100644 --- a/spec/support/helpers/email_helpers.rb +++ b/spec/support/helpers/email_helpers.rb @@ -94,7 +94,8 @@ module EmailHelpers port: credential.smtp_port, user_name: credential.smtp_username, password: credential.smtp_password, - domain: service_desk_setting.custom_email.split('@').last + domain: service_desk_setting.custom_email.split('@').last, + authentication: credential.smtp_authentication ) end end diff --git a/spec/workers/bulk_import_worker_spec.rb b/spec/workers/bulk_import_worker_spec.rb index 8b73549e071..b76bd714a70 100644 --- a/spec/workers/bulk_import_worker_spec.rb +++ b/spec/workers/bulk_import_worker_spec.rb @@ -28,5 +28,7 @@ RSpec.describe BulkImportWorker, feature_category: :importers do end end end + + it_behaves_like 'an idempotent worker' end end diff --git a/spec/workers/bulk_imports/pipeline_batch_worker_spec.rb b/spec/workers/bulk_imports/pipeline_batch_worker_spec.rb index 78ce52c41b4..c4cd2ca2384 100644 --- a/spec/workers/bulk_imports/pipeline_batch_worker_spec.rb +++ b/spec/workers/bulk_imports/pipeline_batch_worker_spec.rb @@ -9,9 +9,13 @@ RSpec.describe BulkImports::PipelineBatchWorker, feature_category: :importers do let(:pipeline_class) do Class.new do - def initialize(_); end + def initialize(context) + @context = context + end - def run; end + def run + @context.tracker.finish! + end def self.file_extraction_pipeline? false @@ -35,7 +39,6 @@ RSpec.describe BulkImports::PipelineBatchWorker, feature_category: :importers do before do stub_const('FakePipeline', pipeline_class) - allow(subject).to receive(:jid).and_return('jid') allow(entity).to receive(:pipeline_exists?).with('FakePipeline').and_return(true) allow_next_instance_of(BulkImports::Groups::Stage) do |instance| allow(instance) @@ -44,51 +47,90 @@ RSpec.describe BulkImports::PipelineBatchWorker, feature_category: :importers do end end + include_examples 'an idempotent worker' do + let(:job_args) { batch.id } + let(:tracker) { create(:bulk_import_tracker, :started, entity: entity, pipeline_name: 'FakePipeline') } + + it 'processes the batch once' do + allow_next_instance_of(pipeline_class) do |instance| + expect(instance).to receive(:run).once.and_call_original + end + + perform_multiple(job_args) + + expect(batch.reload).to be_finished + end + end + describe '#perform' do it 'runs the given pipeline batch successfully' do expect(BulkImports::FinishBatchedPipelineWorker).to receive(:perform_async).with(tracker.id) - subject.perform(batch.id) + worker.perform(batch.id) expect(batch.reload).to be_finished end - context 'when tracker is failed' do - let(:tracker) { create(:bulk_import_tracker, :failed) } + context 'with tracker status' do + context 'when tracker is failed' do + let(:tracker) { create(:bulk_import_tracker, :failed) } - it 'skips the batch' do - subject.perform(batch.id) + it 'skips the batch' do + worker.perform(batch.id) - expect(batch.reload).to be_skipped + expect(batch.reload).to be_skipped + end end - end - context 'when tracker is finished' do - let(:tracker) { create(:bulk_import_tracker, :finished) } + context 'when tracker is finished' do + let(:tracker) { create(:bulk_import_tracker, :finished) } - it 'skips the batch' do - subject.perform(batch.id) + it 'skips the batch' do + worker.perform(batch.id) - expect(batch.reload).to be_skipped + expect(batch.reload).to be_skipped + end end end - context 'when batch status is started' do - let(:batch) { create(:bulk_import_batch_tracker, :started, tracker: tracker) } + context 'with batch status' do + context 'when batch status is started' do + let(:batch) { create(:bulk_import_batch_tracker, :started, tracker: tracker) } + + it 'finishes the batch' do + worker.perform(batch.id) + + expect(batch.reload).to be_finished + end + end + + context 'when batch status is created' do + let(:batch) { create(:bulk_import_batch_tracker, :created, tracker: tracker) } + + it 'finishes the batch' do + worker.perform(batch.id) + + expect(batch.reload).to be_finished + end + end - it 'runs the given pipeline batch successfully' do - subject.perform(batch.id) + context 'when batch status is finished' do + let(:batch) { create(:bulk_import_batch_tracker, :finished, tracker: tracker) } - expect(batch.reload).to be_finished + it 'stays finished' do + worker.perform(batch.id) + + expect(batch.reload).to be_finished + end end end context 'when exclusive lease cannot be obtained' do it 'does not run the pipeline' do - expect(subject).to receive(:try_obtain_lease).and_return(false) - expect(subject).not_to receive(:run) + expect(worker).to receive(:try_obtain_lease).and_return(false) + expect(worker).not_to receive(:run) - subject.perform(batch.id) + worker.perform(batch.id) end end @@ -104,44 +146,54 @@ RSpec.describe BulkImports::PipelineBatchWorker, feature_category: :importers do expect(described_class).to receive(:perform_in).with(60, batch.id) expect(BulkImports::FinishBatchedPipelineWorker).not_to receive(:perform_async).with(tracker.id) - subject.perform(batch.id) + worker.perform(batch.id) expect(batch.reload).to be_created end end - context 'when pipeline is not retryable' do - it 'fails the batch and creates a failure record' do + context 'when pipeline raises an error' do + it 'keeps batch status as `started` and lets the error bubble up' do allow_next_instance_of(pipeline_class) do |instance| allow(instance).to receive(:run).and_raise(StandardError, 'Something went wrong') end - expect(Gitlab::ErrorTracking).to receive(:track_exception).with( - instance_of(StandardError), - hash_including( - batch_id: batch.id, - tracker_id: tracker.id, - pipeline_class: 'FakePipeline', - pipeline_step: 'pipeline_batch_worker_run' - ) - ) - - expect(BulkImports::Failure).to receive(:create).with( - bulk_import_entity_id: entity.id, - pipeline_class: 'FakePipeline', - pipeline_step: 'pipeline_batch_worker_run', - exception_class: 'StandardError', - exception_message: 'Something went wrong', - correlation_id_value: anything - ) - - expect(BulkImports::FinishBatchedPipelineWorker).to receive(:perform_async).with(tracker.id) - - subject.perform(batch.id) - - expect(batch.reload).to be_failed + expect { worker.perform(batch.id) }.to raise_exception(StandardError) + + expect(batch.reload).to be_started end end end end + + describe '.sidekiq_retries_exhausted' do + it 'sets batch status to failed' do + job = { 'args' => [batch.id] } + + expect(Gitlab::ErrorTracking).to receive(:track_exception).with( + instance_of(StandardError), + hash_including( + batch_id: batch.id, + tracker_id: tracker.id, + pipeline_class: 'FakePipeline', + pipeline_step: 'pipeline_batch_worker_run' + ) + ) + + expect(BulkImports::Failure).to receive(:create).with( + bulk_import_entity_id: entity.id, + pipeline_class: 'FakePipeline', + pipeline_step: 'pipeline_batch_worker_run', + exception_class: 'StandardError', + exception_message: 'Something went wrong', + correlation_id_value: anything + ) + + expect(BulkImports::FinishBatchedPipelineWorker).to receive(:perform_async).with(tracker.id) + + described_class.sidekiq_retries_exhausted_block.call(job, StandardError.new("Something went wrong")) + + expect(batch.reload).to be_failed + end + end end diff --git a/spec/workers/bulk_imports/pipeline_worker_spec.rb b/spec/workers/bulk_imports/pipeline_worker_spec.rb index 37205026a90..358cea0110d 100644 --- a/spec/workers/bulk_imports/pipeline_worker_spec.rb +++ b/spec/workers/bulk_imports/pipeline_worker_spec.rb @@ -28,6 +28,8 @@ RSpec.describe BulkImports::PipelineWorker, feature_category: :importers do ) end + let(:worker) { described_class.new } + before do stub_const('FakePipeline', pipeline_class) @@ -38,6 +40,21 @@ RSpec.describe BulkImports::PipelineWorker, feature_category: :importers do end end + include_examples 'an idempotent worker' do + let(:job_args) { [pipeline_tracker.id, pipeline_tracker.stage, entity.id] } + + it 'runs the pipeline and sets tracker to finished' do + allow(worker).to receive(:jid).and_return('jid') + + perform_multiple(job_args, worker: worker) + + pipeline_tracker.reload + + expect(pipeline_tracker.status_name).to eq(:finished) + expect(pipeline_tracker.jid).to eq('jid') + end + end + it 'runs the given pipeline successfully' do expect_next_instance_of(Gitlab::Import::Logger) do |logger| expect(logger) @@ -53,9 +70,9 @@ RSpec.describe BulkImports::PipelineWorker, feature_category: :importers do ) end - allow(subject).to receive(:jid).and_return('jid') + allow(worker).to receive(:jid).and_return('jid') - subject.perform(pipeline_tracker.id, pipeline_tracker.stage, entity.id) + worker.perform(pipeline_tracker.id, pipeline_tracker.stage, entity.id) pipeline_tracker.reload @@ -65,29 +82,16 @@ RSpec.describe BulkImports::PipelineWorker, feature_category: :importers do context 'when exclusive lease cannot be obtained' do it 'does not run the pipeline' do - expect(subject).to receive(:try_obtain_lease).and_return(false) - expect(subject).not_to receive(:run) + expect(worker).to receive(:try_obtain_lease).and_return(false) + expect(worker).not_to receive(:run) - subject.perform(pipeline_tracker.id, pipeline_tracker.stage, entity.id) + worker.perform(pipeline_tracker.id, pipeline_tracker.stage, entity.id) end end - context 'when the pipeline raises an exception' do - it 'logs the error' do - pipeline_tracker = create( - :bulk_import_tracker, - entity: entity, - pipeline_name: 'FakePipeline', - status_event: 'enqueue' - ) - - allow(subject).to receive(:jid).and_return('jid') - - expect_next_instance_of(pipeline_class) do |pipeline| - expect(pipeline) - .to receive(:run) - .and_raise(StandardError, 'Error!') - end + describe '.sidekiq_retries_exhausted' do + it 'logs and sets status as failed' do + job = { 'args' => [pipeline_tracker.id, pipeline_tracker.stage, entity.id] } expect_next_instance_of(Gitlab::Import::Logger) do |logger| expect(logger) @@ -100,7 +104,6 @@ RSpec.describe BulkImports::PipelineWorker, feature_category: :importers do 'bulk_import_entity_type' => entity.source_type, 'source_full_path' => entity.source_full_path, 'class' => 'BulkImports::PipelineWorker', - 'exception.backtrace' => anything, 'exception.message' => 'Error!', 'message' => 'Pipeline failed', 'source_version' => entity.bulk_import.source_version_info.to_s, @@ -137,84 +140,71 @@ RSpec.describe BulkImports::PipelineWorker, feature_category: :importers do ) ) - subject.perform(pipeline_tracker.id, pipeline_tracker.stage, entity.id) + expect_next_instance_of(described_class) do |worker| + expect(worker).to receive(:perform_failure).with(pipeline_tracker.id, entity.id, StandardError) + .and_call_original + allow(worker).to receive(:jid).and_return('jid') + end + + described_class.sidekiq_retries_exhausted_block.call(job, StandardError.new('Error!')) pipeline_tracker.reload expect(pipeline_tracker.status_name).to eq(:failed) expect(pipeline_tracker.jid).to eq('jid') end + end - context 'when enqueued pipeline cannot be found' do - shared_examples 'logs the error' do - it 'logs the error' do - expect_next_instance_of(Gitlab::Import::Logger) do |logger| - status = pipeline_tracker.human_status_name - - expect(logger) - .to receive(:error) - .with( - hash_including( - 'bulk_import_entity_id' => entity.id, - 'bulk_import_id' => entity.bulk_import_id, - 'bulk_import_entity_type' => entity.source_type, - 'pipeline_tracker_id' => pipeline_tracker.id, - 'pipeline_tracker_state' => status, - 'pipeline_class' => pipeline_tracker.pipeline_name, - 'source_full_path' => entity.source_full_path, - 'source_version' => entity.bulk_import.source_version_info.to_s, - 'importer' => 'gitlab_migration', - 'message' => "Pipeline in #{status} state instead of expected enqueued state" - ) - ) - end + context 'when pipeline is finished' do + let(:pipeline_tracker) do + create( + :bulk_import_tracker, + :finished, + entity: entity, + pipeline_name: 'FakePipeline' + ) + end - subject.perform(pipeline_tracker.id, pipeline_tracker.stage, entity.id) - end - end + it 'no-ops and returns' do + expect(described_class).not_to receive(:run) - context 'when pipeline is finished' do - let(:pipeline_tracker) do - create( - :bulk_import_tracker, - :finished, - entity: entity, - pipeline_name: 'FakePipeline' - ) - end + worker.perform(pipeline_tracker.id, pipeline_tracker.stage, entity.id) + end + end - include_examples 'logs the error' - end + context 'when pipeline is skipped' do + let(:pipeline_tracker) do + create( + :bulk_import_tracker, + :skipped, + entity: entity, + pipeline_name: 'FakePipeline' + ) + end - context 'when pipeline is skipped' do - let(:pipeline_tracker) do - create( - :bulk_import_tracker, - :skipped, - entity: entity, - pipeline_name: 'FakePipeline' - ) - end + it 'no-ops and returns' do + expect(described_class).not_to receive(:run) - include_examples 'logs the error' - end + worker.perform(pipeline_tracker.id, pipeline_tracker.stage, entity.id) + end + end - context 'when tracker is started' do - it 'marks tracker as failed' do - pipeline_tracker = create( - :bulk_import_tracker, - :started, - entity: entity, - pipeline_name: 'FakePipeline' - ) + context 'when tracker is started' do + it 'runs the pipeline' do + pipeline_tracker = create( + :bulk_import_tracker, + :started, + entity: entity, + pipeline_name: 'FakePipeline' + ) - subject.perform(pipeline_tracker.id, pipeline_tracker.stage, entity.id) + worker.perform(pipeline_tracker.id, pipeline_tracker.stage, entity.id) - expect(pipeline_tracker.reload.failed?).to eq(true) - end - end + expect(pipeline_tracker.reload.finished?).to eq(true) end + end + describe '#perform' do context 'when entity is failed' do it 'marks tracker as skipped and logs the skip' do pipeline_tracker = create( @@ -243,7 +233,7 @@ RSpec.describe BulkImports::PipelineWorker, feature_category: :importers do ) end - subject.perform(pipeline_tracker.id, pipeline_tracker.stage, entity.id) + worker.perform(pipeline_tracker.id, pipeline_tracker.stage, entity.id) expect(pipeline_tracker.reload.status_name).to eq(:skipped) end @@ -264,7 +254,7 @@ RSpec.describe BulkImports::PipelineWorker, feature_category: :importers do end before do - allow(subject).to receive(:jid).and_return('jid') + allow(worker).to receive(:jid).and_return('jid') expect_next_instance_of(pipeline_class) do |pipeline| expect(pipeline) @@ -301,7 +291,7 @@ RSpec.describe BulkImports::PipelineWorker, feature_category: :importers do pipeline_tracker.entity.id ) - subject.perform(pipeline_tracker.id, pipeline_tracker.stage, entity.id) + worker.perform(pipeline_tracker.id, pipeline_tracker.stage, entity.id) pipeline_tracker.reload @@ -354,7 +344,7 @@ RSpec.describe BulkImports::PipelineWorker, feature_category: :importers do allow(status).to receive(:batched?).and_return(false) end - subject.perform(pipeline_tracker.id, pipeline_tracker.stage, entity.id) + worker.perform(pipeline_tracker.id, pipeline_tracker.stage, entity.id) expect(pipeline_tracker.reload.status_name).to eq(:finished) end @@ -377,7 +367,7 @@ RSpec.describe BulkImports::PipelineWorker, feature_category: :importers do entity.id ) - subject.perform(pipeline_tracker.id, pipeline_tracker.stage, entity.id) + worker.perform(pipeline_tracker.id, pipeline_tracker.stage, entity.id) end end @@ -406,7 +396,7 @@ RSpec.describe BulkImports::PipelineWorker, feature_category: :importers do entity.id ) - subject.perform(pipeline_tracker.id, pipeline_tracker.stage, entity.id) + worker.perform(pipeline_tracker.id, pipeline_tracker.stage, entity.id) expect(pipeline_tracker.reload.status_name).to eq(:enqueued) end @@ -415,31 +405,9 @@ RSpec.describe BulkImports::PipelineWorker, feature_category: :importers do context 'when empty export timeout is reached' do let(:created_at) { 10.minutes.ago } - it 'marks as failed and logs the error' do - expect_next_instance_of(Gitlab::Import::Logger) do |logger| - expect(logger) - .to receive(:error) - .with( - hash_including( - 'pipeline_class' => 'NdjsonPipeline', - 'bulk_import_entity_id' => entity.id, - 'bulk_import_id' => entity.bulk_import_id, - 'bulk_import_entity_type' => entity.source_type, - 'source_full_path' => entity.source_full_path, - 'class' => 'BulkImports::PipelineWorker', - 'exception.backtrace' => anything, - 'exception.class' => 'BulkImports::Pipeline::ExpiredError', - 'exception.message' => 'Empty export status on source instance', - 'importer' => 'gitlab_migration', - 'message' => 'Pipeline failed', - 'source_version' => entity.bulk_import.source_version_info.to_s - ) - ) - end - - subject.perform(pipeline_tracker.id, pipeline_tracker.stage, entity.id) - - expect(pipeline_tracker.reload.status_name).to eq(:failed) + it 'raises sidekiq error' do + expect { worker.perform(pipeline_tracker.id, pipeline_tracker.stage, entity.id) } + .to raise_exception(BulkImports::Pipeline::ExpiredError) end end @@ -449,17 +417,8 @@ RSpec.describe BulkImports::PipelineWorker, feature_category: :importers do it 'falls back to entity created_at' do entity.update!(created_at: 10.minutes.ago) - expect_next_instance_of(Gitlab::Import::Logger) do |logger| - expect(logger) - .to receive(:error) - .with( - hash_including('exception.message' => 'Empty export status on source instance') - ) - end - - subject.perform(pipeline_tracker.id, pipeline_tracker.stage, entity.id) - - expect(pipeline_tracker.reload.status_name).to eq(:failed) + expect { worker.perform(pipeline_tracker.id, pipeline_tracker.stage, entity.id) } + .to raise_exception(BulkImports::Pipeline::ExpiredError) end end end @@ -471,28 +430,8 @@ RSpec.describe BulkImports::PipelineWorker, feature_category: :importers do allow(status).to receive(:error).and_return('Error!') end - expect_next_instance_of(Gitlab::Import::Logger) do |logger| - expect(logger) - .to receive(:error) - .with( - hash_including( - 'pipeline_class' => 'NdjsonPipeline', - 'bulk_import_entity_id' => entity.id, - 'bulk_import_id' => entity.bulk_import_id, - 'bulk_import_entity_type' => entity.source_type, - 'source_full_path' => entity.source_full_path, - 'exception.backtrace' => anything, - 'exception.class' => 'BulkImports::Pipeline::FailedError', - 'exception.message' => 'Export from source instance failed: Error!', - 'importer' => 'gitlab_migration', - 'source_version' => entity.bulk_import.source_version_info.to_s - ) - ) - end - - subject.perform(pipeline_tracker.id, pipeline_tracker.stage, entity.id) - - expect(pipeline_tracker.reload.status_name).to eq(:failed) + expect { worker.perform(pipeline_tracker.id, pipeline_tracker.stage, entity.id) } + .to raise_exception(BulkImports::Pipeline::FailedError) end end @@ -512,7 +451,7 @@ RSpec.describe BulkImports::PipelineWorker, feature_category: :importers do it 'enqueues pipeline batches' do expect(BulkImports::PipelineBatchWorker).to receive(:perform_async).twice - subject.perform(pipeline_tracker.id, pipeline_tracker.stage, entity.id) + worker.perform(pipeline_tracker.id, pipeline_tracker.stage, entity.id) pipeline_tracker.reload @@ -525,9 +464,9 @@ RSpec.describe BulkImports::PipelineWorker, feature_category: :importers do let(:batches_count) { 0 } it 'marks tracker as finished' do - expect(subject).not_to receive(:enqueue_batches) + expect(worker).not_to receive(:enqueue_batches) - subject.perform(pipeline_tracker.id, pipeline_tracker.stage, entity.id) + worker.perform(pipeline_tracker.id, pipeline_tracker.stage, entity.id) expect(pipeline_tracker.reload.status_name).to eq(:finished) end diff --git a/spec/workers/every_sidekiq_worker_spec.rb b/spec/workers/every_sidekiq_worker_spec.rb index 94fe0e72ff8..6e1da58549c 100644 --- a/spec/workers/every_sidekiq_worker_spec.rb +++ b/spec/workers/every_sidekiq_worker_spec.rb @@ -140,8 +140,8 @@ RSpec.describe 'Every Sidekiq worker', feature_category: :shared do 'BulkImportWorker' => false, 'BulkImports::ExportRequestWorker' => 5, 'BulkImports::EntityWorker' => false, - 'BulkImports::PipelineWorker' => false, - 'BulkImports::PipelineBatchWorker' => false, + 'BulkImports::PipelineWorker' => 3, + 'BulkImports::PipelineBatchWorker' => 3, 'BulkImports::FinishProjectImportWorker' => 5, 'Chaos::CpuSpinWorker' => 3, 'Chaos::DbSpinWorker' => 3, |