From aaabf6eb167d1a0bb2357f331bd411923ede37a6 Mon Sep 17 00:00:00 2001 From: GitLab Bot Date: Thu, 10 Aug 2023 15:10:45 +0000 Subject: Add latest changes from gitlab-org/gitlab@master --- spec/db/schema_spec.rb | 9 +- .../__snapshots__/package_list_row_spec.js.snap | 38 ++-- .../components/list/package_list_row_spec.js | 37 +++- .../components/work_item_created_updated_spec.js | 38 +++- .../work_items/components/work_item_detail_spec.js | 31 +-- spec/lib/gitlab/git/repository_spec.rb | 6 + .../gitlab/gitaly_client/commit_service_spec.rb | 18 ++ spec/models/ci/build_spec.rb | 22 ++ spec/models/ci/job_artifact_spec.rb | 78 ------- spec/models/ci/runner_spec.rb | 40 ++-- spec/models/commit_status_spec.rb | 22 ++ .../concerns/ci/partitionable/switch_spec.rb | 20 ++ spec/models/concerns/ci/partitionable_spec.rb | 10 + spec/models/repository_spec.rb | 18 ++ .../api/metrics/user_starred_dashboards_spec.rb | 57 ------ spec/services/ci/delete_objects_service_spec.rb | 225 ++++++--------------- .../create_service_spec.rb | 73 ------- .../delete_service_spec.rb | 41 ---- .../ci/partitioning_testing/schema_helpers.rb | 11 +- 19 files changed, 303 insertions(+), 491 deletions(-) delete mode 100644 spec/services/metrics/users_starred_dashboards/create_service_spec.rb delete mode 100644 spec/services/metrics/users_starred_dashboards/delete_service_spec.rb (limited to 'spec') diff --git a/spec/db/schema_spec.rb b/spec/db/schema_spec.rb index 6eb80df3877..932353f0bd5 100644 --- a/spec/db/schema_spec.rb +++ b/spec/db/schema_spec.rb @@ -44,7 +44,7 @@ RSpec.describe 'Database schema', feature_category: :database do broadcast_messages: %w[namespace_id], chat_names: %w[chat_id team_id user_id integration_id], chat_teams: %w[team_id], - ci_builds: %w[erased_by_id trigger_request_id partition_id], + ci_builds: %w[project_id runner_id user_id erased_by_id trigger_request_id partition_id], ci_namespace_monthly_usages: %w[namespace_id], ci_pipeline_variables: %w[partition_id], ci_pipelines: %w[partition_id], @@ -87,7 +87,7 @@ RSpec.describe 'Database schema', feature_category: :database do oauth_access_grants: %w[resource_owner_id application_id], oauth_access_tokens: %w[resource_owner_id application_id], oauth_applications: %w[owner_id], - p_ci_builds: %w[project_id runner_id user_id erased_by_id trigger_request_id partition_id], + p_ci_builds: %w[erased_by_id trigger_request_id partition_id], p_batched_git_ref_updates_deletions: %w[project_id partition_id], product_analytics_events_experimental: %w[event_id txn_id user_id], project_build_artifacts_size_refreshes: %w[last_job_artifact_id], @@ -195,18 +195,23 @@ RSpec.describe 'Database schema', feature_category: :database do IGNORED_LIMIT_ENUMS = { 'Analytics::CycleAnalytics::Stage' => %w[start_event_identifier end_event_identifier], 'Ci::Bridge' => %w[failure_reason], + 'Ci::Bridge::Partitioned' => %w[failure_reason], 'Ci::Build' => %w[failure_reason], + 'Ci::Build::Partitioned' => %w[failure_reason], 'Ci::BuildMetadata' => %w[timeout_source], 'Ci::BuildTraceChunk' => %w[data_store], 'Ci::DailyReportResult' => %w[param_type], 'Ci::JobArtifact' => %w[file_type], 'Ci::Pipeline' => %w[source config_source failure_reason], 'Ci::Processable' => %w[failure_reason], + 'Ci::Processable::Partitioned' => %w[failure_reason], 'Ci::Runner' => %w[access_level], 'Ci::Stage' => %w[status], 'Clusters::Cluster' => %w[platform_type provider_type], 'CommitStatus' => %w[failure_reason], + 'CommitStatus::Partitioned' => %w[failure_reason], 'GenericCommitStatus' => %w[failure_reason], + 'GenericCommitStatus::Partitioned' => %w[failure_reason], 'InternalId' => %w[usage], 'List' => %w[list_type], 'NotificationSetting' => %w[level], diff --git a/spec/frontend/packages_and_registries/package_registry/components/list/__snapshots__/package_list_row_spec.js.snap b/spec/frontend/packages_and_registries/package_registry/components/list/__snapshots__/package_list_row_spec.js.snap index 7a488a74dcb..e0e6c101029 100644 --- a/spec/frontend/packages_and_registries/package_registry/components/list/__snapshots__/package_list_row_spec.js.snap +++ b/spec/frontend/packages_and_registries/package_registry/components/list/__snapshots__/package_list_row_spec.js.snap @@ -104,35 +104,33 @@ exports[`packages_list_row renders 1`] = `
- - - Delete package - - + + + Delete package + + + +
diff --git a/spec/frontend/packages_and_registries/package_registry/components/list/package_list_row_spec.js b/spec/frontend/packages_and_registries/package_registry/components/list/package_list_row_spec.js index 523d5f855fc..9f8fd4e28e7 100644 --- a/spec/frontend/packages_and_registries/package_registry/components/list/package_list_row_spec.js +++ b/spec/frontend/packages_and_registries/package_registry/components/list/package_list_row_spec.js @@ -34,7 +34,8 @@ describe('packages_list_row', () => { const packageWithTags = { ...packageWithoutTags, tags: { nodes: packageTags() } }; const findPackageTags = () => wrapper.findComponent(PackageTags); - const findDeleteDropdown = () => wrapper.findByTestId('action-delete'); + const findDeleteDropdown = () => wrapper.findByTestId('delete-dropdown'); + const findDeleteButton = () => wrapper.findByTestId('action-delete'); const findPackageType = () => wrapper.findByTestId('package-type'); const findPackageLink = () => wrapper.findByTestId('details-link'); const findWarningIcon = () => wrapper.findByTestId('warning-icon'); @@ -103,7 +104,7 @@ describe('packages_list_row', () => { }); }); - describe('delete button', () => { + describe('delete dropdown', () => { it('does not exist when package cannot be destroyed', () => { mountComponent({ packageEntity: { ...packageWithoutTags, canDestroy: false }, @@ -112,19 +113,39 @@ describe('packages_list_row', () => { expect(findDeleteDropdown().exists()).toBe(false); }); - it('exists and has the correct props', () => { - mountComponent({ packageEntity: packageWithoutTags }); + it('exists when package can be destroyed', () => { + mountComponent(); - expect(findDeleteDropdown().exists()).toBe(true); - expect(findDeleteDropdown().attributes()).toMatchObject({ - variant: 'danger', + expect(findDeleteDropdown().props()).toMatchObject({ + category: 'tertiary', + icon: 'ellipsis_v', + textSrOnly: true, + noCaret: true, + toggleText: 'More actions', }); }); + }); + + describe('delete button', () => { + it('does not exist when package cannot be destroyed', () => { + mountComponent({ + packageEntity: { ...packageWithoutTags, canDestroy: false }, + }); + + expect(findDeleteButton().exists()).toBe(false); + }); + + it('exists and has the correct text', () => { + mountComponent({ packageEntity: packageWithoutTags }); + + expect(findDeleteButton().exists()).toBe(true); + expect(findDeleteButton().text()).toBe('Delete package'); + }); it('emits the delete event when the delete button is clicked', () => { mountComponent({ packageEntity: packageWithoutTags }); - findDeleteDropdown().vm.$emit('click'); + findDeleteButton().vm.$emit('action'); expect(wrapper.emitted('delete')).toHaveLength(1); }); diff --git a/spec/frontend/work_items/components/work_item_created_updated_spec.js b/spec/frontend/work_items/components/work_item_created_updated_spec.js index 428b33aecfa..f77c5481906 100644 --- a/spec/frontend/work_items/components/work_item_created_updated_spec.js +++ b/spec/frontend/work_items/components/work_item_created_updated_spec.js @@ -1,10 +1,11 @@ -import { GlAvatarLink, GlSprintf } from '@gitlab/ui'; +import { GlAvatarLink, GlSprintf, GlLoadingIcon } from '@gitlab/ui'; import { shallowMount } from '@vue/test-utils'; import Vue from 'vue'; import VueApollo from 'vue-apollo'; import createMockApollo from 'helpers/mock_apollo_helper'; import waitForPromises from 'helpers/wait_for_promises'; import WorkItemCreatedUpdated from '~/work_items/components/work_item_created_updated.vue'; +import ConfidentialityBadge from '~/vue_shared/components/confidentiality_badge.vue'; import WorkItemTypeIcon from '~/work_items/components/work_item_type_icon.vue'; import workItemByIidQuery from '~/work_items/graphql/work_item_by_iid.query.graphql'; import { workItemByIidResponseFactory, mockAssignees } from '../mock_data'; @@ -20,11 +21,20 @@ describe('WorkItemCreatedUpdated component', () => { const findCreatedAtText = () => findCreatedAt().text().replace(/\s+/g, ' '); const findWorkItemTypeIcon = () => wrapper.findComponent(WorkItemTypeIcon); - - const createComponent = async ({ workItemIid = '1', author = null, updatedAt } = {}) => { + const findConfidentialityBadge = () => wrapper.findComponent(ConfidentialityBadge); + const findLoadingIcon = () => wrapper.findComponent(GlLoadingIcon); + + const createComponent = async ({ + workItemIid = '1', + author = null, + updatedAt, + confidential = false, + updateInProgress = false, + } = {}) => { const workItemQueryResponse = workItemByIidResponseFactory({ author, updatedAt, + confidential, }); successHandler = jest.fn().mockResolvedValue(workItemQueryResponse); @@ -34,7 +44,7 @@ describe('WorkItemCreatedUpdated component', () => { provide: { fullPath: '/some/project', }, - propsData: { workItemIid }, + propsData: { workItemIid, updateInProgress }, stubs: { GlAvatarLink, GlSprintf, @@ -88,4 +98,24 @@ describe('WorkItemCreatedUpdated component', () => { expect(findUpdatedAt().exists()).toBe(false); }); + + describe('confidential badge', () => { + it('renders badge when the work item is confidential', async () => { + await createComponent({ confidential: true }); + + expect(findConfidentialityBadge().exists()).toBe(true); + }); + + it('does not render badge when the work item is confidential', async () => { + await createComponent({ confidential: false }); + + expect(findConfidentialityBadge().exists()).toBe(false); + }); + + it('shows loading icon badge when the work item is confidential', async () => { + await createComponent({ updateInProgress: true }); + + expect(findLoadingIcon().exists()).toBe(true); + }); + }); }); diff --git a/spec/frontend/work_items/components/work_item_detail_spec.js b/spec/frontend/work_items/components/work_item_detail_spec.js index 07cbd975878..d3c7c9e2074 100644 --- a/spec/frontend/work_items/components/work_item_detail_spec.js +++ b/spec/frontend/work_items/components/work_item_detail_spec.js @@ -1,7 +1,5 @@ import { GlAlert, - GlBadge, - GlLoadingIcon, GlSkeletonLoader, GlButton, GlEmptyState, @@ -68,7 +66,6 @@ describe('WorkItemDetail component', () => { const findAlert = () => wrapper.findComponent(GlAlert); const findEmptyState = () => wrapper.findComponent(GlEmptyState); const findSkeleton = () => wrapper.findComponent(GlSkeletonLoader); - const findLoadingIcon = () => wrapper.findComponent(GlLoadingIcon); const findWorkItemActions = () => wrapper.findComponent(WorkItemActions); const findWorkItemTitle = () => wrapper.findComponent(WorkItemTitle); const findCreatedUpdated = () => wrapper.findComponent(WorkItemCreatedUpdated); @@ -316,27 +313,7 @@ describe('WorkItemDetail component', () => { `( 'when work item has $context', ({ handlerMock, confidentialityMock, confidentialityFailureMock, inputVariables }) => { - it('renders confidential badge when work item is confidential', async () => { - createComponent({ - handler: jest.fn().mockResolvedValue(confidentialWorkItem), - confidentialityMock, - }); - - await waitForPromises(); - - const confidentialBadge = wrapper.findComponent(GlBadge); - expect(confidentialBadge.exists()).toBe(true); - expect(confidentialBadge.props()).toMatchObject({ - variant: 'warning', - icon: 'eye-slash', - }); - expect(confidentialBadge.attributes('title')).toBe( - 'Only project members with at least the Reporter role, the author, and assignees can view or be notified about this task.', - ); - expect(confidentialBadge.text()).toBe('Confidential'); - }); - - it('renders gl-loading-icon while update mutation is in progress', async () => { + it('sends updateInProgress props to child component', async () => { createComponent({ handler: handlerMock, confidentialityMock, @@ -348,10 +325,10 @@ describe('WorkItemDetail component', () => { await nextTick(); - expect(findLoadingIcon().exists()).toBe(true); + expect(findCreatedUpdated().props('updateInProgress')).toBe(true); }); - it('emits workItemUpdated and shows confidentiality badge when mutation is successful', async () => { + it('emits workItemUpdated when mutation is successful', async () => { createComponent({ handler: handlerMock, confidentialityMock, @@ -366,7 +343,6 @@ describe('WorkItemDetail component', () => { expect(confidentialityMock[1]).toHaveBeenCalledWith({ input: inputVariables, }); - expect(findLoadingIcon().exists()).toBe(false); }); it('shows an alert when mutation fails', async () => { @@ -384,7 +360,6 @@ describe('WorkItemDetail component', () => { expect(findAlert().exists()).toBe(true); expect(findAlert().text()).toBe(errorMessage); - expect(findLoadingIcon().exists()).toBe(false); }); }, ); diff --git a/spec/lib/gitlab/git/repository_spec.rb b/spec/lib/gitlab/git/repository_spec.rb index 2b9a3fd8091..d3c7fcc465a 100644 --- a/spec/lib/gitlab/git/repository_spec.rb +++ b/spec/lib/gitlab/git/repository_spec.rb @@ -2573,6 +2573,12 @@ RSpec.describe Gitlab::Git::Repository, feature_category: :source_code_managemen end end + describe '#get_patch_id' do + it_behaves_like 'wrapping gRPC errors', Gitlab::GitalyClient::CommitService, :get_patch_id do + subject { repository.get_patch_id('HEAD~', 'HEAD') } + end + end + def create_remote_branch(remote_name, branch_name, source_branch_name) source_branch = repository.find_branch(source_branch_name) repository.write_ref("refs/remotes/#{remote_name}/#{branch_name}", source_branch.dereferenced_target.sha) diff --git a/spec/lib/gitlab/gitaly_client/commit_service_spec.rb b/spec/lib/gitlab/gitaly_client/commit_service_spec.rb index 740073d5e4e..2ee9d85c723 100644 --- a/spec/lib/gitlab/gitaly_client/commit_service_spec.rb +++ b/spec/lib/gitlab/gitaly_client/commit_service_spec.rb @@ -1108,4 +1108,22 @@ RSpec.describe Gitlab::GitalyClient::CommitService, feature_category: :gitaly do expect(signatures[large_signed_text][:signed_text].size).to eq(4971878) end end + + describe '#get_patch_id' do + it 'returns patch_id of given revisions' do + expect(client.get_patch_id('HEAD~', 'HEAD')).to eq('45435e5d7b339dd76d939508c7687701d0c17fff') + end + + context 'when one of the param is invalid' do + it 'raises an GRPC::InvalidArgument error' do + expect { client.get_patch_id('HEAD', nil) }.to raise_error(GRPC::InvalidArgument) + end + end + + context 'when two revisions are the same' do + it 'raises an GRPC::FailedPrecondition error' do + expect { client.get_patch_id('HEAD', 'HEAD') }.to raise_error(GRPC::FailedPrecondition) + end + end + end end diff --git a/spec/models/ci/build_spec.rb b/spec/models/ci/build_spec.rb index 54c0c086144..90edbf530cf 100644 --- a/spec/models/ci/build_spec.rb +++ b/spec/models/ci/build_spec.rb @@ -5566,4 +5566,26 @@ RSpec.describe Ci::Build, feature_category: :continuous_integration, factory_def end end end + + describe 'routing table switch' do + context 'with ff disabled' do + before do + stub_feature_flags(ci_partitioning_use_ci_builds_routing_table: false) + end + + it 'uses the legacy table' do + expect(described_class.table_name).to eq('ci_builds') + end + end + + context 'with ff enabled' do + before do + stub_feature_flags(ci_partitioning_use_ci_builds_routing_table: true) + end + + it 'uses the routing table' do + expect(described_class.table_name).to eq('p_ci_builds') + end + end + end end diff --git a/spec/models/ci/job_artifact_spec.rb b/spec/models/ci/job_artifact_spec.rb index 498af80dbb6..83c233fa942 100644 --- a/spec/models/ci/job_artifact_spec.rb +++ b/spec/models/ci/job_artifact_spec.rb @@ -824,82 +824,4 @@ RSpec.describe Ci::JobArtifact, feature_category: :build_artifacts do it { is_expected.to eq(artifact.file.filename) } end - - describe '#to_deleted_object_attrs' do - let(:pick_up_at) { nil } - let(:expire_at) { nil } - let(:file_final_path) { nil } - - let(:artifact) do - create( - :ci_job_artifact, - :archive, - :remote_store, - file_final_path: file_final_path, - expire_at: expire_at - ) - end - - subject(:attributes) { artifact.to_deleted_object_attrs(pick_up_at) } - - before do - stub_artifacts_object_storage - end - - shared_examples_for 'returning attributes for object deletion' do - it 'returns the file store' do - expect(attributes[:file_store]).to eq(artifact.file_store) - end - - context 'when pick_up_at is present' do - let(:pick_up_at) { 2.hours.ago } - - it 'returns the pick_up_at value' do - expect(attributes[:pick_up_at]).to eq(pick_up_at) - end - end - - context 'when pick_up_at is not present' do - context 'and expire_at is present' do - let(:expire_at) { 4.hours.ago } - - it 'sets expire_at as pick_up_at' do - expect(attributes[:pick_up_at]).to eq(expire_at) - end - end - - context 'and expire_at is not present' do - it 'sets current time as pick_up_at' do - freeze_time do - expect(attributes[:pick_up_at]).to eq(Time.current) - end - end - end - end - end - - context 'when file_final_path is present' do - let(:file_final_path) { 'some/hash/path/to/randomfile' } - - it 'returns the store_dir and file based on the file_final_path' do - expect(attributes).to include( - store_dir: 'some/hash/path/to', - file: 'randomfile' - ) - end - - it_behaves_like 'returning attributes for object deletion' - end - - context 'when file_final_path is not present' do - it 'returns the uploader default store_dir and file_identifier' do - expect(attributes).to include( - store_dir: artifact.file.store_dir.to_s, - file: artifact.file_identifier - ) - end - - it_behaves_like 'returning attributes for object deletion' - end - end end diff --git a/spec/models/ci/runner_spec.rb b/spec/models/ci/runner_spec.rb index 50e2ded695c..56e69cc2b9c 100644 --- a/spec/models/ci/runner_spec.rb +++ b/spec/models/ci/runner_spec.rb @@ -1968,19 +1968,29 @@ RSpec.describe Ci::Runner, type: :model, feature_category: :runner do end describe '.with_upgrade_status' do - subject { described_class.with_upgrade_status(upgrade_status) } + subject(:scope) { described_class.with_upgrade_status(upgrade_status) } - let_it_be(:runner_14_0_0) { create(:ci_runner, version: '14.0.0') } - let_it_be(:runner_14_1_0) { create(:ci_runner, version: '14.1.0') } - let_it_be(:runner_14_1_1) { create(:ci_runner, version: '14.1.1') } - let_it_be(:runner_version_14_0_0) { create(:ci_runner_version, version: '14.0.0', status: :available) } - let_it_be(:runner_version_14_1_0) { create(:ci_runner_version, version: '14.1.0', status: :recommended) } - let_it_be(:runner_version_14_1_1) { create(:ci_runner_version, version: '14.1.1', status: :unavailable) } + let_it_be(:runner_14_0_0) { create(:ci_runner) } + let_it_be(:runner_14_1_0_and_14_0_0) { create(:ci_runner) } + let_it_be(:runner_14_1_0) { create(:ci_runner) } + let_it_be(:runner_14_1_1) { create(:ci_runner) } + + before_all do + create(:ci_runner_machine, runner: runner_14_1_0_and_14_0_0, version: '14.0.0') + create(:ci_runner_machine, runner: runner_14_1_0_and_14_0_0, version: '14.1.0') + create(:ci_runner_machine, runner: runner_14_0_0, version: '14.0.0') + create(:ci_runner_machine, runner: runner_14_1_0, version: '14.1.0') + create(:ci_runner_machine, runner: runner_14_1_1, version: '14.1.1') + + create(:ci_runner_version, version: '14.0.0', status: :available) + create(:ci_runner_version, version: '14.1.0', status: :recommended) + create(:ci_runner_version, version: '14.1.1', status: :unavailable) + end context ':unavailable' do let(:upgrade_status) { :unavailable } - it 'returns runners whose version is assigned :unavailable' do + it 'returns runners with runner managers whose version is assigned :unavailable' do is_expected.to contain_exactly(runner_14_1_1) end end @@ -1988,23 +1998,27 @@ RSpec.describe Ci::Runner, type: :model, feature_category: :runner do context ':available' do let(:upgrade_status) { :available } - it 'returns runners whose version is assigned :available' do - is_expected.to contain_exactly(runner_14_0_0) + it 'returns runners with runner managers whose version is assigned :available' do + is_expected.to contain_exactly(runner_14_0_0, runner_14_1_0_and_14_0_0) end end context ':recommended' do let(:upgrade_status) { :recommended } - it 'returns runners whose version is assigned :recommended' do - is_expected.to contain_exactly(runner_14_1_0) + it 'returns runners with runner managers whose version is assigned :recommended' do + is_expected.to contain_exactly(runner_14_1_0_and_14_0_0, runner_14_1_0) end end describe 'composed with other scopes' do subject { described_class.active(false).with_upgrade_status(:available) } - let(:inactive_runner_14_0_0) { create(:ci_runner, version: '14.0.0', active: false) } + before do + create(:ci_runner_machine, runner: inactive_runner_14_0_0, version: '14.0.0') + end + + let(:inactive_runner_14_0_0) { create(:ci_runner, active: false) } it 'returns runner matching the composed scope' do is_expected.to contain_exactly(inactive_runner_14_0_0) diff --git a/spec/models/commit_status_spec.rb b/spec/models/commit_status_spec.rb index ac356bcd65a..9ce9f0e13b5 100644 --- a/spec/models/commit_status_spec.rb +++ b/spec/models/commit_status_spec.rb @@ -1067,4 +1067,26 @@ RSpec.describe CommitStatus, feature_category: :continuous_integration do it_behaves_like 'having enum with nil value' end + + describe 'routing table switch' do + context 'with ff disabled' do + before do + stub_feature_flags(ci_partitioning_use_ci_builds_routing_table: false) + end + + it 'uses the legacy table' do + expect(described_class.table_name).to eq('ci_builds') + end + end + + context 'with ff enabled' do + before do + stub_feature_flags(ci_partitioning_use_ci_builds_routing_table: true) + end + + it 'uses the routing table' do + expect(described_class.table_name).to eq('p_ci_builds') + end + end + end end diff --git a/spec/models/concerns/ci/partitionable/switch_spec.rb b/spec/models/concerns/ci/partitionable/switch_spec.rb index 551ae111fa4..4aaf69bb585 100644 --- a/spec/models/concerns/ci/partitionable/switch_spec.rb +++ b/spec/models/concerns/ci/partitionable/switch_spec.rb @@ -38,6 +38,7 @@ RSpec.describe Ci::Partitionable::Switch, :aggregate_failures do id serial NOT NULL PRIMARY KEY, job_id int, partition_id int NOT NULL DEFAULT 1, + type text, expanded_environment_name text); CREATE TABLE _test_p_ci_jobs_metadata ( @@ -89,6 +90,25 @@ RSpec.describe Ci::Partitionable::Switch, :aggregate_failures do it { expect(partitioned_model.sequence_name).to eq('_test_ci_jobs_metadata_id_seq') } + context 'with singe table inheritance' do + let(:child_model) do + Class.new(model) do + def self.name + 'TestSwitchJobMetadataChild' + end + end + end + + it 'adds a Partitioned model for each descendant' do + expect(model::Partitioned).not_to eq(child_model::Partitioned) + end + + it 'uses the parent name in STI queries' do + recorder = ActiveRecord::QueryRecorder.new { child_model.all.load } + expect(recorder.log).to include(/"type" = 'TestSwitchJobMetadataChild'/) + end + end + context 'when switching the tables' do before do stub_feature_flags(table_rollout_flag => false) diff --git a/spec/models/concerns/ci/partitionable_spec.rb b/spec/models/concerns/ci/partitionable_spec.rb index d41654e547e..6daafc78cff 100644 --- a/spec/models/concerns/ci/partitionable_spec.rb +++ b/spec/models/concerns/ci/partitionable_spec.rb @@ -25,7 +25,11 @@ RSpec.describe Ci::Partitionable do end context 'with through options' do + let(:disable_partitionable_switch) { nil } + before do + stub_env('DISABLE_PARTITIONABLE_SWITCH', disable_partitionable_switch) + allow(ActiveSupport::DescendantsTracker).to receive(:store_inherited) stub_const("#{described_class}::Testing::PARTITIONABLE_MODELS", [ci_model.name]) @@ -39,6 +43,12 @@ RSpec.describe Ci::Partitionable do it { expect(ci_model.routing_table_name_flag).to eq(:some_flag) } it { expect(ci_model.ancestors).to include(described_class::Switch) } + + context 'when DISABLE_PARTITIONABLE_SWITCH is set' do + let(:disable_partitionable_switch) { true } + + it { expect(ci_model.ancestors).not_to include(described_class::Switch) } + end end context 'with partitioned options' do diff --git a/spec/models/repository_spec.rb b/spec/models/repository_spec.rb index 9f9417c2483..72c966f8ca5 100644 --- a/spec/models/repository_spec.rb +++ b/spec/models/repository_spec.rb @@ -3804,4 +3804,22 @@ RSpec.describe Repository, feature_category: :source_code_management do include_examples 'does not delete branch' end end + + describe '#get_patch_id' do + it 'returns patch_id of given revisions' do + expect(repository.get_patch_id('HEAD~', 'HEAD')).to eq('45435e5d7b339dd76d939508c7687701d0c17fff') + end + + context 'when one of the param is invalid' do + it 'raises an ArgumentError error' do + expect { repository.get_patch_id('HEAD', nil) }.to raise_error(ArgumentError) + end + end + + context 'when two revisions are the same' do + it 'raises an Gitlab::Git::CommandError error' do + expect { repository.get_patch_id('HEAD', 'HEAD') }.to raise_error(Gitlab::Git::CommandError) + end + end + end end diff --git a/spec/requests/api/metrics/user_starred_dashboards_spec.rb b/spec/requests/api/metrics/user_starred_dashboards_spec.rb index 2ac7d3cffc8..bdeba777350 100644 --- a/spec/requests/api/metrics/user_starred_dashboards_spec.rb +++ b/spec/requests/api/metrics/user_starred_dashboards_spec.rb @@ -15,10 +15,6 @@ RSpec.describe API::Metrics::UserStarredDashboards, feature_category: :metrics d } end - before do - stub_feature_flags(remove_monitor_metrics: false) - end - describe 'POST /projects/:id/metrics/user_starred_dashboards' do before do project.add_reporter(user) @@ -26,13 +22,6 @@ RSpec.describe API::Metrics::UserStarredDashboards, feature_category: :metrics d context 'with correct permissions' do context 'with invalid parameters' do - it 'returns error message' do - post api(url, user), params: { dashboard_path: '' } - - expect(response).to have_gitlab_http_status(:bad_request) - expect(json_response['error']).to eq('dashboard_path is empty') - end - context 'user is missing' do it 'returns 404 not found' do post api(url, nil), params: params @@ -60,10 +49,6 @@ RSpec.describe API::Metrics::UserStarredDashboards, feature_category: :metrics d end context 'when metrics dashboard feature is unavailable' do - before do - stub_feature_flags(remove_monitor_metrics: true) - end - it 'returns 404 not found' do post api(url, user), params: params @@ -83,44 +68,6 @@ RSpec.describe API::Metrics::UserStarredDashboards, feature_category: :metrics d end context 'with correct permissions' do - context 'with valid parameters' do - context 'dashboard_path as url param url escaped' do - it 'deletes given user starred metrics dashboard', :aggregate_failures do - delete api(url, user), params: params - - expect(response).to have_gitlab_http_status(:ok) - expect(json_response['deleted_rows']).to eq(1) - expect(::Metrics::UsersStarredDashboard.all.pluck(:dashboard_path)).not_to include(dashboard) - end - end - - context 'dashboard_path in request body unescaped' do - let(:params) do - { - dashboard_path: dashboard - } - end - - it 'deletes given user starred metrics dashboard', :aggregate_failures do - delete api(url, user), params: params - - expect(response).to have_gitlab_http_status(:ok) - expect(json_response['deleted_rows']).to eq(1) - expect(::Metrics::UsersStarredDashboard.all.pluck(:dashboard_path)).not_to include(dashboard) - end - end - - context 'dashboard_path has not been specified' do - it 'deletes all starred dashboards for that user within given project', :aggregate_failures do - delete api(url, user), params: {} - - expect(response).to have_gitlab_http_status(:ok) - expect(json_response['deleted_rows']).to eq(2) - expect(::Metrics::UsersStarredDashboard.all).to contain_exactly(other_user_starred_dashboard, other_project_starred_dashboard) - end - end - end - context 'with invalid parameters' do context 'user is missing' do it 'returns 404 not found' do @@ -149,10 +96,6 @@ RSpec.describe API::Metrics::UserStarredDashboards, feature_category: :metrics d end context 'when metrics dashboard feature is unavailable' do - before do - stub_feature_flags(remove_monitor_metrics: true) - end - it 'returns 404 not found' do delete api(url, user), params: params diff --git a/spec/services/ci/delete_objects_service_spec.rb b/spec/services/ci/delete_objects_service_spec.rb index a64c0fa4076..939b72cef3b 100644 --- a/spec/services/ci/delete_objects_service_spec.rb +++ b/spec/services/ci/delete_objects_service_spec.rb @@ -4,203 +4,108 @@ require 'spec_helper' RSpec.describe Ci::DeleteObjectsService, :aggregate_failures, feature_category: :continuous_integration do let(:service) { described_class.new } + let(:artifact) { create(:ci_job_artifact, :archive) } let(:data) { [artifact] } describe '#execute' do - shared_examples_for 'deleting objects' do |store_type| - before do - Ci::DeletedObject.bulk_import(data) - # We disable the check because the specs are wrapped in a transaction - allow(service).to receive(:transaction_open?).and_return(false) - end + before do + Ci::DeletedObject.bulk_import(data) + # We disable the check because the specs are wrapped in a transaction + allow(service).to receive(:transaction_open?).and_return(false) + end - subject(:execute) { service.execute } + subject(:execute) { service.execute } - it 'deletes records' do - expect { execute }.to change { Ci::DeletedObject.count }.by(-1) - end + it 'deletes records' do + expect { execute }.to change { Ci::DeletedObject.count }.by(-1) + end - it 'deletes files' do - if store_type == :object_storage - expect { execute } - .to change { fog_connection.directories.get(bucket).files.any? } - else - expect { execute }.to change { artifact.file.exists? } - end - end + it 'deletes files' do + expect { execute }.to change { artifact.file.exists? } + end - context 'when trying to execute without records' do - let(:data) { [] } + context 'when trying to execute without records' do + let(:data) { [] } - it 'does not change the number of objects' do - expect { execute }.not_to change { Ci::DeletedObject.count } - end + it 'does not change the number of objects' do + expect { execute }.not_to change { Ci::DeletedObject.count } end + end - context 'when trying to remove the same file multiple times' do - let(:objects) { Ci::DeletedObject.all.to_a } - - before do - expect(service).to receive(:load_next_batch).twice.and_return(objects) - end + context 'when trying to remove the same file multiple times' do + let(:objects) { Ci::DeletedObject.all.to_a } - it 'executes successfully' do - 2.times { expect(service.execute).to be_truthy } - end + before do + expect(service).to receive(:load_next_batch).twice.and_return(objects) end - context 'with artifacts both ready and not ready for deletion' do - let(:data) { [] } - - let_it_be(:past_ready) { create(:ci_deleted_object, pick_up_at: 2.days.ago) } - let_it_be(:ready) { create(:ci_deleted_object, pick_up_at: 1.day.ago) } - - it 'skips records with pick_up_at in the future' do - not_ready = create(:ci_deleted_object, pick_up_at: 1.day.from_now) - - expect { execute }.to change { Ci::DeletedObject.count }.from(3).to(1) - expect(not_ready.reload.present?).to be_truthy - end - - it 'limits the number of records removed' do - stub_const("#{described_class}::BATCH_SIZE", 1) - - expect { execute }.to change { Ci::DeletedObject.count }.by(-1) - end - - it 'removes records in order' do - stub_const("#{described_class}::BATCH_SIZE", 1) - - execute - - expect { past_ready.reload }.to raise_error(ActiveRecord::RecordNotFound) - expect(ready.reload.present?).to be_truthy - end + it 'executes successfully' do + 2.times { expect(service.execute).to be_truthy } + end + end - it 'updates pick_up_at timestamp' do - allow(service).to receive(:destroy_everything) + context 'with artifacts both ready and not ready for deletion' do + let(:data) { [] } - execute + let_it_be(:past_ready) { create(:ci_deleted_object, pick_up_at: 2.days.ago) } + let_it_be(:ready) { create(:ci_deleted_object, pick_up_at: 1.day.ago) } - expect(past_ready.reload.pick_up_at).to be_like_time(10.minutes.from_now) - end + it 'skips records with pick_up_at in the future' do + not_ready = create(:ci_deleted_object, pick_up_at: 1.day.from_now) - it 'does not delete objects for which file deletion has failed' do - expect(past_ready) - .to receive(:delete_file_from_storage) - .and_return(false) + expect { execute }.to change { Ci::DeletedObject.count }.from(3).to(1) + expect(not_ready.reload.present?).to be_truthy + end - expect(service) - .to receive(:load_next_batch) - .and_return([past_ready, ready]) + it 'limits the number of records removed' do + stub_const("#{described_class}::BATCH_SIZE", 1) - expect { execute }.to change { Ci::DeletedObject.count }.from(2).to(1) - expect(past_ready.reload.present?).to be_truthy - end + expect { execute }.to change { Ci::DeletedObject.count }.by(-1) end - context 'with an open database transaction' do - it 'raises an exception and does not remove records' do - expect(service).to receive(:transaction_open?).and_return(true) + it 'removes records in order' do + stub_const("#{described_class}::BATCH_SIZE", 1) - expect { execute } - .to raise_error(Ci::DeleteObjectsService::TransactionInProgressError) - .and change { Ci::DeletedObject.count }.by(0) - end + execute + + expect { past_ready.reload }.to raise_error(ActiveRecord::RecordNotFound) + expect(ready.reload.present?).to be_truthy end - end - context 'when local storage is used' do - let(:artifact) { create(:ci_job_artifact, :archive) } + it 'updates pick_up_at timestamp' do + allow(service).to receive(:destroy_everything) - it_behaves_like 'deleting objects', :local_storage - end + execute - context 'when object storage is used' do - shared_examples_for 'deleting objects from object storage' do - let!(:fog_file) do - fog_connection.directories.new(key: bucket).files.create( # rubocop:disable Rails/SaveBang - key: fog_file_path, - body: 'content' - ) - end - - let(:artifact) do - create( - :ci_job_artifact, - :remote_store - ).tap do |a| - a.update_column(:file, 'artifacts.zip') - a.reload - end - end - - context 'and object was direct uploaded to its final location' do - let(:upload_path) { 'some/path/to/randomfile' } - - before do - artifact.update_column(:file_final_path, upload_path) - artifact.reload - end - - it_behaves_like 'deleting objects', :object_storage - end - - context 'and object was not direct uploaded to its final location' do - let(:upload_path) do - File.join( - artifact.file.store_dir.to_s, - artifact.file_identifier - ) - end - - it_behaves_like 'deleting objects', :object_storage - end + expect(past_ready.reload.pick_up_at).to be_like_time(10.minutes.from_now) end - context 'and bucket prefix is not configured' do - let(:fog_connection) do - stub_artifacts_object_storage - end + it 'does not delete objects for which file deletion has failed' do + expect(past_ready) + .to receive(:delete_file_from_storage) + .and_return(false) - let(:bucket) { 'artifacts' } - let(:fog_file_path) { upload_path } + expect(service) + .to receive(:load_next_batch) + .and_return([past_ready, ready]) - it_behaves_like 'deleting objects from object storage' + expect { execute }.to change { Ci::DeletedObject.count }.from(2).to(1) + expect(past_ready.reload.present?).to be_truthy end + end - context 'and bucket prefix is configured' do - let(:fog_config) do - Gitlab.config.artifacts.object_store.tap do |config| - config[:remote_directory] = bucket - config[:bucket_prefix] = bucket_prefix - end - end - - let(:fog_connection) do - stub_object_storage_uploader( - config: fog_config, - uploader: JobArtifactUploader - ) - end - - let(:bucket_prefix) { 'my/artifacts' } - let(:bucket) { 'main-bucket' } - let(:fog_file_path) { File.join(bucket_prefix, upload_path) } - - it_behaves_like 'deleting objects from object storage' + context 'with an open database transaction' do + it 'raises an exception and does not remove records' do + expect(service).to receive(:transaction_open?).and_return(true) + + expect { execute } + .to raise_error(Ci::DeleteObjectsService::TransactionInProgressError) + .and change { Ci::DeletedObject.count }.by(0) end end end describe '#remaining_batches_count' do - let(:artifact) do - create( - :ci_job_artifact, - :archive - ) - end - subject { service.remaining_batches_count(max_batch_count: 3) } context 'when there is less than one batch size' do diff --git a/spec/services/metrics/users_starred_dashboards/create_service_spec.rb b/spec/services/metrics/users_starred_dashboards/create_service_spec.rb deleted file mode 100644 index e08bdca8410..00000000000 --- a/spec/services/metrics/users_starred_dashboards/create_service_spec.rb +++ /dev/null @@ -1,73 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -RSpec.describe Metrics::UsersStarredDashboards::CreateService, feature_category: :metrics do - let_it_be(:user) { create(:user) } - - let(:dashboard_path) { 'config/prometheus/common_metrics.yml' } - let(:service_instance) { described_class.new(user, project, dashboard_path) } - let(:project) { create(:project) } - let(:starred_dashboard_params) do - { - user: user, - project: project, - dashboard_path: dashboard_path - } - end - - shared_examples 'prevented starred dashboard creation' do |message| - it 'returns error response', :aggregate_failures do - expect(Metrics::UsersStarredDashboard).not_to receive(:new) - - response = service_instance.execute - - expect(response.status).to be :error - expect(response.message).to eql message - end - end - - describe '.execute' do - context 'with anonymous user' do - it_behaves_like 'prevented starred dashboard creation', 'You are not authorized to add star to this dashboard' - end - - context 'with reporter user' do - before do - project.add_reporter(user) - end - - context 'incorrect dashboard_path' do - let(:dashboard_path) { 'something_incorrect.yml' } - - it_behaves_like 'prevented starred dashboard creation', 'Dashboard with requested path can not be found' - end - - context 'with valid dashboard path' do - it 'creates starred dashboard and returns success response', :aggregate_failures do - expect_next_instance_of(Metrics::UsersStarredDashboard, starred_dashboard_params) do |starred_dashboard| - expect(starred_dashboard).to receive(:save).and_return true - end - - response = service_instance.execute - - expect(response.status).to be :success - end - - context 'Metrics::UsersStarredDashboard has validation errors' do - it 'returns error response', :aggregate_failures do - expect_next_instance_of(Metrics::UsersStarredDashboard, starred_dashboard_params) do |starred_dashboard| - expect(starred_dashboard).to receive(:save).and_return(false) - expect(starred_dashboard).to receive(:errors).and_return(double(messages: { base: ['Model validation error'] })) - end - - response = service_instance.execute - - expect(response.status).to be :error - expect(response.message).to eql(base: ['Model validation error']) - end - end - end - end - end -end diff --git a/spec/services/metrics/users_starred_dashboards/delete_service_spec.rb b/spec/services/metrics/users_starred_dashboards/delete_service_spec.rb deleted file mode 100644 index 8c4bcecc239..00000000000 --- a/spec/services/metrics/users_starred_dashboards/delete_service_spec.rb +++ /dev/null @@ -1,41 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -RSpec.describe Metrics::UsersStarredDashboards::DeleteService, feature_category: :metrics do - subject(:service_instance) { described_class.new(user, project, dashboard_path) } - - let_it_be(:user) { create(:user) } - let_it_be(:project) { create(:project) } - - describe '#execute' do - let_it_be(:user_starred_dashboard_1) { create(:metrics_users_starred_dashboard, user: user, project: project, dashboard_path: 'dashboard_1') } - let_it_be(:user_starred_dashboard_2) { create(:metrics_users_starred_dashboard, user: user, project: project) } - let_it_be(:other_user_starred_dashboard) { create(:metrics_users_starred_dashboard, project: project) } - let_it_be(:other_project_starred_dashboard) { create(:metrics_users_starred_dashboard, user: user) } - - context 'without dashboard_path' do - let(:dashboard_path) { nil } - - it 'does not scope user starred dashboards by dashboard path' do - result = service_instance.execute - - expect(result.success?).to be_truthy - expect(result.payload[:deleted_rows]).to be(2) - expect(Metrics::UsersStarredDashboard.all).to contain_exactly(other_user_starred_dashboard, other_project_starred_dashboard) - end - end - - context 'with dashboard_path' do - let(:dashboard_path) { 'dashboard_1' } - - it 'does scope user starred dashboards by dashboard path' do - result = service_instance.execute - - expect(result.success?).to be_truthy - expect(result.payload[:deleted_rows]).to be(1) - expect(Metrics::UsersStarredDashboard.all).to contain_exactly(user_starred_dashboard_2, other_user_starred_dashboard, other_project_starred_dashboard) - end - end - end -end diff --git a/spec/support/helpers/models/ci/partitioning_testing/schema_helpers.rb b/spec/support/helpers/models/ci/partitioning_testing/schema_helpers.rb index df653c853b9..a47aaffdb43 100644 --- a/spec/support/helpers/models/ci/partitioning_testing/schema_helpers.rb +++ b/spec/support/helpers/models/ci/partitioning_testing/schema_helpers.rb @@ -6,15 +6,12 @@ module Ci module_function def with_routing_tables - previous_table_name = CommitStatus.table_name - CommitStatus.table_name = :p_ci_builds - CommitStatus.descendants.each(&:reset_table_name) + # previous_table_name = Model.table_name + # Model.table_name = routing_table_name yield - - ensure - CommitStatus.table_name = previous_table_name - CommitStatus.descendants.each(&:reset_table_name) + # ensure + # Model.table_name = previous_table_name end def setup(connection: Ci::ApplicationRecord.connection) -- cgit v1.2.3