diff options
Diffstat (limited to 'spec')
57 files changed, 904 insertions, 317 deletions
diff --git a/spec/controllers/projects/branches_controller_spec.rb b/spec/controllers/projects/branches_controller_spec.rb index 02b3d5269a6..52a20fa8d07 100644 --- a/spec/controllers/projects/branches_controller_spec.rb +++ b/spec/controllers/projects/branches_controller_spec.rb @@ -331,7 +331,7 @@ describe Projects::BranchesController do let(:branch) { "feature" } it 'returns JSON response with message' do - expect(json_response).to eql("message" => 'Branch was removed') + expect(json_response).to eql("message" => 'Branch was deleted') end it { expect(response).to have_gitlab_http_status(200) } @@ -341,7 +341,7 @@ describe Projects::BranchesController do let(:branch) { "improve/awesome" } it 'returns JSON response with message' do - expect(json_response).to eql('message' => 'Branch was removed') + expect(json_response).to eql('message' => 'Branch was deleted') end it { expect(response).to have_gitlab_http_status(200) } @@ -351,7 +351,7 @@ describe Projects::BranchesController do let(:branch) { 'improve%2Fawesome' } it 'returns JSON response with message' do - expect(json_response).to eql('message' => 'Branch was removed') + expect(json_response).to eql('message' => 'Branch was deleted') end it { expect(response).to have_gitlab_http_status(200) } diff --git a/spec/controllers/projects/error_tracking_controller_spec.rb b/spec/controllers/projects/error_tracking_controller_spec.rb index 729e71b87a6..6464398cea1 100644 --- a/spec/controllers/projects/error_tracking_controller_spec.rb +++ b/spec/controllers/projects/error_tracking_controller_spec.rb @@ -20,18 +20,6 @@ describe Projects::ErrorTrackingController do expect(response).to render_template(:index) end - context 'with feature flag disabled' do - before do - stub_feature_flags(error_tracking: false) - end - - it 'returns 404' do - get :index, params: project_params - - expect(response).to have_gitlab_http_status(:not_found) - end - end - context 'with insufficient permissions' do before do project.add_guest(user) diff --git a/spec/controllers/projects/settings/operations_controller_spec.rb b/spec/controllers/projects/settings/operations_controller_spec.rb index 810f5bb64ba..d989ec22481 100644 --- a/spec/controllers/projects/settings/operations_controller_spec.rb +++ b/spec/controllers/projects/settings/operations_controller_spec.rb @@ -41,18 +41,6 @@ describe Projects::Settings::OperationsController do end end - context 'with feature flag disabled' do - before do - stub_feature_flags(error_tracking: false) - end - - it 'renders 404' do - get :show, params: project_params(project) - - expect(response).to have_gitlab_http_status(:not_found) - end - end - context 'with insufficient permissions' do before do project.add_reporter(user) @@ -121,18 +109,6 @@ describe Projects::Settings::OperationsController do end end - context 'with feature flag disabled' do - before do - stub_feature_flags(error_tracking: false) - end - - it 'renders 404' do - patch :update, params: project_params(project) - - expect(response).to have_gitlab_http_status(:not_found) - end - end - context 'with insufficient permissions' do before do project.add_reporter(user) diff --git a/spec/controllers/projects_controller_spec.rb b/spec/controllers/projects_controller_spec.rb index f84f069f4db..9801ed19957 100644 --- a/spec/controllers/projects_controller_spec.rb +++ b/spec/controllers/projects_controller_spec.rb @@ -955,6 +955,59 @@ describe ProjectsController do end end + describe 'GET resolve' do + shared_examples 'resolvable endpoint' do + it 'redirects to the project page' do + get :resolve, params: { id: project.id } + + expect(response).to have_gitlab_http_status(302) + expect(response).to redirect_to(project_path(project)) + end + end + + context 'with an authenticated user' do + before do + sign_in(user) + end + + context 'when user has access to the project' do + before do + project.add_developer(user) + end + + it_behaves_like 'resolvable endpoint' + end + + context 'when user has no access to the project' do + it 'gives 404 for existing project' do + get :resolve, params: { id: project.id } + + expect(response).to have_gitlab_http_status(404) + end + end + + it 'gives 404 for non-existing project' do + get :resolve, params: { id: '0' } + + expect(response).to have_gitlab_http_status(404) + end + end + + context 'non authenticated user' do + context 'with a public project' do + let(:project) { public_project } + + it_behaves_like 'resolvable endpoint' + end + + it 'gives 404 for private project' do + get :resolve, params: { id: project.id } + + expect(response).to have_gitlab_http_status(404) + end + end + end + def project_moved_message(redirect_route, project) "Project '#{redirect_route.path}' was moved to '#{project.full_path}'. Please update any links and bookmarks that may still have the old path." end diff --git a/spec/controllers/uploads_controller_spec.rb b/spec/controllers/uploads_controller_spec.rb index 19142aa1272..5fbb71eca96 100644 --- a/spec/controllers/uploads_controller_spec.rb +++ b/spec/controllers/uploads_controller_spec.rb @@ -12,6 +12,12 @@ shared_examples 'content not cached without revalidation and no-store' do end end +shared_examples 'content publicly cached' do + it 'ensures content is publicly cached' do + expect(subject['Cache-Control']).to eq('max-age=300, public') + end +end + describe UploadsController do let!(:user) { create(:user, avatar: fixture_file_upload("spec/fixtures/dk.png", "image/png")) } @@ -184,7 +190,7 @@ describe UploadsController do expect(response).to have_gitlab_http_status(200) end - it_behaves_like 'content not cached without revalidation and no-store' do + it_behaves_like 'content publicly cached' do subject do get :show, params: { model: 'user', mounted_as: 'avatar', id: user.id, filename: 'dk.png' } @@ -201,7 +207,7 @@ describe UploadsController do expect(response).to have_gitlab_http_status(200) end - it_behaves_like 'content not cached without revalidation' do + it_behaves_like 'content publicly cached' do subject do get :show, params: { model: 'user', mounted_as: 'avatar', id: user.id, filename: 'dk.png' } @@ -537,7 +543,7 @@ describe UploadsController do expect(response).to have_gitlab_http_status(200) end - it_behaves_like 'content not cached without revalidation' do + it_behaves_like 'content publicly cached' do subject do get :show, params: { model: 'appearance', mounted_as: 'header_logo', id: appearance.id, filename: 'dk.png' } @@ -557,7 +563,7 @@ describe UploadsController do expect(response).to have_gitlab_http_status(200) end - it_behaves_like 'content not cached without revalidation' do + it_behaves_like 'content publicly cached' do subject do get :show, params: { model: 'appearance', mounted_as: 'logo', id: appearance.id, filename: 'dk.png' } diff --git a/spec/factories/clusters/clusters.rb b/spec/factories/clusters/clusters.rb index 3e2c0df8afb..a2e5f4862db 100644 --- a/spec/factories/clusters/clusters.rb +++ b/spec/factories/clusters/clusters.rb @@ -59,5 +59,9 @@ FactoryBot.define do trait :with_installed_helm do application_helm factory: %i(clusters_applications_helm installed) end + + trait :with_domain do + domain 'example.com' + end end end diff --git a/spec/features/groups_spec.rb b/spec/features/groups_spec.rb index d01fc04311a..00d81b26ce2 100644 --- a/spec/features/groups_spec.rb +++ b/spec/features/groups_spec.rb @@ -154,7 +154,7 @@ describe 'Group' do end describe 'group edit', :js do - let(:group) { create(:group) } + let(:group) { create(:group, :public) } let(:path) { edit_group_path(group) } let(:new_name) { 'new-name' } @@ -163,6 +163,8 @@ describe 'Group' do end it_behaves_like 'dirty submit form', [{ form: '.js-general-settings-form', input: 'input[name="group[name]"]' }, + { form: '.js-general-settings-form', input: '#group_visibility_level_0' }, + { form: '.js-general-permissions-form', input: '#group_request_access_enabled' }, { form: '.js-general-permissions-form', input: 'input[name="group[two_factor_grace_period]"]' }] it 'saves new settings' do diff --git a/spec/features/issuables/markdown_references/internal_references_spec.rb b/spec/features/issuables/markdown_references/internal_references_spec.rb index 9613e22bf24..23385ba65fc 100644 --- a/spec/features/issuables/markdown_references/internal_references_spec.rb +++ b/spec/features/issuables/markdown_references/internal_references_spec.rb @@ -64,11 +64,13 @@ describe "Internal references", :js do it "shows references" do page.within("#merge-requests .merge-requests-title") do - expect(page).to have_content("1 Related Merge Request") + expect(page).to have_content("Related merge requests") + expect(page).to have_css(".mr-count-badge") end page.within("#merge-requests ul") do expect(page).to have_content(private_project_merge_request.title) + expect(page).to have_css(".merge-request-status") end expect(page).to have_content("mentioned in merge request #{private_project_merge_request.to_reference(public_project)}") diff --git a/spec/features/issues/user_creates_branch_and_merge_request_spec.rb b/spec/features/issues/user_creates_branch_and_merge_request_spec.rb index 32bc851f00f..693ad89069c 100644 --- a/spec/features/issues/user_creates_branch_and_merge_request_spec.rb +++ b/spec/features/issues/user_creates_branch_and_merge_request_spec.rb @@ -141,7 +141,7 @@ describe 'User creates branch and merge request on issue page', :js do it 'disables the create branch button' do expect(page).to have_css('.create-mr-dropdown-wrap .unavailable:not(.hidden)') expect(page).to have_css('.create-mr-dropdown-wrap .available.hidden', visible: false) - expect(page).to have_content /1 Related Merge Request/ + expect(page).to have_content /Related merge requests/ end end diff --git a/spec/features/markdown/math_spec.rb b/spec/features/markdown/math_spec.rb index 6a23d6b78ab..678ce80b382 100644 --- a/spec/features/markdown/math_spec.rb +++ b/spec/features/markdown/math_spec.rb @@ -16,7 +16,7 @@ describe 'Math rendering', :js do visit project_issue_path(project, issue) - expect(page).to have_selector('.katex .mord.mathit', text: 'b') - expect(page).to have_selector('.katex-display .mord.mathit', text: 'b') + expect(page).to have_selector('.katex .mord.mathdefault', text: 'b') + expect(page).to have_selector('.katex-display .mord.mathdefault', text: 'b') end end diff --git a/spec/features/merge_request/user_accepts_merge_request_spec.rb b/spec/features/merge_request/user_accepts_merge_request_spec.rb index 01aeed93947..00ac7c72a11 100644 --- a/spec/features/merge_request/user_accepts_merge_request_spec.rb +++ b/spec/features/merge_request/user_accepts_merge_request_spec.rb @@ -25,7 +25,7 @@ describe 'User accepts a merge request', :js do end it 'accepts a merge request' do - check('Remove source branch') + check('Delete source branch') click_button('Merge') expect(page).to have_content('The changes were merged into') @@ -60,7 +60,7 @@ describe 'User accepts a merge request', :js do end it 'accepts a merge request' do - check('Remove source branch') + check('Delete source branch') click_button('Merge') expect(page).to have_content('The changes were merged into') diff --git a/spec/features/merge_request/user_merges_when_pipeline_succeeds_spec.rb b/spec/features/merge_request/user_merges_when_pipeline_succeeds_spec.rb index 29b3d2b629b..6e54aa6006b 100644 --- a/spec/features/merge_request/user_merges_when_pipeline_succeeds_spec.rb +++ b/spec/features/merge_request/user_merges_when_pipeline_succeeds_spec.rb @@ -33,7 +33,7 @@ describe 'Merge request > User merges when pipeline succeeds', :js do click_button "Merge when pipeline succeeds" expect(page).to have_content "Set by #{user.name} to be merged automatically when the pipeline succeeds" - expect(page).to have_content "The source branch will not be removed" + expect(page).to have_content "The source branch will not be deleted" expect(page).to have_selector ".js-cancel-auto-merge" visit project_merge_request_path(project, merge_request) # Needed to refresh the page expect(page).to have_content /enabled an automatic merge when the pipeline for \h{8} succeeds/i @@ -94,7 +94,7 @@ describe 'Merge request > User merges when pipeline succeeds', :js do click_link 'Merge when pipeline succeeds' expect(page).to have_content "Set by #{user.name} to be merged automatically when the pipeline succeeds" - expect(page).to have_content "The source branch will not be removed" + expect(page).to have_content "The source branch will not be deleted" expect(page).to have_link "Cancel automatic merge" end end @@ -127,10 +127,10 @@ describe 'Merge request > User merges when pipeline succeeds', :js do expect(page).to have_content "canceled the automatic merge" end - it 'allows to remove source branch' do - click_link "Remove source branch" + it 'allows to delete source branch' do + click_link "Delete source branch" - expect(page).to have_content "The source branch will be removed" + expect(page).to have_content "The source branch will be deleted" end context 'when pipeline succeeds' do diff --git a/spec/features/merge_request/user_sees_merge_widget_spec.rb b/spec/features/merge_request/user_sees_merge_widget_spec.rb index d8ebd3c92af..afb978d7c45 100644 --- a/spec/features/merge_request/user_sees_merge_widget_spec.rb +++ b/spec/features/merge_request/user_sees_merge_widget_spec.rb @@ -316,7 +316,7 @@ describe 'Merge request > User sees merge widget', :js do it 'user cannot remove source branch' do expect(page).not_to have_field('remove-source-branch-input') - expect(page).to have_content('Removes source branch') + expect(page).to have_content('Deletes source branch') end end diff --git a/spec/features/merge_requests/user_squashes_merge_request_spec.rb b/spec/features/merge_requests/user_squashes_merge_request_spec.rb index ec1153b7f7f..47f9f10815c 100644 --- a/spec/features/merge_requests/user_squashes_merge_request_spec.rb +++ b/spec/features/merge_requests/user_squashes_merge_request_spec.rb @@ -38,7 +38,7 @@ describe 'User squashes a merge request', :js do def accept_mr expect(page).to have_button('Merge') - uncheck 'Remove source branch' + uncheck 'Delete source branch' click_on 'Merge' end diff --git a/spec/features/projects/labels/update_prioritization_spec.rb b/spec/features/projects/labels/update_prioritization_spec.rb index 055a0c83a11..d36f043f880 100644 --- a/spec/features/projects/labels/update_prioritization_spec.rb +++ b/spec/features/projects/labels/update_prioritization_spec.rb @@ -125,7 +125,7 @@ describe 'Prioritize labels' do wait_for_requests end - page.within('.breadcrumbs-container') do + page.within('.top-area') do expect(page).to have_link('New label') end end diff --git a/spec/features/projects/settings/operations_settings_spec.rb b/spec/features/projects/settings/operations_settings_spec.rb index 1f2328a6dd8..06290c67c70 100644 --- a/spec/features/projects/settings/operations_settings_spec.rb +++ b/spec/features/projects/settings/operations_settings_spec.rb @@ -8,32 +8,16 @@ describe 'Projects > Settings > For a forked project', :js do let(:role) { :maintainer } before do - stub_feature_flags(error_tracking: true) sign_in(user) project.add_role(user, role) end describe 'Sidebar > Operations' do - context 'when sidebar feature flag enabled' do - it 'renders the settings link in the sidebar' do - visit project_path(project) - wait_for_requests + it 'renders the settings link in the sidebar' do + visit project_path(project) + wait_for_requests - expect(page).to have_selector('a[title="Operations"]', visible: false) - end - end - - context 'when sidebar feature flag disabled' do - before do - stub_feature_flags(error_tracking: false) - end - - it 'does not render the settings link in the sidebar' do - visit project_path(project) - wait_for_requests - - expect(page).not_to have_selector('a[title="Operations"]', visible: false) - end + expect(page).to have_selector('a[title="Operations"]', visible: false) end end end diff --git a/spec/features/protected_branches_spec.rb b/spec/features/protected_branches_spec.rb index 63c38a25f4b..0aff916ec83 100644 --- a/spec/features/protected_branches_spec.rb +++ b/spec/features/protected_branches_spec.rb @@ -97,7 +97,7 @@ describe 'Protected Branches', :js do set_protected_branch_name('some-branch') click_on "Protect" - within(".protected-branches-list") { expect(page).to have_content('branch was removed') } + within(".protected-branches-list") { expect(page).to have_content('branch was deleted') } end end diff --git a/spec/helpers/application_helper_spec.rb b/spec/helpers/application_helper_spec.rb index 4135f31e051..b81249a1e29 100644 --- a/spec/helpers/application_helper_spec.rb +++ b/spec/helpers/application_helper_spec.rb @@ -168,6 +168,21 @@ describe ApplicationHelper do end end + describe '#client_class_list' do + it 'returns string containing CSS classes representing client browser and platform' do + class_list = helper.client_class_list + expect(class_list).to eq('gl-browser-generic gl-platform-other') + end + end + + describe '#client_js_flags' do + it 'returns map containing JS flags representing client browser and platform' do + flags_list = helper.client_js_flags + expect(flags_list[:isGeneric]).to eq(true) + expect(flags_list[:isOther]).to eq(true) + end + end + describe '#autocomplete_data_sources' do let(:project) { create(:project) } let(:noteable_type) { Issue } diff --git a/spec/javascripts/diffs/components/compare_versions_spec.js b/spec/javascripts/diffs/components/compare_versions_spec.js index 50135b0cf86..a976c6b837f 100644 --- a/spec/javascripts/diffs/components/compare_versions_spec.js +++ b/spec/javascripts/diffs/components/compare_versions_spec.js @@ -22,10 +22,10 @@ describe('CompareVersions', () => { const treeListBtn = vm.$el.querySelector('.js-toggle-tree-list'); expect(treeListBtn).not.toBeNull(); - expect(treeListBtn.dataset.originalTitle).toBe('Toggle file browser'); + expect(treeListBtn.dataset.originalTitle).toBe('Hide file browser'); expect(treeListBtn.querySelectorAll('svg use').length).not.toBe(0); expect(treeListBtn.querySelector('svg use').getAttribute('xlink:href')).toContain( - '#hamburger', + '#collapse-left', ); }); diff --git a/spec/javascripts/diffs/store/utils_spec.js b/spec/javascripts/diffs/store/utils_spec.js index ea86844ddca..3641946518b 100644 --- a/spec/javascripts/diffs/store/utils_spec.js +++ b/spec/javascripts/diffs/store/utils_spec.js @@ -251,45 +251,40 @@ describe('DiffsStoreUtils', () => { describe('trimFirstCharOfLineContent', () => { it('trims the line when it starts with a space', () => { expect(utils.trimFirstCharOfLineContent({ rich_text: ' diff' })).toEqual({ - discussions: [], rich_text: 'diff', }); }); it('trims the line when it starts with a +', () => { expect(utils.trimFirstCharOfLineContent({ rich_text: '+diff' })).toEqual({ - discussions: [], rich_text: 'diff', }); }); it('trims the line when it starts with a -', () => { expect(utils.trimFirstCharOfLineContent({ rich_text: '-diff' })).toEqual({ - discussions: [], rich_text: 'diff', }); }); it('does not trims the line when it starts with a letter', () => { expect(utils.trimFirstCharOfLineContent({ rich_text: 'diff' })).toEqual({ - discussions: [], rich_text: 'diff', }); }); it('does not modify the provided object', () => { const lineObj = { - discussions: [], rich_text: ' diff', }; utils.trimFirstCharOfLineContent(lineObj); - expect(lineObj).toEqual({ discussions: [], rich_text: ' diff' }); + expect(lineObj).toEqual({ rich_text: ' diff' }); }); it('handles a undefined or null parameter', () => { - expect(utils.trimFirstCharOfLineContent()).toEqual({ discussions: [] }); + expect(utils.trimFirstCharOfLineContent()).toEqual({}); }); }); diff --git a/spec/javascripts/dirty_submit/dirty_submit_collection_spec.js b/spec/javascripts/dirty_submit/dirty_submit_collection_spec.js index 08ffc44605f..47be0b3ce9d 100644 --- a/spec/javascripts/dirty_submit/dirty_submit_collection_spec.js +++ b/spec/javascripts/dirty_submit/dirty_submit_collection_spec.js @@ -1,5 +1,5 @@ import DirtySubmitCollection from '~/dirty_submit/dirty_submit_collection'; -import { setInput, createForm } from './helper'; +import { setInputValue, createForm } from './helper'; describe('DirtySubmitCollection', () => { it('disables submits until there are changes', done => { @@ -14,11 +14,11 @@ describe('DirtySubmitCollection', () => { expect(submit.disabled).toBe(true); - return setInput(input, `${originalValue} changes`) + return setInputValue(input, `${originalValue} changes`) .then(() => { expect(submit.disabled).toBe(false); }) - .then(() => setInput(input, originalValue)) + .then(() => setInputValue(input, originalValue)) .then(() => { expect(submit.disabled).toBe(true); }) diff --git a/spec/javascripts/dirty_submit/dirty_submit_form_spec.js b/spec/javascripts/dirty_submit/dirty_submit_form_spec.js index 093fec97951..ae2a785de52 100644 --- a/spec/javascripts/dirty_submit/dirty_submit_form_spec.js +++ b/spec/javascripts/dirty_submit/dirty_submit_form_spec.js @@ -1,14 +1,14 @@ import DirtySubmitForm from '~/dirty_submit/dirty_submit_form'; -import { setInput, createForm } from './helper'; +import { getInputValue, setInputValue, createForm } from './helper'; function expectToToggleDisableOnDirtyUpdate(submit, input) { - const originalValue = input.value; + const originalValue = getInputValue(input); expect(submit.disabled).toBe(true); - return setInput(input, `${originalValue} changes`) + return setInputValue(input, `${originalValue} changes`) .then(() => expect(submit.disabled).toBe(false)) - .then(() => setInput(input, originalValue)) + .then(() => setInputValue(input, originalValue)) .then(() => expect(submit.disabled).toBe(true)); } @@ -33,4 +33,24 @@ describe('DirtySubmitForm', () => { .then(done) .catch(done.fail); }); + + it('disables submit until there are changes for radio inputs', done => { + const { form, input, submit } = createForm('radio'); + + new DirtySubmitForm(form); // eslint-disable-line no-new + + return expectToToggleDisableOnDirtyUpdate(submit, input) + .then(done) + .catch(done.fail); + }); + + it('disables submit until there are changes for checkbox inputs', done => { + const { form, input, submit } = createForm('checkbox'); + + new DirtySubmitForm(form); // eslint-disable-line no-new + + return expectToToggleDisableOnDirtyUpdate(submit, input) + .then(done) + .catch(done.fail); + }); }); diff --git a/spec/javascripts/dirty_submit/helper.js b/spec/javascripts/dirty_submit/helper.js index 6d1e643553c..b51783cb915 100644 --- a/spec/javascripts/dirty_submit/helper.js +++ b/spec/javascripts/dirty_submit/helper.js @@ -1,25 +1,42 @@ import DirtySubmitForm from '~/dirty_submit/dirty_submit_form'; import setTimeoutPromiseHelper from '../helpers/set_timeout_promise_helper'; -export function setInput(element, value) { - element.value = value; +function isCheckableType(type) { + return /^(radio|checkbox)$/.test(type); +} + +export function setInputValue(element, value) { + const { type } = element; + let eventType; + + if (isCheckableType(type)) { + element.checked = !element.checked; + eventType = 'change'; + } else { + element.value = value; + eventType = 'input'; + } element.dispatchEvent( - new Event('input', { + new Event(eventType, { bubbles: true, - cancelable: true, }), ); return setTimeoutPromiseHelper(DirtySubmitForm.THROTTLE_DURATION); } -export function createForm() { +export function getInputValue(input) { + return isCheckableType(input.type) ? input.checked : input.value; +} + +export function createForm(type = 'text') { const form = document.createElement('form'); form.innerHTML = ` - <input type="text" value="original" class="js-input" name="input" /> + <input type="${type}" name="${type}" class="js-input"/> <button type="submit" class="js-dirty-submit"></button> `; + const input = form.querySelector('.js-input'); const submit = form.querySelector('.js-dirty-submit'); diff --git a/spec/javascripts/lib/utils/common_utils_spec.js b/spec/javascripts/lib/utils/common_utils_spec.js index 0dc7e93539a..121c4040212 100644 --- a/spec/javascripts/lib/utils/common_utils_spec.js +++ b/spec/javascripts/lib/utils/common_utils_spec.js @@ -3,6 +3,25 @@ import * as commonUtils from '~/lib/utils/common_utils'; import MockAdapter from 'axios-mock-adapter'; import { faviconDataUrl, overlayDataUrl, faviconWithOverlayDataUrl } from './mock_data'; +const PIXEL_TOLERANCE = 0.2; + +/** + * Loads a data URL as the src of an + * {@link https://developer.mozilla.org/en-US/docs/Web/API/HTMLImageElement/Image|Image} + * and resolves to that Image once loaded. + * + * @param url + * @returns {Promise} + */ +const urlToImage = url => + new Promise(resolve => { + const img = new Image(); + img.onload = function() { + resolve(img); + }; + img.src = url; + }); + describe('common_utils', () => { describe('parseUrl', () => { it('returns an anchor tag with url', () => { @@ -513,8 +532,9 @@ describe('common_utils', () => { it('should return the favicon with the overlay', done => { commonUtils .createOverlayIcon(faviconDataUrl, overlayDataUrl) - .then(url => { - expect(url).toEqual(faviconWithOverlayDataUrl); + .then(url => Promise.all([urlToImage(url), urlToImage(faviconWithOverlayDataUrl)])) + .then(([actual, expected]) => { + expect(actual).toImageDiffEqual(expected, PIXEL_TOLERANCE); done(); }) .catch(done.fail); @@ -536,10 +556,10 @@ describe('common_utils', () => { it('should set page favicon to provided favicon overlay', done => { commonUtils .setFaviconOverlay(overlayDataUrl) - .then(() => { - expect(document.getElementById('favicon').getAttribute('href')).toEqual( - faviconWithOverlayDataUrl, - ); + .then(() => document.getElementById('favicon').getAttribute('href')) + .then(url => Promise.all([urlToImage(url), urlToImage(faviconWithOverlayDataUrl)])) + .then(([actual, expected]) => { + expect(actual).toImageDiffEqual(expected, PIXEL_TOLERANCE); done(); }) .catch(done.fail); @@ -582,10 +602,10 @@ describe('common_utils', () => { commonUtils .setCiStatusFavicon(BUILD_URL) - .then(() => { - const favicon = document.getElementById('favicon'); - - expect(favicon.getAttribute('href')).toEqual(faviconWithOverlayDataUrl); + .then(() => document.getElementById('favicon').getAttribute('href')) + .then(url => Promise.all([urlToImage(url), urlToImage(faviconWithOverlayDataUrl)])) + .then(([actual, expected]) => { + expect(actual).toImageDiffEqual(expected, PIXEL_TOLERANCE); done(); }) .catch(done.fail); diff --git a/spec/javascripts/matchers.js b/spec/javascripts/matchers.js index 0d465510fd3..406527b08a3 100644 --- a/spec/javascripts/matchers.js +++ b/spec/javascripts/matchers.js @@ -1,3 +1,5 @@ +import pixelmatch from 'pixelmatch'; + export default { toContainText: () => ({ compare(vm, text) { @@ -54,4 +56,41 @@ export default { return result; }, }), + toImageDiffEqual: () => { + const getImageData = img => { + const canvas = document.createElement('canvas'); + canvas.width = img.width; + canvas.height = img.height; + canvas.getContext('2d').drawImage(img, 0, 0); + return canvas.getContext('2d').getImageData(0, 0, img.width, img.height).data; + }; + + return { + compare(actual, expected, threshold = 0.1) { + if (actual.height !== expected.height || actual.width !== expected.width) { + return { + pass: false, + message: `Expected image dimensions (h x w) of ${expected.height}x${expected.width}. + Received an image with ${actual.height}x${actual.width}`, + }; + } + + const { width, height } = actual; + const differentPixels = pixelmatch( + getImageData(actual), + getImageData(expected), + null, + width, + height, + { threshold }, + ); + + return { + pass: differentPixels < 20, + message: `${differentPixels} pixels differ more than ${threshold * + 100} percent between input and output.`, + }; + }, + }; + }, }; diff --git a/spec/javascripts/notes/stores/mutation_spec.js b/spec/javascripts/notes/stores/mutation_spec.js index 3fbae82f16c..b6b2c7d60a5 100644 --- a/spec/javascripts/notes/stores/mutation_spec.js +++ b/spec/javascripts/notes/stores/mutation_spec.js @@ -179,11 +179,11 @@ describe('Notes Store mutations', () => { diff_file: { file_hash: 'a', }, - truncated_diff_lines: ['a'], + truncated_diff_lines: [{ text: '+a', rich_text: '+<span>a</span>' }], }, ]); - expect(state.discussions[0].truncated_diff_lines).toEqual(['a']); + expect(state.discussions[0].truncated_diff_lines).toEqual([{ rich_text: '<span>a</span>' }]); }); it('adds empty truncated_diff_lines when not in discussion', () => { @@ -420,9 +420,12 @@ describe('Notes Store mutations', () => { ], }; - mutations.SET_DISCUSSION_DIFF_LINES(state, { discussionId: 1, diffLines: ['test'] }); + mutations.SET_DISCUSSION_DIFF_LINES(state, { + discussionId: 1, + diffLines: [{ text: '+a', rich_text: '+<span>a</span>' }], + }); - expect(state.discussions[0].truncated_diff_lines).toEqual(['test']); + expect(state.discussions[0].truncated_diff_lines).toEqual([{ rich_text: '<span>a</span>' }]); }); it('keeps reactivity of discussion', () => { @@ -435,7 +438,10 @@ describe('Notes Store mutations', () => { ]); const discussion = state.discussions[0]; - mutations.SET_DISCUSSION_DIFF_LINES(state, { discussionId: 1, diffLines: ['test'] }); + mutations.SET_DISCUSSION_DIFF_LINES(state, { + discussionId: 1, + diffLines: [{ rich_text: '<span>a</span>' }], + }); discussion.expanded = true; diff --git a/spec/javascripts/test_bundle.js b/spec/javascripts/test_bundle.js index 96c0844f83c..547379dabed 100644 --- a/spec/javascripts/test_bundle.js +++ b/spec/javascripts/test_bundle.js @@ -187,6 +187,7 @@ if (process.env.BABEL_ENV === 'coverage') { './terminal/terminal_bundle.js', './users/users_bundle.js', './issue_show/index.js', + './pages/admin/application_settings/show/index.js', ]; describe('Uncovered files', function() { diff --git a/spec/javascripts/vue_mr_widget/components/states/mr_widget_merge_when_pipeline_succeeds_spec.js b/spec/javascripts/vue_mr_widget/components/states/mr_widget_merge_when_pipeline_succeeds_spec.js index d46ad0acc9b..b9718a78fa4 100644 --- a/spec/javascripts/vue_mr_widget/components/states/mr_widget_merge_when_pipeline_succeeds_spec.js +++ b/spec/javascripts/vue_mr_widget/components/states/mr_widget_merge_when_pipeline_succeeds_spec.js @@ -121,14 +121,14 @@ describe('MRWidgetMergeWhenPipelineSucceeds', () => { expect(vm.$el.innerText).toContain('to be merged automatically when the pipeline succeeds'); expect(vm.$el.innerText).toContain('The changes will be merged into'); expect(vm.$el.innerText).toContain(targetBranch); - expect(vm.$el.innerText).toContain('The source branch will not be removed'); + expect(vm.$el.innerText).toContain('The source branch will not be deleted'); expect(vm.$el.querySelector('.js-cancel-auto-merge').innerText).toContain( 'Cancel automatic merge', ); expect(vm.$el.querySelector('.js-cancel-auto-merge').getAttribute('disabled')).toBeFalsy(); expect(vm.$el.querySelector('.js-remove-source-branch').innerText).toContain( - 'Remove source branch', + 'Delete source branch', ); expect(vm.$el.querySelector('.js-remove-source-branch').getAttribute('disabled')).toBeFalsy(); @@ -143,19 +143,19 @@ describe('MRWidgetMergeWhenPipelineSucceeds', () => { }); }); - it('should show source branch will be removed text when it source branch set to remove', done => { + it('should show source branch will be deleted text when it source branch set to remove', done => { vm.mr.shouldRemoveSourceBranch = true; Vue.nextTick(() => { const normalizedText = vm.$el.innerText.replace(/\s+/g, ' '); - expect(normalizedText).toContain('The source branch will be removed'); - expect(normalizedText).not.toContain('The source branch will not be removed'); + expect(normalizedText).toContain('The source branch will be deleted'); + expect(normalizedText).not.toContain('The source branch will not be deleted'); done(); }); }); - it('should not show remove source branch button when user not able to remove source branch', done => { + it('should not show delete source branch button when user not able to delete source branch', done => { vm.mr.currentUserId = 4; Vue.nextTick(() => { @@ -164,7 +164,7 @@ describe('MRWidgetMergeWhenPipelineSucceeds', () => { }); }); - it('should disable remove source branch button when the action is in progress', done => { + it('should disable delete source branch button when the action is in progress', done => { vm.isRemovingSourceBranch = true; Vue.nextTick(() => { diff --git a/spec/javascripts/vue_mr_widget/components/states/mr_widget_merged_spec.js b/spec/javascripts/vue_mr_widget/components/states/mr_widget_merged_spec.js index da5cb752c6f..1683da805b9 100644 --- a/spec/javascripts/vue_mr_widget/components/states/mr_widget_merged_spec.js +++ b/spec/javascripts/vue_mr_widget/components/states/mr_widget_merged_spec.js @@ -128,7 +128,7 @@ describe('MRWidgetMerged', () => { new Promise(resolve => { resolve({ data: { - message: 'Branch was removed', + message: 'Branch was deleted', }, }); }), @@ -157,8 +157,8 @@ describe('MRWidgetMerged', () => { expect(vm.$el.textContent).toContain(targetBranch); }); - it('renders information about branch being removed', () => { - expect(vm.$el.textContent).toContain('The source branch has been removed'); + it('renders information about branch being deleted', () => { + expect(vm.$el.textContent).toContain('The source branch has been deleted'); }); it('shows revert and cherry-pick buttons', () => { @@ -189,24 +189,24 @@ describe('MRWidgetMerged', () => { expect(selectors.mergeCommitShaLink.href).toBe(vm.mr.mergeCommitPath); }); - it('should not show source branch removed text', done => { + it('should not show source branch deleted text', done => { vm.mr.sourceBranchRemoved = false; Vue.nextTick(() => { - expect(vm.$el.innerText).toContain('You can remove source branch now'); - expect(vm.$el.innerText).not.toContain('The source branch has been removed'); + expect(vm.$el.innerText).toContain('You can delete the source branch now'); + expect(vm.$el.innerText).not.toContain('The source branch has been deleted'); done(); }); }); - it('should show source branch removing text', done => { + it('should show source branch deleting text', done => { vm.mr.isRemovingSourceBranch = true; vm.mr.sourceBranchRemoved = false; Vue.nextTick(() => { - expect(vm.$el.innerText).toContain('The source branch is being removed'); - expect(vm.$el.innerText).not.toContain('You can remove source branch now'); - expect(vm.$el.innerText).not.toContain('The source branch has been removed'); + expect(vm.$el.innerText).toContain('The source branch is being deleted'); + expect(vm.$el.innerText).not.toContain('You can delete the source branch now'); + expect(vm.$el.innerText).not.toContain('The source branch has been deleted'); done(); }); }); diff --git a/spec/javascripts/vue_mr_widget/mr_widget_options_spec.js b/spec/javascripts/vue_mr_widget/mr_widget_options_spec.js index 99b80df766a..34b90c23c19 100644 --- a/spec/javascripts/vue_mr_widget/mr_widget_options_spec.js +++ b/spec/javascripts/vue_mr_widget/mr_widget_options_spec.js @@ -453,7 +453,7 @@ describe('mrWidgetOptions', () => { vm.$nextTick(() => { const tooltip = vm.$el.querySelector('.fa-question-circle'); - expect(vm.$el.textContent).toContain('Removes source branch'); + expect(vm.$el.textContent).toContain('Deletes source branch'); expect(tooltip.getAttribute('data-original-title')).toBe( 'A user with write access to the source branch selected this option', ); @@ -468,8 +468,8 @@ describe('mrWidgetOptions', () => { vm.mr.state = 'merged'; vm.$nextTick(() => { - expect(vm.$el.textContent).toContain('The source branch has been removed'); - expect(vm.$el.textContent).not.toContain('Removes source branch'); + expect(vm.$el.textContent).toContain('The source branch has been deleted'); + expect(vm.$el.textContent).not.toContain('Deletes source branch'); done(); }); diff --git a/spec/lib/api/helpers/pagination_spec.rb b/spec/lib/api/helpers/pagination_spec.rb index 0a7682d906b..2890aa4ae38 100644 --- a/spec/lib/api/helpers/pagination_spec.rb +++ b/spec/lib/api/helpers/pagination_spec.rb @@ -237,26 +237,89 @@ describe API::Helpers::Pagination do .and_return({ page: 1, per_page: 2 }) end - it 'returns appropriate amount of resources' do - expect(subject.paginate(resource).count).to eq 2 + shared_examples 'response with pagination headers' do + it 'adds appropriate headers' do + expect_header('X-Total', '3') + expect_header('X-Total-Pages', '2') + expect_header('X-Per-Page', '2') + expect_header('X-Page', '1') + expect_header('X-Next-Page', '2') + expect_header('X-Prev-Page', '') + + expect_header('Link', anything) do |_key, val| + expect(val).to include('rel="first"') + expect(val).to include('rel="last"') + expect(val).to include('rel="next"') + expect(val).not_to include('rel="prev"') + end + + subject.paginate(resource) + end end - it 'adds appropriate headers' do - expect_header('X-Total', '3') - expect_header('X-Total-Pages', '2') - expect_header('X-Per-Page', '2') - expect_header('X-Page', '1') - expect_header('X-Next-Page', '2') - expect_header('X-Prev-Page', '') + shared_examples 'paginated response' do + it 'returns appropriate amount of resources' do + expect(subject.paginate(resource).count).to eq 2 + end - expect_header('Link', anything) do |_key, val| - expect(val).to include('rel="first"') - expect(val).to include('rel="last"') - expect(val).to include('rel="next"') - expect(val).not_to include('rel="prev"') + it 'executes only one SELECT COUNT query' do + expect { subject.paginate(resource) }.to make_queries_matching(/SELECT COUNT/, 1) end + end - subject.paginate(resource) + context 'when the api_kaminari_count_with_limit feature flag is unset' do + it_behaves_like 'paginated response' + it_behaves_like 'response with pagination headers' + end + + context 'when the api_kaminari_count_with_limit feature flag is disabled' do + before do + stub_feature_flags(api_kaminari_count_with_limit: false) + end + + it_behaves_like 'paginated response' + it_behaves_like 'response with pagination headers' + end + + context 'when the api_kaminari_count_with_limit feature flag is enabled' do + before do + stub_feature_flags(api_kaminari_count_with_limit: true) + end + + context 'when resources count is less than MAX_COUNT_LIMIT' do + before do + stub_const("::Kaminari::ActiveRecordRelationMethods::MAX_COUNT_LIMIT", 4) + end + + it_behaves_like 'paginated response' + it_behaves_like 'response with pagination headers' + end + + context 'when resources count is more than MAX_COUNT_LIMIT' do + before do + stub_const("::Kaminari::ActiveRecordRelationMethods::MAX_COUNT_LIMIT", 2) + end + + it_behaves_like 'paginated response' + + it 'does not return the X-Total and X-Total-Pages headers' do + expect_no_header('X-Total') + expect_no_header('X-Total-Pages') + expect_header('X-Per-Page', '2') + expect_header('X-Page', '1') + expect_header('X-Next-Page', '2') + expect_header('X-Prev-Page', '') + + expect_header('Link', anything) do |_key, val| + expect(val).to include('rel="first"') + expect(val).not_to include('rel="last"') + expect(val).to include('rel="next"') + expect(val).not_to include('rel="prev"') + end + + subject.paginate(resource) + end + end end end @@ -348,6 +411,10 @@ describe API::Helpers::Pagination do expect(subject).to receive(:header).with(*args, &block) end + def expect_no_header(*args, &block) + expect(subject).not_to receive(:header).with(*args) + end + def expect_message(method) expect(subject).to receive(method) .at_least(:once).and_return(value) diff --git a/spec/lib/gitlab/background_migration/populate_cluster_kubernetes_namespace_table_spec.rb b/spec/lib/gitlab/background_migration/populate_cluster_kubernetes_namespace_table_spec.rb index 8e3cb36d313..812e0cc6947 100644 --- a/spec/lib/gitlab/background_migration/populate_cluster_kubernetes_namespace_table_spec.rb +++ b/spec/lib/gitlab/background_migration/populate_cluster_kubernetes_namespace_table_spec.rb @@ -2,92 +2,88 @@ require 'spec_helper' -# rubocop:disable RSpec/FactoriesInMigrationSpecs describe Gitlab::BackgroundMigration::PopulateClusterKubernetesNamespaceTable, :migration, schema: 20181022173835 do + include MigrationHelpers::ClusterHelpers + let(:migration) { described_class.new } - let(:clusters) { create_list(:cluster, 10, :project, :provided_by_gcp) } + let(:clusters_table) { table(:clusters) } + let(:cluster_projects_table) { table(:cluster_projects) } + let(:cluster_kubernetes_namespaces_table) { table(:clusters_kubernetes_namespaces) } + let(:projects_table) { table(:projects) } + let(:namespaces_table) { table(:namespaces) } + let(:provider_gcp_table) { table(:cluster_providers_gcp) } + let(:platform_kubernetes_table) { table(:cluster_platforms_kubernetes) } before do - clusters + create_cluster_project_list(10) end shared_examples 'consistent kubernetes namespace attributes' do it 'should populate namespace and service account information' do - subject + migration.perform clusters_with_namespace.each do |cluster| - project = cluster.project - cluster_project = cluster.cluster_projects.first + cluster_project = cluster_projects_table.find_by(cluster_id: cluster.id) + project = projects_table.find(cluster_project.project_id) + kubernetes_namespace = cluster_kubernetes_namespaces_table.find_by(cluster_id: cluster.id) namespace = "#{project.path}-#{project.id}" - kubernetes_namespace = cluster.reload.kubernetes_namespace expect(kubernetes_namespace).to be_present - expect(kubernetes_namespace.cluster_project).to eq(cluster_project) - expect(kubernetes_namespace.project).to eq(cluster_project.project) - expect(kubernetes_namespace.cluster).to eq(cluster_project.cluster) + expect(kubernetes_namespace.cluster_project_id).to eq(cluster_project.id) + expect(kubernetes_namespace.project_id).to eq(cluster_project.project_id) + expect(kubernetes_namespace.cluster_id).to eq(cluster_project.cluster_id) expect(kubernetes_namespace.namespace).to eq(namespace) expect(kubernetes_namespace.service_account_name).to eq("#{namespace}-service-account") end end end - subject { migration.perform } - context 'when no Clusters::Project has a Clusters::KubernetesNamespace' do - let(:cluster_projects) { Clusters::Project.all } + let(:cluster_projects) { cluster_projects_table.all } it 'should create a Clusters::KubernetesNamespace per Clusters::Project' do expect do - subject - end.to change(Clusters::KubernetesNamespace, :count).by(cluster_projects.count) + migration.perform + end.to change(Clusters::KubernetesNamespace, :count).by(cluster_projects_table.count) end it_behaves_like 'consistent kubernetes namespace attributes' do - let(:clusters_with_namespace) { clusters } + let(:clusters_with_namespace) { clusters_table.all } end end context 'when every Clusters::Project has Clusters::KubernetesNamespace' do before do - clusters.each do |cluster| - create(:cluster_kubernetes_namespace, - cluster_project: cluster.cluster_projects.first, - cluster: cluster, - project: cluster.project) - end + create_kubernetes_namespace(clusters_table.all) end it 'should not create any Clusters::KubernetesNamespace' do expect do - subject + migration.perform end.not_to change(Clusters::KubernetesNamespace, :count) end end context 'when only some Clusters::Project have Clusters::KubernetesNamespace related' do - let(:with_kubernetes_namespace) { clusters.first(6) } - let(:with_no_kubernetes_namespace) { clusters.last(4) } + let(:with_kubernetes_namespace) { clusters_table.first(6) } + let(:with_no_kubernetes_namespace) { clusters_table.last(4) } before do - with_kubernetes_namespace.each do |cluster| - create(:cluster_kubernetes_namespace, - cluster_project: cluster.cluster_projects.first, - cluster: cluster, - project: cluster.project) - end + create_kubernetes_namespace(with_kubernetes_namespace) end it 'creates limited number of Clusters::KubernetesNamespace' do expect do - subject + migration.perform end.to change(Clusters::KubernetesNamespace, :count).by(with_no_kubernetes_namespace.count) end it 'should not modify clusters with Clusters::KubernetesNamespace' do - subject + migration.perform with_kubernetes_namespace.each do |cluster| - expect(cluster.kubernetes_namespaces.count).to eq(1) + kubernetes_namespace = cluster_kubernetes_namespaces_table.where(cluster_id: cluster.id) + expect(kubernetes_namespace.count).to eq(1) end end @@ -96,4 +92,3 @@ describe Gitlab::BackgroundMigration::PopulateClusterKubernetesNamespaceTable, : end end end -# rubocop:enable RSpec/FactoriesInMigrationSpecs diff --git a/spec/lib/gitlab/ci/pipeline/chain/populate_spec.rb b/spec/lib/gitlab/ci/pipeline/chain/populate_spec.rb index 1b014ecfaa4..3459939267a 100644 --- a/spec/lib/gitlab/ci/pipeline/chain/populate_spec.rb +++ b/spec/lib/gitlab/ci/pipeline/chain/populate_spec.rb @@ -79,6 +79,31 @@ describe Gitlab::Ci::Pipeline::Chain::Populate do end end + describe 'pipeline protect' do + subject { step.perform! } + + context 'when ref is protected' do + before do + allow(project).to receive(:protected_for?).with('master').and_return(true) + allow(project).to receive(:protected_for?).with('refs/heads/master').and_return(true) + end + + it 'does not protect the pipeline' do + subject + + expect(pipeline.protected).to eq(true) + end + end + + context 'when ref is not protected' do + it 'does not protect the pipeline' do + subject + + expect(pipeline.protected).to eq(false) + end + end + end + context 'when pipeline has validation errors' do let(:pipeline) do build(:ci_pipeline, project: project, ref: nil) diff --git a/spec/lib/gitlab/kubernetes/helm/pod_spec.rb b/spec/lib/gitlab/kubernetes/helm/pod_spec.rb index 2dd3a570a1d..9cb79148028 100644 --- a/spec/lib/gitlab/kubernetes/helm/pod_spec.rb +++ b/spec/lib/gitlab/kubernetes/helm/pod_spec.rb @@ -30,7 +30,7 @@ describe Gitlab::Kubernetes::Helm::Pod do it 'should generate the appropriate specifications for the container' do container = subject.generate.spec.containers.first expect(container.name).to eq('helm') - expect(container.image).to eq('registry.gitlab.com/gitlab-org/cluster-integration/helm-install-image/releases/2.11.0-kube-1.11.0') + expect(container.image).to eq('registry.gitlab.com/gitlab-org/cluster-integration/helm-install-image/releases/2.12.2-kube-1.11.0') expect(container.env.count).to eq(3) expect(container.env.map(&:name)).to match_array([:HELM_VERSION, :TILLER_NAMESPACE, :COMMAND_SCRIPT]) expect(container.command).to match_array(["/bin/sh"]) diff --git a/spec/lib/gitlab/sidekiq_logging/structured_logger_spec.rb b/spec/lib/gitlab/sidekiq_logging/structured_logger_spec.rb index f773f370ee2..a9d15f1d522 100644 --- a/spec/lib/gitlab/sidekiq_logging/structured_logger_spec.rb +++ b/spec/lib/gitlab/sidekiq_logging/structured_logger_spec.rb @@ -82,15 +82,36 @@ describe Gitlab::SidekiqLogging::StructuredLogger do end.to raise_error(ArgumentError) end end + + context 'when the job args are bigger than the maximum allowed' do + it 'keeps args from the front until they exceed the limit' do + Timecop.freeze(timestamp) do + job['args'] = [ + 1, + 2, + 'a' * (described_class::MAXIMUM_JOB_ARGUMENTS_LENGTH / 2), + 'b' * (described_class::MAXIMUM_JOB_ARGUMENTS_LENGTH / 2), + 3 + ] + + expected_args = job['args'].take(3) + ['...'] + + expect(logger).to receive(:info).with(start_payload.merge('args' => expected_args)).ordered + expect(logger).to receive(:info).with(end_payload.merge('args' => expected_args)).ordered + expect(subject).to receive(:log_job_start).and_call_original + expect(subject).to receive(:log_job_done).and_call_original + + subject.call(job, 'test_queue') { } + end + end + end end context 'with SIDEKIQ_LOG_ARGUMENTS disabled' do - it 'logs start and end of job' do + it 'logs start and end of job without args' do Timecop.freeze(timestamp) do - start_payload.delete('args') - - expect(logger).to receive(:info).with(start_payload).ordered - expect(logger).to receive(:info).with(end_payload).ordered + expect(logger).to receive(:info).with(start_payload.except('args')).ordered + expect(logger).to receive(:info).with(end_payload.except('args')).ordered expect(subject).to receive(:log_job_start).and_call_original expect(subject).to receive(:log_job_done).and_call_original diff --git a/spec/lib/gitlab/tracing/grpc_interceptor_spec.rb b/spec/lib/gitlab/tracing/grpc_interceptor_spec.rb new file mode 100644 index 00000000000..7f5aecb7baa --- /dev/null +++ b/spec/lib/gitlab/tracing/grpc_interceptor_spec.rb @@ -0,0 +1,47 @@ +# frozen_string_literal: true + +require 'fast_spec_helper' + +describe Gitlab::Tracing::GRPCInterceptor do + subject { described_class.instance } + + shared_examples_for "a grpc interceptor method" do + let(:custom_error) { Class.new(StandardError) } + + it 'yields' do + expect { |b| method.call(kwargs, &b) }.to yield_control + end + + it 'propagates exceptions' do + expect { method.call(kwargs) { raise custom_error } }.to raise_error(custom_error) + end + end + + describe '#request_response' do + let(:method) { subject.method(:request_response) } + let(:kwargs) { { request: {}, call: {}, method: 'grc_method', metadata: {} } } + + it_behaves_like 'a grpc interceptor method' + end + + describe '#client_streamer' do + let(:method) { subject.method(:client_streamer) } + let(:kwargs) { { requests: [], call: {}, method: 'grc_method', metadata: {} } } + + it_behaves_like 'a grpc interceptor method' + end + + describe '#server_streamer' do + let(:method) { subject.method(:server_streamer) } + let(:kwargs) { { request: {}, call: {}, method: 'grc_method', metadata: {} } } + + it_behaves_like 'a grpc interceptor method' + end + + describe '#bidi_streamer' do + let(:method) { subject.method(:bidi_streamer) } + let(:kwargs) { { requests: [], call: {}, method: 'grc_method', metadata: {} } } + + it_behaves_like 'a grpc interceptor method' + end +end diff --git a/spec/lib/gitlab/tracing/rack_middleware_spec.rb b/spec/lib/gitlab/tracing/rack_middleware_spec.rb new file mode 100644 index 00000000000..13d4d8a89f7 --- /dev/null +++ b/spec/lib/gitlab/tracing/rack_middleware_spec.rb @@ -0,0 +1,62 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe Gitlab::Tracing::RackMiddleware do + using RSpec::Parameterized::TableSyntax + + describe '#call' do + context 'for normal middleware flow' do + let(:fake_app) { -> (env) { fake_app_response } } + subject { described_class.new(fake_app) } + let(:request) { } + + context 'for 200 responses' do + let(:fake_app_response) { [200, { 'Content-Type': 'text/plain' }, ['OK']] } + + it 'delegates correctly' do + expect(subject.call(Rack::MockRequest.env_for("/"))).to eq(fake_app_response) + end + end + + context 'for 500 responses' do + let(:fake_app_response) { [500, { 'Content-Type': 'text/plain' }, ['Error']] } + + it 'delegates correctly' do + expect(subject.call(Rack::MockRequest.env_for("/"))).to eq(fake_app_response) + end + end + end + + context 'when an application is raising an exception' do + let(:custom_error) { Class.new(StandardError) } + let(:fake_app) { ->(env) { raise custom_error } } + + subject { described_class.new(fake_app) } + + it 'delegates propagates exceptions correctly' do + expect { subject.call(Rack::MockRequest.env_for("/")) }.to raise_error(custom_error) + end + end + end + + describe '.build_sanitized_url_from_env' do + def env_for_url(url) + env = Rack::MockRequest.env_for(input_url) + env['action_dispatch.parameter_filter'] = [/token/] + + env + end + + where(:input_url, :output_url) do + '/gitlab-org/gitlab-ce' | 'http://example.org/gitlab-org/gitlab-ce' + '/gitlab-org/gitlab-ce?safe=1' | 'http://example.org/gitlab-org/gitlab-ce?safe=1' + '/gitlab-org/gitlab-ce?private_token=secret' | 'http://example.org/gitlab-org/gitlab-ce?private_token=%5BFILTERED%5D' + '/gitlab-org/gitlab-ce?mixed=1&private_token=secret' | 'http://example.org/gitlab-org/gitlab-ce?mixed=1&private_token=%5BFILTERED%5D' + end + + with_them do + it { expect(described_class.build_sanitized_url_from_env(env_for_url(input_url))).to eq(output_url) } + end + end +end diff --git a/spec/lib/gitlab/tracing/sidekiq/client_middleware_spec.rb b/spec/lib/gitlab/tracing/sidekiq/client_middleware_spec.rb new file mode 100644 index 00000000000..3755860b5ba --- /dev/null +++ b/spec/lib/gitlab/tracing/sidekiq/client_middleware_spec.rb @@ -0,0 +1,43 @@ +# frozen_string_literal: true + +require 'fast_spec_helper' + +describe Gitlab::Tracing::Sidekiq::ClientMiddleware do + describe '#call' do + let(:worker_class) { 'test_worker_class' } + let(:job) do + { + 'class' => "jobclass", + 'queue' => "jobqueue", + 'retry' => 0, + 'args' => %w{1 2 3} + } + end + let(:queue) { 'test_queue' } + let(:redis_pool) { double("redis_pool") } + let(:custom_error) { Class.new(StandardError) } + let(:span) { OpenTracing.start_span('test', ignore_active_scope: true) } + + subject { described_class.new } + + it 'yields' do + expect(subject).to receive(:in_tracing_span).with( + operation_name: "sidekiq:jobclass", + tags: { + "component" => "sidekiq", + "span.kind" => "client", + "sidekiq.queue" => "jobqueue", + "sidekiq.jid" => nil, + "sidekiq.retry" => "0", + "sidekiq.args" => "1, 2, 3" + } + ).and_yield(span) + + expect { |b| subject.call(worker_class, job, queue, redis_pool, &b) }.to yield_control + end + + it 'propagates exceptions' do + expect { subject.call(worker_class, job, queue, redis_pool) { raise custom_error } }.to raise_error(custom_error) + end + end +end diff --git a/spec/lib/gitlab/tracing/sidekiq/server_middleware_spec.rb b/spec/lib/gitlab/tracing/sidekiq/server_middleware_spec.rb new file mode 100644 index 00000000000..c3087de785a --- /dev/null +++ b/spec/lib/gitlab/tracing/sidekiq/server_middleware_spec.rb @@ -0,0 +1,43 @@ +# frozen_string_literal: true + +require 'fast_spec_helper' + +describe Gitlab::Tracing::Sidekiq::ServerMiddleware do + describe '#call' do + let(:worker_class) { 'test_worker_class' } + let(:job) do + { + 'class' => "jobclass", + 'queue' => "jobqueue", + 'retry' => 0, + 'args' => %w{1 2 3} + } + end + let(:queue) { 'test_queue' } + let(:custom_error) { Class.new(StandardError) } + let(:span) { OpenTracing.start_span('test', ignore_active_scope: true) } + subject { described_class.new } + + it 'yields' do + expect(subject).to receive(:in_tracing_span).with( + hash_including( + operation_name: "sidekiq:jobclass", + tags: { + "component" => "sidekiq", + "span.kind" => "server", + "sidekiq.queue" => "jobqueue", + "sidekiq.jid" => nil, + "sidekiq.retry" => "0", + "sidekiq.args" => "1, 2, 3" + } + ) + ).and_yield(span) + + expect { |b| subject.call(worker_class, job, queue, &b) }.to yield_control + end + + it 'propagates exceptions' do + expect { subject.call(worker_class, job, queue) { raise custom_error } }.to raise_error(custom_error) + end + end +end diff --git a/spec/migrations/rename_more_reserved_project_names_spec.rb b/spec/migrations/rename_more_reserved_project_names_spec.rb index baf16c2ce53..80ae209e9d1 100644 --- a/spec/migrations/rename_more_reserved_project_names_spec.rb +++ b/spec/migrations/rename_more_reserved_project_names_spec.rb @@ -37,9 +37,8 @@ describe RenameMoreReservedProjectNames, :delete do .to receive(:execute) .and_raise(Projects::AfterRenameService::RenameFailedError) - allow(Projects::AfterRenameService) - .to receive(:new) - .with(project) + expect(migration) + .to receive(:after_rename_service) .and_return(service) end diff --git a/spec/migrations/rename_reserved_project_names_spec.rb b/spec/migrations/rename_reserved_project_names_spec.rb index 7818aa0d560..93e5c032287 100644 --- a/spec/migrations/rename_reserved_project_names_spec.rb +++ b/spec/migrations/rename_reserved_project_names_spec.rb @@ -41,9 +41,8 @@ describe RenameReservedProjectNames, :migration, schema: :latest do .to receive(:execute) .and_raise(Projects::AfterRenameService::RenameFailedError) - allow(Projects::AfterRenameService) - .to receive(:new) - .with(project) + expect(migration) + .to receive(:after_rename_service) .and_return(service) end diff --git a/spec/models/appearance_spec.rb b/spec/models/appearance_spec.rb index ec2e7d672f0..cc76a2019ec 100644 --- a/spec/models/appearance_spec.rb +++ b/spec/models/appearance_spec.rb @@ -36,6 +36,13 @@ describe Appearance do expect(subject.send("#{logo_type}_path")).to be_nil end + it 'returns the path when the upload has been orphaned' do + appearance.send(logo_type).upload.destroy + appearance.reload + + expect(appearance.send("#{logo_type}_path")).to eq(expected_path) + end + it 'returns a local path using the system route' do expect(appearance.send("#{logo_type}_path")).to eq(expected_path) end diff --git a/spec/models/clusters/applications/runner_spec.rb b/spec/models/clusters/applications/runner_spec.rb index 3d0735c6d0b..8ad41e997c2 100644 --- a/spec/models/clusters/applications/runner_spec.rb +++ b/spec/models/clusters/applications/runner_spec.rb @@ -18,7 +18,7 @@ describe Clusters::Applications::Runner do let(:application) { create(:clusters_applications_runner, :scheduled, version: '0.1.30') } it 'updates the application version' do - expect(application.reload.version).to eq('0.1.43') + expect(application.reload.version).to eq('0.1.45') end end end @@ -46,7 +46,7 @@ describe Clusters::Applications::Runner do it 'should be initialized with 4 arguments' do expect(subject.name).to eq('runner') expect(subject.chart).to eq('runner/gitlab-runner') - expect(subject.version).to eq('0.1.43') + expect(subject.version).to eq('0.1.45') expect(subject).to be_rbac expect(subject.repository).to eq('https://charts.gitlab.io') expect(subject.files).to eq(gitlab_runner.files) @@ -64,7 +64,7 @@ describe Clusters::Applications::Runner do let(:gitlab_runner) { create(:clusters_applications_runner, :errored, runner: ci_runner, version: '0.1.13') } it 'should be initialized with the locked version' do - expect(subject.version).to eq('0.1.43') + expect(subject.version).to eq('0.1.45') end end end diff --git a/spec/models/clusters/cluster_spec.rb b/spec/models/clusters/cluster_spec.rb index f447e64b029..0161db740ee 100644 --- a/spec/models/clusters/cluster_spec.rb +++ b/spec/models/clusters/cluster_spec.rb @@ -113,7 +113,7 @@ describe Clusters::Cluster do end end - describe 'validation' do + describe 'validations' do subject { cluster.valid? } context 'when validates name' do @@ -252,6 +252,31 @@ describe Clusters::Cluster do end end end + + describe 'domain validation' do + let(:cluster) { build(:cluster) } + + subject { cluster } + + context 'when cluster has domain' do + let(:cluster) { build(:cluster, :with_domain) } + + it { is_expected.to be_valid } + end + + context 'when cluster has an invalid domain' do + let(:cluster) { build(:cluster, domain: 'not-valid-domain') } + + it 'should add an error on domain' do + expect(subject).not_to be_valid + expect(subject.errors[:domain].first).to eq('is not a fully qualified domain name') + end + end + + context 'when cluster does not have a domain' do + it { is_expected.to be_valid } + end + end end describe '.ancestor_clusters_for_clusterable' do diff --git a/spec/models/issue_spec.rb b/spec/models/issue_spec.rb index 6f900a60213..5d18e085a6f 100644 --- a/spec/models/issue_spec.rb +++ b/spec/models/issue_spec.rb @@ -721,39 +721,28 @@ describe Issue do end end - describe '#check_for_spam' do - let(:project) { create :project, visibility_level: visibility_level } - let(:issue) { create :issue, project: project } + describe '#check_for_spam?' do + using RSpec::Parameterized::TableSyntax - subject do - issue.assign_attributes(description: description) - issue.check_for_spam? + where(:visibility_level, :confidential, :new_attributes, :check_for_spam?) do + Gitlab::VisibilityLevel::PUBLIC | false | { description: 'woo' } | true + Gitlab::VisibilityLevel::PUBLIC | false | { title: 'woo' } | true + Gitlab::VisibilityLevel::PUBLIC | true | { confidential: false } | true + Gitlab::VisibilityLevel::PUBLIC | true | { description: 'woo' } | false + Gitlab::VisibilityLevel::PUBLIC | false | { title: 'woo', confidential: true } | false + Gitlab::VisibilityLevel::PUBLIC | false | { description: 'original description' } | false + Gitlab::VisibilityLevel::INTERNAL | false | { description: 'woo' } | false + Gitlab::VisibilityLevel::PRIVATE | false | { description: 'woo' } | false end - context 'when project is public and spammable attributes changed' do - let(:visibility_level) { Gitlab::VisibilityLevel::PUBLIC } - let(:description) { 'woo' } + with_them do + it 'checks for spam on issues that can be seen anonymously' do + project = create(:project, visibility_level: visibility_level) + issue = create(:issue, project: project, confidential: confidential, description: 'original description') - it 'returns true' do - is_expected.to be_truthy - end - end - - context 'when project is private' do - let(:visibility_level) { Gitlab::VisibilityLevel::PRIVATE } - let(:description) { issue.description } - - it 'returns false' do - is_expected.to be_falsey - end - end - - context 'when spammable attributes have not changed' do - let(:visibility_level) { Gitlab::VisibilityLevel::PUBLIC } - let(:description) { issue.description } + issue.assign_attributes(new_attributes) - it 'returns false' do - is_expected.to be_falsey + expect(issue.check_for_spam?).to eq(check_for_spam?) end end end diff --git a/spec/requests/api/groups_spec.rb b/spec/requests/api/groups_spec.rb index c9dfc5c4a7e..7176bc23e34 100644 --- a/spec/requests/api/groups_spec.rb +++ b/spec/requests/api/groups_spec.rb @@ -382,6 +382,20 @@ describe API::Groups do expect(response_project_ids(json_response, 'shared_projects')) .to contain_exactly(projects[:public].id, projects[:internal].id) end + + it 'avoids N+1 queries' do + get api("/groups/#{group1.id}", admin) + + control_count = ActiveRecord::QueryRecorder.new do + get api("/groups/#{group1.id}", admin) + end.count + + create(:project, namespace: group1) + + expect do + get api("/groups/#{group1.id}", admin) + end.not_to exceed_query_limit(control_count) + end end context "when authenticated as admin" do diff --git a/spec/requests/api/submodules_spec.rb b/spec/requests/api/submodules_spec.rb index c482a85c68f..064392fb185 100644 --- a/spec/requests/api/submodules_spec.rb +++ b/spec/requests/api/submodules_spec.rb @@ -64,7 +64,7 @@ describe API::Submodules do expect(response).to have_gitlab_http_status(400) end - it 'returns the commmit' do + it 'returns the commit' do head_commit = project.repository.commit.id put api(route(submodule), user), params: params @@ -81,7 +81,7 @@ describe API::Submodules do let(:branch) { 'submodule_inside_folder' } let(:encoded_submodule) { CGI.escape(submodule) } - it 'returns the commmit' do + it 'returns the commit' do expect(Submodules::UpdateService) .to receive(:new) .with(any_args, hash_including(submodule: submodule)) diff --git a/spec/requests/api/tags_spec.rb b/spec/requests/api/tags_spec.rb index d09b6fe72b1..fffe878ddbd 100644 --- a/spec/requests/api/tags_spec.rb +++ b/spec/requests/api/tags_spec.rb @@ -54,6 +54,18 @@ describe API::Tags do end end + context 'searching' do + it 'only returns searched tags' do + get api("#{route}", user), params: { search: 'v1.1.0' } + + expect(response).to have_gitlab_http_status(200) + expect(response).to include_pagination_headers + expect(json_response).to be_an Array + expect(json_response.size).to eq(1) + expect(json_response[0]['name']).to eq('v1.1.0') + end + end + shared_examples_for 'repository tags' do it 'returns the repository tags' do get api(route, current_user) diff --git a/spec/requests/lfs_locks_api_spec.rb b/spec/requests/lfs_locks_api_spec.rb index 28cb90e450e..c63fbcdd84e 100644 --- a/spec/requests/lfs_locks_api_spec.rb +++ b/spec/requests/lfs_locks_api_spec.rb @@ -132,6 +132,17 @@ describe 'Git LFS File Locking API' do expect(json_response['lock'].keys).to match_array(%w(id path locked_at owner)) end + + context 'when a maintainer uses force' do + let(:authorization) { authorize_user(maintainer) } + + it 'deletes the lock' do + project.add_maintainer(maintainer) + post_lfs_json url, { force: true }, headers + + expect(response).to have_gitlab_http_status(200) + end + end end end diff --git a/spec/routing/project_routing_spec.rb b/spec/routing/project_routing_spec.rb index 5c3b37ef11c..a0d01fc8263 100644 --- a/spec/routing/project_routing_spec.rb +++ b/spec/routing/project_routing_spec.rb @@ -122,6 +122,10 @@ describe 'project routing' do route_to('projects#preview_markdown', namespace_id: 'gitlab', id: 'gitlabhq') ) end + + it 'to #resolve' do + expect(get('/projects/1')).to route_to('projects#resolve', id: '1') + end end # members_namespace_project_autocomplete_sources_path GET /:project_id/autocomplete_sources/members(.:format) projects/autocomplete_sources#members diff --git a/spec/services/projects/after_rename_service_spec.rb b/spec/services/projects/after_rename_service_spec.rb index 59c08b30f9f..bc5366a3339 100644 --- a/spec/services/projects/after_rename_service_spec.rb +++ b/spec/services/projects/after_rename_service_spec.rb @@ -4,53 +4,45 @@ require 'spec_helper' describe Projects::AfterRenameService do let(:rugged_config) { rugged_repo(project.repository).config } + let(:legacy_storage) { Storage::LegacyProject.new(project) } + let(:hashed_storage) { Storage::HashedProject.new(project) } + let!(:path_before_rename) { project.path } + let!(:full_path_before_rename) { project.full_path } + let!(:path_after_rename) { "#{project.path}-renamed" } + let!(:full_path_after_rename) { "#{project.full_path}-renamed" } describe '#execute' do context 'using legacy storage' do - let(:project) { create(:project, :repository, :legacy_storage) } - let(:gitlab_shell) { Gitlab::Shell.new } + let(:project) { create(:project, :repository, :wiki_repo, :legacy_storage) } let(:project_storage) { project.send(:storage) } + let(:gitlab_shell) { Gitlab::Shell.new } before do # Project#gitlab_shell returns a new instance of Gitlab::Shell on every # call. This makes testing a bit easier. allow(project).to receive(:gitlab_shell).and_return(gitlab_shell) - allow(project) - .to receive(:previous_changes) - .and_return('path' => ['foo']) - - allow(project) - .to receive(:path_was) - .and_return('foo') - stub_feature_flags(skip_hashed_storage_upgrade: false) end it 'renames a repository' do stub_container_registry_config(enabled: false) - expect(gitlab_shell).to receive(:mv_repository) - .ordered - .with(project.repository_storage, "#{project.namespace.full_path}/foo", "#{project.full_path}") - .and_return(true) - - expect(gitlab_shell).to receive(:mv_repository) - .ordered - .with(project.repository_storage, "#{project.namespace.full_path}/foo.wiki", "#{project.full_path}.wiki") - .and_return(true) - expect_any_instance_of(SystemHooksService) .to receive(:execute_hooks_for) .with(project, :rename) expect_any_instance_of(Gitlab::UploadsTransfer) .to receive(:rename_project) - .with('foo', project.path, project.namespace.full_path) + .with(path_before_rename, path_after_rename, project.namespace.full_path) - expect(project).to receive(:expire_caches_before_rename) + expect_repository_exist("#{full_path_before_rename}.git") + expect_repository_exist("#{full_path_before_rename}.wiki.git") - described_class.new(project).execute + service_execute + + expect_repository_exist("#{full_path_after_rename}.git") + expect_repository_exist("#{full_path_after_rename}.wiki.git") end context 'container registry with images' do @@ -63,8 +55,7 @@ describe Projects::AfterRenameService do end it 'raises a RenameFailedError' do - expect { described_class.new(project).execute } - .to raise_error(described_class::RenameFailedError) + expect { service_execute }.to raise_error(described_class::RenameFailedError) end end @@ -76,7 +67,7 @@ describe Projects::AfterRenameService do it 'moves pages folder to new location' do expect_any_instance_of(Gitlab::PagesTransfer).to receive(:rename_project) - described_class.new(project).execute + service_execute end end @@ -88,14 +79,12 @@ describe Projects::AfterRenameService do it 'moves uploads folder to new location' do expect_any_instance_of(Gitlab::UploadsTransfer).to receive(:rename_project) - described_class.new(project).execute + service_execute end end it 'updates project full path in .git/config' do - allow(project_storage).to receive(:rename_repo).and_return(true) - - described_class.new(project).execute + service_execute expect(rugged_config['gitlab.fullpath']).to eq(project.full_path) end @@ -103,13 +92,25 @@ describe Projects::AfterRenameService do it 'updates storage location' do allow(project_storage).to receive(:rename_repo).and_return(true) - described_class.new(project).execute + service_execute expect(project.project_repository).to have_attributes( disk_path: project.disk_path, shard_name: project.repository_storage ) end + + context 'with hashed storage upgrade when renaming enabled' do + it 'calls HashedStorageMigrationService with correct options' do + stub_application_setting(hashed_storage_enabled: true) + + expect_next_instance_of(::Projects::HashedStorageMigrationService) do |service| + expect(service).to receive(:execute).and_return(true) + end + + service_execute + end + end end context 'using hashed storage' do @@ -123,25 +124,11 @@ describe Projects::AfterRenameService do # Project#gitlab_shell returns a new instance of Gitlab::Shell on every # call. This makes testing a bit easier. allow(project).to receive(:gitlab_shell).and_return(gitlab_shell) - allow(project).to receive(:previous_changes).and_return('path' => ['foo']) stub_feature_flags(skip_hashed_storage_upgrade: false) stub_application_setting(hashed_storage_enabled: true) end - context 'migration to hashed storage' do - it 'calls HashedStorageMigrationService with correct options' do - project = create(:project, :repository, :legacy_storage) - allow(project).to receive(:previous_changes).and_return('path' => ['foo']) - - expect_next_instance_of(::Projects::HashedStorageMigrationService) do |service| - expect(service).to receive(:execute).and_return(true) - end - - described_class.new(project).execute - end - end - it 'renames a repository' do stub_container_registry_config(enabled: false) @@ -153,7 +140,7 @@ describe Projects::AfterRenameService do expect(project).to receive(:expire_caches_before_rename) - described_class.new(project).execute + service_execute end context 'container registry with images' do @@ -166,7 +153,7 @@ describe Projects::AfterRenameService do end it 'raises a RenameFailedError' do - expect { described_class.new(project).execute } + expect { service_execute } .to raise_error(described_class::RenameFailedError) end end @@ -175,38 +162,46 @@ describe Projects::AfterRenameService do it 'moves pages folder to new location' do expect_any_instance_of(Gitlab::PagesTransfer).to receive(:rename_project) - described_class.new(project).execute + service_execute end end context 'attachments' do + let(:uploader) { create(:upload, :issuable_upload, :with_file, model: project) } + let(:file_uploader) { build(:file_uploader, project: project) } + let(:legacy_storage_path) { File.join(file_uploader.root, legacy_storage.disk_path) } + let(:hashed_storage_path) { File.join(file_uploader.root, hashed_storage.disk_path) } + it 'keeps uploads folder location unchanged' do expect_any_instance_of(Gitlab::UploadsTransfer).not_to receive(:rename_project) - described_class.new(project).execute + service_execute end context 'when not rolled out' do let(:project) { create(:project, :repository, storage_version: 1, skip_disk_validation: true) } - it 'moves pages folder to hashed storage' do - expect_next_instance_of(Projects::HashedStorage::MigrateAttachmentsService) do |service| - expect(service).to receive(:execute) - end + it 'moves attachments folder to hashed storage' do + expect(File.directory?(legacy_storage_path)).to be_truthy + expect(File.directory?(hashed_storage_path)).to be_falsey - described_class.new(project).execute + service_execute + expect(project.reload.hashed_storage?(:attachments)).to be_truthy + + expect(File.directory?(legacy_storage_path)).to be_falsey + expect(File.directory?(hashed_storage_path)).to be_truthy end end end it 'updates project full path in .git/config' do - described_class.new(project).execute + service_execute expect(rugged_config['gitlab.fullpath']).to eq(project.full_path) end it 'updates storage location' do - described_class.new(project).execute + service_execute expect(project.project_repository).to have_attributes( disk_path: project.disk_path, @@ -215,4 +210,21 @@ describe Projects::AfterRenameService do end end end + + def service_execute + # AfterRenameService is called by UpdateService after a successful model.update + # the initialization will include before and after paths values + project.update(path: path_after_rename) + + described_class.new(project, path_before: path_before_rename, full_path_before: full_path_before_rename).execute + end + + def expect_repository_exist(full_path_with_extension) + expect( + gitlab_shell.exists?( + project.repository_storage, + full_path_with_extension + ) + ).to be_truthy + end end diff --git a/spec/support/helpers/test_env.rb b/spec/support/helpers/test_env.rb index d352a7cdf1a..f485eb7b0eb 100644 --- a/spec/support/helpers/test_env.rb +++ b/spec/support/helpers/test_env.rb @@ -160,11 +160,12 @@ module TestEnv def setup_gitaly socket_path = Gitlab::GitalyClient.address('default').sub(/\Aunix:/, '') gitaly_dir = File.dirname(socket_path) + install_gitaly_args = [gitaly_dir, repos_path, gitaly_url].compact.join(',') component_timed_setup('Gitaly', install_dir: gitaly_dir, version: Gitlab::GitalyClient.expected_server_version, - task: "gitlab:gitaly:install[#{gitaly_dir},#{repos_path}]") do + task: "gitlab:gitaly:install[#{install_gitaly_args}]") do Gitlab::SetupHelper.create_gitaly_configuration(gitaly_dir, { 'default' => repos_path }, force: true) start_gitaly(gitaly_dir) @@ -215,6 +216,10 @@ module TestEnv # The process can already be gone if the test run was INTerrupted. end + def gitaly_url + ENV.fetch('GITALY_REPO_URL', nil) + end + def setup_factory_repo setup_repo(factory_repo_path, factory_repo_path_bare, factory_repo_name, BRANCH_SHA) diff --git a/spec/support/migrations_helpers/cluster_helpers.rb b/spec/support/migrations_helpers/cluster_helpers.rb new file mode 100644 index 00000000000..b54af15c29e --- /dev/null +++ b/spec/support/migrations_helpers/cluster_helpers.rb @@ -0,0 +1,71 @@ +# frozen_string_literal: true + +module MigrationHelpers + module ClusterHelpers + # Creates a list of cluster projects. + def create_cluster_project_list(quantity) + group = namespaces_table.create(name: 'gitlab-org', path: 'gitlab-org') + + quantity.times do |id| + create_cluster_project(group, id) + end + end + + # Creates dependencies for a cluster project: + # - Group + # - Project + # - Cluster + # - Project - cluster relationship + # - GCP provider + # - Platform Kubernetes + def create_cluster_project(group, id) + project = projects_table.create!( + name: "project-#{id}", + path: "project-#{id}", + namespace_id: group.id + ) + + cluster = clusters_table.create( + name: 'test-cluster', + cluster_type: 3, + provider_type: :gcp, + platform_type: :kubernetes + ) + + cluster_projects_table.create(project_id: project.id, cluster_id: cluster.id) + + provider_gcp_table.create!( + gcp_project_id: "test-gcp-project-#{id}", + endpoint: '111.111.111.111', + cluster_id: cluster.id, + status: 3, + num_nodes: 1, + zone: 'us-central1-a' + ) + + platform_kubernetes_table.create( + cluster_id: cluster.id, + api_url: 'https://kubernetes.example.com', + encrypted_token: 'a' * 40, + encrypted_token_iv: 'a' * 40 + ) + end + + # Creates a Kubernetes namespace for a list of clusters + def create_kubernetes_namespace(clusters) + clusters.each do |cluster| + cluster_project = cluster_projects_table.find_by(cluster_id: cluster.id) + project = projects_table.find(cluster_project.project_id) + namespace = "#{project.path}-#{project.id}" + + cluster_kubernetes_namespaces_table.create( + cluster_project_id: cluster_project.id, + cluster_id: cluster.id, + project_id: cluster_project.project_id, + namespace: namespace, + service_account_name: "#{namespace}-service-account" + ) + end + end + end +end diff --git a/spec/support/shared_examples/dirty_submit_form_shared_examples.rb b/spec/support/shared_examples/dirty_submit_form_shared_examples.rb index ba363593120..52a2ee49495 100644 --- a/spec/support/shared_examples/dirty_submit_form_shared_examples.rb +++ b/spec/support/shared_examples/dirty_submit_form_shared_examples.rb @@ -1,24 +1,36 @@ shared_examples 'dirty submit form' do |selector_args| selectors = selector_args.is_a?(Array) ? selector_args : [selector_args] + def expect_disabled_state(form, submit, is_disabled = true) + disabled_selector = is_disabled == true ? '[disabled]' : ':not([disabled])' + + form.find(".js-dirty-submit#{disabled_selector}", match: :first) + + expect(submit.disabled?).to be is_disabled + end + selectors.each do |selector| - it "disables #{selector[:form]} submit until there are changes", :js do + it "disables #{selector[:form]} submit until there are changes on #{selector[:input]}", :js do form = find(selector[:form]) submit = form.first('.js-dirty-submit') input = form.first(selector[:input]) + is_radio = input[:type] == 'radio' + is_checkbox = input[:type] == 'checkbox' + is_checkable = is_radio || is_checkbox original_value = input.value + original_checkable = form.find("input[name='#{input[:name]}'][checked]") if is_radio + original_checkable = input if is_checkbox expect(submit.disabled?).to be true + expect(input.checked?).to be false - input.set("#{original_value} changes") + is_checkable ? input.click : input.set("#{original_value} changes") - form.find('.js-dirty-submit:not([disabled])', match: :first) - expect(submit.disabled?).to be false + expect_disabled_state(form, submit, false) - input.set(original_value) + is_checkable ? original_checkable.click : input.set(original_value) - form.find('.js-dirty-submit[disabled]', match: :first) - expect(submit.disabled?).to be true + expect_disabled_state(form, submit) end end end diff --git a/spec/support/shared_examples/features/editable_merge_request_shared_examples.rb b/spec/support/shared_examples/features/editable_merge_request_shared_examples.rb index a096627ee62..eef0327c9a6 100644 --- a/spec/support/shared_examples/features/editable_merge_request_shared_examples.rb +++ b/spec/support/shared_examples/features/editable_merge_request_shared_examples.rb @@ -129,12 +129,12 @@ RSpec.shared_examples 'an editable merge request' do expect(merge_request.merge_params['force_remove_source_branch']).to be_truthy visit edit_project_merge_request_path(target_project, merge_request) - uncheck 'Remove source branch when merge request is accepted' + uncheck 'Delete source branch when merge request is accepted' click_button 'Save changes' expect(page).to have_unchecked_field 'remove-source-branch-input' - expect(page).to have_content 'Remove source branch' + expect(page).to have_content 'Delete source branch' end end end diff --git a/spec/uploaders/personal_file_uploader_spec.rb b/spec/uploaders/personal_file_uploader_spec.rb index 2896e9a112d..97758f0243e 100644 --- a/spec/uploaders/personal_file_uploader_spec.rb +++ b/spec/uploaders/personal_file_uploader_spec.rb @@ -4,19 +4,13 @@ describe PersonalFileUploader do let(:model) { create(:personal_snippet) } let(:uploader) { described_class.new(model) } let(:upload) { create(:upload, :personal_snippet_upload) } - let(:identifier) { %r{\h+/\S+} } subject { uploader } - it_behaves_like 'builds correct paths' do - let(:patterns) do - { - store_dir: %r[uploads/-/system/personal_snippet/\d+], - upload_path: identifier, - absolute_path: %r[#{CarrierWave.root}/uploads/-/system/personal_snippet/\d+/#{identifier}] - } - end - end + it_behaves_like 'builds correct paths', + store_dir: %r[uploads/-/system/personal_snippet/\d+], + upload_path: %r[\h+/\S+], + absolute_path: %r[#{CarrierWave.root}/uploads/-/system/personal_snippet\/\d+\/\h+\/\S+$] context "object_store is REMOTE" do before do @@ -25,13 +19,17 @@ describe PersonalFileUploader do include_context 'with storage', described_class::Store::REMOTE - it_behaves_like 'builds correct paths' do - let(:patterns) do - { - store_dir: %r[\d+/\h+], - upload_path: identifier - } - end + it_behaves_like 'builds correct paths', + store_dir: %r[\d+/\h+], + upload_path: %r[^personal_snippet\/\d+\/\h+\/<filename>] + end + + describe '#upload_paths' do + it 'builds correct paths for both local and remote storage' do + paths = uploader.upload_paths('test.jpg') + + expect(paths.first).to match(%r[\h+\/test.jpg]) + expect(paths.second).to match(%r[^personal_snippet\/\d+\/\h+\/test.jpg]) end end diff --git a/spec/views/projects/settings/operations/show.html.haml_spec.rb b/spec/views/projects/settings/operations/show.html.haml_spec.rb index 752fd82c5e8..8e34521c7c8 100644 --- a/spec/views/projects/settings/operations/show.html.haml_spec.rb +++ b/spec/views/projects/settings/operations/show.html.haml_spec.rb @@ -13,8 +13,6 @@ describe 'projects/settings/operations/show' do describe 'Operations > Error Tracking' do before do - stub_feature_flags(error_tracking: true) - project.add_reporter(user) allow(view).to receive(:error_tracking_setting) |