diff options
Diffstat (limited to 'spec')
17 files changed, 374 insertions, 251 deletions
diff --git a/spec/factories/groups.rb b/spec/factories/groups.rb index 065eb36375a..5d232a9d09a 100644 --- a/spec/factories/groups.rb +++ b/spec/factories/groups.rb @@ -61,5 +61,35 @@ FactoryBot.define do trait :allow_descendants_override_disabled_shared_runners do allow_descendants_override_disabled_shared_runners { true } end + + # Construct a hierarchy underneath the group. + # Each group will have `children` amount of children, + # and `depth` levels of descendants. + trait :with_hierarchy do + transient do + children { 4 } + depth { 4 } + end + + after(:create) do |group, evaluator| + def create_graph(parent: nil, children: 4, depth: 4) + return unless depth > 1 + + children.times do + factory_name = parent.model_name.singular + child = FactoryBot.create(factory_name, parent: parent) + create_graph(parent: child, children: children, depth: depth - 1) + end + + parent + end + + create_graph( + parent: group, + children: evaluator.children, + depth: evaluator.depth + ) + end + end end end diff --git a/spec/factories/namespaces.rb b/spec/factories/namespaces.rb index 0ec977b8234..398e4f8b4f9 100644 --- a/spec/factories/namespaces.rb +++ b/spec/factories/namespaces.rb @@ -34,36 +34,6 @@ FactoryBot.define do association :namespace_settings, factory: :namespace_settings end - # Construct a hierarchy underneath the namespace. - # Each namespace will have `children` amount of children, - # and `depth` levels of descendants. - trait :with_hierarchy do - transient do - children { 4 } - depth { 4 } - end - - after(:create) do |namespace, evaluator| - def create_graph(parent: nil, children: 4, depth: 4) - return unless depth > 1 - - children.times do - factory_name = parent.model_name.singular - child = FactoryBot.create(factory_name, parent: parent) - create_graph(parent: child, children: children, depth: depth - 1) - end - - parent - end - - create_graph( - parent: namespace, - children: evaluator.children, - depth: evaluator.depth - ) - end - end - trait :shared_runners_disabled do shared_runners_enabled { false } end diff --git a/spec/lib/gitlab/database/rename_reserved_paths_migration/v1/rename_base_spec.rb b/spec/lib/gitlab/database/rename_reserved_paths_migration/v1/rename_base_spec.rb index 76b1be1e497..757da2d9092 100644 --- a/spec/lib/gitlab/database/rename_reserved_paths_migration/v1/rename_base_spec.rb +++ b/spec/lib/gitlab/database/rename_reserved_paths_migration/v1/rename_base_spec.rb @@ -81,7 +81,7 @@ RSpec.describe Gitlab::Database::RenameReservedPathsMigration::V1::RenameBase, : end describe '#rename_path_for_routable' do - context 'for namespaces' do + context 'for personal namespaces' do let(:namespace) { create(:namespace, path: 'the-path') } it "renames namespaces called the-path" do @@ -119,13 +119,16 @@ RSpec.describe Gitlab::Database::RenameReservedPathsMigration::V1::RenameBase, : expect(project.route.reload.path).to eq('the-path-but-not-really/the-project') end + end - context "the-path namespace -> subgroup -> the-path0 project" do + context 'for groups' do + context "the-path group -> subgroup -> the-path0 project" do it "updates the route of the project correctly" do - subgroup = create(:group, path: "subgroup", parent: namespace) + group = create(:group, path: 'the-path') + subgroup = create(:group, path: "subgroup", parent: group) project = create(:project, :repository, path: "the-path0", namespace: subgroup) - subject.rename_path_for_routable(migration_namespace(namespace)) + subject.rename_path_for_routable(migration_namespace(group)) expect(project.route.reload.path).to eq("the-path0/subgroup/the-path0") end @@ -158,23 +161,27 @@ RSpec.describe Gitlab::Database::RenameReservedPathsMigration::V1::RenameBase, : end describe '#perform_rename' do - describe 'for namespaces' do - let(:namespace) { create(:namespace, path: 'the-path') } - + context 'for personal namespaces' do it 'renames the path' do + namespace = create(:namespace, path: 'the-path') + subject.perform_rename(migration_namespace(namespace), 'the-path', 'renamed') expect(namespace.reload.path).to eq('renamed') + expect(namespace.reload.route.path).to eq('renamed') end + end - it 'renames all the routes for the namespace' do - child = create(:group, path: 'child', parent: namespace) + context 'for groups' do + it 'renames all the routes for the group' do + group = create(:group, path: 'the-path') + child = create(:group, path: 'child', parent: group) project = create(:project, :repository, namespace: child, path: 'the-project') - other_one = create(:namespace, path: 'the-path-is-similar') + other_one = create(:group, path: 'the-path-is-similar') - subject.perform_rename(migration_namespace(namespace), 'the-path', 'renamed') + subject.perform_rename(migration_namespace(group), 'the-path', 'renamed') - expect(namespace.reload.route.path).to eq('renamed') + expect(group.reload.route.path).to eq('renamed') expect(child.reload.route.path).to eq('renamed/child') expect(project.reload.route.path).to eq('renamed/child/the-project') expect(other_one.reload.route.path).to eq('the-path-is-similar') diff --git a/spec/migrations/schedule_populate_personal_snippet_statistics_spec.rb b/spec/migrations/schedule_populate_personal_snippet_statistics_spec.rb index 8678361fc64..faa993dfbc7 100644 --- a/spec/migrations/schedule_populate_personal_snippet_statistics_spec.rb +++ b/spec/migrations/schedule_populate_personal_snippet_statistics_spec.rb @@ -6,11 +6,15 @@ require Rails.root.join('db', 'post_migrate', '20200714075739_schedule_populate_ RSpec.describe SchedulePopulatePersonalSnippetStatistics do let(:users) { table(:users) } + let(:namespaces) { table(:namespaces) } let(:snippets) { table(:snippets) } let(:projects) { table(:projects) } - let(:user1) { users.create!(id: 1, email: 'user1@example.com', projects_limit: 10, username: 'test1', name: 'Test1', state: 'active') } - let(:user2) { users.create!(id: 2, email: 'user2@example.com', projects_limit: 10, username: 'test2', name: 'Test2', state: 'active') } - let(:user3) { users.create!(id: 3, email: 'user3@example.com', projects_limit: 10, username: 'test3', name: 'Test3', state: 'active') } + let!(:user1) { users.create!(id: 1, email: 'user1@example.com', projects_limit: 10, username: 'test1', name: 'Test1', state: 'active') } + let!(:user2) { users.create!(id: 2, email: 'user2@example.com', projects_limit: 10, username: 'test2', name: 'Test2', state: 'active') } + let!(:user3) { users.create!(id: 3, email: 'user3@example.com', projects_limit: 10, username: 'test3', name: 'Test3', state: 'active') } + let!(:namespace1) { namespaces.create!(id: 1, owner_id: user1.id, name: 'test1', path: 'test1') } + let!(:namespace2) { namespaces.create!(id: 2, owner_id: user2.id, name: 'test2', path: 'test2') } + let!(:namespace3) { namespaces.create!(id: 3, owner_id: user3.id, name: 'test3', path: 'test3') } def create_snippet(id, user_id, type = 'PersonalSnippet') params = { diff --git a/spec/models/group_spec.rb b/spec/models/group_spec.rb index 528ce661370..96d02eacf8a 100644 --- a/spec/models/group_spec.rb +++ b/spec/models/group_spec.rb @@ -64,6 +64,59 @@ RSpec.describe Group do it { is_expected.to validate_presence_of :two_factor_grace_period } it { is_expected.to validate_numericality_of(:two_factor_grace_period).is_greater_than_or_equal_to(0) } + context 'validating the parent of a group' do + context 'when the group has no parent' do + it 'allows a group to have no parent associated with it' do + group = build(:group) + + expect(group).to be_valid + end + end + + context 'when the group has a parent' do + it 'does not allow a group to have a namespace as its parent' do + group = build(:group, parent: build(:namespace)) + + expect(group).not_to be_valid + expect(group.errors[:parent_id].first).to eq('a group cannot have a user namespace as its parent') + end + + it 'allows a group to have another group as its parent' do + group = build(:group, parent: build(:group)) + + expect(group).to be_valid + end + end + + context 'when the feature flag `validate_namespace_parent_type` is disabled' do + before do + stub_feature_flags(validate_namespace_parent_type: false) + end + + context 'when the group has no parent' do + it 'allows a group to have no parent associated with it' do + group = build(:group) + + expect(group).to be_valid + end + end + + context 'when the group has a parent' do + it 'allows a group to have a namespace as its parent' do + group = build(:group, parent: build(:namespace)) + + expect(group).to be_valid + end + + it 'allows a group to have another group as its parent' do + group = build(:group, parent: build(:group)) + + expect(group).to be_valid + end + end + end + end + describe 'path validation' do it 'rejects paths reserved on the root namespace when the group has no parent' do group = build(:group, path: 'api') diff --git a/spec/models/namespace/traversal_hierarchy_spec.rb b/spec/models/namespace/traversal_hierarchy_spec.rb index 71b0e974106..83e6d704640 100644 --- a/spec/models/namespace/traversal_hierarchy_spec.rb +++ b/spec/models/namespace/traversal_hierarchy_spec.rb @@ -3,41 +3,41 @@ require 'spec_helper' RSpec.describe Namespace::TraversalHierarchy, type: :model do - let_it_be(:root, reload: true) { create(:namespace, :with_hierarchy) } + let_it_be(:root, reload: true) { create(:group, :with_hierarchy) } describe '.for_namespace' do - let(:hierarchy) { described_class.for_namespace(namespace) } + let(:hierarchy) { described_class.for_namespace(group) } context 'with root group' do - let(:namespace) { root } + let(:group) { root } it { expect(hierarchy.root).to eq root } end context 'with child group' do - let(:namespace) { root.children.first.children.first } + let(:group) { root.children.first.children.first } it { expect(hierarchy.root).to eq root } end context 'with group outside of hierarchy' do - let(:namespace) { create(:namespace) } + let(:group) { create(:namespace) } it { expect(hierarchy.root).not_to eq root } end end describe '.new' do - let(:hierarchy) { described_class.new(namespace) } + let(:hierarchy) { described_class.new(group) } context 'with root group' do - let(:namespace) { root } + let(:group) { root } it { expect(hierarchy.root).to eq root } end context 'with child group' do - let(:namespace) { root.children.first } + let(:group) { root.children.first } it { expect { hierarchy }.to raise_error(StandardError, 'Must specify a root node') } end diff --git a/spec/models/namespace_spec.rb b/spec/models/namespace_spec.rb index 7cdf66f42aa..3f36ff433b4 100644 --- a/spec/models/namespace_spec.rb +++ b/spec/models/namespace_spec.rb @@ -32,14 +32,68 @@ RSpec.describe Namespace do it { is_expected.to validate_presence_of(:owner) } it { is_expected.to validate_numericality_of(:max_artifacts_size).only_integer.is_greater_than(0) } + context 'validating the parent of a namespace' do + context 'when the namespace has no parent' do + it 'allows a namespace to have no parent associated with it' do + namespace = build(:namespace) + + expect(namespace).to be_valid + end + end + + context 'when the namespace has a parent' do + it 'does not allow a namespace to have a group as its parent' do + namespace = build(:namespace, parent: build(:group)) + + expect(namespace).not_to be_valid + expect(namespace.errors[:parent_id].first).to eq('a user namespace cannot have a parent') + end + + it 'does not allow a namespace to have another namespace as its parent' do + namespace = build(:namespace, parent: build(:namespace)) + + expect(namespace).not_to be_valid + expect(namespace.errors[:parent_id].first).to eq('a user namespace cannot have a parent') + end + end + + context 'when the feature flag `validate_namespace_parent_type` is disabled' do + before do + stub_feature_flags(validate_namespace_parent_type: false) + end + + context 'when the namespace has no parent' do + it 'allows a namespace to have no parent associated with it' do + namespace = build(:namespace) + + expect(namespace).to be_valid + end + end + + context 'when the namespace has a parent' do + it 'allows a namespace to have a group as its parent' do + namespace = build(:namespace, parent: build(:group)) + + expect(namespace).to be_valid + end + + it 'allows a namespace to have another namespace as its parent' do + namespace = build(:namespace, parent: build(:namespace)) + + expect(namespace).to be_valid + end + end + end + end + it 'does not allow too deep nesting' do ancestors = (1..21).to_a - nested = build(:namespace, parent: namespace) + group = build(:group) - allow(nested).to receive(:ancestors).and_return(ancestors) + allow(group).to receive(:ancestors).and_return(ancestors) - expect(nested).not_to be_valid - expect(nested.errors[:parent_id].first).to eq('has too deep level of nesting') + expect(group).not_to be_valid + expect(group.errors[:parent_id].first).to eq('has too deep level of nesting') end describe 'reserved path validation' do @@ -172,45 +226,45 @@ RSpec.describe Namespace do end describe '.search' do - let_it_be(:first_namespace) { build(:namespace, name: 'my first namespace', path: 'old-path').tap(&:save!) } - let_it_be(:parent_namespace) { build(:namespace, name: 'my parent namespace', path: 'parent-path').tap(&:save!) } - let_it_be(:second_namespace) { build(:namespace, name: 'my second namespace', path: 'new-path', parent: parent_namespace).tap(&:save!) } - let_it_be(:project_with_same_path) { create(:project, id: second_namespace.id, path: first_namespace.path) } + let_it_be(:first_group) { build(:group, name: 'my first namespace', path: 'old-path').tap(&:save!) } + let_it_be(:parent_group) { build(:group, name: 'my parent namespace', path: 'parent-path').tap(&:save!) } + let_it_be(:second_group) { build(:group, name: 'my second namespace', path: 'new-path', parent: parent_group).tap(&:save!) } + let_it_be(:project_with_same_path) { create(:project, id: second_group.id, path: first_group.path) } it 'returns namespaces with a matching name' do - expect(described_class.search('my first namespace')).to eq([first_namespace]) + expect(described_class.search('my first namespace')).to eq([first_group]) end it 'returns namespaces with a partially matching name' do - expect(described_class.search('first')).to eq([first_namespace]) + expect(described_class.search('first')).to eq([first_group]) end it 'returns namespaces with a matching name regardless of the casing' do - expect(described_class.search('MY FIRST NAMESPACE')).to eq([first_namespace]) + expect(described_class.search('MY FIRST NAMESPACE')).to eq([first_group]) end it 'returns namespaces with a matching path' do - expect(described_class.search('old-path')).to eq([first_namespace]) + expect(described_class.search('old-path')).to eq([first_group]) end it 'returns namespaces with a partially matching path' do - expect(described_class.search('old')).to eq([first_namespace]) + expect(described_class.search('old')).to eq([first_group]) end it 'returns namespaces with a matching path regardless of the casing' do - expect(described_class.search('OLD-PATH')).to eq([first_namespace]) + expect(described_class.search('OLD-PATH')).to eq([first_group]) end it 'returns namespaces with a matching route path' do - expect(described_class.search('parent-path/new-path', include_parents: true)).to eq([second_namespace]) + expect(described_class.search('parent-path/new-path', include_parents: true)).to eq([second_group]) end it 'returns namespaces with a partially matching route path' do - expect(described_class.search('parent-path/new', include_parents: true)).to eq([second_namespace]) + expect(described_class.search('parent-path/new', include_parents: true)).to eq([second_group]) end it 'returns namespaces with a matching route path regardless of the casing' do - expect(described_class.search('PARENT-PATH/NEW-PATH', include_parents: true)).to eq([second_namespace]) + expect(described_class.search('PARENT-PATH/NEW-PATH', include_parents: true)).to eq([second_group]) end end @@ -309,12 +363,13 @@ RSpec.describe Namespace do describe '.top_most' do let_it_be(:namespace) { create(:namespace) } - let_it_be(:sub_namespace) { create(:namespace, parent: namespace) } + let_it_be(:group) { create(:group) } + let_it_be(:subgroup) { create(:group, parent: group) } subject { described_class.top_most.ids } - it 'only contains root namespace' do - is_expected.to eq([namespace.id]) + it 'only contains root namespaces' do + is_expected.to contain_exactly(group.id, namespace.id) end end @@ -1259,14 +1314,14 @@ RSpec.describe Namespace do using RSpec::Parameterized::TableSyntax shared_examples_for 'fetching closest setting' do - let!(:root_namespace) { create(:namespace) } - let!(:namespace) { create(:namespace, parent: root_namespace) } + let!(:parent) { create(:group) } + let!(:group) { create(:group, parent: parent) } - let(:setting) { namespace.closest_setting(setting_name) } + let(:setting) { group.closest_setting(setting_name) } before do - root_namespace.update_attribute(setting_name, root_setting) - namespace.update_attribute(setting_name, child_setting) + parent.update_attribute(setting_name, root_setting) + group.update_attribute(setting_name, child_setting) end it 'returns closest non-nil value' do @@ -1357,30 +1412,30 @@ RSpec.describe Namespace do context 'with a parent' do context 'when parent has shared runners disabled' do - let(:parent) { create(:namespace, :shared_runners_disabled) } - let(:sub_namespace) { build(:namespace, shared_runners_enabled: true, parent_id: parent.id) } + let(:parent) { create(:group, :shared_runners_disabled) } + let(:group) { build(:group, shared_runners_enabled: true, parent_id: parent.id) } it 'is invalid' do - expect(sub_namespace).to be_invalid - expect(sub_namespace.errors[:shared_runners_enabled]).to include('cannot be enabled because parent group has shared Runners disabled') + expect(group).to be_invalid + expect(group.errors[:shared_runners_enabled]).to include('cannot be enabled because parent group has shared Runners disabled') end end context 'when parent has shared runners disabled but allows override' do - let(:parent) { create(:namespace, :shared_runners_disabled, :allow_descendants_override_disabled_shared_runners) } - let(:sub_namespace) { build(:namespace, shared_runners_enabled: true, parent_id: parent.id) } + let(:parent) { create(:group, :shared_runners_disabled, :allow_descendants_override_disabled_shared_runners) } + let(:group) { build(:group, shared_runners_enabled: true, parent_id: parent.id) } it 'is valid' do - expect(sub_namespace).to be_valid + expect(group).to be_valid end end context 'when parent has shared runners enabled' do - let(:parent) { create(:namespace, shared_runners_enabled: true) } - let(:sub_namespace) { build(:namespace, shared_runners_enabled: true, parent_id: parent.id) } + let(:parent) { create(:group, shared_runners_enabled: true) } + let(:group) { build(:group, shared_runners_enabled: true, parent_id: parent.id) } it 'is valid' do - expect(sub_namespace).to be_valid + expect(group).to be_valid end end end @@ -1410,30 +1465,30 @@ RSpec.describe Namespace do context 'with a parent' do context 'when parent does not allow shared runners' do - let(:parent) { create(:namespace, :shared_runners_disabled) } - let(:sub_namespace) { build(:namespace, :shared_runners_disabled, :allow_descendants_override_disabled_shared_runners, parent_id: parent.id) } + let(:parent) { create(:group, :shared_runners_disabled) } + let(:group) { build(:group, :shared_runners_disabled, :allow_descendants_override_disabled_shared_runners, parent_id: parent.id) } it 'is invalid' do - expect(sub_namespace).to be_invalid - expect(sub_namespace.errors[:allow_descendants_override_disabled_shared_runners]).to include('cannot be enabled because parent group does not allow it') + expect(group).to be_invalid + expect(group.errors[:allow_descendants_override_disabled_shared_runners]).to include('cannot be enabled because parent group does not allow it') end end context 'when parent allows shared runners and setting to true' do - let(:parent) { create(:namespace, shared_runners_enabled: true) } - let(:sub_namespace) { build(:namespace, :shared_runners_disabled, :allow_descendants_override_disabled_shared_runners, parent_id: parent.id) } + let(:parent) { create(:group, shared_runners_enabled: true) } + let(:group) { build(:group, :shared_runners_disabled, :allow_descendants_override_disabled_shared_runners, parent_id: parent.id) } it 'is valid' do - expect(sub_namespace).to be_valid + expect(group).to be_valid end end context 'when parent allows shared runners and setting to false' do - let(:parent) { create(:namespace, shared_runners_enabled: true) } - let(:sub_namespace) { build(:namespace, :shared_runners_disabled, allow_descendants_override_disabled_shared_runners: false, parent_id: parent.id) } + let(:parent) { create(:group, shared_runners_enabled: true) } + let(:group) { build(:group, :shared_runners_disabled, allow_descendants_override_disabled_shared_runners: false, parent_id: parent.id) } it 'is valid' do - expect(sub_namespace).to be_valid + expect(group).to be_valid end end end diff --git a/spec/models/onboarding_progress_spec.rb b/spec/models/onboarding_progress_spec.rb index 8da1bf773e7..779312c9fa0 100644 --- a/spec/models/onboarding_progress_spec.rb +++ b/spec/models/onboarding_progress_spec.rb @@ -106,7 +106,7 @@ RSpec.describe OnboardingProgress do end context 'when not given a root namespace' do - let(:namespace) { create(:namespace, parent: build(:namespace)) } + let(:namespace) { create(:group, parent: build(:group)) } it 'does not add a record for the namespace' do expect { onboard }.not_to change(described_class, :count).from(0) diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index c33c6c05c24..59657e68d2b 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -145,7 +145,7 @@ RSpec.describe Project, factory_default: :keep do end it_behaves_like 'model with wiki' do - let_it_be(:container) { create(:project, :wiki_repo) } + let_it_be(:container) { create(:project, :wiki_repo, namespace: create(:group)) } let(:container_without_wiki) { create(:project) } end diff --git a/spec/policies/project_policy_spec.rb b/spec/policies/project_policy_spec.rb index cf5a606bd8e..b07940760dd 100644 --- a/spec/policies/project_policy_spec.rb +++ b/spec/policies/project_policy_spec.rb @@ -105,6 +105,10 @@ RSpec.describe ProjectPolicy do context 'pipeline feature' do let(:project) { private_project } + before do + private_project.add_developer(current_user) + end + describe 'for unconfirmed user' do let(:current_user) { create(:user, confirmed_at: nil) } diff --git a/spec/requests/api/ci/runner/jobs_artifacts_spec.rb b/spec/requests/api/ci/runner/jobs_artifacts_spec.rb index 4d8da50f8f0..9369b6aa464 100644 --- a/spec/requests/api/ci/runner/jobs_artifacts_spec.rb +++ b/spec/requests/api/ci/runner/jobs_artifacts_spec.rb @@ -17,9 +17,9 @@ RSpec.describe API::Ci::Runner, :clean_gitlab_redis_shared_state do end describe '/api/v4/jobs' do - let(:root_namespace) { create(:namespace) } - let(:namespace) { create(:namespace, parent: root_namespace) } - let(:project) { create(:project, namespace: namespace, shared_runners_enabled: false) } + let(:parent_group) { create(:group) } + let(:group) { create(:group, parent: parent_group) } + let(:project) { create(:project, namespace: group, shared_runners_enabled: false) } let(:pipeline) { create(:ci_pipeline, project: project, ref: 'master') } let(:runner) { create(:ci_runner, :project, projects: [project]) } let(:user) { create(:user) } @@ -78,7 +78,7 @@ RSpec.describe API::Ci::Runner, :clean_gitlab_redis_shared_state do before do stub_application_setting(max_artifacts_size: application_max_size) - root_namespace.update!(max_artifacts_size: sample_max_size) + parent_group.update!(max_artifacts_size: sample_max_size) end it_behaves_like 'failed request' @@ -90,8 +90,8 @@ RSpec.describe API::Ci::Runner, :clean_gitlab_redis_shared_state do before do stub_application_setting(max_artifacts_size: application_max_size) - root_namespace.update!(max_artifacts_size: root_namespace_max_size) - namespace.update!(max_artifacts_size: sample_max_size) + parent_group.update!(max_artifacts_size: root_namespace_max_size) + group.update!(max_artifacts_size: sample_max_size) end it_behaves_like 'failed request' @@ -104,8 +104,8 @@ RSpec.describe API::Ci::Runner, :clean_gitlab_redis_shared_state do before do stub_application_setting(max_artifacts_size: application_max_size) - root_namespace.update!(max_artifacts_size: root_namespace_max_size) - namespace.update!(max_artifacts_size: child_namespace_max_size) + parent_group.update!(max_artifacts_size: root_namespace_max_size) + group.update!(max_artifacts_size: child_namespace_max_size) project.update!(max_artifacts_size: sample_max_size) end diff --git a/spec/requests/api/ci/runner/jobs_put_spec.rb b/spec/requests/api/ci/runner/jobs_put_spec.rb index f4c99307b1a..b5d2c4608c5 100644 --- a/spec/requests/api/ci/runner/jobs_put_spec.rb +++ b/spec/requests/api/ci/runner/jobs_put_spec.rb @@ -17,9 +17,8 @@ RSpec.describe API::Ci::Runner, :clean_gitlab_redis_shared_state do end describe '/api/v4/jobs' do - let(:root_namespace) { create(:namespace) } - let(:namespace) { create(:namespace, parent: root_namespace) } - let(:project) { create(:project, namespace: namespace, shared_runners_enabled: false) } + let(:group) { create(:group, :nested) } + let(:project) { create(:project, namespace: group, shared_runners_enabled: false) } let(:pipeline) { create(:ci_pipeline, project: project, ref: 'master') } let(:runner) { create(:ci_runner, :project, projects: [project]) } let(:user) { create(:user) } diff --git a/spec/requests/api/ci/runner/jobs_request_post_spec.rb b/spec/requests/api/ci/runner/jobs_request_post_spec.rb index 3cc27d0e1eb..def3f9f50f8 100644 --- a/spec/requests/api/ci/runner/jobs_request_post_spec.rb +++ b/spec/requests/api/ci/runner/jobs_request_post_spec.rb @@ -17,9 +17,8 @@ RSpec.describe API::Ci::Runner, :clean_gitlab_redis_shared_state do end describe '/api/v4/jobs' do - let(:root_namespace) { create(:namespace) } - let(:namespace) { create(:namespace, parent: root_namespace) } - let(:project) { create(:project, namespace: namespace, shared_runners_enabled: false) } + let(:group) { create(:group, :nested) } + let(:project) { create(:project, namespace: group, shared_runners_enabled: false) } let(:pipeline) { create(:ci_pipeline, project: project, ref: 'master') } let(:runner) { create(:ci_runner, :project, projects: [project]) } let(:user) { create(:user) } diff --git a/spec/requests/api/ci/runner/jobs_trace_spec.rb b/spec/requests/api/ci/runner/jobs_trace_spec.rb index 5b7a33d23d8..659cf055023 100644 --- a/spec/requests/api/ci/runner/jobs_trace_spec.rb +++ b/spec/requests/api/ci/runner/jobs_trace_spec.rb @@ -17,9 +17,8 @@ RSpec.describe API::Ci::Runner, :clean_gitlab_redis_shared_state do end describe '/api/v4/jobs' do - let(:root_namespace) { create(:namespace) } - let(:namespace) { create(:namespace, parent: root_namespace) } - let(:project) { create(:project, namespace: namespace, shared_runners_enabled: false) } + let(:group) { create(:group, :nested) } + let(:project) { create(:project, namespace: group, shared_runners_enabled: false) } let(:pipeline) { create(:ci_pipeline, project: project, ref: 'master') } let(:runner) { create(:ci_runner, :project, projects: [project]) } let(:user) { create(:user) } diff --git a/spec/services/onboarding_progress_service_spec.rb b/spec/services/onboarding_progress_service_spec.rb index abfe960984b..ef4f4f0d822 100644 --- a/spec/services/onboarding_progress_service_spec.rb +++ b/spec/services/onboarding_progress_service_spec.rb @@ -45,8 +45,7 @@ RSpec.describe OnboardingProgressService do end describe '#execute' do - let(:namespace) { create(:namespace, parent: root_namespace) } - let(:root_namespace) { nil } + let(:namespace) { create(:namespace) } let(:action) { :namespace_action } subject(:execute_service) { described_class.new(namespace).execute(action: :subscription_created) } @@ -64,16 +63,16 @@ RSpec.describe OnboardingProgressService do end context 'when the namespace is not the root' do - let(:root_namespace) { build(:namespace) } + let(:group) { create(:group, :nested) } before do - OnboardingProgress.onboard(root_namespace) + OnboardingProgress.onboard(group) end - it 'registers a namespace onboarding progress action for the root namespace' do + it 'does not register a namespace onboarding progress action' do execute_service - expect(OnboardingProgress.completed?(root_namespace, :subscription_created)).to eq(true) + expect(OnboardingProgress.completed?(group, :subscription_created)).to be(nil) end end @@ -83,7 +82,7 @@ RSpec.describe OnboardingProgressService do it 'does not register a namespace onboarding progress action' do execute_service - expect(OnboardingProgress.completed?(root_namespace, :subscription_created)).to be(nil) + expect(OnboardingProgress.completed?(namespace, :subscription_created)).to be(nil) end end end diff --git a/spec/support/matchers/background_migrations_matchers.rb b/spec/support/matchers/background_migrations_matchers.rb index 0144a044f6c..08bbbcc7438 100644 --- a/spec/support/matchers/background_migrations_matchers.rb +++ b/spec/support/matchers/background_migrations_matchers.rb @@ -1,7 +1,17 @@ # frozen_string_literal: true +RSpec::Matchers.define :be_background_migration_with_arguments do |arguments| + define_method :matches? do |migration| + expect do + Gitlab::BackgroundMigration.perform(migration, arguments) + end.not_to raise_error + end +end + RSpec::Matchers.define :be_scheduled_delayed_migration do |delay, *expected| - match do |migration| + define_method :matches? do |migration| + expect(migration).to be_background_migration_with_arguments(expected) + BackgroundMigrationWorker.jobs.any? do |job| job['args'] == [migration, expected] && job['at'].to_i == (delay.to_i + Time.now.to_i) @@ -16,7 +26,9 @@ RSpec::Matchers.define :be_scheduled_delayed_migration do |delay, *expected| end RSpec::Matchers.define :be_scheduled_migration do |*expected| - match do |migration| + define_method :matches? do |migration| + expect(migration).to be_background_migration_with_arguments(expected) + BackgroundMigrationWorker.jobs.any? do |job| args = job['args'].size == 1 ? [BackgroundMigrationWorker.jobs[0]['args'][0], []] : job['args'] args == [migration, expected] @@ -29,7 +41,9 @@ RSpec::Matchers.define :be_scheduled_migration do |*expected| end RSpec::Matchers.define :be_scheduled_migration_with_multiple_args do |*expected| - match do |migration| + define_method :matches? do |migration| + expect(migration).to be_background_migration_with_arguments(expected) + BackgroundMigrationWorker.jobs.any? do |job| args = job['args'].size == 1 ? [BackgroundMigrationWorker.jobs[0]['args'][0], []] : job['args'] args[0] == migration && compare_args(args, expected) diff --git a/spec/support/shared_examples/models/slack_mattermost_notifications_shared_examples.rb b/spec/support/shared_examples/models/slack_mattermost_notifications_shared_examples.rb index e31c39d7e1b..ddf68a798fb 100644 --- a/spec/support/shared_examples/models/slack_mattermost_notifications_shared_examples.rb +++ b/spec/support/shared_examples/models/slack_mattermost_notifications_shared_examples.rb @@ -66,193 +66,183 @@ RSpec.shared_examples 'slack or mattermost notifications' do |service_name| end describe "#execute" do - let(:user) { create(:user) } - let(:project) { create(:project, :repository, :wiki_repo) } - let(:username) { 'slack_username' } - let(:channel) { 'slack_channel' } - let(:issue_service_options) { { title: 'Awesome issue', description: 'please fix' } } - - let(:data) do - Gitlab::DataBuilder::Push.build_sample(project, user) - end + let_it_be(:project) { create(:project, :repository, :wiki_repo) } + let_it_be(:user) { create(:user) } + + let(:chat_service) { described_class.new( { project: project, webhook: webhook_url, branches_to_be_notified: 'all' }.merge(chat_service_params)) } + let(:chat_service_params) { {} } + let(:data) { Gitlab::DataBuilder::Push.build_sample(project, user) } let!(:stubbed_resolved_hostname) do stub_full_request(webhook_url, method: :post).request_pattern.uri_pattern.to_s end - before do - allow(chat_service).to receive_messages( - project: project, - project_id: project.id, - service_hook: true, - webhook: webhook_url - ) + subject(:execute_service) { chat_service.execute(data) } - issue_service = Issues::CreateService.new(project, user, issue_service_options) - @issue = issue_service.execute - @issues_sample_data = issue_service.hook_data(@issue, 'open') - - project.add_developer(user) - opts = { - title: 'Awesome merge_request', - description: 'please fix', - source_branch: 'feature', - target_branch: 'master' - } - merge_service = MergeRequests::CreateService.new(project, - user, opts) - @merge_request = merge_service.execute - @merge_sample_data = merge_service.hook_data(@merge_request, - 'open') - - opts = { - title: "Awesome wiki_page", - content: "Some text describing some thing or another", - format: "md", - message: "user created page: Awesome wiki_page" - } - - @wiki_page = create(:wiki_page, wiki: project.wiki, **opts) - @wiki_page_sample_data = Gitlab::DataBuilder::WikiPage.build(@wiki_page, user, 'create') - end + context 'with username for slack configured' do + let(:chat_service_params) { { username: 'slack_username' } } - it "calls #{service_name} API for push events" do - chat_service.execute(data) + it 'uses the username as an option' do + expect(Slack::Messenger).to execute_with_options(username: 'slack_username') - expect(WebMock).to have_requested(:post, stubbed_resolved_hostname).once + execute_service + end end - it "calls #{service_name} API for issue events" do - chat_service.execute(@issues_sample_data) + context 'push events' do + let(:data) { Gitlab::DataBuilder::Push.build_sample(project, user) } - expect(WebMock).to have_requested(:post, stubbed_resolved_hostname).once - end + it "calls #{service_name} API for push events" do + execute_service - it "calls #{service_name} API for merge requests events" do - chat_service.execute(@merge_sample_data) + expect(WebMock).to have_requested(:post, stubbed_resolved_hostname).once + end - expect(WebMock).to have_requested(:post, stubbed_resolved_hostname).once - end + context 'with event channel' do + let(:chat_service_params) { { push_channel: 'random' } } - it "calls #{service_name} API for wiki page events" do - chat_service.execute(@wiki_page_sample_data) + it 'uses the right channel for push event' do + expect(Slack::Messenger).to execute_with_options(channel: ['random']) - expect(WebMock).to have_requested(:post, stubbed_resolved_hostname).once + execute_service + end + end end - it "calls #{service_name} API for deployment events" do - deployment_event_data = { object_kind: 'deployment' } + context 'issue events' do + let_it_be(:issue) { create(:issue) } + let(:data) { issue.to_hook_data(user) } - chat_service.execute(deployment_event_data) + it "calls #{service_name} API for issue events" do + execute_service - expect(WebMock).to have_requested(:post, stubbed_resolved_hostname).once - end + expect(WebMock).to have_requested(:post, stubbed_resolved_hostname).once + end - it 'uses the username as an option for slack when configured' do - allow(chat_service).to receive(:username).and_return(username) + context 'whith event channel' do + let(:chat_service_params) { { issue_channel: 'random' } } - expect(Slack::Messenger).to execute_with_options(username: username) + it 'uses the right channel for issue event' do + expect(Slack::Messenger).to execute_with_options(channel: ['random']) - chat_service.execute(data) - end + execute_service + end - it 'uses the channel as an option when it is configured' do - allow(chat_service).to receive(:channel).and_return(channel) - expect(Slack::Messenger).to execute_with_options(channel: [channel]) - chat_service.execute(data) - end + context 'for confidential issues' do + before_all do + issue.update!(confidential: true) + end + + it 'falls back to issue channel' do + expect(Slack::Messenger).to execute_with_options(channel: ['random']) + + execute_service + end - context "event channels" do - it "uses the right channel for push event" do - chat_service.update!(push_channel: "random") + context 'and confidential_issue_channel is defined' do + let(:chat_service_params) { { issue_channel: 'random', confidential_issue_channel: 'confidential' } } - expect(Slack::Messenger).to execute_with_options(channel: ['random']) + it 'uses the confidential issue channel when it is defined' do + expect(Slack::Messenger).to execute_with_options(channel: ['confidential']) - chat_service.execute(data) + execute_service + end + end + end end + end - it "uses the right channel for merge request event" do - chat_service.update!(merge_request_channel: "random") + context 'merge request events' do + let_it_be(:merge_request) { create(:merge_request) } + let(:data) { merge_request.to_hook_data(user) } - expect(Slack::Messenger).to execute_with_options(channel: ['random']) + it "calls #{service_name} API for merge requests events" do + execute_service - chat_service.execute(@merge_sample_data) + expect(WebMock).to have_requested(:post, stubbed_resolved_hostname).once end - it "uses the right channel for issue event" do - chat_service.update!(issue_channel: "random") + context 'with event channel' do + let(:chat_service_params) { { merge_request_channel: 'random' } } - expect(Slack::Messenger).to execute_with_options(channel: ['random']) + it 'uses the right channel for merge request event' do + expect(Slack::Messenger).to execute_with_options(channel: ['random']) - chat_service.execute(@issues_sample_data) + execute_service + end end + end - context 'for confidential issues' do - let(:issue_service_options) { { title: 'Secret', confidential: true } } - - it "uses confidential issue channel" do - chat_service.update!(confidential_issue_channel: 'confidential') + context 'wiki page events' do + let_it_be(:wiki_page) { create(:wiki_page, wiki: project.wiki, message: 'user created page: Awesome wiki_page') } + let(:data) { Gitlab::DataBuilder::WikiPage.build(wiki_page, user, 'create') } - expect(Slack::Messenger).to execute_with_options(channel: ['confidential']) + it "calls #{service_name} API for wiki page events" do + execute_service - chat_service.execute(@issues_sample_data) - end + expect(WebMock).to have_requested(:post, stubbed_resolved_hostname).once + end - it 'falls back to issue channel' do - chat_service.update!(issue_channel: 'fallback_channel') + context 'with event channel' do + let(:chat_service_params) { { wiki_page_channel: 'random' } } - expect(Slack::Messenger).to execute_with_options(channel: ['fallback_channel']) + it 'uses the right channel for wiki event' do + expect(Slack::Messenger).to execute_with_options(channel: ['random']) - chat_service.execute(@issues_sample_data) + execute_service end end + end - it "uses the right channel for wiki event" do - chat_service.update!(wiki_page_channel: "random") + context 'deployment events' do + let_it_be(:deployment) { create(:deployment) } + let(:data) { Gitlab::DataBuilder::Deployment.build(deployment) } - expect(Slack::Messenger).to execute_with_options(channel: ['random']) + it "calls #{service_name} API for deployment events" do + execute_service - chat_service.execute(@wiki_page_sample_data) + expect(WebMock).to have_requested(:post, stubbed_resolved_hostname).once end + end - context "note event" do - let(:issue_note) do - create(:note_on_issue, project: project, note: "issue note") - end + context 'note event' do + let_it_be(:issue_note) { create(:note_on_issue, project: project, note: "issue note") } + let(:data) { Gitlab::DataBuilder::Note.build(issue_note, user) } + + it "calls #{service_name} API for note events" do + execute_service - it "uses the right channel" do - chat_service.update!(note_channel: "random") + expect(WebMock).to have_requested(:post, stubbed_resolved_hostname).once + end - note_data = Gitlab::DataBuilder::Note.build(issue_note, user) + context 'with event channel' do + let(:chat_service_params) { { note_channel: 'random' } } + it 'uses the right channel' do expect(Slack::Messenger).to execute_with_options(channel: ['random']) - chat_service.execute(note_data) + execute_service end context 'for confidential notes' do - before do - issue_note.noteable.update!(confidential: true) + before_all do + issue_note.update!(confidential: true) end - it "uses confidential channel" do - chat_service.update!(confidential_note_channel: "confidential") - - note_data = Gitlab::DataBuilder::Note.build(issue_note, user) - - expect(Slack::Messenger).to execute_with_options(channel: ['confidential']) + it 'falls back to note channel' do + expect(Slack::Messenger).to execute_with_options(channel: ['random']) - chat_service.execute(note_data) + execute_service end - it 'falls back to note channel' do - chat_service.update!(note_channel: "fallback_channel") - - note_data = Gitlab::DataBuilder::Note.build(issue_note, user) + context 'and confidential_note_channel is defined' do + let(:chat_service_params) { { note_channel: 'random', confidential_note_channel: 'confidential' } } - expect(Slack::Messenger).to execute_with_options(channel: ['fallback_channel']) + it 'uses confidential channel' do + expect(Slack::Messenger).to execute_with_options(channel: ['confidential']) - chat_service.execute(note_data) + execute_service + end end end end |