diff options
author | GitLab Bot <gitlab-bot@gitlab.com> | 2022-10-17 12:10:08 +0000 |
---|---|---|
committer | GitLab Bot <gitlab-bot@gitlab.com> | 2022-10-17 12:10:08 +0000 |
commit | 8060e5c60901ab0f6b890414dccbdf5d1b95c3ad (patch) | |
tree | fc217fe53f68a45ea225c0d1b966642852d96321 /spec | |
parent | b9b58dba70466949d761132d2d96f0f24c0b469c (diff) | |
download | gitlab-ce-8060e5c60901ab0f6b890414dccbdf5d1b95c3ad.tar.gz |
Add latest changes from gitlab-org/gitlab@master
Diffstat (limited to 'spec')
47 files changed, 1205 insertions, 784 deletions
diff --git a/spec/controllers/groups/runners_controller_spec.rb b/spec/controllers/groups/runners_controller_spec.rb index 1da868c6c4b..6dbf0803892 100644 --- a/spec/controllers/groups/runners_controller_spec.rb +++ b/spec/controllers/groups/runners_controller_spec.rb @@ -72,29 +72,11 @@ RSpec.describe Groups::RunnersController do expect(response).to render_template(:show) end - context 'when runners_finder_all_available is enabled' do - before do - stub_feature_flags(runners_finder_all_available: true) - end - - it 'renders show with 200 status code instance runner' do - get :show, params: { group_id: group, id: instance_runner } - - expect(response).to have_gitlab_http_status(:ok) - expect(response).to render_template(:show) - end - end - - context 'when runners_finder_all_available is disabled' do - before do - stub_feature_flags(runners_finder_all_available: false) - end - - it 'renders show with a 404 instance runner' do - get :show, params: { group_id: group, id: instance_runner } + it 'renders show with 200 status code instance runner' do + get :show, params: { group_id: group, id: instance_runner } - expect(response).to have_gitlab_http_status(:not_found) - end + expect(response).to have_gitlab_http_status(:ok) + expect(response).to render_template(:show) end it 'renders show with 200 status code project runner' do diff --git a/spec/controllers/groups_controller_spec.rb b/spec/controllers/groups_controller_spec.rb index c4e4eeec953..5bbe236077c 100644 --- a/spec/controllers/groups_controller_spec.rb +++ b/spec/controllers/groups_controller_spec.rb @@ -229,7 +229,7 @@ RSpec.describe GroupsController, factory_default: :keep do sign_in(user) expect do - post :create, params: { group: { name: 'new_group', path: "new_group" } } + post :create, params: { group: { name: 'new_group', path: 'new_group' } } end.to change { Group.count }.by(1) expect(response).to have_gitlab_http_status(:found) @@ -240,13 +240,31 @@ RSpec.describe GroupsController, factory_default: :keep do sign_in(create(:admin)) expect do - post :create, params: { group: { name: 'new_group', path: "new_group" } } + post :create, params: { group: { name: 'new_group', path: 'new_group' } } end.to change { Group.count }.by(1) expect(response).to have_gitlab_http_status(:found) end end + context 'when creating chat team' do + before do + stub_mattermost_setting(enabled: true) + end + + it 'triggers Mattermost::CreateTeamService' do + sign_in(user) + + expect_next_instance_of(::Mattermost::CreateTeamService) do |service| + expect(service).to receive(:execute).and_return({ name: 'test-chat-team', id: 1 }) + end + + post :create, params: { group: { name: 'new_group', path: 'new_group', create_chat_team: 1 } } + + expect(response).to have_gitlab_http_status(:found) + end + end + context 'when creating subgroups' do [true, false].each do |can_create_group_status| context "and can_create_group is #{can_create_group_status}" do diff --git a/spec/factories/users/namespace_user_callouts.rb b/spec/factories/users/namespace_user_callouts.rb deleted file mode 100644 index fded63d0cce..00000000000 --- a/spec/factories/users/namespace_user_callouts.rb +++ /dev/null @@ -1,10 +0,0 @@ -# frozen_string_literal: true - -FactoryBot.define do - factory :namespace_callout, class: 'Users::NamespaceCallout' do - feature_name { :invite_members_banner } - - user - namespace - end -end diff --git a/spec/features/groups/group_runners_spec.rb b/spec/features/groups/group_runners_spec.rb index 97c9103a054..e9807c487d5 100644 --- a/spec/features/groups/group_runners_spec.rb +++ b/spec/features/groups/group_runners_spec.rb @@ -119,45 +119,27 @@ RSpec.describe "Group Runners" do create(:ci_runner, :instance, description: 'runner-baz', contacted_at: Time.zone.now) end - context "when runners_finder_all_available is enabled" do - before do - stub_feature_flags(runners_finder_all_available: true) - - visit group_runners_path(group) - end - - context "when selecting 'Show only inherited'" do - before do - find("[data-testid='runner-membership-toggle'] button").click - - wait_for_requests - end - - it_behaves_like 'shows runner in list' do - let(:runner) { instance_runner } - end - - it 'shows runner details page' do - click_link("##{instance_runner.id} (#{instance_runner.short_sha})") - - expect(current_url).to include(group_runner_path(group, instance_runner)) - expect(page).to have_content "#{s_('Runners|Description')} runner-baz" - end - end + before do + visit group_runners_path(group) end - context "when runners_finder_all_available is disabled" do + context "when selecting 'Show only inherited'" do before do - stub_feature_flags(runners_finder_all_available: false) + find("[data-testid='runner-membership-toggle'] button").click - visit group_runners_path(group) + wait_for_requests end - it "does not display 'Show only inherited' toggle" do - expect(page).not_to have_content(s_('Runners|Show only inherited')) + it_behaves_like 'shows runner in list' do + let(:runner) { instance_runner } end - it_behaves_like 'shows no runners registered' + it 'shows runner details page' do + click_link("##{instance_runner.id} (#{instance_runner.short_sha})") + + expect(current_url).to include(group_runner_path(group, instance_runner)) + expect(page).to have_content "#{s_('Runners|Description')} runner-baz" + end end end diff --git a/spec/finders/ci/runners_finder_spec.rb b/spec/finders/ci/runners_finder_spec.rb index 62663cd5c27..18eecd0f073 100644 --- a/spec/finders/ci/runners_finder_spec.rb +++ b/spec/finders/ci/runners_finder_spec.rb @@ -322,27 +322,11 @@ RSpec.describe Ci::RunnersFinder do context 'with :all_available membership' do let(:membership) { :all_available } - context 'with runners_finder_all_available FF disabled' do - before do - stub_feature_flags(runners_finder_all_available: false) - end - - it 'returns no runners' do - expect(subject).to be_empty - end - end - - context 'with runners_finder_all_available FF enabled' do - before do - stub_feature_flags(runners_finder_all_available: [target_group]) - end - - it 'returns runners available to group' do - expect(subject).to match_array([runner_project_7, runner_project_6, runner_project_5, - runner_project_4, runner_project_3, runner_project_2, - runner_project_1, runner_sub_group_4, runner_sub_group_3, - runner_sub_group_2, runner_sub_group_1, runner_group, runner_instance]) - end + it 'returns runners available to group' do + expect(subject).to match_array([runner_project_7, runner_project_6, runner_project_5, + runner_project_4, runner_project_3, runner_project_2, + runner_project_1, runner_sub_group_4, runner_sub_group_3, + runner_sub_group_2, runner_sub_group_1, runner_group, runner_instance]) end end @@ -448,14 +432,8 @@ RSpec.describe Ci::RunnersFinder do context 'with :all_available membership' do let(:membership) { :all_available } - context 'with runners_finder_all_available FF enabled' do - before do - stub_feature_flags(runners_finder_all_available: [target_group]) - end - - it 'returns no runners' do - expect(subject).to be_empty - end + it 'returns no runners' do + expect(subject).to be_empty end end end diff --git a/spec/frontend/access_tokens/components/new_access_token_app_spec.js b/spec/frontend/access_tokens/components/new_access_token_app_spec.js index d12d200d214..b4af11169ad 100644 --- a/spec/frontend/access_tokens/components/new_access_token_app_spec.js +++ b/spec/frontend/access_tokens/components/new_access_token_app_spec.js @@ -22,6 +22,8 @@ describe('~/access_tokens/components/new_access_token_app', () => { }); }; + const findButtonEl = () => document.querySelector('[type=submit]'); + const triggerSuccess = async (newToken = 'new token') => { wrapper .findComponent(DomElementListener) @@ -41,7 +43,7 @@ describe('~/access_tokens/components/new_access_token_app', () => { <input type="text" id="expires_at" value="2022-01-01"/> <input type="text" value='1'/> <input type="checkbox" checked/> - <input type="submit" value="Create"/> + <button type="submit" value="Create" class="disabled" disabled="disabled"/> </form>`, ); @@ -120,10 +122,10 @@ describe('~/access_tokens/components/new_access_token_app', () => { }); it('should not reset the submit button value', async () => { - expect(document.querySelector('input[type=submit]').value).toBe('Create'); + expect(findButtonEl().value).toBe('Create'); await triggerSuccess(); - expect(document.querySelector('input[type=submit]').value).toBe('Create'); + expect(findButtonEl().value).toBe('Create'); }); }); }); @@ -162,6 +164,17 @@ describe('~/access_tokens/components/new_access_token_app', () => { expect(wrapper.findComponent(GlAlert).exists()).toBe(false); }); + + it('should enable the submit button', async () => { + const button = findButtonEl(); + expect(button).toBeDisabled(); + expect(button.className).toBe('disabled'); + + await triggerError(); + + expect(button).not.toBeDisabled(); + expect(button.className).toBe(''); + }); }); describe('before error or success', () => { diff --git a/spec/frontend/runner/group_runners/group_runners_app_spec.js b/spec/frontend/runner/group_runners/group_runners_app_spec.js index ce91f5a6ba6..5f355e27d9e 100644 --- a/spec/frontend/runner/group_runners/group_runners_app_spec.js +++ b/spec/frontend/runner/group_runners/group_runners_app_spec.js @@ -159,55 +159,30 @@ describe('GroupRunnersApp', () => { }); describe('show all available runners toggle', () => { - describe('when runners_finder_all_available is enabled', () => { - it('shows the membership toggle', () => { - createComponent({ - provide: { - glFeatures: { runnersFinderAllAvailable: true }, - }, - }); - - expect(findRunnerMembershipToggle().exists()).toBe(true); - }); - - it('sets the membership toggle', () => { - setWindowLocation(`?membership[]=${MEMBERSHIP_ALL_AVAILABLE}`); - createComponent({ - provide: { - glFeatures: { runnersFinderAllAvailable: true }, - }, - }); - - expect(findRunnerMembershipToggle().props('value')).toBe(MEMBERSHIP_ALL_AVAILABLE); - }); - - it('requests filter', async () => { - createComponent({ - provide: { - glFeatures: { runnersFinderAllAvailable: true }, - }, - }); + it('shows the membership toggle', () => { + createComponent(); + expect(findRunnerMembershipToggle().exists()).toBe(true); + }); - findRunnerMembershipToggle().vm.$emit('input', MEMBERSHIP_ALL_AVAILABLE); + it('sets the membership toggle', () => { + setWindowLocation(`?membership[]=${MEMBERSHIP_ALL_AVAILABLE}`); - await waitForPromises(); + createComponent(); - expect(mockGroupRunnersHandler).toHaveBeenLastCalledWith( - expect.objectContaining({ - membership: MEMBERSHIP_ALL_AVAILABLE, - }), - ); - }); + expect(findRunnerMembershipToggle().props('value')).toBe(MEMBERSHIP_ALL_AVAILABLE); }); - describe('when runners_finder_all_available is disabled', () => { - beforeEach(() => { - createComponent(); - }); + it('requests filter', async () => { + createComponent(); + findRunnerMembershipToggle().vm.$emit('input', MEMBERSHIP_ALL_AVAILABLE); - it('does not show the membership toggle', () => { - expect(findRunnerMembershipToggle().exists()).toBe(false); - }); + await waitForPromises(); + + expect(mockGroupRunnersHandler).toHaveBeenLastCalledWith( + expect.objectContaining({ + membership: MEMBERSHIP_ALL_AVAILABLE, + }), + ); }); }); diff --git a/spec/frontend/token_access/token_access_spec.js b/spec/frontend/token_access/token_access_spec.js index e6aef41cc8b..c55ac32b6a6 100644 --- a/spec/frontend/token_access/token_access_spec.js +++ b/spec/frontend/token_access/token_access_spec.js @@ -4,7 +4,7 @@ import VueApollo from 'vue-apollo'; import createMockApollo from 'helpers/mock_apollo_helper'; import { mountExtended, shallowMountExtended } from 'helpers/vue_test_utils_helper'; import waitForPromises from 'helpers/wait_for_promises'; -import createFlash from '~/flash'; +import { createAlert } from '~/flash'; import TokenAccess from '~/token_access/components/token_access.vue'; import addProjectCIJobTokenScopeMutation from '~/token_access/graphql/mutations/add_project_ci_job_token_scope.mutation.graphql'; import removeProjectCIJobTokenScopeMutation from '~/token_access/graphql/mutations/remove_project_ci_job_token_scope.mutation.graphql'; @@ -144,7 +144,7 @@ describe('TokenAccess component', () => { await waitForPromises(); - expect(createFlash).toHaveBeenCalled(); + expect(createAlert).toHaveBeenCalled(); }); }); @@ -187,7 +187,7 @@ describe('TokenAccess component', () => { await waitForPromises(); - expect(createFlash).toHaveBeenCalled(); + expect(createAlert).toHaveBeenCalled(); }); }); }); diff --git a/spec/frontend/vue_merge_request_widget/components/approvals/approvals_spec.js b/spec/frontend/vue_merge_request_widget/components/approvals/approvals_spec.js index 612fc2112d0..1f3b6dce620 100644 --- a/spec/frontend/vue_merge_request_widget/components/approvals/approvals_spec.js +++ b/spec/frontend/vue_merge_request_widget/components/approvals/approvals_spec.js @@ -1,7 +1,7 @@ import { nextTick } from 'vue'; import { GlButton, GlSprintf } from '@gitlab/ui'; import { shallowMount } from '@vue/test-utils'; -import createFlash from '~/flash'; +import { createAlert } from '~/flash'; import Approvals from '~/vue_merge_request_widget/components/approvals/approvals.vue'; import ApprovalsSummary from '~/vue_merge_request_widget/components/approvals/approvals_summary.vue'; import ApprovalsSummaryOptional from '~/vue_merge_request_widget/components/approvals/approvals_summary_optional.vue'; @@ -129,7 +129,7 @@ describe('MRWidget approvals', () => { }); it('flashes error', () => { - expect(createFlash).toHaveBeenCalledWith({ message: FETCH_ERROR }); + expect(createAlert).toHaveBeenCalledWith({ message: FETCH_ERROR }); }); }); @@ -268,7 +268,7 @@ describe('MRWidget approvals', () => { }); it('flashes error message', () => { - expect(createFlash).toHaveBeenCalledWith({ message: APPROVE_ERROR }); + expect(createAlert).toHaveBeenCalledWith({ message: APPROVE_ERROR }); }); }); }); @@ -319,7 +319,7 @@ describe('MRWidget approvals', () => { }); it('flashes error message', () => { - expect(createFlash).toHaveBeenCalledWith({ message: UNAPPROVE_ERROR }); + expect(createAlert).toHaveBeenCalledWith({ message: UNAPPROVE_ERROR }); }); }); }); diff --git a/spec/frontend/vue_merge_request_widget/deployment/deployment_actions_spec.js b/spec/frontend/vue_merge_request_widget/deployment/deployment_actions_spec.js index d3e17065d89..58dadb2c679 100644 --- a/spec/frontend/vue_merge_request_widget/deployment/deployment_actions_spec.js +++ b/spec/frontend/vue_merge_request_widget/deployment/deployment_actions_spec.js @@ -1,6 +1,6 @@ import { mount } from '@vue/test-utils'; import waitForPromises from 'helpers/wait_for_promises'; -import createFlash from '~/flash'; +import { createAlert } from '~/flash'; import { confirmAction } from '~/lib/utils/confirm_via_gl_modal/confirm_via_gl_modal'; import { visitUrl } from '~/lib/utils/url_utility'; import { @@ -168,7 +168,7 @@ describe('DeploymentAction component', () => { }); it('should not throw an error', () => { - expect(createFlash).not.toHaveBeenCalled(); + expect(createAlert).not.toHaveBeenCalled(); }); describe('response includes redirect_url', () => { @@ -225,9 +225,8 @@ describe('DeploymentAction component', () => { finderFn().trigger('click'); }); - it('should call createFlash with error message', () => { - expect(createFlash).toHaveBeenCalled(); - expect(createFlash).toHaveBeenCalledWith({ + it('should call createAlert with error message', () => { + expect(createAlert).toHaveBeenCalledWith({ message: actionButtonMocks[configConst].errorMessage, }); }); diff --git a/spec/graphql/mutations/namespace/package_settings/update_spec.rb b/spec/graphql/mutations/namespace/package_settings/update_spec.rb index 631e02ff3dc..09ac1c99b10 100644 --- a/spec/graphql/mutations/namespace/package_settings/update_spec.rb +++ b/spec/graphql/mutations/namespace/package_settings/update_spec.rb @@ -26,8 +26,29 @@ RSpec.describe Mutations::Namespace::PackageSettings::Update do RSpec.shared_examples 'updating the namespace package setting' do it_behaves_like 'updating the namespace package setting attributes', - from: { maven_duplicates_allowed: true, maven_duplicate_exception_regex: 'SNAPSHOT', generic_duplicates_allowed: true, generic_duplicate_exception_regex: 'foo' }, - to: { maven_duplicates_allowed: false, maven_duplicate_exception_regex: 'RELEASE', generic_duplicates_allowed: false, generic_duplicate_exception_regex: 'bar' } + from: { + maven_duplicates_allowed: true, + maven_duplicate_exception_regex: 'SNAPSHOT', + generic_duplicates_allowed: true, + generic_duplicate_exception_regex: 'foo', + maven_package_requests_forwarding: nil, + lock_maven_package_requests_forwarding: false, + npm_package_requests_forwarding: nil, + lock_npm_package_requests_forwarding: false, + pypi_package_requests_forwarding: nil, + lock_pypi_package_requests_forwarding: false + }, to: { + maven_duplicates_allowed: false, + maven_duplicate_exception_regex: 'RELEASE', + generic_duplicates_allowed: false, + generic_duplicate_exception_regex: 'bar', + maven_package_requests_forwarding: true, + lock_maven_package_requests_forwarding: true, + npm_package_requests_forwarding: true, + lock_npm_package_requests_forwarding: true, + pypi_package_requests_forwarding: true, + lock_pypi_package_requests_forwarding: true + } it_behaves_like 'returning a success' @@ -59,11 +80,19 @@ RSpec.describe Mutations::Namespace::PackageSettings::Update do context 'with existing namespace package setting' do let_it_be(:package_settings) { create(:namespace_package_setting, namespace: namespace) } let_it_be(:params) do - { namespace_path: namespace.full_path, + { + namespace_path: namespace.full_path, maven_duplicates_allowed: false, maven_duplicate_exception_regex: 'RELEASE', generic_duplicates_allowed: false, - generic_duplicate_exception_regex: 'bar' } + generic_duplicate_exception_regex: 'bar', + maven_package_requests_forwarding: true, + lock_maven_package_requests_forwarding: true, + npm_package_requests_forwarding: true, + lock_npm_package_requests_forwarding: true, + pypi_package_requests_forwarding: true, + lock_pypi_package_requests_forwarding: true + } end where(:user_role, :shared_examples_name) do diff --git a/spec/graphql/types/namespace/package_settings_type_spec.rb b/spec/graphql/types/namespace/package_settings_type_spec.rb index f63a0a7010f..5039f2d6153 100644 --- a/spec/graphql/types/namespace/package_settings_type_spec.rb +++ b/spec/graphql/types/namespace/package_settings_type_spec.rb @@ -14,4 +14,24 @@ RSpec.describe GitlabSchema.types['PackageSettings'] do it { is_expected.to have_graphql_type(Types::UntrustedRegexp) } end + + it 'includes package setting fields' do + expected_fields = %w[ + maven_duplicates_allowed + maven_duplicate_exception_regex + generic_duplicates_allowed + generic_duplicate_exception_regex + maven_package_requests_forwarding + lock_maven_package_requests_forwarding + npm_package_requests_forwarding + lock_npm_package_requests_forwarding + pypi_package_requests_forwarding + lock_pypi_package_requests_forwarding + maven_package_requests_forwarding_locked + npm_package_requests_forwarding_locked + pypi_package_requests_forwarding_locked + ] + + expect(described_class).to include_graphql_fields(*expected_fields) + end end diff --git a/spec/helpers/events_helper_spec.rb b/spec/helpers/events_helper_spec.rb index cc6804f0355..7005b3dc53e 100644 --- a/spec/helpers/events_helper_spec.rb +++ b/spec/helpers/events_helper_spec.rb @@ -24,6 +24,45 @@ RSpec.describe EventsHelper do end end + describe '#localized_action_name' do + it 'handles all valid design events' do + created, updated, destroyed = %i[created updated destroyed].map do |trait| + event = build(:design_event, trait) + helper.localized_action_name(event) + end + + expect(created).to eq(_('added')) + expect(updated).to eq(_('updated')) + expect(destroyed).to eq(_('removed')) + end + + context 'handles correct base actions' do + using RSpec::Parameterized::TableSyntax + + where(:trait, :localized_action_name) do + :created | s_('Event|created') + :updated | s_('Event|opened') + :closed | s_('Event|closed') + :reopened | s_('Event|opened') + :commented | s_('Event|commented on') + :merged | s_('Event|accepted') + :joined | s_('Event|joined') + :left | s_('Event|left') + :destroyed | s_('Event|destroyed') + :expired | s_('Event|removed due to membership expiration from') + :approved | s_('Event|approved') + end + + with_them do + it 'with correct name and method' do + event = build(:event, trait) + + expect(helper.localized_action_name(event)).to eq(localized_action_name) + end + end + end + end + describe '#event_commit_title' do let(:message) { 'foo & bar ' + 'A' * 70 + '\n' + 'B' * 80 } diff --git a/spec/initializers/memory_watchdog_spec.rb b/spec/initializers/memory_watchdog_spec.rb index 56f995b5cd3..36f96131c3d 100644 --- a/spec/initializers/memory_watchdog_spec.rb +++ b/spec/initializers/memory_watchdog_spec.rb @@ -4,7 +4,7 @@ require 'fast_spec_helper' RSpec.describe 'memory watchdog' do subject(:run_initializer) do - load Rails.root.join('config/initializers/memory_watchdog.rb') + load rails_root_join('config/initializers/memory_watchdog.rb') end context 'when GITLAB_MEMORY_WATCHDOG_ENABLED is truthy' do @@ -17,6 +17,7 @@ RSpec.describe 'memory watchdog' do context 'when runtime is an application' do let(:watchdog) { instance_double(Gitlab::Memory::Watchdog) } let(:background_task) { instance_double(Gitlab::BackgroundTask) } + let(:logger) { Gitlab::AppLogger } before do allow(Gitlab::Runtime).to receive(:application?).and_return(true) @@ -28,16 +29,65 @@ RSpec.describe 'memory watchdog' do run_initializer end - shared_examples 'starts watchdog with handler' do |handler_class| - it "uses the #{handler_class} and starts the watchdog" do - expect(Gitlab::Memory::Watchdog).to receive(:new).with( - handler: an_instance_of(handler_class), - logger: Gitlab::AppLogger).and_return(watchdog) - expect(Gitlab::BackgroundTask).to receive(:new).with(watchdog).and_return(background_task) - expect(background_task).to receive(:start) - expect(Gitlab::Cluster::LifecycleEvents).to receive(:on_worker_start).and_yield + shared_examples 'starts configured watchdog' do |handler_class| + let(:configuration) { Gitlab::Memory::Watchdog::Configuration.new } + let(:watchdog_monitors_params) do + { + Gitlab::Memory::Watchdog::Monitor::HeapFragmentation => { + max_heap_fragmentation: max_heap_fragmentation, + max_strikes: max_strikes + }, + Gitlab::Memory::Watchdog::Monitor::UniqueMemoryGrowth => { + max_mem_growth: max_mem_growth, + max_strikes: max_strikes + } + } + end + + shared_examples 'configures and starts watchdog' do + it "correctly configures and starts watchdog", :aggregate_failures do + expect(watchdog).to receive(:configure).and_yield(configuration) + + watchdog_monitors_params.each do |monitor_class, params| + expect(configuration.monitors).to receive(:use).with(monitor_class, **params) + end + + expect(Gitlab::Memory::Watchdog).to receive(:new).and_return(watchdog) + expect(Gitlab::BackgroundTask).to receive(:new).with(watchdog).and_return(background_task) + expect(background_task).to receive(:start) + expect(Gitlab::Cluster::LifecycleEvents).to receive(:on_worker_start).and_yield + + run_initializer + + expect(configuration.handler).to be_an_instance_of(handler_class) + expect(configuration.logger).to eq(logger) + expect(configuration.sleep_time_seconds).to eq(sleep_time_seconds) + end + end + + context 'when settings are not passed through the environment' do + let(:max_strikes) { 5 } + let(:max_heap_fragmentation) { 0.5 } + let(:max_mem_growth) { 3.0 } + let(:sleep_time_seconds) { 60 } + + include_examples 'configures and starts watchdog' + end + + context 'when settings are passed through the environment' do + let(:max_strikes) { 6 } + let(:max_heap_fragmentation) { 0.4 } + let(:max_mem_growth) { 2.0 } + let(:sleep_time_seconds) { 50 } + + before do + stub_env('GITLAB_MEMWD_MAX_STRIKES', 6) + stub_env('GITLAB_MEMWD_SLEEP_TIME_SEC', 50) + stub_env('GITLAB_MEMWD_MAX_MEM_GROWTH', 2.0) + stub_env('GITLAB_MEMWD_MAX_HEAP_FRAG', 0.4) + end - run_initializer + include_examples 'configures and starts watchdog' end end @@ -59,7 +109,7 @@ RSpec.describe 'memory watchdog' do allow(Gitlab::Runtime).to receive(:puma?).and_return(true) end - it_behaves_like 'starts watchdog with handler', Gitlab::Memory::Watchdog::PumaHandler + it_behaves_like 'starts configured watchdog', Gitlab::Memory::Watchdog::PumaHandler end # rubocop: enable RSpec/VerifiedDoubles @@ -68,11 +118,11 @@ RSpec.describe 'memory watchdog' do allow(Gitlab::Runtime).to receive(:sidekiq?).and_return(true) end - it_behaves_like 'starts watchdog with handler', Gitlab::Memory::Watchdog::TermProcessHandler + it_behaves_like 'starts configured watchdog', Gitlab::Memory::Watchdog::TermProcessHandler end context 'when other runtime' do - it_behaves_like 'starts watchdog with handler', Gitlab::Memory::Watchdog::NullHandler + it_behaves_like 'starts configured watchdog', Gitlab::Memory::Watchdog::NullHandler end end diff --git a/spec/lib/gitlab/bitbucket_import/importer_spec.rb b/spec/lib/gitlab/bitbucket_import/importer_spec.rb index a9534101567..186d4e1fb42 100644 --- a/spec/lib/gitlab/bitbucket_import/importer_spec.rb +++ b/spec/lib/gitlab/bitbucket_import/importer_spec.rb @@ -58,16 +58,15 @@ RSpec.describe Gitlab::BitbucketImport::Importer do issues end - let(:project_identifier) { 'namespace/repo' } - let(:data) { { 'token' => 'token' } } + let_it_be(:project_identifier) { 'namespace/repo' } - let(:project) do + let_it_be_with_reload(:project) do create( :project, :repository, import_source: project_identifier, import_url: "https://bitbucket.org/#{project_identifier}.git", - import_data_attributes: { credentials: data } + import_data_attributes: { credentials: { 'token' => 'token' } } ) end @@ -80,6 +79,14 @@ RSpec.describe Gitlab::BitbucketImport::Importer do } end + let(:last_issue_data) do + { + page: 1, + pagelen: 1, + values: [sample_issues_statuses.last] + } + end + let(:counter) { double('counter', increment: true) } subject { described_class.new(project) } @@ -245,6 +252,13 @@ RSpec.describe Gitlab::BitbucketImport::Importer do stub_request( :get, + "https://api.bitbucket.org/2.0/repositories/#{project_identifier}/issues?pagelen=1&sort=-created_on&state=ALL" + ).to_return(status: 200, + headers: { "Content-Type" => "application/json" }, + body: last_issue_data.to_json) + + stub_request( + :get, "https://api.bitbucket.org/2.0/repositories/#{project_identifier}/issues?pagelen=50&sort=created_on" ).to_return(status: 200, headers: { "Content-Type" => "application/json" }, @@ -344,6 +358,12 @@ RSpec.describe Gitlab::BitbucketImport::Importer do end describe 'issue import' do + it 'allocates internal ids' do + expect(Issue).to receive(:track_project_iid!).with(project, 6) + + importer.execute + end + it 'maps reporters to anonymous if bitbucket reporter is nil' do allow(importer).to receive(:import_wiki) importer.execute @@ -363,6 +383,29 @@ RSpec.describe Gitlab::BitbucketImport::Importer do expect(project.issues.map(&:work_item_type_id).uniq).to contain_exactly(WorkItems::Type.default_issue_type.id) end + + context 'with issue comments' do + let(:inline_note) do + instance_double(Bitbucket::Representation::Comment, note: 'Hello world', author: 'someuser', created_at: Time.now, updated_at: Time.now) + end + + before do + allow_next_instance_of(Bitbucket::Client) do |instance| + allow(instance).to receive(:issue_comments).and_return([inline_note]) + end + end + + it 'imports issue comments' do + allow(importer).to receive(:import_wiki) + importer.execute + + comment = project.notes.first + expect(project.notes.size).to eq(7) + expect(comment.note).to include(inline_note.note) + expect(comment.note).to include(inline_note.author) + expect(importer.errors).to be_empty + end + end end context 'metrics' do diff --git a/spec/lib/gitlab/github_import/importer/attachments/issues_importer_spec.rb b/spec/lib/gitlab/github_import/importer/attachments/issues_importer_spec.rb new file mode 100644 index 00000000000..85bc67376d3 --- /dev/null +++ b/spec/lib/gitlab/github_import/importer/attachments/issues_importer_spec.rb @@ -0,0 +1,61 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Gitlab::GithubImport::Importer::Attachments::IssuesImporter do + subject(:importer) { described_class.new(project, client) } + + let_it_be(:project) { create(:project) } + + let(:client) { instance_double(Gitlab::GithubImport::Client) } + + describe '#sequential_import', :clean_gitlab_redis_cache do + let_it_be(:issue_1) { create(:issue, project: project) } + let_it_be(:issue_2) { create(:issue, project: project) } + + let(:importer_stub) { instance_double('Gitlab::GithubImport::Importer::NoteAttachmentsImporter') } + let(:importer_attrs) { [instance_of(Gitlab::GithubImport::Representation::NoteText), project, client] } + + it 'imports each project issue attachments' do + expect_next_instances_of( + Gitlab::GithubImport::Importer::NoteAttachmentsImporter, 2, false, *importer_attrs + ) do |note_attachments_importer| + expect(note_attachments_importer).to receive(:execute) + end + + importer.sequential_import + end + + context 'when issue is already processed' do + it "doesn't import this issue attachments" do + importer.mark_as_imported(issue_1) + + expect_next_instance_of( + Gitlab::GithubImport::Importer::NoteAttachmentsImporter, *importer_attrs + ) do |note_attachments_importer| + expect(note_attachments_importer).to receive(:execute) + end + + importer.sequential_import + end + end + end + + describe '#sidekiq_worker_class' do + it { expect(importer.sidekiq_worker_class).to eq(Gitlab::GithubImport::Attachments::ImportIssueWorker) } + end + + describe '#collection_method' do + it { expect(importer.collection_method).to eq(:issue_attachments) } + end + + describe '#object_type' do + it { expect(importer.object_type).to eq(:issue_attachment) } + end + + describe '#id_for_already_imported_cache' do + let(:issue) { build_stubbed(:issue) } + + it { expect(importer.id_for_already_imported_cache(issue)).to eq(issue.id) } + end +end diff --git a/spec/lib/gitlab/github_import/importer/attachments/merge_requests_importer_spec.rb b/spec/lib/gitlab/github_import/importer/attachments/merge_requests_importer_spec.rb new file mode 100644 index 00000000000..e4718c2d17c --- /dev/null +++ b/spec/lib/gitlab/github_import/importer/attachments/merge_requests_importer_spec.rb @@ -0,0 +1,61 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Gitlab::GithubImport::Importer::Attachments::MergeRequestsImporter do + subject(:importer) { described_class.new(project, client) } + + let_it_be(:project) { create(:project) } + + let(:client) { instance_double(Gitlab::GithubImport::Client) } + + describe '#sequential_import', :clean_gitlab_redis_cache do + let_it_be(:merge_request_1) { create(:merge_request, source_project: project, target_branch: 'feature1') } + let_it_be(:merge_request_2) { create(:merge_request, source_project: project, target_branch: 'feature2') } + + let(:importer_stub) { instance_double('Gitlab::GithubImport::Importer::NoteAttachmentsImporter') } + let(:importer_attrs) { [instance_of(Gitlab::GithubImport::Representation::NoteText), project, client] } + + it 'imports each project merge request attachments' do + expect_next_instances_of( + Gitlab::GithubImport::Importer::NoteAttachmentsImporter, 2, false, *importer_attrs + ) do |note_attachments_importer| + expect(note_attachments_importer).to receive(:execute) + end + + importer.sequential_import + end + + context 'when merge request is already processed' do + it "doesn't import this merge request attachments" do + importer.mark_as_imported(merge_request_1) + + expect_next_instance_of( + Gitlab::GithubImport::Importer::NoteAttachmentsImporter, *importer_attrs + ) do |note_attachments_importer| + expect(note_attachments_importer).to receive(:execute) + end + + importer.sequential_import + end + end + end + + describe '#sidekiq_worker_class' do + it { expect(importer.sidekiq_worker_class).to eq(Gitlab::GithubImport::Attachments::ImportMergeRequestWorker) } + end + + describe '#collection_method' do + it { expect(importer.collection_method).to eq(:merge_request_attachments) } + end + + describe '#object_type' do + it { expect(importer.object_type).to eq(:merge_request_attachment) } + end + + describe '#id_for_already_imported_cache' do + let(:merge_request) { build_stubbed(:merge_request) } + + it { expect(importer.id_for_already_imported_cache(merge_request)).to eq(merge_request.id) } + end +end diff --git a/spec/lib/gitlab/github_import/importer/attachments/notes_importer_spec.rb b/spec/lib/gitlab/github_import/importer/attachments/notes_importer_spec.rb index f66a7b2eb8f..7ed353e1b71 100644 --- a/spec/lib/gitlab/github_import/importer/attachments/notes_importer_spec.rb +++ b/spec/lib/gitlab/github_import/importer/attachments/notes_importer_spec.rb @@ -38,14 +38,6 @@ RSpec.describe Gitlab::GithubImport::Importer::Attachments::NotesImporter do end end - describe '#representation_class' do - it { expect(importer.representation_class).to eq(Gitlab::GithubImport::Representation::NoteText) } - end - - describe '#importer_class' do - it { expect(importer.importer_class).to eq(Gitlab::GithubImport::Importer::NoteAttachmentsImporter) } - end - describe '#sidekiq_worker_class' do it { expect(importer.sidekiq_worker_class).to eq(Gitlab::GithubImport::Attachments::ImportNoteWorker) } end diff --git a/spec/lib/gitlab/github_import/importer/attachments/releases_importer_spec.rb b/spec/lib/gitlab/github_import/importer/attachments/releases_importer_spec.rb index 584e6301f4c..b989345ae09 100644 --- a/spec/lib/gitlab/github_import/importer/attachments/releases_importer_spec.rb +++ b/spec/lib/gitlab/github_import/importer/attachments/releases_importer_spec.rb @@ -37,14 +37,6 @@ RSpec.describe Gitlab::GithubImport::Importer::Attachments::ReleasesImporter do end end - describe '#representation_class' do - it { expect(importer.representation_class).to eq(Gitlab::GithubImport::Representation::NoteText) } - end - - describe '#importer_class' do - it { expect(importer.importer_class).to eq(Gitlab::GithubImport::Importer::NoteAttachmentsImporter) } - end - describe '#sidekiq_worker_class' do it { expect(importer.sidekiq_worker_class).to eq(Gitlab::GithubImport::Attachments::ImportReleaseWorker) } end diff --git a/spec/lib/gitlab/github_import/importer/note_attachments_importer_spec.rb b/spec/lib/gitlab/github_import/importer/note_attachments_importer_spec.rb index d4c5612e906..7d4e3c3bcce 100644 --- a/spec/lib/gitlab/github_import/importer/note_attachments_importer_spec.rb +++ b/spec/lib/gitlab/github_import/importer/note_attachments_importer_spec.rb @@ -7,6 +7,7 @@ RSpec.describe Gitlab::GithubImport::Importer::NoteAttachmentsImporter do let_it_be(:project) { create(:project) } + let(:note_text) { Gitlab::GithubImport::Representation::NoteText.from_db_record(record) } let(:client) { instance_double('Gitlab::GithubImport::Client') } let(:doc_url) { 'https://github.com/nickname/public-test-repo/files/9020437/git-cheat-sheet.txt' } @@ -22,6 +23,17 @@ RSpec.describe Gitlab::GithubImport::Importer::NoteAttachmentsImporter do TEXT end + shared_examples 'updates record description' do + it do + importer.execute + + record.reload + expect(record.description).to start_with("Some text...\n\n[special-doc](/uploads/") + expect(record.description).to include('![image.jpeg](/uploads/') + expect(record.description).to include('<img width="248" alt="tag-image" src="/uploads') + end + end + describe '#execute' do let(:downloader_stub) { instance_double(Gitlab::GithubImport::AttachmentsDownloader) } let(:tmp_stub_doc) { Tempfile.create('attachment_download_test.txt') } @@ -40,37 +52,33 @@ RSpec.describe Gitlab::GithubImport::Importer::NoteAttachmentsImporter do end context 'when importing release attachments' do - let(:release) { create(:release, project: project, description: text) } - let(:note_text) { Gitlab::GithubImport::Representation::NoteText.from_db_record(release) } + let(:record) { create(:release, project: project, description: text) } + + it_behaves_like 'updates record description' + end - it 'updates release description with new attachment url' do - expect(UploadService).to receive(:new) - .with(project, tmp_stub_doc, FileUploader).and_call_original - expect(UploadService).to receive(:new) - .with(project, tmp_stub_image, FileUploader).and_call_original - expect(UploadService).to receive(:new) - .with(project, tmp_stub_image_tag, FileUploader).and_call_original + context 'when importing issue attachments' do + let(:record) { create(:issue, project: project, description: text) } - importer.execute + it_behaves_like 'updates record description' + end - release.reload - expect(release.description).to start_with("Some text...\n\n[special-doc](/uploads/") - expect(release.description).to include('![image.jpeg](/uploads/') - expect(release.description).to include('<img width="248" alt="tag-image" src="/uploads') - end + context 'when importing merge request attachments' do + let(:record) { create(:merge_request, source_project: project, description: text) } + + it_behaves_like 'updates record description' end context 'when importing note attachments' do - let(:note) { create(:note, project: project, note: text) } - let(:note_text) { Gitlab::GithubImport::Representation::NoteText.from_db_record(note) } + let(:record) { create(:note, project: project, note: text) } it 'updates note text with new attachment urls' do importer.execute - note.reload - expect(note.note).to start_with("Some text...\n\n[special-doc](/uploads/") - expect(note.note).to include('![image.jpeg](/uploads/') - expect(note.note).to include('<img width="248" alt="tag-image" src="/uploads') + record.reload + expect(record.note).to start_with("Some text...\n\n[special-doc](/uploads/") + expect(record.note).to include('![image.jpeg](/uploads/') + expect(record.note).to include('<img width="248" alt="tag-image" src="/uploads') end end end diff --git a/spec/lib/gitlab/github_import/representation/note_text_spec.rb b/spec/lib/gitlab/github_import/representation/note_text_spec.rb index adced223032..8b57c9a0373 100644 --- a/spec/lib/gitlab/github_import/representation/note_text_spec.rb +++ b/spec/lib/gitlab/github_import/representation/note_text_spec.rb @@ -4,7 +4,7 @@ require 'spec_helper' RSpec.describe Gitlab::GithubImport::Representation::NoteText do shared_examples 'a Note text data' do |match_record_type| - it 'returns an instance of ReleaseAttachments' do + it 'returns an instance of NoteText' do expect(representation).to be_an_instance_of(described_class) end @@ -30,6 +30,22 @@ RSpec.describe Gitlab::GithubImport::Representation::NoteText do end end + context 'with Issue' do + let(:record) { build_stubbed(:issue, id: 42, description: 'Some text here..') } + + it_behaves_like 'a Note text data', 'Issue' do + let(:representation) { described_class.from_db_record(record) } + end + end + + context 'with MergeRequest' do + let(:record) { build_stubbed(:merge_request, id: 42, description: 'Some text here..') } + + it_behaves_like 'a Note text data', 'MergeRequest' do + let(:representation) { described_class.from_db_record(record) } + end + end + context 'with Note' do let(:record) { build_stubbed(:note, id: 42, note: 'Some text here..') } diff --git a/spec/lib/gitlab/memory/watchdog/configuration_spec.rb b/spec/lib/gitlab/memory/watchdog/configuration_spec.rb new file mode 100644 index 00000000000..892a4b06ad0 --- /dev/null +++ b/spec/lib/gitlab/memory/watchdog/configuration_spec.rb @@ -0,0 +1,121 @@ +# frozen_string_literal: true + +require 'fast_spec_helper' +require_dependency 'gitlab/cluster/lifecycle_events' + +RSpec.describe Gitlab::Memory::Watchdog::Configuration do + subject(:configuration) { described_class.new } + + describe '#initialize' do + it 'initialize monitors' do + expect(configuration.monitors).to be_an_instance_of(described_class::MonitorStack) + end + end + + describe '#handler' do + context 'when handler is not set' do + it 'defaults to NullHandler' do + expect(configuration.handler).to be(Gitlab::Memory::Watchdog::NullHandler.instance) + end + end + end + + describe '#logger' do + context 'when logger is not set, defaults to stdout logger' do + it 'defaults to Logger' do + expect(configuration.logger).to be_an_instance_of(::Gitlab::Logger) + end + end + end + + describe '#sleep_time_seconds' do + context 'when sleep_time_seconds is not set' do + it 'defaults to SLEEP_TIME_SECONDS' do + expect(configuration.sleep_time_seconds).to eq(described_class::DEFAULT_SLEEP_TIME_SECONDS) + end + end + end + + describe '#monitors' do + context 'when monitors are configured to be used' do + let(:payload1) do + { + message: 'monitor_1_text', + memwd_max_strikes: 5, + memwd_cur_strikes: 0 + } + end + + let(:payload2) do + { + message: 'monitor_2_text', + memwd_max_strikes: 0, + memwd_cur_strikes: 1 + } + end + + let(:monitor_class_1) do + Struct.new(:threshold_violated, :payload) do + def call + { threshold_violated: !!threshold_violated, payload: payload || {} } + end + + def self.name + 'Monitor1' + end + end + end + + let(:monitor_class_2) do + Struct.new(:threshold_violated, :payload) do + def call + { threshold_violated: !!threshold_violated, payload: payload || {} } + end + + def self.name + 'Monitor2' + end + end + end + + context 'when two monitors are configured to be used' do + before do + configuration.monitors.use monitor_class_1, false, { message: 'monitor_1_text' }, max_strikes: 5 + configuration.monitors.use monitor_class_2, true, { message: 'monitor_2_text' }, max_strikes: 0 + end + + it 'calls each monitor and returns correct results', :aggregate_failures do + payloads = [] + thresholds = [] + strikes = [] + monitor_names = [] + + configuration.monitors.call_each do |result| + payloads << result.payload + thresholds << result.threshold_violated? + strikes << result.strikes_exceeded? + monitor_names << result.monitor_name + end + + expect(payloads).to eq([payload1, payload2]) + expect(thresholds).to eq([false, true]) + expect(strikes).to eq([false, true]) + expect(monitor_names).to eq([:monitor1, :monitor2]) + end + end + + context 'when same monitor class is configured to be used twice' do + before do + configuration.monitors.use monitor_class_1, max_strikes: 1 + configuration.monitors.use monitor_class_1, max_strikes: 1 + end + + it 'calls same monitor only once' do + expect do |b| + configuration.monitors.call_each(&b) + end.to yield_control.once + end + end + end + end +end diff --git a/spec/lib/gitlab/memory/watchdog/monitor/heap_fragmentation_spec.rb b/spec/lib/gitlab/memory/watchdog/monitor/heap_fragmentation_spec.rb new file mode 100644 index 00000000000..dad19cfd588 --- /dev/null +++ b/spec/lib/gitlab/memory/watchdog/monitor/heap_fragmentation_spec.rb @@ -0,0 +1,60 @@ +# frozen_string_literal: true + +require 'fast_spec_helper' +require 'support/shared_examples/lib/gitlab/memory/watchdog/monitor_result_shared_examples' +require 'prometheus/client' + +RSpec.describe Gitlab::Memory::Watchdog::Monitor::HeapFragmentation do + let(:heap_frag_limit_gauge) { instance_double(::Prometheus::Client::Gauge) } + let(:max_heap_fragmentation) { 0.2 } + let(:fragmentation) { 0.3 } + + subject(:monitor) do + described_class.new(max_heap_fragmentation: max_heap_fragmentation) + end + + before do + allow(Gitlab::Metrics).to receive(:gauge) + .with(:gitlab_memwd_heap_frag_limit, anything) + .and_return(heap_frag_limit_gauge) + allow(heap_frag_limit_gauge).to receive(:set) + + allow(Gitlab::Metrics::Memory).to receive(:gc_heap_fragmentation).and_return(fragmentation) + end + + describe '#initialize' do + it 'sets the heap fragmentation limit gauge' do + expect(heap_frag_limit_gauge).to receive(:set).with({}, max_heap_fragmentation) + + monitor + end + end + + describe '#call' do + it 'gets gc_heap_fragmentation' do + expect(Gitlab::Metrics::Memory).to receive(:gc_heap_fragmentation) + + monitor.call + end + + context 'when process exceeds threshold' do + let(:fragmentation) { max_heap_fragmentation + 0.1 } + let(:payload) do + { + message: 'heap fragmentation limit exceeded', + memwd_cur_heap_frag: fragmentation, + memwd_max_heap_frag: max_heap_fragmentation + } + end + + include_examples 'returns Watchdog Monitor result', threshold_violated: true + end + + context 'when process does not exceed threshold' do + let(:fragmentation) { max_heap_fragmentation - 0.1 } + let(:payload) { {} } + + include_examples 'returns Watchdog Monitor result', threshold_violated: false + end + end +end diff --git a/spec/lib/gitlab/memory/watchdog/monitor/unique_memory_growth_spec.rb b/spec/lib/gitlab/memory/watchdog/monitor/unique_memory_growth_spec.rb new file mode 100644 index 00000000000..22494af4425 --- /dev/null +++ b/spec/lib/gitlab/memory/watchdog/monitor/unique_memory_growth_spec.rb @@ -0,0 +1,62 @@ +# frozen_string_literal: true + +require 'fast_spec_helper' +require 'support/shared_examples/lib/gitlab/memory/watchdog/monitor_result_shared_examples' +require_dependency 'gitlab/cluster/lifecycle_events' + +RSpec.describe Gitlab::Memory::Watchdog::Monitor::UniqueMemoryGrowth do + let(:primary_memory) { 2048 } + let(:worker_memory) { 0 } + let(:max_mem_growth) { 2 } + + subject(:monitor) do + described_class.new(max_mem_growth: max_mem_growth) + end + + before do + allow(Gitlab::Metrics::System).to receive(:memory_usage_uss_pss).and_return({ uss: worker_memory }) + allow(Gitlab::Metrics::System).to receive(:memory_usage_uss_pss).with( + pid: Gitlab::Cluster::PRIMARY_PID + ).and_return({ uss: primary_memory }) + end + + describe '#call' do + it 'gets memory_usage_uss_pss' do + expect(Gitlab::Metrics::System).to receive(:memory_usage_uss_pss).with(no_args) + expect(Gitlab::Metrics::System).to receive(:memory_usage_uss_pss).with(pid: Gitlab::Cluster::PRIMARY_PID) + + monitor.call + end + + context 'when monitor is called twice' do + it 'reference memory is calculated only once' do + expect(Gitlab::Metrics::System).to receive(:memory_usage_uss_pss).with(no_args).twice + expect(Gitlab::Metrics::System).to receive(:memory_usage_uss_pss).with(pid: Gitlab::Cluster::PRIMARY_PID).once + + monitor.call + monitor.call + end + end + + context 'when process exceeds threshold' do + let(:worker_memory) { max_mem_growth * primary_memory + 1 } + let(:payload) do + { + message: 'memory limit exceeded', + memwd_max_uss_bytes: max_mem_growth * primary_memory, + memwd_ref_uss_bytes: primary_memory, + memwd_uss_bytes: worker_memory + } + end + + include_examples 'returns Watchdog Monitor result', threshold_violated: true + end + + context 'when process does not exceed threshold' do + let(:worker_memory) { max_mem_growth * primary_memory - 1 } + let(:payload) { {} } + + include_examples 'returns Watchdog Monitor result', threshold_violated: false + end + end +end diff --git a/spec/lib/gitlab/memory/watchdog/monitor_state_spec.rb b/spec/lib/gitlab/memory/watchdog/monitor_state_spec.rb new file mode 100644 index 00000000000..ace1353c6e3 --- /dev/null +++ b/spec/lib/gitlab/memory/watchdog/monitor_state_spec.rb @@ -0,0 +1,72 @@ +# frozen_string_literal: true + +require 'fast_spec_helper' + +RSpec.describe Gitlab::Memory::Watchdog::MonitorState do + let(:max_strikes) { 2 } + let(:payload) { { message: 'DummyMessage' } } + let(:threshold_violated) { true } + let(:monitor) { monitor_class.new(threshold_violated, payload) } + let(:monitor_class) do + Struct.new(:threshold_violated, :payload) do + def call + { threshold_violated: threshold_violated, payload: payload } + end + + def self.name + 'MonitorName' + end + end + end + + subject(:monitor_state) { described_class.new(monitor, max_strikes: max_strikes) } + + shared_examples 'returns correct result' do + it 'returns correct result', :aggregate_failures do + result = monitor_state.call + + expect(result).to be_an_instance_of(described_class::Result) + expect(result.strikes_exceeded?).to eq(strikes_exceeded) + expect(result.threshold_violated?).to eq(threshold_violated) + expect(result.payload).to eq(expected_payload) + expect(result.monitor_name).to eq(:monitor_name) + end + end + + describe '#call' do + let(:strikes_exceeded) { false } + let(:curr_strikes) { 0 } + let(:expected_payload) do + { + memwd_max_strikes: max_strikes, + memwd_cur_strikes: curr_strikes + }.merge(payload) + end + + context 'when threshold is not violated' do + let(:threshold_violated) { false } + + include_examples 'returns correct result' + end + + context 'when threshold is violated' do + let(:curr_strikes) { 1 } + let(:threshold_violated) { true } + + include_examples 'returns correct result' + + context 'when strikes_exceeded' do + let(:max_strikes) { 0 } + let(:strikes_exceeded) { true } + + include_examples 'returns correct result' + end + end + end + + describe '#monitor_class' do + subject { monitor_state.monitor_class } + + it { is_expected.to eq(monitor_class) } + end +end diff --git a/spec/lib/gitlab/memory/watchdog_spec.rb b/spec/lib/gitlab/memory/watchdog_spec.rb index beb49660022..84e9a577afb 100644 --- a/spec/lib/gitlab/memory/watchdog_spec.rb +++ b/spec/lib/gitlab/memory/watchdog_spec.rb @@ -1,35 +1,35 @@ # frozen_string_literal: true require 'spec_helper' -require_relative '../../../../lib/gitlab/cluster/lifecycle_events' -RSpec.describe Gitlab::Memory::Watchdog, :aggregate_failures, :prometheus do +RSpec.describe Gitlab::Memory::Watchdog, :aggregate_failures do context 'watchdog' do - let(:logger) { instance_double(::Logger) } + let(:configuration) { instance_double(described_class::Configuration) } let(:handler) { instance_double(described_class::NullHandler) } - - let(:heap_frag_limit_gauge) { instance_double(::Prometheus::Client::Gauge) } + let(:logger) { instance_double(::Logger) } + let(:sleep_time_seconds) { 60 } + let(:threshold_violated) { false } let(:violations_counter) { instance_double(::Prometheus::Client::Counter) } let(:violations_handled_counter) { instance_double(::Prometheus::Client::Counter) } - - let(:sleep_time) { 0.1 } - let(:max_heap_fragmentation) { 0.2 } - let(:max_mem_growth) { 2 } - - # Defaults that will not trigger any events. - let(:fragmentation) { 0 } - let(:worker_memory) { 0 } - let(:primary_memory) { 0 } - let(:max_strikes) { 0 } - - # Tests should set this to control the number of loop iterations in `call`. let(:watchdog_iterations) { 1 } + let(:name) { :monitor_name } + let(:payload) { { message: 'dummy_text' } } + let(:max_strikes) { 2 } + let(:monitor_class) do + Struct.new(:threshold_violated, :payload) do + def call + { threshold_violated: threshold_violated, payload: payload } + end + + def self.name + 'MonitorName' + end + end + end subject(:watchdog) do - described_class.new(handler: handler, logger: logger, sleep_time_seconds: sleep_time, - max_strikes: max_strikes, max_mem_growth: max_mem_growth, - max_heap_fragmentation: max_heap_fragmentation).tap do |instance| - # We need to defuse `sleep` and stop the internal loop after N iterations. + described_class.new.tap do |instance| + # We need to defuse `sleep` and stop the internal loop after 1 iteration iterations = 0 allow(instance).to receive(:sleep) do instance.stop if (iterations += 1) > watchdog_iterations @@ -38,9 +38,6 @@ RSpec.describe Gitlab::Memory::Watchdog, :aggregate_failures, :prometheus do end def stub_prometheus_metrics - allow(Gitlab::Metrics).to receive(:gauge) - .with(:gitlab_memwd_heap_frag_limit, anything) - .and_return(heap_frag_limit_gauge) allow(Gitlab::Metrics).to receive(:counter) .with(:gitlab_memwd_violations_total, anything, anything) .and_return(violations_counter) @@ -48,318 +45,195 @@ RSpec.describe Gitlab::Memory::Watchdog, :aggregate_failures, :prometheus do .with(:gitlab_memwd_violations_handled_total, anything, anything) .and_return(violations_handled_counter) - allow(heap_frag_limit_gauge).to receive(:set) allow(violations_counter).to receive(:increment) allow(violations_handled_counter).to receive(:increment) end - before do - stub_prometheus_metrics - - allow(handler).to receive(:call).and_return(true) - - allow(logger).to receive(:warn) - allow(logger).to receive(:info) - - allow(Gitlab::Metrics::Memory).to receive(:gc_heap_fragmentation).and_return(fragmentation) - allow(Gitlab::Metrics::System).to receive(:memory_usage_uss_pss).and_return({ uss: worker_memory }) - allow(Gitlab::Metrics::System).to receive(:memory_usage_uss_pss).with( - pid: Gitlab::Cluster::PRIMARY_PID - ).and_return({ uss: primary_memory }) - - allow(::Prometheus::PidProvider).to receive(:worker_id).and_return('worker_1') - end - - context 'when created' do - it 'sets the heap fragmentation limit gauge' do - expect(heap_frag_limit_gauge).to receive(:set).with({}, max_heap_fragmentation) + describe '#initialize' do + it 'initialize new configuration' do + expect(described_class::Configuration).to receive(:new) watchdog end - - context 'when no settings are set in the environment' do - it 'initializes with defaults' do - watchdog = described_class.new(handler: handler, logger: logger) - - expect(watchdog.max_heap_fragmentation).to eq(described_class::DEFAULT_MAX_HEAP_FRAG) - expect(watchdog.max_mem_growth).to eq(described_class::DEFAULT_MAX_MEM_GROWTH) - expect(watchdog.max_strikes).to eq(described_class::DEFAULT_MAX_STRIKES) - expect(watchdog.sleep_time_seconds).to eq(described_class::DEFAULT_SLEEP_TIME_SECONDS) - end - end - - context 'when settings are passed through the environment' do - before do - stub_env('GITLAB_MEMWD_MAX_HEAP_FRAG', 1) - stub_env('GITLAB_MEMWD_MAX_STRIKES', 2) - stub_env('GITLAB_MEMWD_SLEEP_TIME_SEC', 3) - stub_env('GITLAB_MEMWD_MAX_MEM_GROWTH', 4) - end - - it 'initializes with these settings' do - watchdog = described_class.new(handler: handler, logger: logger) - - expect(watchdog.max_heap_fragmentation).to eq(1) - expect(watchdog.max_strikes).to eq(2) - expect(watchdog.sleep_time_seconds).to eq(3) - expect(watchdog.max_mem_growth).to eq(4) - end - end end - shared_examples 'has strikes left' do |stat| - context 'when process has not exceeded allowed number of strikes' do - let(:watchdog_iterations) { max_strikes } - - it 'does not signal the handler' do - expect(handler).not_to receive(:call) - - watchdog.call - end - - it 'does not log any events' do - expect(logger).not_to receive(:warn) - - watchdog.call - end - - it 'increments the violations counter' do - expect(violations_counter).to receive(:increment).with(reason: stat).exactly(watchdog_iterations) - - watchdog.call + describe '#call' do + before do + stub_prometheus_metrics + allow(Gitlab::Metrics::System).to receive(:memory_usage_rss).at_least(:once).and_return(1024) + allow(::Prometheus::PidProvider).to receive(:worker_id).and_return('worker_1') + + watchdog.configure do |config| + config.handler = handler + config.logger = logger + config.sleep_time_seconds = sleep_time_seconds + config.monitors.use monitor_class, threshold_violated, payload, max_strikes: max_strikes end - it 'does not increment violations handled counter' do - expect(violations_handled_counter).not_to receive(:increment) - - watchdog.call - end + allow(handler).to receive(:call).and_return(true) + allow(logger).to receive(:info) + allow(logger).to receive(:warn) end - end - shared_examples 'no strikes left' do |stat| - it 'signals the handler and resets strike counter' do - expect(handler).to receive(:call).and_return(true) + it 'logs start message once' do + expect(logger).to receive(:info).once + .with( + pid: Process.pid, + worker_id: 'worker_1', + memwd_handler_class: handler.class.name, + memwd_sleep_time_s: sleep_time_seconds, + memwd_rss_bytes: 1024, + message: 'started') watchdog.call - - expect(watchdog.strikes(stat.to_sym)).to eq(0) end - it 'increments both the violations and violations handled counters' do - expect(violations_counter).to receive(:increment).with(reason: stat).exactly(watchdog_iterations) - expect(violations_handled_counter).to receive(:increment).with(reason: stat) + it 'waits for check interval seconds' do + expect(watchdog).to receive(:sleep).with(sleep_time_seconds) watchdog.call end - context 'when enforce_memory_watchdog ops toggle is off' do + context 'when gitlab_memory_watchdog ops toggle is off' do before do - stub_feature_flags(enforce_memory_watchdog: false) + stub_feature_flags(gitlab_memory_watchdog: false) end - it 'always uses the NullHandler' do - expect(handler).not_to receive(:call) - expect(described_class::NullHandler.instance).to receive(:call).and_return(true) - - watchdog.call + it 'does not trigger any monitor' do + expect(configuration).not_to receive(:monitors) end end - context 'when handler result is true' do - it 'considers the event handled and stops itself' do - expect(handler).to receive(:call).once.and_return(true) - expect(logger).to receive(:info).with(hash_including(message: 'stopped')) + context 'when process does not exceed threshold' do + it 'does not increment violations counters' do + expect(violations_counter).not_to receive(:increment) + expect(violations_handled_counter).not_to receive(:increment) watchdog.call end - end - - context 'when handler result is false' do - let(:max_strikes) { 0 } # to make sure the handler fires each iteration - let(:watchdog_iterations) { 3 } - it 'keeps running' do - expect(violations_counter).to receive(:increment).exactly(watchdog_iterations) - expect(violations_handled_counter).to receive(:increment).exactly(watchdog_iterations) - # Return true the third time to terminate the daemon. - expect(handler).to receive(:call).and_return(false, false, true) + it 'does not log violation' do + expect(logger).not_to receive(:warn) watchdog.call end - end - end - - context 'when monitoring memory growth' do - let(:primary_memory) { 2048 } - - context 'when process does not exceed threshold' do - let(:worker_memory) { max_mem_growth * primary_memory - 1 } - it 'does not signal the handler' do + it 'does not execute handler' do expect(handler).not_to receive(:call) watchdog.call end end - context 'when process exceeds threshold permanently' do - let(:worker_memory) { max_mem_growth * primary_memory + 1 } - let(:max_strikes) { 3 } - - it_behaves_like 'has strikes left', 'mem_growth' + context 'when process exceeds threshold' do + let(:threshold_violated) { true } - context 'when process exceeds the allowed number of strikes' do - let(:watchdog_iterations) { max_strikes + 1 } + it 'increments violations counter' do + expect(violations_counter).to receive(:increment).with(reason: name) - it_behaves_like 'no strikes left', 'mem_growth' + watchdog.call + end - it 'only reads reference memory once' do - expect(Gitlab::Metrics::System).to receive(:memory_usage_uss_pss) - .with(pid: Gitlab::Cluster::PRIMARY_PID) - .once + context 'when process does not exceed the allowed number of strikes' do + it 'does not increment handled violations counter' do + expect(violations_handled_counter).not_to receive(:increment) watchdog.call end - it 'logs the event' do - expect(Gitlab::Metrics::System).to receive(:memory_usage_rss).at_least(:once).and_return(1024) - expect(logger).to receive(:warn).with({ - message: 'memory limit exceeded', - pid: Process.pid, - worker_id: 'worker_1', - memwd_handler_class: 'RSpec::Mocks::InstanceVerifyingDouble', - memwd_sleep_time_s: sleep_time, - memwd_max_uss_bytes: max_mem_growth * primary_memory, - memwd_ref_uss_bytes: primary_memory, - memwd_uss_bytes: worker_memory, - memwd_rss_bytes: 1024, - memwd_max_strikes: max_strikes, - memwd_cur_strikes: max_strikes + 1 - }) + it 'does not log violation' do + expect(logger).not_to receive(:warn) watchdog.call end - end - end - context 'when process exceeds threshold temporarily' do - let(:worker_memory) { max_mem_growth * primary_memory } - let(:max_strikes) { 1 } - let(:watchdog_iterations) { 4 } + it 'does not execute handler' do + expect(handler).not_to receive(:call) - before do - allow(Gitlab::Metrics::System).to receive(:memory_usage_uss_pss).and_return( - { uss: worker_memory - 0.1 }, - { uss: worker_memory + 0.2 }, - { uss: worker_memory - 0.1 }, - { uss: worker_memory + 0.1 } - ) - allow(Gitlab::Metrics::System).to receive(:memory_usage_uss_pss).with( - pid: Gitlab::Cluster::PRIMARY_PID - ).and_return({ uss: primary_memory }) + watchdog.call + end end - it 'does not signal the handler' do - expect(handler).not_to receive(:call) + context 'when monitor exceeds the allowed number of strikes' do + let(:max_strikes) { 0 } - watchdog.call - end - end - end + it 'increments handled violations counter' do + expect(violations_handled_counter).to receive(:increment).with(reason: name) - context 'when monitoring heap fragmentation' do - context 'when process does not exceed threshold' do - let(:fragmentation) { max_heap_fragmentation - 0.1 } - - it 'does not signal the handler' do - expect(handler).not_to receive(:call) - - watchdog.call - end - end - - context 'when process exceeds threshold permanently' do - let(:fragmentation) { max_heap_fragmentation + 0.1 } - let(:max_strikes) { 3 } - - it_behaves_like 'has strikes left', 'heap_frag' + watchdog.call + end - context 'when process exceeds the allowed number of strikes' do - let(:watchdog_iterations) { max_strikes + 1 } + it 'logs violation' do + expect(logger).to receive(:warn) + .with( + pid: Process.pid, + worker_id: 'worker_1', + memwd_handler_class: handler.class.name, + memwd_sleep_time_s: sleep_time_seconds, + memwd_rss_bytes: 1024, + memwd_cur_strikes: 1, + memwd_max_strikes: max_strikes, + message: 'dummy_text') - it_behaves_like 'no strikes left', 'heap_frag' + watchdog.call + end - it 'logs the event' do - expect(Gitlab::Metrics::System).to receive(:memory_usage_rss).at_least(:once).and_return(1024) - expect(logger).to receive(:warn).with({ - message: 'heap fragmentation limit exceeded', - pid: Process.pid, - worker_id: 'worker_1', - memwd_handler_class: 'RSpec::Mocks::InstanceVerifyingDouble', - memwd_sleep_time_s: sleep_time, - memwd_max_heap_frag: max_heap_fragmentation, - memwd_cur_heap_frag: fragmentation, - memwd_max_strikes: max_strikes, - memwd_cur_strikes: max_strikes + 1, - memwd_rss_bytes: 1024 - }) + it 'executes handler' do + expect(handler).to receive(:call) watchdog.call end - end - end - context 'when process exceeds threshold temporarily' do - let(:fragmentation) { max_heap_fragmentation } - let(:max_strikes) { 1 } - let(:watchdog_iterations) { 4 } + context 'when enforce_memory_watchdog ops toggle is off' do + before do + stub_feature_flags(enforce_memory_watchdog: false) + end - before do - allow(Gitlab::Metrics::Memory).to receive(:gc_heap_fragmentation).and_return( - fragmentation - 0.1, - fragmentation + 0.2, - fragmentation - 0.1, - fragmentation + 0.1 - ) - end + it 'always uses the NullHandler' do + expect(handler).not_to receive(:call) + expect(described_class::NullHandler.instance).to receive(:call).and_return(true) - it 'does not signal the handler' do - expect(handler).not_to receive(:call) + watchdog.call + end + end - watchdog.call + context 'when multiple monitors exceeds allowed number of strikes' do + before do + watchdog.configure do |config| + config.handler = handler + config.logger = logger + config.sleep_time_seconds = sleep_time_seconds + config.monitors.use monitor_class, threshold_violated, payload, max_strikes: max_strikes + config.monitors.use monitor_class, threshold_violated, payload, max_strikes: max_strikes + end + end + + it 'only calls the handler once' do + expect(handler).to receive(:call).once.and_return(true) + + watchdog.call + end + end end end - end - - context 'when both memory fragmentation and growth exceed thresholds' do - let(:fragmentation) { max_heap_fragmentation + 0.1 } - let(:primary_memory) { 2048 } - let(:worker_memory) { max_mem_growth * primary_memory + 1 } - let(:watchdog_iterations) { max_strikes + 1 } - it 'only calls the handler once' do - expect(handler).to receive(:call).once.and_return(true) + it 'logs stop message once' do + expect(logger).to receive(:info).once + .with( + pid: Process.pid, + worker_id: 'worker_1', + memwd_handler_class: handler.class.name, + memwd_sleep_time_s: sleep_time_seconds, + memwd_rss_bytes: 1024, + message: 'stopped') watchdog.call end end - context 'when gitlab_memory_watchdog ops toggle is off' do - before do - stub_feature_flags(gitlab_memory_watchdog: false) - end - - it 'does not monitor heap fragmentation' do - expect(Gitlab::Metrics::Memory).not_to receive(:gc_heap_fragmentation) - - watchdog.call - end - - it 'does not monitor memory growth' do - expect(Gitlab::Metrics::System).not_to receive(:memory_usage_uss_pss) - - watchdog.call + describe '#configure' do + it 'yields block' do + expect { |b| watchdog.configure(&b) }.to yield_control end end end diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index d3cf0c79a56..b08c85339d1 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -140,7 +140,6 @@ RSpec.describe User do it { is_expected.to have_many(:timelogs) } it { is_expected.to have_many(:callouts).class_name('Users::Callout') } it { is_expected.to have_many(:group_callouts).class_name('Users::GroupCallout') } - it { is_expected.to have_many(:namespace_callouts).class_name('Users::NamespaceCallout') } it { is_expected.to have_many(:project_callouts).class_name('Users::ProjectCallout') } describe '#user_detail' do @@ -6666,96 +6665,6 @@ RSpec.describe User do end end - describe 'Users::NamespaceCallout' do - describe '#dismissed_callout_for_namespace?' do - let_it_be(:user, refind: true) { create(:user) } - let_it_be(:namespace) { create(:namespace) } - let_it_be(:feature_name) { Users::NamespaceCallout.feature_names.each_key.first } - - let(:query) do - { feature_name: feature_name, namespace: namespace } - end - - def have_dismissed_callout - be_dismissed_callout_for_namespace(**query) - end - - context 'when no callout dismissal record exists' do - it 'returns false when no ignore_dismissal_earlier_than provided' do - expect(user).not_to have_dismissed_callout - end - end - - context 'when dismissed callout exists' do - before_all do - create(:namespace_callout, - user: user, - namespace_id: namespace.id, - feature_name: feature_name, - dismissed_at: 4.months.ago) - end - - it 'returns true when no ignore_dismissal_earlier_than provided' do - expect(user).to have_dismissed_callout - end - - it 'returns true when ignore_dismissal_earlier_than is earlier than dismissed_at' do - query[:ignore_dismissal_earlier_than] = 6.months.ago - - expect(user).to have_dismissed_callout - end - - it 'returns false when ignore_dismissal_earlier_than is later than dismissed_at' do - query[:ignore_dismissal_earlier_than] = 2.months.ago - - expect(user).not_to have_dismissed_callout - end - end - end - - describe '#find_or_initialize_namespace_callout' do - let_it_be(:user, refind: true) { create(:user) } - let_it_be(:namespace) { create(:namespace) } - let_it_be(:feature_name) { Users::NamespaceCallout.feature_names.each_key.first } - - subject(:callout_with_source) do - user.find_or_initialize_namespace_callout(feature_name, namespace.id) - end - - context 'when callout exists' do - let!(:callout) do - create(:namespace_callout, user: user, feature_name: feature_name, namespace_id: namespace.id) - end - - it 'returns existing callout' do - expect(callout_with_source).to eq(callout) - end - end - - context 'when callout does not exist' do - context 'when feature name is valid' do - it 'initializes a new callout' do - expect(callout_with_source) - .to be_a_new(Users::NamespaceCallout) - .and be_valid - end - end - - context 'when feature name is not valid' do - let(:feature_name) { 'notvalid' } - - it 'initializes a new callout' do - expect(callout_with_source).to be_a_new(Users::NamespaceCallout) - end - - it 'is not valid' do - expect(callout_with_source).not_to be_valid - end - end - end - end - end - describe '#dismissed_callout_for_group?' do let_it_be(:user, refind: true) { create(:user) } let_it_be(:group) { create(:group) } diff --git a/spec/models/users/namespace_callout_spec.rb b/spec/models/users/namespace_callout_spec.rb deleted file mode 100644 index f8207f2abc8..00000000000 --- a/spec/models/users/namespace_callout_spec.rb +++ /dev/null @@ -1,39 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -RSpec.describe Users::NamespaceCallout do - let_it_be(:user) { create_default(:user) } - let_it_be(:namespace) { create_default(:namespace) } - let_it_be(:callout) { create(:namespace_callout) } - - it_behaves_like 'having unique enum values' - - describe 'relationships' do - it { is_expected.to belong_to(:namespace) } - end - - describe 'validations' do - it { is_expected.to validate_presence_of(:namespace) } - it { is_expected.to validate_presence_of(:user) } - it { is_expected.to validate_presence_of(:feature_name) } - - specify do - is_expected.to validate_uniqueness_of(:feature_name) - .scoped_to(:user_id, :namespace_id) - .ignoring_case_sensitivity - end - - it { is_expected.to allow_value(:web_hook_disabled).for(:feature_name) } - - it 'rejects invalid feature names' do - expect { callout.feature_name = :non_existent_feature }.to raise_error(ArgumentError) - end - end - - describe '#source_feature_name' do - it 'provides string based off source and feature' do - expect(callout.source_feature_name).to eq "#{callout.feature_name}_#{callout.namespace_id}" - end - end -end diff --git a/spec/policies/group_policy_spec.rb b/spec/policies/group_policy_spec.rb index 184f7d676ba..c65933c5208 100644 --- a/spec/policies/group_policy_spec.rb +++ b/spec/policies/group_policy_spec.rb @@ -1241,21 +1241,7 @@ RSpec.describe GroupPolicy do let(:current_user) { admin } context 'when admin mode is enabled', :enable_admin_mode do - context 'with runners_finder_all_available FF disabled' do - before do - stub_feature_flags(runners_finder_all_available: false) - end - - specify { is_expected.to be_disallowed(:read_group_all_available_runners) } - end - - context 'with runners_finder_all_available FF enabled' do - before do - stub_feature_flags(runners_finder_all_available: [group]) - end - - specify { is_expected.to be_allowed(:read_group_all_available_runners) } - end + specify { is_expected.to be_allowed(:read_group_all_available_runners) } end context 'when admin mode is disabled' do @@ -1266,21 +1252,7 @@ RSpec.describe GroupPolicy do context 'with owner' do let(:current_user) { owner } - context 'with runners_finder_all_available FF disabled' do - before do - stub_feature_flags(runners_finder_all_available: false) - end - - specify { is_expected.to be_disallowed(:read_group_all_available_runners) } - end - - context 'with runners_finder_all_available FF enabled' do - before do - stub_feature_flags(runners_finder_all_available: [group]) - end - - specify { is_expected.to be_allowed(:read_group_all_available_runners) } - end + specify { is_expected.to be_allowed(:read_group_all_available_runners) } end context 'with maintainer' do diff --git a/spec/requests/api/graphql/mutations/namespace/package_settings/update_spec.rb b/spec/requests/api/graphql/mutations/namespace/package_settings/update_spec.rb index 194e42bf59d..567d8799d93 100644 --- a/spec/requests/api/graphql/mutations/namespace/package_settings/update_spec.rb +++ b/spec/requests/api/graphql/mutations/namespace/package_settings/update_spec.rb @@ -14,7 +14,13 @@ RSpec.describe 'Updating the package settings' do maven_duplicates_allowed: false, maven_duplicate_exception_regex: 'foo-.*', generic_duplicates_allowed: false, - generic_duplicate_exception_regex: 'bar-.*' + generic_duplicate_exception_regex: 'bar-.*', + maven_package_requests_forwarding: true, + lock_maven_package_requests_forwarding: true, + npm_package_requests_forwarding: true, + lock_npm_package_requests_forwarding: true, + pypi_package_requests_forwarding: true, + lock_pypi_package_requests_forwarding: true } end @@ -26,6 +32,12 @@ RSpec.describe 'Updating the package settings' do mavenDuplicateExceptionRegex genericDuplicatesAllowed genericDuplicateExceptionRegex + mavenPackageRequestsForwarding + lockMavenPackageRequestsForwarding + npmPackageRequestsForwarding + lockNpmPackageRequestsForwarding + pypiPackageRequestsForwarding + lockPypiPackageRequestsForwarding } errors QL @@ -46,6 +58,12 @@ RSpec.describe 'Updating the package settings' do expect(package_settings_response['mavenDuplicateExceptionRegex']).to eq(params[:maven_duplicate_exception_regex]) expect(package_settings_response['genericDuplicatesAllowed']).to eq(params[:generic_duplicates_allowed]) expect(package_settings_response['genericDuplicateExceptionRegex']).to eq(params[:generic_duplicate_exception_regex]) + expect(package_settings_response['mavenPackageRequestsForwarding']).to eq(params[:maven_package_requests_forwarding]) + expect(package_settings_response['lockMavenPackageRequestsForwarding']).to eq(params[:lock_maven_package_requests_forwarding]) + expect(package_settings_response['pypiPackageRequestsForwarding']).to eq(params[:pypi_package_requests_forwarding]) + expect(package_settings_response['lockPypiPackageRequestsForwarding']).to eq(params[:lock_pypi_package_requests_forwarding]) + expect(package_settings_response['npmPackageRequestsForwarding']).to eq(params[:npm_package_requests_forwarding]) + expect(package_settings_response['lockNpmPackageRequestsForwarding']).to eq(params[:lock_npm_package_requests_forwarding]) end end @@ -75,8 +93,29 @@ RSpec.describe 'Updating the package settings' do RSpec.shared_examples 'accepting the mutation request updating the package settings' do it_behaves_like 'updating the namespace package setting attributes', - from: { maven_duplicates_allowed: true, maven_duplicate_exception_regex: 'SNAPSHOT', generic_duplicates_allowed: true, generic_duplicate_exception_regex: 'foo' }, - to: { maven_duplicates_allowed: false, maven_duplicate_exception_regex: 'foo-.*', generic_duplicates_allowed: false, generic_duplicate_exception_regex: 'bar-.*' } + from: { + maven_duplicates_allowed: true, + maven_duplicate_exception_regex: 'SNAPSHOT', + generic_duplicates_allowed: true, + generic_duplicate_exception_regex: 'foo', + maven_package_requests_forwarding: nil, + lock_maven_package_requests_forwarding: false, + npm_package_requests_forwarding: nil, + lock_npm_package_requests_forwarding: false, + pypi_package_requests_forwarding: nil, + lock_pypi_package_requests_forwarding: false + }, to: { + maven_duplicates_allowed: false, + maven_duplicate_exception_regex: 'foo-.*', + generic_duplicates_allowed: false, + generic_duplicate_exception_regex: 'bar-.*', + maven_package_requests_forwarding: true, + lock_maven_package_requests_forwarding: true, + npm_package_requests_forwarding: true, + lock_npm_package_requests_forwarding: true, + pypi_package_requests_forwarding: true, + lock_pypi_package_requests_forwarding: true + } it_behaves_like 'returning a success' it_behaves_like 'rejecting invalid regex' diff --git a/spec/requests/api/helm_packages_spec.rb b/spec/requests/api/helm_packages_spec.rb index 5212e225351..6bd81f64913 100644 --- a/spec/requests/api/helm_packages_spec.rb +++ b/spec/requests/api/helm_packages_spec.rb @@ -73,6 +73,17 @@ RSpec.describe API::HelmPackages do end end + context 'with access to package registry for everyone' do + let(:snowplow_gitlab_standard_context) { { project: project, namespace: project.namespace } } + + before do + project.update!(visibility: Gitlab::VisibilityLevel::PRIVATE) + project.project_feature.update!(package_registry_access_level: ProjectFeature::PUBLIC) + end + + it_behaves_like 'process helm download content request', :anonymous, :success + end + context 'when an invalid token is passed' do let(:headers) { basic_auth_header(user.username, 'wrong') } diff --git a/spec/requests/groups/settings/access_tokens_controller_spec.rb b/spec/requests/groups/settings/access_tokens_controller_spec.rb index eabdef3c41e..cf728b3935f 100644 --- a/spec/requests/groups/settings/access_tokens_controller_spec.rb +++ b/spec/requests/groups/settings/access_tokens_controller_spec.rb @@ -87,4 +87,19 @@ RSpec.describe Groups::Settings::AccessTokensController do it_behaves_like 'feature unavailable' it_behaves_like 'PUT resource access tokens available' end + + describe '#index' do + let_it_be(:resource_access_tokens) { create_list(:personal_access_token, 3, user: bot_user) } + + before do + get group_settings_access_tokens_path(resource) + end + + it 'includes details of the active group access tokens' do + active_resource_access_tokens = + ::GroupAccessTokenSerializer.new.represent(resource_access_tokens.reverse, group: resource) + + expect(assigns(:active_resource_access_tokens).to_json).to eq(active_resource_access_tokens.to_json) + end + end end diff --git a/spec/requests/projects/settings/access_tokens_controller_spec.rb b/spec/requests/projects/settings/access_tokens_controller_spec.rb index 780d1b8caef..48114834c65 100644 --- a/spec/requests/projects/settings/access_tokens_controller_spec.rb +++ b/spec/requests/projects/settings/access_tokens_controller_spec.rb @@ -88,4 +88,19 @@ RSpec.describe Projects::Settings::AccessTokensController do it_behaves_like 'feature unavailable' it_behaves_like 'PUT resource access tokens available' end + + describe '#index' do + let_it_be(:resource_access_tokens) { create_list(:personal_access_token, 3, user: bot_user) } + + before do + get project_settings_access_tokens_path(resource) + end + + it 'includes details of the active project access tokens' do + active_resource_access_tokens = + ::ProjectAccessTokenSerializer.new.represent(resource_access_tokens.reverse, project: resource) + + expect(assigns(:active_resource_access_tokens).to_json).to eq(active_resource_access_tokens.to_json) + end + end end diff --git a/spec/requests/users/namespace_callouts_spec.rb b/spec/requests/users/namespace_callouts_spec.rb deleted file mode 100644 index 5a4e269eefb..00000000000 --- a/spec/requests/users/namespace_callouts_spec.rb +++ /dev/null @@ -1,57 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -RSpec.describe 'Namespace callouts' do - let_it_be(:user) { create(:user) } - - before do - sign_in(user) - end - - describe 'POST /-/users/namespace_callouts' do - let(:params) { { feature_name: feature_name, namespace_id: user.namespace.id } } - - subject { post namespace_callouts_path, params: params, headers: { 'ACCEPT' => 'application/json' } } - - context 'with valid feature name and group' do - let(:feature_name) { Users::NamespaceCallout.feature_names.each_key.first } - - context 'when callout entry does not exist' do - it 'creates a callout entry with dismissed state' do - expect { subject }.to change { Users::NamespaceCallout.count }.by(1) - end - - it 'returns success' do - subject - - expect(response).to have_gitlab_http_status(:ok) - end - end - - context 'when callout entry already exists' do - let!(:callout) do - create(:namespace_callout, - feature_name: Users::GroupCallout.feature_names.each_key.first, - user: user, - namespace: user.namespace) - end - - it 'returns success', :aggregate_failures do - expect { subject }.not_to change { Users::NamespaceCallout.count } - expect(response).to have_gitlab_http_status(:ok) - end - end - end - - context 'with invalid feature name' do - let(:feature_name) { 'bogus_feature_name' } - - it 'returns bad request' do - subject - - expect(response).to have_gitlab_http_status(:bad_request) - end - end - end -end diff --git a/spec/serializers/group_access_token_entity_spec.rb b/spec/serializers/group_access_token_entity_spec.rb index 05609dc3c7a..586eb0a8588 100644 --- a/spec/serializers/group_access_token_entity_spec.rb +++ b/spec/serializers/group_access_token_entity_spec.rb @@ -18,7 +18,7 @@ RSpec.describe GroupAccessTokenEntity do expected_revoke_path = Gitlab::Routing.url_helpers .revoke_group_settings_access_token_path( { id: token, - group_id: group.path }) + group_id: group.full_path }) expect(json).to( include( @@ -39,7 +39,7 @@ RSpec.describe GroupAccessTokenEntity do expected_revoke_path = Gitlab::Routing.url_helpers .revoke_group_settings_access_token_path( { id: token, - group_id: group.path }) + group_id: group.full_path }) expect(json).to( include( diff --git a/spec/serializers/project_access_token_entity_spec.rb b/spec/serializers/project_access_token_entity_spec.rb index 4b5b4d4d77d..8af09b0a45d 100644 --- a/spec/serializers/project_access_token_entity_spec.rb +++ b/spec/serializers/project_access_token_entity_spec.rb @@ -18,7 +18,7 @@ RSpec.describe ProjectAccessTokenEntity do expected_revoke_path = Gitlab::Routing.url_helpers .revoke_namespace_project_settings_access_token_path( { id: token, - namespace_id: project.namespace.path, + namespace_id: project.namespace.full_path, project_id: project.path }) expect(json).to( @@ -42,7 +42,7 @@ RSpec.describe ProjectAccessTokenEntity do expected_revoke_path = Gitlab::Routing.url_helpers .revoke_namespace_project_settings_access_token_path( { id: token, - namespace_id: project.namespace.path, + namespace_id: project.namespace.full_path, project_id: project.path }) expect(json).to( diff --git a/spec/services/namespaces/package_settings/update_service_spec.rb b/spec/services/namespaces/package_settings/update_service_spec.rb index ed385f1cd7f..10926c5ef57 100644 --- a/spec/services/namespaces/package_settings/update_service_spec.rb +++ b/spec/services/namespaces/package_settings/update_service_spec.rb @@ -33,8 +33,29 @@ RSpec.describe ::Namespaces::PackageSettings::UpdateService do shared_examples 'updating the namespace package setting' do it_behaves_like 'updating the namespace package setting attributes', - from: { maven_duplicates_allowed: true, maven_duplicate_exception_regex: 'SNAPSHOT', generic_duplicates_allowed: true, generic_duplicate_exception_regex: 'foo' }, - to: { maven_duplicates_allowed: false, maven_duplicate_exception_regex: 'RELEASE', generic_duplicates_allowed: false, generic_duplicate_exception_regex: 'bar' } + from: { + maven_duplicates_allowed: true, + maven_duplicate_exception_regex: 'SNAPSHOT', + generic_duplicates_allowed: true, + generic_duplicate_exception_regex: 'foo', + maven_package_requests_forwarding: true, + lock_maven_package_requests_forwarding: false, + npm_package_requests_forwarding: nil, + lock_npm_package_requests_forwarding: false, + pypi_package_requests_forwarding: nil, + lock_pypi_package_requests_forwarding: false + }, to: { + maven_duplicates_allowed: false, + maven_duplicate_exception_regex: 'RELEASE', + generic_duplicates_allowed: false, + generic_duplicate_exception_regex: 'bar', + maven_package_requests_forwarding: true, + lock_maven_package_requests_forwarding: true, + npm_package_requests_forwarding: true, + lock_npm_package_requests_forwarding: true, + pypi_package_requests_forwarding: true, + lock_pypi_package_requests_forwarding: true + } it_behaves_like 'returning a success' @@ -63,10 +84,18 @@ RSpec.describe ::Namespaces::PackageSettings::UpdateService do context 'with existing namespace package setting' do let_it_be(:package_settings) { create(:namespace_package_setting, namespace: namespace) } let_it_be(:params) do - { maven_duplicates_allowed: false, + { + maven_duplicates_allowed: false, maven_duplicate_exception_regex: 'RELEASE', generic_duplicates_allowed: false, - generic_duplicate_exception_regex: 'bar' } + generic_duplicate_exception_regex: 'bar', + maven_package_requests_forwarding: true, + lock_maven_package_requests_forwarding: true, + npm_package_requests_forwarding: true, + lock_npm_package_requests_forwarding: true, + pypi_package_requests_forwarding: true, + lock_pypi_package_requests_forwarding: true + } end where(:user_role, :shared_examples_name) do diff --git a/spec/services/users/dismiss_namespace_callout_service_spec.rb b/spec/services/users/dismiss_namespace_callout_service_spec.rb deleted file mode 100644 index fbcdb66c9e8..00000000000 --- a/spec/services/users/dismiss_namespace_callout_service_spec.rb +++ /dev/null @@ -1,24 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -RSpec.describe Users::DismissNamespaceCalloutService do - describe '#execute' do - let_it_be(:user) { create(:user) } - - let(:params) { { feature_name: feature_name, namespace_id: user.namespace.id } } - let(:feature_name) { Users::NamespaceCallout.feature_names.each_key.first } - - subject(:execute) do - described_class.new( - container: nil, current_user: user, params: params - ).execute - end - - it_behaves_like 'dismissing user callout', Users::NamespaceCallout - - it 'sets the namespace_id' do - expect(execute.namespace_id).to eq(user.namespace.id) - end - end -end diff --git a/spec/support/shared_examples/features/access_tokens_shared_examples.rb b/spec/support/shared_examples/features/access_tokens_shared_examples.rb index 07e64446ab3..cd255abd7a8 100644 --- a/spec/support/shared_examples/features/access_tokens_shared_examples.rb +++ b/spec/support/shared_examples/features/access_tokens_shared_examples.rb @@ -10,11 +10,11 @@ end RSpec.shared_examples 'resource access tokens creation' do |resource_type| def active_resource_access_tokens - find('.table.active-tokens') + find("[data-testid='active-tokens']") end def created_resource_access_token - find('#created-personal-access-token').value + find_field('new-access-token').value end it 'allows creation of an access token', :aggregate_failures do @@ -106,7 +106,7 @@ end RSpec.shared_examples 'active resource access tokens' do def active_resource_access_tokens - find('.table.active-tokens') + find("[data-testid='active-tokens']") end it 'shows active access tokens' do @@ -129,24 +129,22 @@ RSpec.shared_examples 'active resource access tokens' do end RSpec.shared_examples 'inactive resource access tokens' do |no_active_tokens_text| - def no_resource_access_tokens_message - find('.settings-message') + def active_resource_access_tokens + find("[data-testid='active-tokens']") end it 'allows revocation of an active token' do visit resource_settings_access_tokens_path accept_gl_confirm(button_text: 'Revoke') { click_on 'Revoke' } - expect(page).to have_selector('.settings-message') - expect(no_resource_access_tokens_message).to have_text(no_active_tokens_text) + expect(active_resource_access_tokens).to have_text(no_active_tokens_text) end it 'removes expired tokens from active section' do resource_access_token.update!(expires_at: 5.days.ago) visit resource_settings_access_tokens_path - expect(page).to have_selector('.settings-message') - expect(no_resource_access_tokens_message).to have_text(no_active_tokens_text) + expect(active_resource_access_tokens).to have_text(no_active_tokens_text) end context 'when resource access token creation is not allowed' do @@ -158,8 +156,7 @@ RSpec.shared_examples 'inactive resource access tokens' do |no_active_tokens_tex visit resource_settings_access_tokens_path accept_gl_confirm(button_text: 'Revoke') { click_on 'Revoke' } - expect(page).to have_selector('.settings-message') - expect(no_resource_access_tokens_message).to have_text(no_active_tokens_text) + expect(active_resource_access_tokens).to have_text(no_active_tokens_text) end end end diff --git a/spec/support/shared_examples/lib/gitlab/memory/watchdog/monitor_result_shared_examples.rb b/spec/support/shared_examples/lib/gitlab/memory/watchdog/monitor_result_shared_examples.rb new file mode 100644 index 00000000000..98c0e7d506b --- /dev/null +++ b/spec/support/shared_examples/lib/gitlab/memory/watchdog/monitor_result_shared_examples.rb @@ -0,0 +1,10 @@ +# frozen_string_literal: true + +RSpec.shared_examples 'returns Watchdog Monitor result' do |threshold_violated:| + it 'returns if threshold is violated and payload' do + result = monitor.call + + expect(result[:threshold_violated]).to eq(threshold_violated) + expect(result[:payload]).to eq(payload) + end +end diff --git a/spec/support/shared_examples/requests/access_tokens_controller_shared_examples.rb b/spec/support/shared_examples/requests/access_tokens_controller_shared_examples.rb index 017e6274cb0..59e641e2af6 100644 --- a/spec/support/shared_examples/requests/access_tokens_controller_shared_examples.rb +++ b/spec/support/shared_examples/requests/access_tokens_controller_shared_examples.rb @@ -2,18 +2,13 @@ RSpec.shared_examples 'GET resource access tokens available' do let_it_be(:active_resource_access_token) { create(:personal_access_token, user: bot_user) } - let_it_be(:inactive_resource_access_token) { create(:personal_access_token, :revoked, user: bot_user) } it 'retrieves active resource access tokens' do subject - expect(assigns(:active_resource_access_tokens)).to contain_exactly(active_resource_access_token) - end - - it 'retrieves inactive resource access tokens' do - subject - - expect(assigns(:inactive_resource_access_tokens)).to contain_exactly(inactive_resource_access_token) + token_entities = assigns(:active_resource_access_tokens) + expect(token_entities.length).to eq(1) + expect(token_entities[0][:name]).to eq(active_resource_access_token.name) end it 'lists all available scopes' do @@ -21,15 +16,6 @@ RSpec.shared_examples 'GET resource access tokens available' do expect(assigns(:scopes)).to eq(Gitlab::Auth.resource_bot_scopes) end - - it 'retrieves newly created personal access token value' do - token_value = 'random-value' - allow(PersonalAccessToken).to receive(:redis_getdel).with("#{user.id}:#{resource.id}").and_return(token_value) - - subject - - expect(assigns(:new_resource_access_token)).to eq(token_value) - end end RSpec.shared_examples 'POST resource access tokens available' do @@ -37,10 +23,13 @@ RSpec.shared_examples 'POST resource access tokens available' do PersonalAccessToken.order(:created_at).last end - it 'returns success message' do + it 'renders JSON with a token' do subject - expect(flash[:notice]).to match('Your new access token has been created.') + parsed_body = Gitlab::Json.parse(response.body) + expect(parsed_body['new_token']).not_to be_blank + expect(parsed_body['errors']).to be_blank + expect(response).to have_gitlab_http_status(:success) end it 'creates resource access token' do @@ -59,12 +48,6 @@ RSpec.shared_examples 'POST resource access tokens available' do expect(created_token.user).to be_project_bot end - it 'stores newly created token redis store' do - expect(PersonalAccessToken).to receive(:redis_store!) - - subject - end - it { expect { subject }.to change { User.count }.by(1) } it { expect { subject }.to change { PersonalAccessToken.count }.by(1) } @@ -87,10 +70,13 @@ RSpec.shared_examples 'POST resource access tokens available' do expect { subject }.not_to change { User.count } end - it 'shows a failure alert' do + it 'renders JSON with an error' do subject - expect(flash[:alert]).to match("Failed to create new access token: Failed!") + parsed_body = Gitlab::Json.parse(response.body) + expect(parsed_body['new_token']).to be_blank + expect(parsed_body['errors']).to contain_exactly('Failed!') + expect(response).to have_gitlab_http_status(:unprocessable_entity) end end end diff --git a/spec/support/shared_examples/requests/api/helm_packages_shared_examples.rb b/spec/support/shared_examples/requests/api/helm_packages_shared_examples.rb index 06ed0448b50..8bf6b162508 100644 --- a/spec/support/shared_examples/requests/api/helm_packages_shared_examples.rb +++ b/spec/support/shared_examples/requests/api/helm_packages_shared_examples.rb @@ -247,6 +247,15 @@ RSpec.shared_examples 'handling helm chart index requests' do end end + context 'with access to package registry for everyone' do + before do + project.update!(visibility: Gitlab::VisibilityLevel::PRIVATE) + project.project_feature.update!(package_registry_access_level: ProjectFeature::PUBLIC) + end + + it_behaves_like 'process helm service index request', :anonymous, :success + end + context 'when an invalid token is passed' do let(:headers) { basic_auth_header(user.username, 'wrong') } diff --git a/spec/workers/every_sidekiq_worker_spec.rb b/spec/workers/every_sidekiq_worker_spec.rb index 2f652ac6472..0c2ed26459a 100644 --- a/spec/workers/every_sidekiq_worker_spec.rb +++ b/spec/workers/every_sidekiq_worker_spec.rb @@ -260,6 +260,8 @@ RSpec.describe 'Every Sidekiq worker' do 'Gitlab::GithubImport::ImportReleaseAttachmentsWorker' => 5, 'Gitlab::GithubImport::Attachments::ImportReleaseWorker' => 5, 'Gitlab::GithubImport::Attachments::ImportNoteWorker' => 5, + 'Gitlab::GithubImport::Attachments::ImportIssueWorker' => 5, + 'Gitlab::GithubImport::Attachments::ImportMergeRequestWorker' => 5, 'Gitlab::GithubImport::ImportDiffNoteWorker' => 5, 'Gitlab::GithubImport::ImportIssueWorker' => 5, 'Gitlab::GithubImport::ImportIssueEventWorker' => 5, diff --git a/spec/workers/gitlab/github_import/attachments/import_issue_worker_spec.rb b/spec/workers/gitlab/github_import/attachments/import_issue_worker_spec.rb new file mode 100644 index 00000000000..6d617755861 --- /dev/null +++ b/spec/workers/gitlab/github_import/attachments/import_issue_worker_spec.rb @@ -0,0 +1,34 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Gitlab::GithubImport::Attachments::ImportIssueWorker do + subject(:worker) { described_class.new } + + describe '#import' do + let(:import_state) { create(:import_state, :started) } + + let(:project) do + instance_double('Project', full_path: 'foo/bar', id: 1, import_state: import_state) + end + + let(:client) { instance_double('Gitlab::GithubImport::Client') } + + it 'imports an issue attachments' do + expect_next_instance_of( + Gitlab::GithubImport::Importer::NoteAttachmentsImporter, + an_instance_of(Gitlab::GithubImport::Representation::NoteText), + project, + client + ) do |note_attachments_importer| + expect(note_attachments_importer).to receive(:execute) + end + + expect(Gitlab::GithubImport::ObjectCounter) + .to receive(:increment) + .and_call_original + + worker.import(project, client, {}) + end + end +end diff --git a/spec/workers/gitlab/github_import/attachments/import_merge_request_worker_spec.rb b/spec/workers/gitlab/github_import/attachments/import_merge_request_worker_spec.rb new file mode 100644 index 00000000000..66dfc027e6e --- /dev/null +++ b/spec/workers/gitlab/github_import/attachments/import_merge_request_worker_spec.rb @@ -0,0 +1,34 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Gitlab::GithubImport::Attachments::ImportMergeRequestWorker do + subject(:worker) { described_class.new } + + describe '#import' do + let(:import_state) { create(:import_state, :started) } + + let(:project) do + instance_double('Project', full_path: 'foo/bar', id: 1, import_state: import_state) + end + + let(:client) { instance_double('Gitlab::GithubImport::Client') } + + it 'imports an merge request attachments' do + expect_next_instance_of( + Gitlab::GithubImport::Importer::NoteAttachmentsImporter, + an_instance_of(Gitlab::GithubImport::Representation::NoteText), + project, + client + ) do |note_attachments_importer| + expect(note_attachments_importer).to receive(:execute) + end + + expect(Gitlab::GithubImport::ObjectCounter) + .to receive(:increment) + .and_call_original + + worker.import(project, client, {}) + end + end +end diff --git a/spec/workers/gitlab/github_import/stage/import_attachments_worker_spec.rb b/spec/workers/gitlab/github_import/stage/import_attachments_worker_spec.rb index 33e2443d106..ecfece735af 100644 --- a/spec/workers/gitlab/github_import/stage/import_attachments_worker_spec.rb +++ b/spec/workers/gitlab/github_import/stage/import_attachments_worker_spec.rb @@ -5,8 +5,7 @@ require 'spec_helper' RSpec.describe Gitlab::GithubImport::Stage::ImportAttachmentsWorker do subject(:worker) { described_class.new } - let(:project) { create(:project) } - let!(:group) { create(:group, projects: [project]) } + let_it_be(:project) { create(:project) } let(:settings) { ::Gitlab::GithubImport::Settings.new(project) } let(:stage_enabled) { true } @@ -15,28 +14,42 @@ RSpec.describe Gitlab::GithubImport::Stage::ImportAttachmentsWorker do end describe '#import' do - let(:releases_importer) { instance_double('Gitlab::GithubImport::Importer::Attachments::ReleasesImporter') } - let(:notes_importer) { instance_double('Gitlab::GithubImport::Importer::Attachments::NotesImporter') } let(:client) { instance_double('Gitlab::GithubImport::Client') } - let(:releases_waiter) { Gitlab::JobWaiter.new(2, '123') } - let(:notes_waiter) { Gitlab::JobWaiter.new(3, '234') } - - it 'imports release attachments' do - expect(Gitlab::GithubImport::Importer::Attachments::ReleasesImporter) - .to receive(:new) - .with(project, client) - .and_return(releases_importer) - expect(releases_importer).to receive(:execute).and_return(releases_waiter) - - expect(Gitlab::GithubImport::Importer::Attachments::NotesImporter) - .to receive(:new) - .with(project, client) - .and_return(notes_importer) - expect(notes_importer).to receive(:execute).and_return(notes_waiter) + let(:importers) do + [ + { + klass: Gitlab::GithubImport::Importer::Attachments::ReleasesImporter, + double: instance_double('Gitlab::GithubImport::Importer::Attachments::ReleasesImporter'), + waiter: Gitlab::JobWaiter.new(2, '123') + }, + { + klass: Gitlab::GithubImport::Importer::Attachments::NotesImporter, + double: instance_double('Gitlab::GithubImport::Importer::Attachments::NotesImporter'), + waiter: Gitlab::JobWaiter.new(3, '234') + }, + { + klass: Gitlab::GithubImport::Importer::Attachments::IssuesImporter, + double: instance_double('Gitlab::GithubImport::Importer::Attachments::IssuesImporter'), + waiter: Gitlab::JobWaiter.new(4, '345') + }, + { + klass: Gitlab::GithubImport::Importer::Attachments::MergeRequestsImporter, + double: instance_double('Gitlab::GithubImport::Importer::Attachments::MergeRequestsImporter'), + waiter: Gitlab::JobWaiter.new(5, '456') + } + ] + end + + it 'imports attachments' do + importers.each do |importer| + expect_next_instance_of(importer[:klass], project, client) do |instance| + expect(instance).to receive(:execute).and_return(importer[:waiter]) + end + end expect(Gitlab::GithubImport::AdvanceStageWorker) .to receive(:perform_async) - .with(project.id, { '123' => 2, '234' => 3 }, :protected_branches) + .with(project.id, { '123' => 2, '234' => 3, '345' => 4, '456' => 5 }, :protected_branches) worker.import(client, project) end @@ -45,8 +58,7 @@ RSpec.describe Gitlab::GithubImport::Stage::ImportAttachmentsWorker do let(:stage_enabled) { false } it 'skips release attachments import and calls next stage' do - expect(Gitlab::GithubImport::Importer::Attachments::ReleasesImporter).not_to receive(:new) - expect(Gitlab::GithubImport::Importer::Attachments::NotesImporter).not_to receive(:new) + importers.each { |importer| expect(importer[:klass]).not_to receive(:new) } expect(Gitlab::GithubImport::AdvanceStageWorker) .to receive(:perform_async).with(project.id, {}, :protected_branches) diff --git a/spec/workers/pages/invalidate_domain_cache_worker_spec.rb b/spec/workers/pages/invalidate_domain_cache_worker_spec.rb index 9658aacd093..b9c27c54fa1 100644 --- a/spec/workers/pages/invalidate_domain_cache_worker_spec.rb +++ b/spec/workers/pages/invalidate_domain_cache_worker_spec.rb @@ -197,6 +197,36 @@ RSpec.describe Pages::InvalidateDomainCacheWorker do end end + context 'when project features change' do + it_behaves_like 'clears caches with', + event_class: Projects::ProjectFeaturesChangedEvent, + event_data: { + project_id: 1, + namespace_id: 2, + root_namespace_id: 3, + 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) + end + end + context 'when namespace based cache keys are duplicated' do # de-dups namespace cache keys it_behaves_like 'clears caches with', |