diff options
author | GitLab Bot <gitlab-bot@gitlab.com> | 2022-10-13 03:08:53 +0300 |
---|---|---|
committer | GitLab Bot <gitlab-bot@gitlab.com> | 2022-10-13 03:08:53 +0300 |
commit | 0cd52ae4aff38b1fc95dfc9488d61fa0b374eafc (patch) | |
tree | beab0e0c164566a3c0c6fc6e2232eacf5a8fdf8f /spec | |
parent | a9acc0c2fb0f280077edc7a70dbf383c55252af8 (diff) |
Add latest changes from gitlab-org/gitlab@master
Diffstat (limited to 'spec')
-rw-r--r-- | spec/controllers/import/github_controller_spec.rb | 33 | ||||
-rw-r--r-- | spec/controllers/projects/milestones_controller_spec.rb | 20 | ||||
-rw-r--r-- | spec/features/milestone_spec.rb | 6 | ||||
-rw-r--r-- | spec/features/oauth_registration_spec.rb | 175 | ||||
-rw-r--r-- | spec/frontend/vue_shared/components/gitlab_version_check_spec.js | 18 | ||||
-rw-r--r-- | spec/helpers/releases_helper_spec.rb | 6 | ||||
-rw-r--r-- | spec/lib/gitlab/ci/pipeline/chain/cancel_pending_pipelines_spec.rb | 37 | ||||
-rw-r--r-- | spec/lib/gitlab/git_access_spec.rb | 24 | ||||
-rw-r--r-- | spec/models/hooks/web_hook_spec.rb | 106 | ||||
-rw-r--r-- | spec/models/pool_repository_spec.rb | 18 | ||||
-rw-r--r-- | spec/requests/api/import_github_spec.rb | 38 | ||||
-rw-r--r-- | spec/services/import/github/cancel_project_import_service_spec.rb | 56 | ||||
-rw-r--r-- | spec/services/web_hooks/log_execution_service_spec.rb | 33 | ||||
-rw-r--r-- | spec/support/shared_examples/requests/api/hooks_shared_examples.rb | 1 |
14 files changed, 401 insertions, 170 deletions
diff --git a/spec/controllers/import/github_controller_spec.rb b/spec/controllers/import/github_controller_spec.rb index 47e7ff98812..592e2b44192 100644 --- a/spec/controllers/import/github_controller_spec.rb +++ b/spec/controllers/import/github_controller_spec.rb @@ -321,4 +321,37 @@ RSpec.describe Import::GithubController do expect(json_response[0]['stats']).to include('imported') end end + + describe "POST cancel" do + let_it_be(:project) { create(:project, :import_started, import_type: 'github', import_url: 'https://fake.url') } + + context 'when project import was canceled' do + before do + allow(Import::Github::CancelProjectImportService) + .to receive(:new).with(project, user) + .and_return(double(execute: { status: :success, project: project })) + end + + it 'returns success' do + post :cancel, params: { project_id: project.id } + + expect(response).to have_gitlab_http_status(:ok) + end + end + + context 'when project import was not canceled' do + before do + allow(Import::Github::CancelProjectImportService) + .to receive(:new).with(project, user) + .and_return(double(execute: { status: :error, message: 'The import cannot be canceled because it is finished', http_status: :bad_request })) + end + + it 'returns error' do + post :cancel, params: { project_id: project.id } + + expect(response).to have_gitlab_http_status(:bad_request) + expect(json_response['errors']).to eq('The import cannot be canceled because it is finished') + end + end + end end diff --git a/spec/controllers/projects/milestones_controller_spec.rb b/spec/controllers/projects/milestones_controller_spec.rb index b62353784b3..28da7eff8fc 100644 --- a/spec/controllers/projects/milestones_controller_spec.rb +++ b/spec/controllers/projects/milestones_controller_spec.rb @@ -44,6 +44,26 @@ RSpec.describe Projects::MilestonesController do end end + describe "#create" do + it 'does not redirect without redirect_path' do + post :create, params: { namespace_id: project.namespace.id, project_id: project.id, milestone: { title: 'test' } } + + expect(response).to redirect_to(project_milestone_path(project, project.milestones.last)) + end + + it 'redirects when given a redirect_path' do + post :create, params: { namespace_id: project.namespace.id, project_id: project.id, redirect_path: 'new_release', milestone: { title: 'test' } } + + expect(response).to redirect_to(new_project_release_path(project)) + end + + it 'will not redirect when given a redirect_path with an error' do + post :create, params: { namespace_id: project.namespace.id, project_id: project.id, redirect_path: 'new_release', milestone: { title: nil } } + + expect(response).to have_gitlab_http_status(:ok) + end + end + describe "#index" do context "as html" do def render_index(project:, page:, search_title: '') diff --git a/spec/features/milestone_spec.rb b/spec/features/milestone_spec.rb index 3aaffad314e..98d623902a5 100644 --- a/spec/features/milestone_spec.rb +++ b/spec/features/milestone_spec.rb @@ -28,6 +28,12 @@ RSpec.describe 'Milestone' do expect(find('[data-testid="no-issues-alert"]')).to have_content('Assign some issues to this milestone.') expect(page).to have_content('Nov 16, 2016–Dec 16, 2016') end + + it 'passes redirect_path through to form' do + visit new_project_milestone_path(project, redirect_path: 'new_release') + + expect(find('#redirect_path', visible: :all)[:value]).to eq('new_release') + end end describe 'Open a milestone with closed issues' do diff --git a/spec/features/oauth_registration_spec.rb b/spec/features/oauth_registration_spec.rb index cb8343b8065..0a35b5a7e42 100644 --- a/spec/features/oauth_registration_spec.rb +++ b/spec/features/oauth_registration_spec.rb @@ -3,126 +3,139 @@ require 'spec_helper' RSpec.describe 'OAuth Registration', :js, :allow_forgery_protection do - include DeviseHelpers include LoginHelpers include TermsHelper using RSpec::Parameterized::TableSyntax + let(:uid) { 'my-uid' } + let(:email) { 'user@example.com' } + around do |example| with_omniauth_full_host { example.run } end - context 'when the user registers using single-sign on provider' do - let(:uid) { 'my-uid' } - let(:email) { 'user@example.com' } - - where(:provider, :additional_info) do - :github | {} - :twitter | {} - :bitbucket | {} - :gitlab | {} - :google_oauth2 | {} - :facebook | {} - :cas3 | {} - :auth0 | {} - :authentiq | {} - :salesforce | { extra: { email_verified: true } } - :dingtalk | {} - :alicloud | {} + where(:provider, :additional_info) do + :github | {} + :twitter | {} + :bitbucket | {} + :gitlab | {} + :google_oauth2 | {} + :facebook | {} + :cas3 | {} + :auth0 | {} + :authentiq | {} + :salesforce | { extra: { email_verified: true } } + :dingtalk | {} + :alicloud | {} + end + + with_them do + before do + stub_omniauth_provider(provider) + stub_feature_flags(update_oauth_registration_flow: true) end - with_them do + context 'when block_auto_created_users is true' do before do - stub_omniauth_provider(provider) - stub_feature_flags(update_oauth_registration_flow: true) + stub_omniauth_setting(block_auto_created_users: true) end - context 'when block_auto_created_users is true' do - before do - stub_omniauth_setting(block_auto_created_users: true) - end + it 'redirects back to the sign-in page' do + register_via(provider, uid, email, additional_info: additional_info) - it 'redirects back to the sign-in page' do - register_via(provider, uid, email, additional_info: additional_info) + expect(page).to have_current_path new_user_session_path + expect(page).to have_content('Your account is pending approval') + end + end - expect(page).to have_current_path new_user_session_path - expect(page).to have_content('Your account is pending approval') - end + context 'when block_auto_created_users is false' do + before do + stub_omniauth_setting(block_auto_created_users: false) + end + + it 'redirects to the initial welcome path' do + register_via(provider, uid, email, additional_info: additional_info) + + expect(page).to have_current_path users_sign_up_welcome_path + expect(page).to have_content('Welcome to GitLab, mockuser!') end - context 'when block_auto_created_users is false' do + context 'when terms are enforced' do before do - stub_omniauth_setting(block_auto_created_users: false) + enforce_terms end - it 'redirects to the initial welcome path' do + it 'auto accepts terms and redirects to the initial welcome path' do register_via(provider, uid, email, additional_info: additional_info) expect(page).to have_current_path users_sign_up_welcome_path expect(page).to have_content('Welcome to GitLab, mockuser!') end + end - context 'when terms are enforced' do - before do - enforce_terms - end + context 'when provider does not send a verified email address' do + let(:email) { 'temp-email-for-oauth@email.com' } - it 'auto accepts terms and redirects to the initial welcome path' do - register_via(provider, uid, email, additional_info: additional_info) + it 'redirects to the profile path' do + register_via(provider, uid, email, additional_info: additional_info) - expect(page).to have_current_path users_sign_up_welcome_path - expect(page).to have_content('Welcome to GitLab, mockuser!') - end + expect(page).to have_current_path profile_path + expect(page).to have_content('Please complete your profile with email address') end + end - context 'when provider does not send a verified email address' do - let(:email) { 'temp-email-for-oauth@email.com' } + context 'when registering via an invitation email' do + let_it_be(:owner) { create(:user) } + let_it_be(:group) { create(:group, name: 'Owned') } + let_it_be(:project) { create(:project, :repository, namespace: group) } + + let(:invite_email) { generate(:email) } + let(:extra_params) { { invite_type: Emails::Members::INITIAL_INVITE } } + let(:group_invite) do + create( + :group_member, :invited, + group: group, + invite_email: invite_email, + created_by: owner + ) + end - it 'redirects to the profile path' do - register_via(provider, uid, email, additional_info: additional_info) + before do + project.add_maintainer(owner) + group.add_owner(owner) + group_invite.generate_invite_token! - expect(page).to have_current_path profile_path - expect(page).to have_content('Please complete your profile with email address') - end + mock_auth_hash(provider, uid, invite_email, additional_info: additional_info) end - context 'when registering via an invitation email' do - let_it_be(:owner) { create(:user) } - let_it_be(:group) { create(:group, name: 'Owned') } - let_it_be(:project) { create(:project, :repository, namespace: group) } - - let(:invite_email) { generate(:email) } - let(:extra_params) { { invite_type: Emails::Members::INITIAL_INVITE } } - let(:group_invite) do - create( - :group_member, :invited, - group: group, - invite_email: invite_email, - created_by: owner - ) - end - - before do - project.add_maintainer(owner) - group.add_owner(owner) - group_invite.generate_invite_token! - - mock_auth_hash(provider, uid, invite_email, additional_info: additional_info) - end - - it 'redirects to the activity page with all the projects/groups invitations accepted' do - visit invite_path(group_invite.raw_invite_token, extra_params) - click_link_or_button "oauth-login-#{provider}" - fill_in_welcome_form - - expect(page).to have_content('You have been granted Owner access to group Owned.') - expect(page).to have_current_path(activity_group_path(group), ignore_query: true) - end + it 'redirects to the activity page with all the projects/groups invitations accepted' do + visit invite_path(group_invite.raw_invite_token, extra_params) + click_link_or_button "oauth-login-#{provider}" + fill_in_welcome_form + + expect(page).to have_content('You have been granted Owner access to group Owned.') + expect(page).to have_current_path(activity_group_path(group), ignore_query: true) end end end end + context 'when update_oauth_registration_flow is disabled' do + before do + stub_omniauth_provider(:github) + stub_omniauth_setting(block_auto_created_users: false) + stub_feature_flags(update_oauth_registration_flow: false) + + enforce_terms + end + + it 'presents the terms page' do + register_via(:github, uid, email) + + expect(page).to have_content('These are the terms') + end + end + def fill_in_welcome_form select 'Software Developer', from: 'user_role' click_button 'Get started!' diff --git a/spec/frontend/vue_shared/components/gitlab_version_check_spec.js b/spec/frontend/vue_shared/components/gitlab_version_check_spec.js index f6bb8f5de6c..5f63d38c532 100644 --- a/spec/frontend/vue_shared/components/gitlab_version_check_spec.js +++ b/spec/frontend/vue_shared/components/gitlab_version_check_spec.js @@ -2,6 +2,7 @@ import { GlBadge } from '@gitlab/ui'; import { shallowMount } from '@vue/test-utils'; import MockAdapter from 'axios-mock-adapter'; import waitForPromises from 'helpers/wait_for_promises'; +import { mockTracking } from 'helpers/tracking_helper'; import { helpPagePath } from '~/helpers/help_page_helper'; import axios from '~/lib/utils/axios_utils'; import GitlabVersionCheck from '~/vue_shared/components/gitlab_version_check.vue'; @@ -93,8 +94,11 @@ describe('GitlabVersionCheck', () => { ${{ code: 200, res: { severity: 'danger' } }} | ${{ title: 'Update ASAP', variant: 'danger' }} `('badge ui', ({ mockResponse, expectedUI }) => { describe(`when response is ${mockResponse.res.severity}`, () => { + let trackingSpy; + beforeEach(async () => { createComponent(mockResponse); + trackingSpy = mockTracking(undefined, wrapper.element, jest.spyOn); await waitForPromises(); // Ensure we wrap up the axios call }); @@ -106,9 +110,23 @@ describe('GitlabVersionCheck', () => { expect(findGlBadge().attributes('variant')).toBe(expectedUI.variant); }); + it(`tracks rendered_version_badge with status ${expectedUI.variant}`, () => { + expect(trackingSpy).toHaveBeenCalledWith(undefined, 'rendered_version_badge', { + label: expectedUI.variant, + }); + }); + it(`link is ${UPGRADE_DOCS_URL}`, () => { expect(findGlBadge().attributes('href')).toBe(UPGRADE_DOCS_URL); }); + + it(`tracks click_version_badge with status ${expectedUI.variant} when badge is clicked`, async () => { + await findGlBadge().vm.$emit('click'); + + expect(trackingSpy).toHaveBeenCalledWith(undefined, 'click_version_badge', { + label: expectedUI.variant, + }); + }); }); }); }); diff --git a/spec/helpers/releases_helper_spec.rb b/spec/helpers/releases_helper_spec.rb index 59a92c067f4..5a9deb5c63b 100644 --- a/spec/helpers/releases_helper_spec.rb +++ b/spec/helpers/releases_helper_spec.rb @@ -49,6 +49,12 @@ RSpec.describe ReleasesHelper do expect(helper.data_for_releases_page[:new_release_path]).to eq(new_project_release_path(project)) end end + + context 'new releases redirect new milestone creation' do + it 'redirects new_milestone_path back to the release page' do + expect(helper.data_for_new_release_page[:new_milestone_path]).to include('redirect_path') + end + end end describe '#data_for_edit_release_page' do diff --git a/spec/lib/gitlab/ci/pipeline/chain/cancel_pending_pipelines_spec.rb b/spec/lib/gitlab/ci/pipeline/chain/cancel_pending_pipelines_spec.rb index b570f2a7f75..31f596e7e70 100644 --- a/spec/lib/gitlab/ci/pipeline/chain/cancel_pending_pipelines_spec.rb +++ b/spec/lib/gitlab/ci/pipeline/chain/cancel_pending_pipelines_spec.rb @@ -141,7 +141,42 @@ RSpec.describe Gitlab::Ci::Pipeline::Chain::CancelPendingPipelines do end end - context 'when the prev pipeline source is webide' do + context 'when the pipeline is a child pipeline' do + let!(:parent_pipeline) { create(:ci_pipeline, project: project, sha: new_commit.sha) } + let(:pipeline) { create(:ci_pipeline, child_of: parent_pipeline) } + + before do + create(:ci_build, :interruptible, :running, pipeline: parent_pipeline) + create(:ci_build, :interruptible, :running, pipeline: parent_pipeline) + end + + it 'does not cancel any builds' do + expect(build_statuses(prev_pipeline)).to contain_exactly('running', 'success', 'created') + expect(build_statuses(parent_pipeline)).to contain_exactly('running', 'running') + + perform + + expect(build_statuses(prev_pipeline)).to contain_exactly('running', 'success', 'created') + expect(build_statuses(parent_pipeline)).to contain_exactly('running', 'running') + end + + context 'when feature flag ci_skip_auto_cancelation_on_child_pipelines is disabled' do + before do + stub_feature_flags(ci_skip_auto_cancelation_on_child_pipelines: false) + end + + it 'does not cancel the parent pipeline' do + expect(build_statuses(parent_pipeline)).to contain_exactly('running', 'running') + + perform + + expect(build_statuses(prev_pipeline)).to contain_exactly('success', 'canceled', 'canceled') + expect(build_statuses(parent_pipeline)).to contain_exactly('running', 'running') + end + end + end + + context 'when the previous pipeline source is webide' do let(:prev_pipeline) { create(:ci_pipeline, :webide, project: project) } it 'does not cancel builds of the previous pipeline' do diff --git a/spec/lib/gitlab/git_access_spec.rb b/spec/lib/gitlab/git_access_spec.rb index cdebbe1539a..7e3a1bf61bc 100644 --- a/spec/lib/gitlab/git_access_spec.rb +++ b/spec/lib/gitlab/git_access_spec.rb @@ -4,7 +4,6 @@ require 'spec_helper' RSpec.describe Gitlab::GitAccess, :aggregate_failures do include TermsHelper - include GitHelpers include AdminModeHelper let(:user) { create(:user) } @@ -789,18 +788,29 @@ RSpec.describe Gitlab::GitAccess, :aggregate_failures do def merge_into_protected_branch @protected_branch_merge_commit ||= begin project.repository.add_branch(user, unprotected_branch, 'feature') - rugged = rugged_repo(project.repository) - target_branch = rugged.rev_parse('feature') + target_branch = TestEnv::BRANCH_SHA['feature'] source_branch = project.repository.create_file( user, 'filename', 'This is the file content', message: 'This is a good commit message', branch_name: unprotected_branch) - author = { email: "email@example.com", time: Time.now, name: "Example Git User" } - - merge_index = rugged.merge_commits(target_branch, source_branch) - Rugged::Commit.create(rugged, author: author, committer: author, message: "commit message", parents: [target_branch, source_branch], tree: merge_index.write_tree(rugged)) + merge_id = project.repository.raw.merge_to_ref( + user, + branch: target_branch, + first_parent_ref: target_branch, + source_sha: source_branch, + target_ref: 'refs/merge-requests/test', + message: 'commit message' + ) + + # We are trying to simulate what the repository would look like + # during the pre-receive hook, before the actual ref is + # written/created. Repository#new_commits relies on there being no + # ref pointing to the merge commit. + project.repository.delete_refs('refs/merge-requests/test') + + merge_id end end diff --git a/spec/models/hooks/web_hook_spec.rb b/spec/models/hooks/web_hook_spec.rb index 036d2effc0f..93c47422f37 100644 --- a/spec/models/hooks/web_hook_spec.rb +++ b/spec/models/hooks/web_hook_spec.rb @@ -170,7 +170,7 @@ RSpec.describe WebHook do end it 'does not async execute non-executable hooks' do - hook.update!(disabled_until: 1.day.from_now) + allow(hook).to receive(:executable?).and_return(false) expect(WebHookService).not_to receive(:new) @@ -238,17 +238,18 @@ RSpec.describe WebHook do [ [0, :not_set, true], [0, :past, true], - [0, :future, false], - [0, :now, false], + [0, :future, true], + [0, :now, true], [1, :not_set, true], [1, :past, true], - [1, :future, false], + [1, :future, true], [3, :not_set, true], [3, :past, true], - [3, :future, false], + [3, :future, true], [4, :not_set, false], - [4, :past, false], - [4, :future, false] + [4, :past, true], # expired suspension + [4, :now, false], # active suspension + [4, :future, false] # active suspension ] end @@ -357,6 +358,7 @@ RSpec.describe WebHook do end it 'makes a hook executable if it is currently backed off' do + hook.recent_failures = 1000 hook.disabled_until = 1.hour.from_now expect { hook.enable! }.to change(hook, :executable?).from(false).to(true) @@ -378,55 +380,71 @@ RSpec.describe WebHook do end describe 'backoff!' do - it 'sets disabled_until to the next backoff' do - expect { hook.backoff! }.to change(hook, :disabled_until).to(hook.next_backoff.from_now) - end + context 'when we have not backed off before' do + it 'does not disable the hook' do + expect { hook.backoff! }.not_to change(hook, :executable?).from(true) + end - it 'increments the backoff count' do - expect { hook.backoff! }.to change(hook, :backoff_count).by(1) + it 'increments the recent_failures count' do + expect { hook.backoff! }.to change(hook, :recent_failures).by(1) + end end - context 'when the hook is permanently disabled' do + context 'when we have exhausted the grace period' do before do - allow(hook).to receive(:permanently_disabled?).and_return(true) + hook.update!(recent_failures: described_class::FAILURE_THRESHOLD) end - it 'does not set disabled_until' do - expect { hook.backoff! }.not_to change(hook, :disabled_until) + it 'sets disabled_until to the next backoff' do + expect { hook.backoff! }.to change(hook, :disabled_until).to(hook.next_backoff.from_now) end - it 'does not increment the backoff count' do - expect { hook.backoff! }.not_to change(hook, :backoff_count) - end - end - - context 'when we have backed off MAX_FAILURES times' do - before do - stub_const("#{described_class}::MAX_FAILURES", 5) - 5.times { hook.backoff! } + it 'increments the backoff count' do + expect { hook.backoff! }.to change(hook, :backoff_count).by(1) end - it 'does not let the backoff count exceed the maximum failure count' do - expect { hook.backoff! }.not_to change(hook, :backoff_count) - end + context 'when the hook is permanently disabled' do + before do + allow(hook).to receive(:permanently_disabled?).and_return(true) + end - it 'does not change disabled_until', :skip_freeze_time do - travel_to(hook.disabled_until - 1.minute) do + it 'does not set disabled_until' do expect { hook.backoff! }.not_to change(hook, :disabled_until) end + + it 'does not increment the backoff count' do + expect { hook.backoff! }.not_to change(hook, :backoff_count) + end end - it 'changes disabled_until when it has elapsed', :skip_freeze_time do - travel_to(hook.disabled_until + 1.minute) do - expect { hook.backoff! }.to change { hook.disabled_until } - expect(hook.backoff_count).to eq(described_class::MAX_FAILURES) + context 'when we have backed off MAX_FAILURES times' do + before do + stub_const("#{described_class}::MAX_FAILURES", 5) + (described_class::FAILURE_THRESHOLD + 5).times { hook.backoff! } + end + + it 'does not let the backoff count exceed the maximum failure count' do + expect { hook.backoff! }.not_to change(hook, :backoff_count) + end + + it 'does not change disabled_until', :skip_freeze_time do + travel_to(hook.disabled_until - 1.minute) do + expect { hook.backoff! }.not_to change(hook, :disabled_until) + end + end + + it 'changes disabled_until when it has elapsed', :skip_freeze_time do + travel_to(hook.disabled_until + 1.minute) do + expect { hook.backoff! }.to change { hook.disabled_until } + expect(hook.backoff_count).to eq(described_class::MAX_FAILURES) + end end end - end - include_examples 'is tolerant of invalid records' do - def run_expectation - expect { hook.backoff! }.to change(hook, :backoff_count).by(1) + include_examples 'is tolerant of invalid records' do + def run_expectation + expect { hook.backoff! }.to change(hook, :backoff_count).by(1) + end end end end @@ -468,8 +486,19 @@ RSpec.describe WebHook do expect(hook).not_to be_temporarily_disabled end + it 'allows FAILURE_THRESHOLD initial failures before we back-off' do + described_class::FAILURE_THRESHOLD.times do + hook.backoff! + expect(hook).not_to be_temporarily_disabled + end + + hook.backoff! + expect(hook).to be_temporarily_disabled + end + context 'when hook has been told to back off' do before do + hook.update!(recent_failures: described_class::FAILURE_THRESHOLD) hook.backoff! end @@ -550,6 +579,7 @@ RSpec.describe WebHook do context 'when hook has been backed off' do before do + hook.update!(recent_failures: described_class::FAILURE_THRESHOLD + 1) hook.disabled_until = 1.hour.from_now end diff --git a/spec/models/pool_repository_spec.rb b/spec/models/pool_repository_spec.rb index c0eb770a414..9861e832bef 100644 --- a/spec/models/pool_repository_spec.rb +++ b/spec/models/pool_repository_spec.rb @@ -26,13 +26,6 @@ RSpec.describe PoolRepository do describe '#unlink_repository' do let(:pool) { create(:pool_repository, :ready) } - let(:alternates_file) { File.join(repository_path, 'objects', 'info', 'alternates') } - - let(:repository_path) do - Gitlab::GitalyClient::StorageSettings.allow_disk_access do - File.join(TestEnv.repos_path, pool.source_project.repository.relative_path) - end - end before do pool.link_repository(pool.source_project.repository) @@ -41,19 +34,17 @@ RSpec.describe PoolRepository do context 'when the last member leaves' do it 'schedules pool removal' do expect(::ObjectPool::DestroyWorker).to receive(:perform_async).with(pool.id).and_call_original + expect(pool.source_project.repository).to receive(:disconnect_alternates).and_call_original pool.unlink_repository(pool.source_project.repository) - - expect(File).not_to exist(alternates_file) end end context 'when skipping disconnect' do it 'does not change the alternates file' do - before = File.read(alternates_file) - pool.unlink_repository(pool.source_project.repository, disconnect: false) + expect(pool.source_project.repository).not_to receive(:disconnect_alternates) - expect(File.read(alternates_file)).to eq(before) + pool.unlink_repository(pool.source_project.repository, disconnect: false) end end @@ -63,10 +54,9 @@ RSpec.describe PoolRepository do pool.link_repository(other_project.repository) expect(::ObjectPool::DestroyWorker).not_to receive(:perform_async).with(pool.id) + expect(pool.source_project.repository).to receive(:disconnect_alternates).and_call_original pool.unlink_repository(pool.source_project.repository) - - expect(File).not_to exist(alternates_file) end end end diff --git a/spec/requests/api/import_github_spec.rb b/spec/requests/api/import_github_spec.rb index d2fa3dabe69..71d3a997ad7 100644 --- a/spec/requests/api/import_github_spec.rb +++ b/spec/requests/api/import_github_spec.rb @@ -89,4 +89,42 @@ RSpec.describe API::ImportGithub do expect(response).to have_gitlab_http_status(:unprocessable_entity) end end + + describe "POST /import/github/cancel" do + let_it_be(:user) { create(:user) } + let_it_be(:project) { create(:project, :import_started, import_type: 'github', import_url: 'https://fake.url') } + + context 'when project import was canceled' do + before do + allow(Import::Github::CancelProjectImportService) + .to receive(:new).with(project, user) + .and_return(double(execute: { status: :success, project: project })) + end + + it 'returns success' do + post api("/import/github/cancel", user), params: { + project_id: project.id + } + + expect(response).to have_gitlab_http_status(:ok) + end + end + + context 'when project import was not canceled' do + before do + allow(Import::Github::CancelProjectImportService) + .to receive(:new).with(project, user) + .and_return(double(execute: { status: :error, message: 'The import cannot be canceled because it is finished', http_status: :bad_request })) + end + + it 'returns error' do + post api("/import/github/cancel", user), params: { + project_id: project.id + } + + expect(response).to have_gitlab_http_status(:bad_request) + expect(json_response['message']).to eq('The import cannot be canceled because it is finished') + end + end + end end diff --git a/spec/services/import/github/cancel_project_import_service_spec.rb b/spec/services/import/github/cancel_project_import_service_spec.rb new file mode 100644 index 00000000000..77b8771ee65 --- /dev/null +++ b/spec/services/import/github/cancel_project_import_service_spec.rb @@ -0,0 +1,56 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Import::Github::CancelProjectImportService do + subject(:import_cancel) { described_class.new(project, project.owner) } + + let_it_be(:user) { create(:user) } + let_it_be_with_reload(:project) { create(:project, :import_started, import_type: 'github', import_url: 'https://fake.url') } + + describe '.execute' do + context 'when user is an owner' do + context 'when import is in progress' do + it 'update import state to be canceled' do + expect(import_cancel.execute).to eq({ status: :success, project: project }) + end + end + + context 'when import is finished' do + let(:expected_result) do + { + status: :error, + http_status: :bad_request, + message: 'The import cannot be canceled because it is finished' + } + end + + before do + project.import_state.finish! + end + + it 'returns error' do + expect(import_cancel.execute).to eq(expected_result) + end + end + end + + context 'when user is not allowed to read project' do + it 'returns 404' do + expect(described_class.new(project, user).execute) + .to eq({ status: :error, http_status: :not_found, message: 'Not Found' }) + end + end + + context 'when user is not allowed to cancel project' do + before do + project.add_developer(user) + end + + it 'returns 403' do + expect(described_class.new(project, user).execute) + .to eq({ status: :error, http_status: :forbidden, message: 'Unauthorized access' }) + end + end + end +end diff --git a/spec/services/web_hooks/log_execution_service_spec.rb b/spec/services/web_hooks/log_execution_service_spec.rb index 7e3f33c5fbb..9ba2c2b0764 100644 --- a/spec/services/web_hooks/log_execution_service_spec.rb +++ b/spec/services/web_hooks/log_execution_service_spec.rb @@ -46,7 +46,8 @@ RSpec.describe WebHooks::LogExecutionService do it 'updates failure state using a lease that ensures fresh state is written' do service = described_class.new(hook: project_hook, log_data: data, response_category: :error) - WebHook.find(project_hook.id).update!(backoff_count: 1) + # Write state somewhere else, so that the hook is out-of-date + WebHook.find(project_hook.id).update!(recent_failures: 5, disabled_until: 10.minutes.from_now, backoff_count: 1) lease = stub_exclusive_lease(lease_key, timeout: described_class::LOCK_TTL) @@ -148,36 +149,10 @@ RSpec.describe WebHooks::LogExecutionService do data[:response_status] = '500' end - it 'does not increment the failure count' do - expect { service.execute }.not_to change(project_hook, :recent_failures) - end - it 'backs off' do - expect { service.execute }.to change(project_hook, :disabled_until) - end + expect(project_hook).to receive(:backoff!) - it 'increases the backoff count' do - expect { service.execute }.to change(project_hook, :backoff_count).by(1) - end - - context 'when the previous cool-off was near the maximum' do - before do - project_hook.update!(disabled_until: 5.minutes.ago, backoff_count: 8) - end - - it 'sets the disabled_until attribute' do - expect { service.execute }.to change(project_hook, :disabled_until).to(1.day.from_now) - end - end - - context 'when we have backed-off many many times' do - before do - project_hook.update!(disabled_until: 5.minutes.ago, backoff_count: 365) - end - - it 'sets the disabled_until attribute' do - expect { service.execute }.to change(project_hook, :disabled_until).to(1.day.from_now) - end + service.execute end end end diff --git a/spec/support/shared_examples/requests/api/hooks_shared_examples.rb b/spec/support/shared_examples/requests/api/hooks_shared_examples.rb index 013945bd578..d666a754d9f 100644 --- a/spec/support/shared_examples/requests/api/hooks_shared_examples.rb +++ b/spec/support/shared_examples/requests/api/hooks_shared_examples.rb @@ -134,6 +134,7 @@ RSpec.shared_examples 'web-hook API endpoints' do |prefix| context 'the hook is backed-off' do before do + WebHook::FAILURE_THRESHOLD.times { hook.backoff! } hook.backoff! end |