rich
', // Note the leading space }; @@ -328,6 +333,12 @@ describe('DiffsStoreUtils', () => { hasForm: false, text: undefined, alreadyPrepared: true, + commentsDisabled: false, + problems: { + brokenLineCode: false, + brokenSymlink: false, + fileOnlyMoved: false, + }, }; let preppedLine; @@ -360,24 +371,35 @@ describe('DiffsStoreUtils', () => { }); it.each` - brokenSymlink - ${false} - ${{}} - ${'anything except `false`'} + brokenSymlink | renamed | added | removed | lineCode | commentsDisabled + ${false} | ${false} | ${0} | ${0} | ${'a'} | ${false} + ${{}} | ${false} | ${1} | ${1} | ${'a'} | ${true} + ${'truthy'} | ${false} | ${1} | ${1} | ${'a'} | ${true} + ${false} | ${true} | ${1} | ${1} | ${'a'} | ${false} + ${false} | ${true} | ${1} | ${0} | ${'a'} | ${false} + ${false} | ${true} | ${0} | ${1} | ${'a'} | ${false} + ${false} | ${true} | ${0} | ${0} | ${'a'} | ${true} `( - "properly assigns each line's `commentsDisabled` as the same value as the parent file's `brokenSymlink` value (`$brokenSymlink`)", - ({ brokenSymlink }) => { - preppedLine = utils.prepareLineForRenamedFile({ - diffViewType: INLINE_DIFF_VIEW_TYPE, - line: sourceLine, + "properly sets a line's `commentsDisabled` to '$commentsDisabled' for file and line settings { brokenSymlink: $brokenSymlink, renamed: $renamed, added: $added, removed: $removed, line_code: $lineCode }", + ({ brokenSymlink, renamed, added, removed, lineCode, commentsDisabled }) => { + const line = { + ...sourceLine, + line_code: lineCode, + }; + const file = { + ...diffFile, + brokenSymlink, + renamed_file: renamed, + added_lines: added, + removed_lines: removed, + }; + const preparedLine = utils.prepareLineForRenamedFile({ index: lineIndex, - diffFile: { - ...diffFile, - brokenSymlink, - }, + diffFile: file, + line, }); - expect(preppedLine.commentsDisabled).toStrictEqual(brokenSymlink); + expect(preparedLine.commentsDisabled).toBe(commentsDisabled); }, ); }); @@ -477,7 +499,7 @@ describe('DiffsStoreUtils', () => { it('adds the `.brokenSymlink` property to each diff file', () => { preparedDiff.diff_files.forEach((file) => { - expect(file).toEqual(expect.objectContaining({ brokenSymlink: false })); + expect(file).toHaveProperty('brokenSymlink', false); }); }); @@ -490,7 +512,7 @@ describe('DiffsStoreUtils', () => { ].flatMap((file) => [...file[INLINE_DIFF_LINES_KEY]]); lines.forEach((line) => { - expect(line.commentsDisabled).toBe(false); + expect(line.problems.brokenSymlink).toBe(false); }); }); }); diff --git a/spec/frontend/fixtures/job_artifacts.rb b/spec/frontend/fixtures/job_artifacts.rb new file mode 100644 index 00000000000..e53cdbbaaa5 --- /dev/null +++ b/spec/frontend/fixtures/job_artifacts.rb @@ -0,0 +1,28 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe 'Job Artifacts (GraphQL fixtures)' do + describe GraphQL::Query, type: :request do + include ApiHelpers + include GraphqlHelpers + include JavaScriptFixturesHelpers + + let_it_be(:project) { create(:project, :repository, :public) } + let_it_be(:pipeline) { create(:ci_pipeline, project: project) } + let_it_be(:user) { create(:user) } + + job_artifacts_query_path = 'artifacts/graphql/queries/get_job_artifacts.query.graphql' + + it "graphql/#{job_artifacts_query_path}.json" do + create(:ci_build, :failed, :artifacts, :trace_artifact, pipeline: pipeline) + create(:ci_build, :success, :artifacts, :trace_artifact, pipeline: pipeline) + + query = get_graphql_query_as_string(job_artifacts_query_path) + + post_graphql(query, current_user: user, variables: { projectPath: project.full_path }) + + expect_graphql_errors_to_be_empty + end + end +end diff --git a/spec/frontend/sidebar/components/reviewers/sidebar_reviewers_inputs_spec.js b/spec/frontend/sidebar/components/reviewers/sidebar_reviewers_inputs_spec.js new file mode 100644 index 00000000000..277ef6d9561 --- /dev/null +++ b/spec/frontend/sidebar/components/reviewers/sidebar_reviewers_inputs_spec.js @@ -0,0 +1,36 @@ +import { shallowMount } from '@vue/test-utils'; +import SidebarReviewersInputs from '~/sidebar/components/reviewers/sidebar_reviewers_inputs.vue'; +import { state } from '~/sidebar/components/reviewers/sidebar_reviewers.vue'; + +let wrapper; + +function factory() { + wrapper = shallowMount(SidebarReviewersInputs); +} + +describe('Sidebar reviewers inputs component', () => { + it('renders hidden input', () => { + state.issuable.reviewers = { + nodes: [ + { + id: 1, + avatarUrl: '', + name: 'root', + username: 'root', + mergeRequestInteraction: { canMerge: true }, + }, + { + id: 2, + avatarUrl: '', + name: 'root', + username: 'root', + mergeRequestInteraction: { canMerge: true }, + }, + ], + }; + + factory(); + + expect(wrapper.findAll('input[type="hidden"]').length).toBe(2); + }); +}); diff --git a/spec/lib/gitlab/database/migration_helpers_spec.rb b/spec/lib/gitlab/database/migration_helpers_spec.rb index f6a2f939ce3..64a36b88625 100644 --- a/spec/lib/gitlab/database/migration_helpers_spec.rb +++ b/spec/lib/gitlab/database/migration_helpers_spec.rb @@ -2526,48 +2526,6 @@ RSpec.describe Gitlab::Database::MigrationHelpers do end end - describe '#with_lock_retries' do - let(:buffer) { StringIO.new } - let(:in_memory_logger) { Gitlab::JsonLogger.new(buffer) } - let(:env) { { 'DISABLE_LOCK_RETRIES' => 'true' } } - - it 'sets the migration class name in the logs' do - model.with_lock_retries(env: env, logger: in_memory_logger) {} - - buffer.rewind - expect(buffer.read).to include("\"class\":\"#{model.class}\"") - end - - where(raise_on_exhaustion: [true, false]) - - with_them do - it 'sets raise_on_exhaustion as requested' do - with_lock_retries = double - expect(Gitlab::Database::WithLockRetries).to receive(:new).and_return(with_lock_retries) - expect(with_lock_retries).to receive(:run).with(raise_on_exhaustion: raise_on_exhaustion) - - model.with_lock_retries(env: env, logger: in_memory_logger, raise_on_exhaustion: raise_on_exhaustion) {} - end - end - - it 'does not raise on exhaustion by default' do - with_lock_retries = double - expect(Gitlab::Database::WithLockRetries).to receive(:new).and_return(with_lock_retries) - expect(with_lock_retries).to receive(:run).with(raise_on_exhaustion: false) - - model.with_lock_retries(env: env, logger: in_memory_logger) {} - end - - it 'defaults to allowing subtransactions' do - with_lock_retries = double - - expect(Gitlab::Database::WithLockRetries).to receive(:new).with(hash_including(allow_savepoints: true)).and_return(with_lock_retries) - expect(with_lock_retries).to receive(:run).with(raise_on_exhaustion: false) - - model.with_lock_retries(env: env, logger: in_memory_logger) {} - end - end - describe '#backfill_iids' do include MigrationsHelpers diff --git a/spec/lib/gitlab/database/migrations/lock_retries_helpers_spec.rb b/spec/lib/gitlab/database/migrations/lock_retries_helpers_spec.rb new file mode 100644 index 00000000000..a8739f6758f --- /dev/null +++ b/spec/lib/gitlab/database/migrations/lock_retries_helpers_spec.rb @@ -0,0 +1,52 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Gitlab::Database::Migrations::LockRetriesHelpers do + let(:model) do + ActiveRecord::Migration.new.extend(described_class) + end + + describe '#with_lock_retries' do + let(:buffer) { StringIO.new } + let(:in_memory_logger) { Gitlab::JsonLogger.new(buffer) } + let(:env) { { 'DISABLE_LOCK_RETRIES' => 'true' } } + + it 'sets the migration class name in the logs' do + model.with_lock_retries(env: env, logger: in_memory_logger) {} + + buffer.rewind + expect(buffer.read).to include("\"class\":\"#{model.class}\"") + end + + where(raise_on_exhaustion: [true, false]) + + with_them do + it 'sets raise_on_exhaustion as requested' do + with_lock_retries = double + expect(Gitlab::Database::WithLockRetries).to receive(:new).and_return(with_lock_retries) + expect(with_lock_retries).to receive(:run).with(raise_on_exhaustion: raise_on_exhaustion) + + model.with_lock_retries(env: env, logger: in_memory_logger, raise_on_exhaustion: raise_on_exhaustion) {} + end + end + + it 'does not raise on exhaustion by default' do + with_lock_retries = double + expect(Gitlab::Database::WithLockRetries).to receive(:new).and_return(with_lock_retries) + expect(with_lock_retries).to receive(:run).with(raise_on_exhaustion: false) + + model.with_lock_retries(env: env, logger: in_memory_logger) {} + end + + it 'defaults to allowing subtransactions' do + with_lock_retries = double + + expect(Gitlab::Database::WithLockRetries) + .to receive(:new).with(hash_including(allow_savepoints: true)).and_return(with_lock_retries) + expect(with_lock_retries).to receive(:run).with(raise_on_exhaustion: false) + + model.with_lock_retries(env: env, logger: in_memory_logger) {} + end + end +end diff --git a/spec/lib/gitlab/import_export/all_models.yml b/spec/lib/gitlab/import_export/all_models.yml index ccc4f1f7149..a130438ce99 100644 --- a/spec/lib/gitlab/import_export/all_models.yml +++ b/spec/lib/gitlab/import_export/all_models.yml @@ -361,6 +361,7 @@ hooks: - web_hook_logs protected_branches: - project +- group - merge_access_levels - push_access_levels - unprotect_access_levels diff --git a/spec/lib/gitlab/import_export/decompressed_archive_size_validator_spec.rb b/spec/lib/gitlab/import_export/decompressed_archive_size_validator_spec.rb index 9af72cc0dea..a6cb74c3c9f 100644 --- a/spec/lib/gitlab/import_export/decompressed_archive_size_validator_spec.rb +++ b/spec/lib/gitlab/import_export/decompressed_archive_size_validator_spec.rb @@ -112,7 +112,7 @@ RSpec.describe Gitlab::ImportExport::DecompressedArchiveSizeValidator do context 'when archive path is not a string' do let(:filepath) { 123 } - let(:error_message) { 'Archive path is not a string' } + let(:error_message) { 'Invalid path' } it 'returns false' do expect(subject.valid?).to eq(false) diff --git a/spec/lib/gitlab/utils_spec.rb b/spec/lib/gitlab/utils_spec.rb index d1fdaf7a9db..80b2ec63af9 100644 --- a/spec/lib/gitlab/utils_spec.rb +++ b/spec/lib/gitlab/utils_spec.rb @@ -63,9 +63,21 @@ RSpec.describe Gitlab::Utils do expect(check_path_traversal!('dir/.foo.rb')).to eq('dir/.foo.rb') end - it 'does nothing for a non-string' do + it 'does nothing for nil' do expect(check_path_traversal!(nil)).to be_nil end + + it 'does nothing for safe HashedPath' do + expect(check_path_traversal!(Gitlab::HashedPath.new('tmp', root_hash: 1))).to eq '6b/86/6b86b273ff34fce19d6b804eff5a3f5747ada4eaa22f1d49c01e52ddb7875b4b/tmp' + end + + it 'raises for unsafe HashedPath' do + expect { check_path_traversal!(Gitlab::HashedPath.new('tmp', '..', 'etc', 'passwd', root_hash: 1)) }.to raise_error(/Invalid path/) + end + + it 'raises for other non-strings' do + expect { check_path_traversal!(%w[/tmp /tmp/../etc/passwd]) }.to raise_error(/Invalid path/) + end end describe '.check_allowed_absolute_path_and_path_traversal!' do diff --git a/spec/models/ci/sources/pipeline_spec.rb b/spec/models/ci/sources/pipeline_spec.rb index 732dd5c3df3..fdc1c111c40 100644 --- a/spec/models/ci/sources/pipeline_spec.rb +++ b/spec/models/ci/sources/pipeline_spec.rb @@ -20,14 +20,14 @@ RSpec.describe Ci::Sources::Pipeline do context 'loose foreign key on ci_sources_pipelines.source_project_id' do it_behaves_like 'cleanup by a loose foreign key' do - let!(:parent) { create(:project) } + let!(:parent) { create(:project, namespace: create(:group)) } let!(:model) { create(:ci_sources_pipeline, source_project: parent) } end end context 'loose foreign key on ci_sources_pipelines.project_id' do it_behaves_like 'cleanup by a loose foreign key' do - let!(:parent) { create(:project) } + let!(:parent) { create(:project, namespace: create(:group)) } let!(:model) { create(:ci_sources_pipeline, project: parent) } end end diff --git a/spec/models/ci/unit_test_spec.rb b/spec/models/ci/unit_test_spec.rb index b3180492a36..e35a4ce40da 100644 --- a/spec/models/ci/unit_test_spec.rb +++ b/spec/models/ci/unit_test_spec.rb @@ -4,7 +4,7 @@ require 'spec_helper' RSpec.describe Ci::UnitTest do it_behaves_like 'cleanup by a loose foreign key' do - let!(:parent) { create(:project) } + let!(:parent) { create(:project, namespace: create(:group)) } let!(:model) { create(:ci_unit_test, project: parent) } end diff --git a/spec/models/group_spec.rb b/spec/models/group_spec.rb index 68c2d1d3995..eb9ef8a21e9 100644 --- a/spec/models/group_spec.rb +++ b/spec/models/group_spec.rb @@ -40,6 +40,7 @@ RSpec.describe Group do it { is_expected.to have_many(:bulk_import_exports).class_name('BulkImports::Export') } it { is_expected.to have_many(:contacts).class_name('CustomerRelations::Contact') } it { is_expected.to have_many(:organizations).class_name('CustomerRelations::Organization') } + it { is_expected.to have_many(:protected_branches) } it { is_expected.to have_one(:crm_settings) } it { is_expected.to have_one(:group_feature) } it { is_expected.to have_one(:harbor_integration) } diff --git a/spec/models/integration_spec.rb b/spec/models/integration_spec.rb index baa3443b4c5..5dc004d661c 100644 --- a/spec/models/integration_spec.rb +++ b/spec/models/integration_spec.rb @@ -60,10 +60,10 @@ RSpec.describe Integration do describe 'Scopes' do describe '.third_party_wikis' do - let!(:integration1) { create(:jira_integration) } - let!(:integration2) { create(:redmine_integration) } - let!(:integration3) { create(:confluence_integration) } - let!(:integration4) { create(:shimo_integration) } + let!(:integration1) { create(:jira_integration, project: project) } + let!(:integration2) { create(:redmine_integration, project: project) } + let!(:integration3) { create(:confluence_integration, project: project) } + let!(:integration4) { create(:shimo_integration, project: project) } it 'returns the right group integration' do expect(described_class.third_party_wikis).to contain_exactly(integration3, integration4) @@ -89,7 +89,7 @@ RSpec.describe Integration do end describe '.by_type' do - let!(:integration1) { create(:jira_integration) } + let!(:integration1) { create(:jira_integration, project: project) } let!(:integration2) { create(:jira_integration) } let!(:integration3) { create(:redmine_integration) } @@ -110,7 +110,7 @@ RSpec.describe Integration do describe '.for_group' do let!(:integration1) { create(:jira_integration, project_id: nil, group_id: group.id) } - let!(:integration2) { create(:jira_integration) } + let!(:integration2) { create(:jira_integration, project: project) } it 'returns the right group integration' do expect(described_class.for_group(group)).to contain_exactly(integration1) @@ -219,7 +219,7 @@ RSpec.describe Integration do describe '.find_or_initialize_non_project_specific_integration' do let!(:integration_1) { create(:jira_integration, project_id: nil, group_id: group.id) } - let!(:integration_2) { create(:jira_integration) } + let!(:integration_2) { create(:jira_integration, project: project) } it 'returns the right integration' do expect(Integration.find_or_initialize_non_project_specific_integration('jira', group_id: group)) @@ -374,7 +374,7 @@ RSpec.describe Integration do context 'when data is stored in properties' do let(:properties) { data_params } let!(:integration) do - create(:jira_integration, :without_properties_callback, properties: properties.merge(additional: 'something')) + create(:jira_integration, :without_properties_callback, project: project, properties: properties.merge(additional: 'something')) end it_behaves_like 'integration creation from an integration' @@ -382,7 +382,7 @@ RSpec.describe Integration do context 'when data are stored in separated fields' do let(:integration) do - create(:jira_integration, data_params.merge(properties: {})) + create(:jira_integration, data_params.merge(properties: {}, project: project)) end it_behaves_like 'integration creation from an integration' @@ -391,7 +391,7 @@ RSpec.describe Integration do context 'when data are stored in both properties and separated fields' do let(:properties) { data_params } let(:integration) do - create(:jira_integration, :without_properties_callback, active: true, properties: properties).tap do |integration| + create(:jira_integration, :without_properties_callback, project: project, active: true, properties: properties).tap do |integration| create(:jira_tracker_data, data_params.merge(integration: integration)) end end @@ -1233,11 +1233,11 @@ RSpec.describe Integration do describe '#attributes' do it 'does not include properties' do - expect(create(:integration).attributes).not_to have_key('properties') + expect(build(:integration, project: project).attributes).not_to have_key('properties') end it 'can be used in assign_attributes without nullifying properties' do - record = create(:integration, :instance, properties: { url: generate(:url) }) + record = build(:integration, :instance, properties: { url: generate(:url) }) attrs = record.attributes @@ -1246,7 +1246,7 @@ RSpec.describe Integration do end describe '#dup' do - let(:original) { create(:integration, properties: { one: 1, two: 2, three: 3 }) } + let(:original) { build(:integration, project: project, properties: { one: 1, two: 2, three: 3 }) } it 'results in distinct ciphertexts, but identical properties' do copy = original.dup @@ -1259,7 +1259,7 @@ RSpec.describe Integration do end context 'when the model supports data-fields' do - let(:original) { create(:jira_integration, username: generate(:username), url: generate(:url)) } + let(:original) { build(:jira_integration, project: project, username: generate(:username), url: generate(:url)) } it 'creates distinct but identical data-fields' do copy = original.dup diff --git a/spec/models/integrations/base_chat_notification_spec.rb b/spec/models/integrations/base_chat_notification_spec.rb index eb503e501d6..e9d931bb564 100644 --- a/spec/models/integrations/base_chat_notification_spec.rb +++ b/spec/models/integrations/base_chat_notification_spec.rb @@ -19,7 +19,7 @@ RSpec.describe Integrations::BaseChatNotification do let_it_be(:project) { create(:project, :repository) } - let(:user) { create(:user) } + let(:user) { build_stubbed(:user) } let(:webhook_url) { 'https://example.gitlab.com/' } let(:data) { Gitlab::DataBuilder::Push.build_sample(subject.project, user) } @@ -44,7 +44,7 @@ RSpec.describe Integrations::BaseChatNotification do context 'with an empty repository' do it 'returns true' do - subject.project = create(:project, :empty_repo) + subject.project = build_stubbed(:project, :empty_repo) expect(chat_integration).to receive(:notify).and_return(true) expect(chat_integration.execute(data)).to be true @@ -61,9 +61,9 @@ RSpec.describe Integrations::BaseChatNotification do end context 'when the data object has a label' do - let_it_be(:label) { create(:label, name: 'Bug') } - let_it_be(:label_2) { create(:label, name: 'Community contribution') } - let_it_be(:label_3) { create(:label, name: 'Backend') } + let_it_be(:label) { create(:label, project: project, name: 'Bug') } + let_it_be(:label_2) { create(:label, project: project, name: 'Community contribution') } + let_it_be(:label_3) { create(:label, project: project, name: 'Backend') } let_it_be(:issue) { create(:labeled_issue, project: project, labels: [label, label_2, label_3]) } let_it_be(:note) { create(:note, noteable: issue, project: project) } @@ -93,7 +93,7 @@ RSpec.describe Integrations::BaseChatNotification do it_behaves_like 'notifies the chat integration' context 'MergeRequest events' do - let(:data) { create(:merge_request, labels: [label]).to_hook_data(user) } + let(:data) { build_stubbed(:merge_request, source_project: project, labels: [label]).to_hook_data(user) } it_behaves_like 'notifies the chat integration' end diff --git a/spec/models/integrations/buildkite_spec.rb b/spec/models/integrations/buildkite_spec.rb index c720dc6d418..ef686c0ae3c 100644 --- a/spec/models/integrations/buildkite_spec.rb +++ b/spec/models/integrations/buildkite_spec.rb @@ -6,7 +6,7 @@ RSpec.describe Integrations::Buildkite, :use_clean_rails_memory_store_caching do include ReactiveCachingHelpers include StubRequests - let(:project) { create(:project) } + let_it_be(:project) { create(:project) } subject(:integration) do described_class.create!( diff --git a/spec/models/integrations/campfire_spec.rb b/spec/models/integrations/campfire_spec.rb index a6bcd22b6f6..ae923cd38fc 100644 --- a/spec/models/integrations/campfire_spec.rb +++ b/spec/models/integrations/campfire_spec.rb @@ -34,8 +34,8 @@ RSpec.describe Integrations::Campfire do end describe "#execute" do - let(:user) { create(:user) } - let(:project) { create(:project, :repository) } + let(:user) { build_stubbed(:user) } + let(:project) { build_stubbed(:project, :repository) } before do @campfire_integration = described_class.new diff --git a/spec/models/integrations/confluence_spec.rb b/spec/models/integrations/confluence_spec.rb index e2f9316bc95..999a532527d 100644 --- a/spec/models/integrations/confluence_spec.rb +++ b/spec/models/integrations/confluence_spec.rb @@ -3,6 +3,8 @@ require 'spec_helper' RSpec.describe Integrations::Confluence do + let_it_be(:project) { create(:project) } + describe 'Validations' do before do subject.active = active @@ -40,7 +42,6 @@ RSpec.describe Integrations::Confluence do describe '#help' do it 'can correctly return a link to the project wiki when active' do - project = create(:project) subject.project = project subject.active = true @@ -62,8 +63,6 @@ RSpec.describe Integrations::Confluence do end describe 'Caching has_confluence on project_settings' do - let(:project) { create(:project) } - subject { project.project_setting.has_confluence? } it 'sets the property to true when integration is active' do diff --git a/spec/models/integrations/discord_spec.rb b/spec/models/integrations/discord_spec.rb index eb90acc73be..138a56d1872 100644 --- a/spec/models/integrations/discord_spec.rb +++ b/spec/models/integrations/discord_spec.rb @@ -25,7 +25,7 @@ RSpec.describe Integrations::Discord do let_it_be(:project) { create(:project, :repository) } - let(:user) { create(:user) } + let(:user) { build_stubbed(:user) } let(:webhook_url) { "https://example.gitlab.com/" } let(:sample_data) do Gitlab::DataBuilder::Push.build_sample(project, user) diff --git a/spec/models/integrations/drone_ci_spec.rb b/spec/models/integrations/drone_ci_spec.rb index f3203a6e69d..59961b37c20 100644 --- a/spec/models/integrations/drone_ci_spec.rb +++ b/spec/models/integrations/drone_ci_spec.rb @@ -7,6 +7,8 @@ RSpec.describe Integrations::DroneCi, :use_clean_rails_memory_store_caching do subject(:integration) { described_class.new } + let_it_be(:project) { create(:project, :repository, name: 'project') } + it_behaves_like Integrations::ResetSecretFields do let(:integration) { subject } end @@ -43,7 +45,6 @@ RSpec.describe Integrations::DroneCi, :use_clean_rails_memory_store_caching do ) end - let(:project) { create(:project, :repository, name: 'project') } let(:path) { project.full_path } let(:drone_url) { 'http://drone.example.com' } let(:sha) { '2ab7834c' } @@ -192,7 +193,7 @@ RSpec.describe Integrations::DroneCi, :use_clean_rails_memory_store_caching do describe "execute" do include_context :drone_ci_integration - let(:user) { create(:user, username: 'username') } + let(:user) { build(:user, username: 'username') } let(:push_sample_data) do Gitlab::DataBuilder::Push.build_sample(project, user) end diff --git a/spec/models/integrations/emails_on_push_spec.rb b/spec/models/integrations/emails_on_push_spec.rb index 15aa105e379..b3fe6bf9506 100644 --- a/spec/models/integrations/emails_on_push_spec.rb +++ b/spec/models/integrations/emails_on_push_spec.rb @@ -87,8 +87,8 @@ RSpec.describe Integrations::EmailsOnPush do end describe '#execute' do + let_it_be(:project) { create(:project, :repository) } let(:push_data) { { object_kind: 'push' } } - let(:project) { create(:project, :repository) } let(:integration) { create(:emails_on_push_integration, project: project) } let(:recipients) { 'test@gitlab.com' } diff --git a/spec/models/integrations/hangouts_chat_spec.rb b/spec/models/integrations/hangouts_chat_spec.rb index 828bcdf5d8f..288478b494e 100644 --- a/spec/models/integrations/hangouts_chat_spec.rb +++ b/spec/models/integrations/hangouts_chat_spec.rb @@ -46,7 +46,7 @@ RSpec.describe Integrations::HangoutsChat do end context 'with issue events' do - let(:issues_sample_data) { create(:issue).to_hook_data(user) } + let(:issues_sample_data) { create(:issue, project: project).to_hook_data(user) } it "adds thread key for issue events" do expect(chat_integration.execute(issues_sample_data)).to be(true) @@ -58,7 +58,7 @@ RSpec.describe Integrations::HangoutsChat do end context 'with merge events' do - let(:merge_sample_data) { create(:merge_request).to_hook_data(user) } + let(:merge_sample_data) { create(:merge_request, source_project: project).to_hook_data(user) } it "adds thread key for merge events" do expect(chat_integration.execute(merge_sample_data)).to be(true) @@ -71,7 +71,7 @@ RSpec.describe Integrations::HangoutsChat do context 'with wiki page events' do let(:wiki_page_sample_data) do - Gitlab::DataBuilder::WikiPage.build(create(:wiki_page, message: 'foo'), user, 'create') + Gitlab::DataBuilder::WikiPage.build(create(:wiki_page, project: project, message: 'foo'), user, 'create') end it "adds thread key for wiki page events" do diff --git a/spec/models/integrations/harbor_spec.rb b/spec/models/integrations/harbor_spec.rb index 9ab37a92e89..b4580028112 100644 --- a/spec/models/integrations/harbor_spec.rb +++ b/spec/models/integrations/harbor_spec.rb @@ -70,7 +70,7 @@ RSpec.describe Integrations::Harbor do end context 'ci variables' do - let(:harbor_integration) { create(:harbor_integration) } + let(:harbor_integration) { build_stubbed(:harbor_integration) } it 'returns vars when harbor_integration is activated' do ci_vars = [ @@ -85,9 +85,12 @@ RSpec.describe Integrations::Harbor do expect(harbor_integration.ci_variables).to match_array(ci_vars) end - it 'returns [] when harbor_integration is inactive' do - harbor_integration.update!(active: false) - expect(harbor_integration.ci_variables).to match_array([]) + context 'when harbor_integration is inactive' do + let(:harbor_integration) { build_stubbed(:harbor_integration, active: false) } + + it 'returns []' do + expect(harbor_integration.ci_variables).to match_array([]) + end end context 'with robot username' do diff --git a/spec/models/integrations/jenkins_spec.rb b/spec/models/integrations/jenkins_spec.rb index 200de1305e2..929473b0f02 100644 --- a/spec/models/integrations/jenkins_spec.rb +++ b/spec/models/integrations/jenkins_spec.rb @@ -3,7 +3,7 @@ require 'spec_helper' RSpec.describe Integrations::Jenkins do - let(:project) { create(:project) } + let_it_be(:project) { create(:project) } let(:jenkins_integration) { described_class.new(jenkins_params) } let(:jenkins_url) { 'http://jenkins.example.com/' } let(:jenkins_hook_url) { jenkins_url + 'project/my_project' } @@ -144,8 +144,7 @@ RSpec.describe Integrations::Jenkins do describe '#test' do it 'returns the right status' do - user = create(:user, username: 'username') - project = create(:project, name: 'project') + user = build(:user, username: 'username') push_sample_data = Gitlab::DataBuilder::Push.build_sample(project, user) jenkins_integration = described_class.create!(jenkins_params) stub_request(:post, jenkins_hook_url).with(headers: { 'Authorization' => jenkins_authorization }) @@ -157,9 +156,9 @@ RSpec.describe Integrations::Jenkins do end describe '#execute' do - let(:user) { create(:user, username: 'username') } - let(:namespace) { create(:group, :private) } - let(:project) { create(:project, :private, name: 'project', namespace: namespace) } + let(:user) { build(:user, username: 'username') } + let_it_be(:namespace) { create(:group, :private) } + let_it_be(:project) { create(:project, :private, name: 'project', namespace: namespace) } let(:push_sample_data) { Gitlab::DataBuilder::Push.build_sample(project, user) } let(:jenkins_integration) { described_class.create!(jenkins_params) } @@ -193,8 +192,6 @@ RSpec.describe Integrations::Jenkins do end describe 'Stored password invalidation' do - let(:project) { create(:project) } - context 'when a password was previously set' do let(:jenkins_integration) do described_class.create!( @@ -249,7 +246,7 @@ RSpec.describe Integrations::Jenkins do context 'when no password was previously set' do let(:jenkins_integration) do described_class.create!( - project: create(:project), + project: project, properties: { jenkins_url: 'http://jenkins.example.com/', username: 'jenkins' diff --git a/spec/models/integrations/jira_spec.rb b/spec/models/integrations/jira_spec.rb index 9f928442b28..4c8c19cb3e5 100644 --- a/spec/models/integrations/jira_spec.rb +++ b/spec/models/integrations/jira_spec.rb @@ -162,7 +162,7 @@ RSpec.describe Integrations::Jira do end describe '#fields' do - let(:integration) { create(:jira_integration) } + let(:integration) { jira_integration } subject(:fields) { integration.fields } @@ -172,7 +172,7 @@ RSpec.describe Integrations::Jira do end describe '#sections' do - let(:integration) { create(:jira_integration) } + let(:integration) { jira_integration } subject(:sections) { integration.sections.map { |s| s[:type] } } @@ -332,28 +332,7 @@ RSpec.describe Integrations::Jira do # we need to make sure we are able to read both from properties and jira_tracker_data table # TODO: change this as part of https://gitlab.com/gitlab-org/gitlab/issues/29404 context 'overriding properties' do - let(:access_params) do - { url: url, api_url: api_url, username: username, password: password, - jira_issue_transition_id: transition_id } - end - - let(:data_params) do - { - url: url, api_url: api_url, - username: username, password: password, - jira_issue_transition_id: transition_id - } - end - shared_examples 'handles jira fields' do - let(:data_params) do - { - url: url, api_url: api_url, - username: username, password: password, - jira_issue_transition_id: transition_id - } - end - context 'reading data' do it 'reads data correctly' do expect(integration.url).to eq(url) @@ -449,32 +428,40 @@ RSpec.describe Integrations::Jira do end # this will be removed as part of https://gitlab.com/gitlab-org/gitlab/issues/29404 - context 'when data are stored in properties' do - let(:properties) { data_params } - let!(:integration) do - create(:jira_integration, :without_properties_callback, properties: properties.merge(additional: 'something')) + context 'with properties' do + let(:data_params) do + { + url: url, api_url: api_url, + username: username, password: password, + jira_issue_transition_id: transition_id + } end - it_behaves_like 'handles jira fields' - end - - context 'when data are stored in separated fields' do - let(:integration) do - create(:jira_integration, data_params.merge(properties: {})) - end - - it_behaves_like 'handles jira fields' - end - - context 'when data are stored in both properties and separated fields' do - let(:properties) { data_params } - let(:integration) do - create(:jira_integration, :without_properties_callback, properties: properties).tap do |integration| - create(:jira_tracker_data, data_params.merge(integration: integration)) + context 'when data are stored in properties' do + let(:integration) do + create(:jira_integration, :without_properties_callback, project: project, properties: data_params.merge(additional: 'something')) end + + it_behaves_like 'handles jira fields' end - it_behaves_like 'handles jira fields' + context 'when data are stored in separated fields' do + let(:integration) do + create(:jira_integration, data_params.merge(properties: {}, project: project)) + end + + it_behaves_like 'handles jira fields' + end + + context 'when data are stored in both properties and separated fields' do + let(:integration) do + create(:jira_integration, :without_properties_callback, properties: data_params, project: project).tap do |integration| + create(:jira_tracker_data, data_params.merge(integration: integration)) + end + end + + it_behaves_like 'handles jira fields' + end end end @@ -872,7 +859,7 @@ RSpec.describe Integrations::Jira do end context 'when resource is a merge request' do - let(:resource) { create(:merge_request) } + let_it_be(:resource) { create(:merge_request, source_project: project) } let(:commit_id) { resource.diff_head_sha } it_behaves_like 'close_issue' @@ -1084,7 +1071,7 @@ RSpec.describe Integrations::Jira do end it 'removes trailing slashes from url' do - integration = described_class.new(url: 'http://jira.test.com/path/') + integration = described_class.new(url: 'http://jira.test.com/path/', project: project) expect(integration.url).to eq('http://jira.test.com/path') end @@ -1105,7 +1092,7 @@ RSpec.describe Integrations::Jira do end context 'generating external URLs' do - let(:integration) { described_class.new(url: 'http://jira.test.com/path/') } + let(:integration) { described_class.new(url: 'http://jira.test.com/path/', project: project) } describe '#web_url' do it 'handles paths, slashes, and query string' do diff --git a/spec/models/integrations/mattermost_slash_commands_spec.rb b/spec/models/integrations/mattermost_slash_commands_spec.rb index b6abe00469b..070adb9ba93 100644 --- a/spec/models/integrations/mattermost_slash_commands_spec.rb +++ b/spec/models/integrations/mattermost_slash_commands_spec.rb @@ -6,9 +6,9 @@ RSpec.describe Integrations::MattermostSlashCommands do it_behaves_like Integrations::BaseSlashCommands describe 'Mattermost API' do - let(:project) { create(:project) } + let_it_be_with_reload(:project) { create(:project) } let(:integration) { project.build_mattermost_slash_commands_integration } - let(:user) { create(:user) } + let(:user) { build_stubbed(:user) } before do session = ::Mattermost::Session.new(nil) diff --git a/spec/models/integrations/microsoft_teams_spec.rb b/spec/models/integrations/microsoft_teams_spec.rb index b6de2bb7176..c61cc732372 100644 --- a/spec/models/integrations/microsoft_teams_spec.rb +++ b/spec/models/integrations/microsoft_teams_spec.rb @@ -17,35 +17,8 @@ RSpec.describe Integrations::MicrosoftTeams do let(:chat_integration) { described_class.new } let(:webhook_url) { 'https://example.gitlab.com/' } - describe 'Validations' do - context 'when integration is active' do - before do - subject.active = true - end - - it { is_expected.to validate_presence_of(:webhook) } - - it_behaves_like 'issue tracker integration URL attribute', :webhook - end - - context 'when integration is inactive' do - before do - subject.active = false - end - - it { is_expected.not_to validate_presence_of(:webhook) } - end - end - - describe '.supported_events' do - it 'does not support deployment_events' do - expect(described_class.supported_events).not_to include('deployment') - end - end - describe "#execute" do - let(:user) { create(:user) } - + let_it_be(:user) { create(:user) } let_it_be(:project) { create(:project, :repository, :wiki_repo) } before do @@ -145,8 +118,8 @@ RSpec.describe Integrations::MicrosoftTeams do end describe "Note events" do - let(:user) { create(:user) } - let(:project) { create(:project, :repository, creator: user) } + let_it_be(:user) { create(:user) } + let_it_be(:project) { create(:project, :repository, creator: user) } before do allow(chat_integration).to receive_messages( @@ -221,8 +194,7 @@ RSpec.describe Integrations::MicrosoftTeams do end describe 'Pipeline events' do - let(:user) { create(:user) } - let(:project) { create(:project, :repository) } + let_it_be_with_reload(:project) { create(:project, :repository) } let(:pipeline) do create(:ci_pipeline, diff --git a/spec/models/integrations/packagist_spec.rb b/spec/models/integrations/packagist_spec.rb index e078debd126..ef86f0565b6 100644 --- a/spec/models/integrations/packagist_spec.rb +++ b/spec/models/integrations/packagist_spec.rb @@ -22,7 +22,7 @@ RSpec.describe Integrations::Packagist do let(:packagist_token) { 'verySecret' } let(:packagist_username) { 'theUser' } let(:packagist_server) { 'https://packagist.example.com' } - let(:project) { create(:project) } + let_it_be(:project) { create(:project) } it_behaves_like Integrations::HasWebHook do let(:integration) { described_class.new(packagist_params) } @@ -34,8 +34,7 @@ RSpec.describe Integrations::Packagist do end describe '#execute' do - let(:user) { create(:user) } - let(:project) { create(:project, :repository) } + let(:user) { create(:user) } let(:push_sample_data) { Gitlab::DataBuilder::Push.build_sample(project, user) } let(:packagist_integration) { described_class.create!(packagist_params) } diff --git a/spec/models/integrations/pipelines_email_spec.rb b/spec/models/integrations/pipelines_email_spec.rb index d70f104b965..37a3849a768 100644 --- a/spec/models/integrations/pipelines_email_spec.rb +++ b/spec/models/integrations/pipelines_email_spec.rb @@ -11,7 +11,7 @@ RSpec.describe Integrations::PipelinesEmail, :mailer do ) end - let(:project) { create(:project, :repository) } + let_it_be_with_reload(:project) { create(:project, :repository) } let(:recipients) { 'test@gitlab.com' } let(:receivers) { [recipients] } diff --git a/spec/models/integrations/prometheus_spec.rb b/spec/models/integrations/prometheus_spec.rb index 3971511872b..b5fc1af368d 100644 --- a/spec/models/integrations/prometheus_spec.rb +++ b/spec/models/integrations/prometheus_spec.rb @@ -308,7 +308,7 @@ RSpec.describe Integrations::Prometheus, :use_clean_rails_memory_store_caching, end context 'cluster belongs to project' do - let(:cluster) { create(:cluster, projects: [project]) } + let_it_be(:cluster) { create(:cluster, projects: [project]) } it 'returns true' do expect(integration.prometheus_available?).to be(true) @@ -319,7 +319,7 @@ RSpec.describe Integrations::Prometheus, :use_clean_rails_memory_store_caching, let_it_be(:group) { create(:group) } let(:project) { create(:project, :with_prometheus_integration, group: group) } - let(:cluster) { create(:cluster_for_group, groups: [group]) } + let_it_be(:cluster) { create(:cluster_for_group, groups: [group]) } it 'returns true' do expect(integration.prometheus_available?).to be(true) diff --git a/spec/models/integrations/pushover_spec.rb b/spec/models/integrations/pushover_spec.rb index 716a00c5bcf..8286fd20669 100644 --- a/spec/models/integrations/pushover_spec.rb +++ b/spec/models/integrations/pushover_spec.rb @@ -29,8 +29,8 @@ RSpec.describe Integrations::Pushover do describe 'Execute' do let(:pushover) { described_class.new } - let(:user) { create(:user) } - let(:project) { create(:project, :repository) } + let(:user) { build_stubbed(:user) } + let(:project) { build_stubbed(:project, :repository) } let(:sample_data) do Gitlab::DataBuilder::Push.build_sample(project, user) end diff --git a/spec/models/integrations/shimo_spec.rb b/spec/models/integrations/shimo_spec.rb index 41f3f3c0c16..be626012ab2 100644 --- a/spec/models/integrations/shimo_spec.rb +++ b/spec/models/integrations/shimo_spec.rb @@ -4,7 +4,7 @@ require 'spec_helper' RSpec.describe ::Integrations::Shimo do describe '#fields' do - let(:shimo_integration) { create(:shimo_integration) } + let(:shimo_integration) { build(:shimo_integration) } it 'returns custom fields' do expect(shimo_integration.fields.pluck(:name)).to eq(%w[external_wiki_url]) @@ -12,7 +12,7 @@ RSpec.describe ::Integrations::Shimo do end describe '#create' do - let(:project) { create(:project, :repository) } + let_it_be(:project) { create(:project, :repository) } let(:external_wiki_url) { 'https://shimo.example.com/desktop' } let(:params) { { active: true, project: project, external_wiki_url: external_wiki_url } } @@ -40,7 +40,7 @@ RSpec.describe ::Integrations::Shimo do end describe 'Caching has_shimo on project_settings' do - let(:project) { create(:project) } + let_it_be(:project) { create(:project) } subject { project.project_setting.has_shimo? } diff --git a/spec/models/integrations/slack_slash_commands_spec.rb b/spec/models/integrations/slack_slash_commands_spec.rb index ff89d2c6a40..22cbaa777cd 100644 --- a/spec/models/integrations/slack_slash_commands_spec.rb +++ b/spec/models/integrations/slack_slash_commands_spec.rb @@ -7,7 +7,7 @@ RSpec.describe Integrations::SlackSlashCommands do describe '#trigger' do context 'when an auth url is generated' do - let(:project) { create(:project) } + let_it_be(:project) { create(:project) } let(:params) do { team_domain: 'http://domain.tld', diff --git a/spec/models/integrations/slack_spec.rb b/spec/models/integrations/slack_spec.rb index ed282f1d39d..affec7864e9 100644 --- a/spec/models/integrations/slack_spec.rb +++ b/spec/models/integrations/slack_spec.rb @@ -6,8 +6,8 @@ RSpec.describe Integrations::Slack do it_behaves_like Integrations::SlackMattermostNotifier, "Slack" describe '#execute' do - let(:slack_integration) { create(:integrations_slack, branches_to_be_notified: 'all', project_id: project.id) } - let(:project) { create_default(:project, :repository, :wiki_repo) } + let_it_be(:project) { create(:project, :repository, :wiki_repo) } + let_it_be(:slack_integration) { create(:integrations_slack, branches_to_be_notified: 'all', project: project) } before do stub_request(:post, slack_integration.webhook) @@ -42,7 +42,7 @@ RSpec.describe Integrations::Slack do end context 'event is not supported for usage log' do - let_it_be(:pipeline) { create(:ci_pipeline) } + let_it_be(:pipeline) { create(:ci_pipeline, project: project) } let(:data) { Gitlab::DataBuilder::Pipeline.build(pipeline) } @@ -54,7 +54,7 @@ RSpec.describe Integrations::Slack do end context 'issue notification' do - let_it_be(:issue) { create(:issue) } + let_it_be(:issue) { create(:issue, project: project) } let(:data) { issue.to_hook_data(user) } @@ -68,7 +68,7 @@ RSpec.describe Integrations::Slack do end context 'deployment notification' do - let_it_be(:deployment) { create(:deployment, user: user) } + let_it_be(:deployment) { create(:deployment, project: project, user: user) } let(:data) { Gitlab::DataBuilder::Deployment.build(deployment, deployment.status, Time.current) } @@ -76,7 +76,7 @@ RSpec.describe Integrations::Slack do end context 'wiki_page notification' do - let(:wiki_page) { create(:wiki_page, wiki: project.wiki, message: 'user created page: Awesome wiki_page') } + let(:wiki_page) { create(:wiki_page, wiki: project.wiki, project: project, message: 'user created page: Awesome wiki_page') } let(:data) { Gitlab::DataBuilder::WikiPage.build(wiki_page, user, 'create') } @@ -90,7 +90,7 @@ RSpec.describe Integrations::Slack do end context 'merge_request notification' do - let_it_be(:merge_request) { create(:merge_request) } + let_it_be(:merge_request) { create(:merge_request, source_project: project) } let(:data) { merge_request.to_hook_data(user) } @@ -98,7 +98,7 @@ RSpec.describe Integrations::Slack do end context 'note notification' do - let_it_be(:issue_note) { create(:note_on_issue, note: 'issue note') } + let_it_be(:issue_note) { create(:note_on_issue, project: project, note: 'issue note') } let(:data) { Gitlab::DataBuilder::Note.build(issue_note, user) } @@ -115,7 +115,7 @@ RSpec.describe Integrations::Slack do end context 'confidential note notification' do - let_it_be(:confidential_issue_note) { create(:note_on_issue, note: 'issue note', confidential: true) } + let_it_be(:confidential_issue_note) { create(:note_on_issue, project: project, note: 'issue note', confidential: true) } let(:data) { Gitlab::DataBuilder::Note.build(confidential_issue_note, user) } @@ -123,7 +123,7 @@ RSpec.describe Integrations::Slack do end context 'confidential issue notification' do - let_it_be(:issue) { create(:issue, confidential: true) } + let_it_be(:issue) { create(:issue, project: project, confidential: true) } let(:data) { issue.to_hook_data(user) } @@ -132,7 +132,7 @@ RSpec.describe Integrations::Slack do end context 'hook data does not include a user' do - let(:data) { Gitlab::DataBuilder::Pipeline.build(create(:ci_pipeline)) } + let(:data) { Gitlab::DataBuilder::Pipeline.build(create(:ci_pipeline, project: project)) } it 'does not increase the usage data counter' do expect(Gitlab::UsageDataCounters::HLLRedisCounter).not_to receive(:track_event) diff --git a/spec/models/integrations/teamcity_spec.rb b/spec/models/integrations/teamcity_spec.rb index da559264c1e..ae3e6658b3c 100644 --- a/spec/models/integrations/teamcity_spec.rb +++ b/spec/models/integrations/teamcity_spec.rb @@ -8,7 +8,7 @@ RSpec.describe Integrations::Teamcity, :use_clean_rails_memory_store_caching do let(:teamcity_url) { 'https://gitlab.teamcity.com' } let(:teamcity_full_url) { 'https://gitlab.teamcity.com/httpAuth/app/rest/builds/branch:unspecified:any,revision:123' } - let(:project) { create(:project) } + let_it_be(:project) { create(:project) } subject(:integration) do described_class.create!( diff --git a/spec/models/integrations/zentao_spec.rb b/spec/models/integrations/zentao_spec.rb index 1a32453819d..2fa4df0e900 100644 --- a/spec/models/integrations/zentao_spec.rb +++ b/spec/models/integrations/zentao_spec.rb @@ -7,15 +7,14 @@ RSpec.describe Integrations::Zentao do let(:api_url) { 'https://jihudemo.zentao.net' } let(:api_token) { 'ZENTAO_TOKEN' } let(:zentao_product_xid) { '3' } - let(:zentao_integration) { create(:zentao_integration) } + let(:zentao_integration) { build(:zentao_integration, project: project) } + let_it_be(:project) { create(:project, :repository) } it_behaves_like Integrations::ResetSecretFields do let(:integration) { zentao_integration } end describe 'set_default_data' do - let(:project) { create(:project, :repository) } - context 'when gitlab.yml was initialized' do it 'is prepopulated with the settings' do settings = { @@ -35,7 +34,6 @@ RSpec.describe Integrations::Zentao do end describe '#create' do - let(:project) { create(:project, :repository) } let(:params) do { project: project, diff --git a/spec/models/project_authorization_spec.rb b/spec/models/project_authorization_spec.rb index 55fe28ceb6f..df89e97a41f 100644 --- a/spec/models/project_authorization_spec.rb +++ b/spec/models/project_authorization_spec.rb @@ -86,6 +86,25 @@ RSpec.describe ProjectAuthorization do end end + shared_examples_for 'does not log any detail' do + it 'does not log any detail' do + expect(Gitlab::AppLogger).not_to receive(:info) + + execute + end + end + + shared_examples_for 'logs the detail' do + it 'logs the detail' do + expect(Gitlab::AppLogger).to receive(:info).with( + entire_size: 3, + message: 'Project authorizations refresh performed with delay' + ) + + execute + end + end + describe '.insert_all_in_batches' do let_it_be(:user) { create(:user) } let_it_be(:project_1) { create(:project) } @@ -100,6 +119,8 @@ RSpec.describe ProjectAuthorization do ] end + subject(:execute) { described_class.insert_all_in_batches(attributes, per_batch_size) } + before do # Configure as if a replica database is enabled allow(::Gitlab::Database::LoadBalancing).to receive(:primary_only?).and_return(false) @@ -110,7 +131,7 @@ RSpec.describe ProjectAuthorization do specify do expect(described_class).not_to receive(:sleep) - described_class.insert_all_in_batches(attributes, per_batch_size) + execute expect(user.project_authorizations.pluck(:user_id, :project_id, :access_level)).to match_array(attributes.map(&:values)) end @@ -123,11 +144,13 @@ RSpec.describe ProjectAuthorization do expect(described_class).to receive(:insert_all).twice.and_call_original expect(described_class).to receive(:sleep).twice - described_class.insert_all_in_batches(attributes, per_batch_size) + execute expect(user.project_authorizations.pluck(:user_id, :project_id, :access_level)).to match_array(attributes.map(&:values)) end + it_behaves_like 'logs the detail' + context 'when the GitLab installation does not have a replica database configured' do before do # Configure as if a replica database is not enabled @@ -135,6 +158,7 @@ RSpec.describe ProjectAuthorization do end it_behaves_like 'inserts the rows in batches, as per the `per_batch` size, without a delay between each batch' + it_behaves_like 'does not log any detail' end end @@ -142,6 +166,7 @@ RSpec.describe ProjectAuthorization do let(:per_batch_size) { 5 } it_behaves_like 'inserts the rows in batches, as per the `per_batch` size, without a delay between each batch' + it_behaves_like 'does not log any detail' end end @@ -154,6 +179,14 @@ RSpec.describe ProjectAuthorization do let(:user_ids) { [user_1.id, user_2.id, user_3.id] } + subject(:execute) do + described_class.delete_all_in_batches_for_project( + project: project, + user_ids: user_ids, + per_batch: per_batch_size + ) + end + before do # Configure as if a replica database is enabled allow(::Gitlab::Database::LoadBalancing).to receive(:primary_only?).and_return(false) @@ -171,11 +204,7 @@ RSpec.describe ProjectAuthorization do specify do expect(described_class).not_to receive(:sleep) - described_class.delete_all_in_batches_for_project( - project: project, - user_ids: user_ids, - per_batch: per_batch_size - ) + execute expect(project.project_authorizations.pluck(:user_id)).not_to include(*user_ids) end @@ -187,15 +216,13 @@ RSpec.describe ProjectAuthorization do it 'removes the project authorizations of the specified users in the current project, with a delay between each batch' do expect(described_class).to receive(:sleep).twice - described_class.delete_all_in_batches_for_project( - project: project, - user_ids: user_ids, - per_batch: per_batch_size - ) + execute expect(project.project_authorizations.pluck(:user_id)).not_to include(*user_ids) end + it_behaves_like 'logs the detail' + context 'when the GitLab installation does not have a replica database configured' do before do # Configure as if a replica database is not enabled @@ -203,6 +230,7 @@ RSpec.describe ProjectAuthorization do end it_behaves_like 'removes the project authorizations of the specified users in the current project, without a delay between each batch' + it_behaves_like 'does not log any detail' end end @@ -210,6 +238,7 @@ RSpec.describe ProjectAuthorization do let(:per_batch_size) { 5 } it_behaves_like 'removes the project authorizations of the specified users in the current project, without a delay between each batch' + it_behaves_like 'does not log any detail' end end @@ -222,6 +251,14 @@ RSpec.describe ProjectAuthorization do let(:project_ids) { [project_1.id, project_2.id, project_3.id] } + subject(:execute) do + described_class.delete_all_in_batches_for_user( + user: user, + project_ids: project_ids, + per_batch: per_batch_size + ) + end + before do # Configure as if a replica database is enabled allow(::Gitlab::Database::LoadBalancing).to receive(:primary_only?).and_return(false) @@ -239,11 +276,7 @@ RSpec.describe ProjectAuthorization do specify do expect(described_class).not_to receive(:sleep) - described_class.delete_all_in_batches_for_user( - user: user, - project_ids: project_ids, - per_batch: per_batch_size - ) + execute expect(user.project_authorizations.pluck(:project_id)).not_to include(*project_ids) end @@ -255,15 +288,13 @@ RSpec.describe ProjectAuthorization do it 'removes the project authorizations of the specified projects from the current user, with a delay between each batch' do expect(described_class).to receive(:sleep).twice - described_class.delete_all_in_batches_for_user( - user: user, - project_ids: project_ids, - per_batch: per_batch_size - ) + execute expect(user.project_authorizations.pluck(:project_id)).not_to include(*project_ids) end + it_behaves_like 'logs the detail' + context 'when the GitLab installation does not have a replica database configured' do before do # Configure as if a replica database is not enabled @@ -271,6 +302,7 @@ RSpec.describe ProjectAuthorization do end it_behaves_like 'removes the project authorizations of the specified projects from the current user, without a delay between each batch' + it_behaves_like 'does not log any detail' end end @@ -278,6 +310,7 @@ RSpec.describe ProjectAuthorization do let(:per_batch_size) { 5 } it_behaves_like 'removes the project authorizations of the specified projects from the current user, without a delay between each batch' + it_behaves_like 'does not log any detail' end end end diff --git a/spec/models/protected_branch_spec.rb b/spec/models/protected_branch_spec.rb index b88367b9ca2..b623d534f29 100644 --- a/spec/models/protected_branch_spec.rb +++ b/spec/models/protected_branch_spec.rb @@ -7,11 +7,54 @@ RSpec.describe ProtectedBranch do describe 'Associations' do it { is_expected.to belong_to(:project) } + it { is_expected.to belong_to(:group) } end describe 'Validation' do - it { is_expected.to validate_presence_of(:project) } it { is_expected.to validate_presence_of(:name) } + + describe '#validate_either_project_or_top_group' do + context 'when protected branch does not have project or group association' do + it 'validate failed' do + subject.assign_attributes(project: nil, group: nil) + subject.validate + + expect(subject.errors).to include(:base) + end + end + + context 'when protected branch is associated with both project and group' do + it 'validate failed' do + subject.assign_attributes(project: build(:project), group: build(:group)) + subject.validate + + expect(subject.errors).to include(:base) + end + end + + context 'when protected branch is associated with a subgroup' do + it 'validate failed' do + subject.assign_attributes(project: nil, group: build(:group, :nested)) + subject.validate + + expect(subject.errors).to include(:base) + end + end + end + end + + describe 'set a group' do + context 'when associated with group' do + it 'create successfully' do + expect { subject.group = build(:group) }.not_to raise_error + end + end + + context 'when associated with other namespace' do + it 'create failed with `ActiveRecord::AssociationTypeMismatch`' do + expect { subject.group = build(:namespace) }.to raise_error(ActiveRecord::AssociationTypeMismatch) + end + end end describe "#matches?" do diff --git a/spec/support/shared_examples/lib/gitlab/event_store_shared_examples.rb b/spec/support/shared_examples/lib/gitlab/event_store_shared_examples.rb index db2f2f2d0f0..e97b9ad969f 100644 --- a/spec/support/shared_examples/lib/gitlab/event_store_shared_examples.rb +++ b/spec/support/shared_examples/lib/gitlab/event_store_shared_examples.rb @@ -15,6 +15,16 @@ RSpec.shared_examples 'subscribes to event' do it_behaves_like 'an idempotent worker' end +RSpec.shared_examples 'ignores the published event' do + include AfterNextHelpers + + it 'does not consume the published event', :sidekiq_inline do + expect_next(described_class).not_to receive(:handle_event) + + ::Gitlab::EventStore.publish(event) + end +end + def consume_event(subscriber:, event:) subscriber.new.perform(event.class.name, event.data) end diff --git a/spec/support/shared_examples/models/concerns/integrations/slack_mattermost_notifier_shared_examples.rb b/spec/support/shared_examples/models/concerns/integrations/slack_mattermost_notifier_shared_examples.rb index 7512a9f2855..974fc8f402a 100644 --- a/spec/support/shared_examples/models/concerns/integrations/slack_mattermost_notifier_shared_examples.rb +++ b/spec/support/shared_examples/models/concerns/integrations/slack_mattermost_notifier_shared_examples.rb @@ -152,7 +152,7 @@ RSpec.shared_examples Integrations::SlackMattermostNotifier do |integration_name end context 'issue events' do - let_it_be(:issue) { create(:issue) } + let_it_be(:issue) { create(:issue, project: project) } let(:data) { issue.to_hook_data(user) } @@ -192,7 +192,7 @@ RSpec.shared_examples Integrations::SlackMattermostNotifier do |integration_name end context 'merge request events' do - let_it_be(:merge_request) { create(:merge_request) } + let_it_be(:merge_request) { create(:merge_request, source_project: project) } let(:data) { merge_request.to_hook_data(user) } @@ -210,7 +210,7 @@ RSpec.shared_examples Integrations::SlackMattermostNotifier do |integration_name end context 'wiki page events' do - let_it_be(:wiki_page) { create(:wiki_page, wiki: project.wiki, message: 'user created page: Awesome wiki_page') } + let_it_be(:wiki_page) { create(:wiki_page, wiki: project.wiki, project: project, message: 'user created page: Awesome wiki_page') } let(:data) { Gitlab::DataBuilder::WikiPage.build(wiki_page, user, 'create') } @@ -228,7 +228,7 @@ RSpec.shared_examples Integrations::SlackMattermostNotifier do |integration_name end context 'deployment events' do - let_it_be(:deployment) { create(:deployment) } + let_it_be(:deployment) { create(:deployment, project: project) } let(:data) { Gitlab::DataBuilder::Deployment.build(deployment, 'created', Time.current) } @@ -275,8 +275,8 @@ RSpec.shared_examples Integrations::SlackMattermostNotifier do |integration_name end describe 'Push events' do - let(:user) { create(:user) } - let(:project) { create(:project, :repository, creator: user) } + let_it_be(:user) { create(:user) } + let_it_be_with_reload(:project) { create(:project, :repository, creator: user) } before do allow(chat_integration).to receive_messages( @@ -327,7 +327,7 @@ RSpec.shared_examples Integrations::SlackMattermostNotifier do |integration_name end context 'on a protected branch' do - before do + before(:all) do create(:protected_branch, :create_branch_on_repository, project: project, name: 'a-protected-branch') end @@ -369,7 +369,7 @@ RSpec.shared_examples Integrations::SlackMattermostNotifier do |integration_name end context 'on a protected branch with protected branches defined using wildcards' do - before do + before(:all) do create(:protected_branch, :create_branch_on_repository, repository_branch_name: '1-stable', project: project, name: '*-stable') end @@ -450,8 +450,8 @@ RSpec.shared_examples Integrations::SlackMattermostNotifier do |integration_name end describe 'Note events' do - let(:user) { create(:user) } - let(:project) { create(:project, :repository, creator: user) } + let_it_be(:user) { create(:user) } + let_it_be_with_reload(:project) { create(:project, :repository, creator: user) } before do allow(chat_integration).to receive_messages( @@ -519,8 +519,8 @@ RSpec.shared_examples Integrations::SlackMattermostNotifier do |integration_name end describe 'Pipeline events' do - let(:user) { create(:user) } - let(:project) { create(:project, :repository, creator: user) } + let_it_be(:user) { create(:user) } + let_it_be_with_reload(:project) { create(:project, :repository, creator: user) } let(:pipeline) do create(:ci_pipeline, project: project, status: status, @@ -582,7 +582,7 @@ RSpec.shared_examples Integrations::SlackMattermostNotifier do |integration_name end context 'on a protected branch' do - before do + before(:all) do create(:protected_branch, :create_branch_on_repository, project: project, name: 'a-protected-branch') end @@ -612,7 +612,7 @@ RSpec.shared_examples Integrations::SlackMattermostNotifier do |integration_name end context 'on a protected branch with protected branches defined usin wildcards' do - before do + before(:all) do create(:protected_branch, :create_branch_on_repository, repository_branch_name: '1-stable', project: project, name: '*-stable') end @@ -673,7 +673,7 @@ RSpec.shared_examples Integrations::SlackMattermostNotifier do |integration_name let_it_be(:user) { create(:user) } let_it_be_with_reload(:project) { create(:project, :repository, creator: user) } - let(:deployment) do + let_it_be(:deployment) do create(:deployment, :success, project: project, sha: project.commit.sha, ref: project.default_branch) end @@ -692,11 +692,11 @@ RSpec.shared_examples Integrations::SlackMattermostNotifier do |integration_name it_behaves_like "triggered #{integration_name} integration", event_type: "deployment" context 'on a protected branch' do - before do + before(:all) do create(:protected_branch, :create_branch_on_repository, project: project, name: 'a-protected-branch') end - let(:deployment) do + let_it_be(:deployment) do create(:deployment, :success, project: project, sha: project.commit.sha, ref: 'a-protected-branch') end diff --git a/spec/views/projects/artifacts/_artifact.html.haml_spec.rb b/spec/views/projects/artifacts/_artifact.html.haml_spec.rb deleted file mode 100644 index 5d930d6b0f2..00000000000 --- a/spec/views/projects/artifacts/_artifact.html.haml_spec.rb +++ /dev/null @@ -1,85 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -RSpec.describe "projects/artifacts/_artifact.html.haml" do - let(:project) { create(:project) } - - describe 'delete button' do - before do - create(:ci_build, :artifacts, project: project) - - allow(view).to receive(:current_user).and_return(user) - assign(:project, project) - end - - context 'with admin' do - let(:user) { build(:admin) } - - context 'when admin mode is enabled', :enable_admin_mode do - it 'has a delete button' do - render_partial - - expect(rendered).to have_link('Delete artifacts', href: project_artifact_path(project, project.job_artifacts.first)) - end - end - - context 'when admin mode is disabled' do - it 'has no delete button' do - project.add_reporter(user) - render_partial - - expect(rendered).not_to have_link('Delete artifacts') - end - end - end - - context 'with owner' do - let(:user) { create(:user) } - let(:project) { build(:project, namespace: user.namespace) } - - it 'has a delete button' do - render_partial - - expect(rendered).to have_link('Delete artifacts', href: project_artifact_path(project, project.job_artifacts.first)) - end - end - - context 'with master' do - let(:user) { create(:user) } - - it 'has a delete button' do - allow_any_instance_of(ProjectTeam).to receive(:max_member_access).and_return(Gitlab::Access::MAINTAINER) - render_partial - - expect(rendered).to have_link('Delete artifacts', href: project_artifact_path(project, project.job_artifacts.first)) - end - end - - context 'with developer' do - let(:user) { build(:user) } - - it 'has no delete button' do - project.add_developer(user) - render_partial - - expect(rendered).not_to have_link('Delete artifacts') - end - end - - context 'with reporter' do - let(:user) { build(:user) } - - it 'has no delete button' do - project.add_reporter(user) - render_partial - - expect(rendered).not_to have_link('Delete artifacts') - end - end - end - - def render_partial - render partial: 'projects/artifacts/artifact', collection: project.job_artifacts, as: :artifact - end -end diff --git a/spec/workers/pages/invalidate_domain_cache_worker_spec.rb b/spec/workers/pages/invalidate_domain_cache_worker_spec.rb index b9c27c54fa1..c786d4658d4 100644 --- a/spec/workers/pages/invalidate_domain_cache_worker_spec.rb +++ b/spec/workers/pages/invalidate_domain_cache_worker_spec.rb @@ -4,9 +4,9 @@ require 'spec_helper' RSpec.describe Pages::InvalidateDomainCacheWorker do shared_examples 'clears caches with' do |event_class:, event_data:, caches:| - let(:event) do - event_class.new(data: event_data) - end + include AfterNextHelpers + + let(:event) { event_class.new(data: event_data) } subject { consume_event(subscriber: described_class, event: event) } @@ -14,9 +14,8 @@ RSpec.describe Pages::InvalidateDomainCacheWorker do it 'clears the cache with Gitlab::Pages::CacheControl' do caches.each do |cache| - expect_next_instance_of(Gitlab::Pages::CacheControl, type: cache[:type], id: cache[:id]) do |cache_control| - expect(cache_control).to receive(:clear_cache) - end + expect_next(Gitlab::Pages::CacheControl, type: cache[:type], id: cache[:id]) + .to receive(:clear_cache) end subject @@ -181,19 +180,17 @@ RSpec.describe Pages::InvalidateDomainCacheWorker do ] end - it 'does not clear the cache when the attributes is not pages related' do - event = Projects::ProjectAttributesChangedEvent.new( - data: { - project_id: 1, - namespace_id: 2, - root_namespace_id: 3, - attributes: ['unknown'] - } - ) - - expect(described_class).not_to receive(:clear_cache) - - ::Gitlab::EventStore.publish(event) + it_behaves_like 'ignores the published event' do + let(:event) do + Projects::ProjectAttributesChangedEvent.new( + data: { + project_id: 1, + namespace_id: 2, + root_namespace_id: 3, + attributes: ['unknown'] + } + ) + end end end @@ -204,26 +201,24 @@ RSpec.describe Pages::InvalidateDomainCacheWorker do project_id: 1, namespace_id: 2, root_namespace_id: 3, - features: ["pages_access_level"] + features: ['pages_access_level'] }, caches: [ { type: :project, id: 1 }, { type: :namespace, id: 3 } ] - it 'does not clear the cache when the features is not pages related' do - event = Projects::ProjectFeaturesChangedEvent.new( - data: { - project_id: 1, - namespace_id: 2, - root_namespace_id: 3, - features: ['unknown'] - } - ) - - expect(described_class).not_to receive(:clear_cache) - - ::Gitlab::EventStore.publish(event) + it_behaves_like 'ignores the published event' do + let(:event) do + Projects::ProjectFeaturesChangedEvent.new( + data: { + project_id: 1, + namespace_id: 2, + root_namespace_id: 3, + features: ['unknown'] + } + ) + end end end