diff options
Diffstat (limited to 'spec')
35 files changed, 823 insertions, 417 deletions
diff --git a/spec/controllers/admin/gitaly_servers_controller_spec.rb b/spec/controllers/admin/gitaly_servers_controller_spec.rb new file mode 100644 index 00000000000..b7378aa37d0 --- /dev/null +++ b/spec/controllers/admin/gitaly_servers_controller_spec.rb @@ -0,0 +1,15 @@ +require 'spec_helper' + +describe Admin::GitalyServersController do + describe '#index' do + before do + sign_in(create(:admin)) + end + + it 'shows the gitaly servers page' do + get :index + + expect(response).to have_gitlab_http_status(200) + end + end +end diff --git a/spec/controllers/groups_controller_spec.rb b/spec/controllers/groups_controller_spec.rb index a9cfd964dd5..492fed42d31 100644 --- a/spec/controllers/groups_controller_spec.rb +++ b/spec/controllers/groups_controller_spec.rb @@ -85,6 +85,30 @@ describe GroupsController do end end + describe 'GET #activity' do + render_views + + before do + sign_in(user) + project + end + + context 'as json' do + it 'includes all projects in event feed' do + 3.times do + project = create(:project, group: group) + create(:event, project: project) + end + + get :activity, id: group.to_param, format: :json + + expect(response).to have_gitlab_http_status(200) + expect(json_response['count']).to eq(3) + expect(assigns(:projects).limit_value).to be_nil + end + end + end + describe 'POST #create' do context 'when creating subgroups', :nested_groups do [true, false].each do |can_create_group_status| diff --git a/spec/controllers/projects/issues_controller_spec.rb b/spec/controllers/projects/issues_controller_spec.rb index 4a2998b4ccd..9656e7f7e74 100644 --- a/spec/controllers/projects/issues_controller_spec.rb +++ b/spec/controllers/projects/issues_controller_spec.rb @@ -102,6 +102,18 @@ describe Projects::IssuesController do expect(response).to redirect_to(namespace_project_issues_path(page: last_page, state: controller.params[:state], scope: controller.params[:scope])) end + + it 'does not use pagination if disabled' do + allow(controller).to receive(:pagination_disabled?).and_return(true) + + get :index, + namespace_id: project.namespace.to_param, + project_id: project, + page: (last_page + 1).to_param + + expect(response).to have_gitlab_http_status(200) + expect(assigns(:issues).size).to eq(2) + end end end diff --git a/spec/features/projects/members/share_with_group_spec.rb b/spec/features/projects/members/share_with_group_spec.rb index 3198798306c..4cf48098401 100644 --- a/spec/features/projects/members/share_with_group_spec.rb +++ b/spec/features/projects/members/share_with_group_spec.rb @@ -122,7 +122,7 @@ feature 'Project > Members > Share with Group', :js do select2 group.id, from: '#link_group_id' fill_in 'expires_at_groups', with: (Time.now + 4.5.days).strftime('%Y-%m-%d') - page.find('body').click + click_on 'share-with-group-tab' find('.btn-create').click end diff --git a/spec/javascripts/api_spec.js b/spec/javascripts/api_spec.js index cc5fa42aafe..cf3a76d0d2e 100644 --- a/spec/javascripts/api_spec.js +++ b/spec/javascripts/api_spec.js @@ -1,3 +1,5 @@ +import MockAdapter from 'axios-mock-adapter'; +import axios from '~/lib/utils/axios_utils'; import Api from '~/api'; describe('Api', () => { @@ -7,20 +9,17 @@ describe('Api', () => { api_version: dummyApiVersion, relative_url_root: dummyUrlRoot, }; - const dummyResponse = 'hello from outer space!'; - const sendDummyResponse = () => { - const deferred = $.Deferred(); - deferred.resolve(dummyResponse); - return deferred.promise(); - }; let originalGon; + let mock; beforeEach(() => { + mock = new MockAdapter(axios); originalGon = window.gon; window.gon = Object.assign({}, dummyGon); }); afterEach(() => { + mock.restore(); window.gon = originalGon; }); @@ -38,15 +37,13 @@ describe('Api', () => { describe('group', () => { it('fetches a group', (done) => { const groupId = '123456'; - const expectedUrl = `${dummyUrlRoot}/api/${dummyApiVersion}/groups/${groupId}.json`; - spyOn(jQuery, 'ajax').and.callFake((request) => { - expect(request.url).toEqual(expectedUrl); - expect(request.dataType).toEqual('json'); - return sendDummyResponse(); + const expectedUrl = `${dummyUrlRoot}/api/${dummyApiVersion}/groups/${groupId}`; + mock.onGet(expectedUrl).reply(200, { + name: 'test', }); Api.group(groupId, (response) => { - expect(response).toBe(dummyResponse); + expect(response.name).toBe('test'); done(); }); }); @@ -57,19 +54,13 @@ describe('Api', () => { const query = 'dummy query'; const options = { unused: 'option' }; const expectedUrl = `${dummyUrlRoot}/api/${dummyApiVersion}/groups.json`; - const expectedData = Object.assign({ - search: query, - per_page: 20, - }, options); - spyOn(jQuery, 'ajax').and.callFake((request) => { - expect(request.url).toEqual(expectedUrl); - expect(request.dataType).toEqual('json'); - expect(request.data).toEqual(expectedData); - return sendDummyResponse(); - }); + mock.onGet(expectedUrl).reply(200, [{ + name: 'test', + }]); Api.groups(query, options, (response) => { - expect(response).toBe(dummyResponse); + expect(response.length).toBe(1); + expect(response[0].name).toBe('test'); done(); }); }); @@ -79,19 +70,13 @@ describe('Api', () => { it('fetches namespaces', (done) => { const query = 'dummy query'; const expectedUrl = `${dummyUrlRoot}/api/${dummyApiVersion}/namespaces.json`; - const expectedData = { - search: query, - per_page: 20, - }; - spyOn(jQuery, 'ajax').and.callFake((request) => { - expect(request.url).toEqual(expectedUrl); - expect(request.dataType).toEqual('json'); - expect(request.data).toEqual(expectedData); - return sendDummyResponse(); - }); + mock.onGet(expectedUrl).reply(200, [{ + name: 'test', + }]); Api.namespaces(query, (response) => { - expect(response).toBe(dummyResponse); + expect(response.length).toBe(1); + expect(response[0].name).toBe('test'); done(); }); }); @@ -103,21 +88,13 @@ describe('Api', () => { const options = { unused: 'option' }; const expectedUrl = `${dummyUrlRoot}/api/${dummyApiVersion}/projects.json`; window.gon.current_user_id = 1; - const expectedData = Object.assign({ - search: query, - per_page: 20, - membership: true, - simple: true, - }, options); - spyOn(jQuery, 'ajax').and.callFake((request) => { - expect(request.url).toEqual(expectedUrl); - expect(request.dataType).toEqual('json'); - expect(request.data).toEqual(expectedData); - return sendDummyResponse(); - }); + mock.onGet(expectedUrl).reply(200, [{ + name: 'test', + }]); Api.projects(query, options, (response) => { - expect(response).toBe(dummyResponse); + expect(response.length).toBe(1); + expect(response[0].name).toBe('test'); done(); }); }); @@ -126,20 +103,13 @@ describe('Api', () => { const query = 'dummy query'; const options = { unused: 'option' }; const expectedUrl = `${dummyUrlRoot}/api/${dummyApiVersion}/projects.json`; - const expectedData = Object.assign({ - search: query, - per_page: 20, - simple: true, - }, options); - spyOn(jQuery, 'ajax').and.callFake((request) => { - expect(request.url).toEqual(expectedUrl); - expect(request.dataType).toEqual('json'); - expect(request.data).toEqual(expectedData); - return sendDummyResponse(); - }); + mock.onGet(expectedUrl).reply(200, [{ + name: 'test', + }]); Api.projects(query, options, (response) => { - expect(response).toBe(dummyResponse); + expect(response.length).toBe(1); + expect(response[0].name).toBe('test'); done(); }); }); @@ -154,16 +124,16 @@ describe('Api', () => { const expectedData = { label: labelData, }; - spyOn(jQuery, 'ajax').and.callFake((request) => { - expect(request.url).toEqual(expectedUrl); - expect(request.dataType).toEqual('json'); - expect(request.type).toEqual('POST'); - expect(request.data).toEqual(expectedData); - return sendDummyResponse(); + mock.onPost(expectedUrl).reply((config) => { + expect(config.data).toBe(JSON.stringify(expectedData)); + + return [200, { + name: 'test', + }]; }); Api.newLabel(namespace, project, labelData, (response) => { - expect(response).toBe(dummyResponse); + expect(response.name).toBe('test'); done(); }); }); @@ -174,19 +144,13 @@ describe('Api', () => { const groupId = '123456'; const query = 'dummy query'; const expectedUrl = `${dummyUrlRoot}/api/${dummyApiVersion}/groups/${groupId}/projects.json`; - const expectedData = { - search: query, - per_page: 20, - }; - spyOn(jQuery, 'ajax').and.callFake((request) => { - expect(request.url).toEqual(expectedUrl); - expect(request.dataType).toEqual('json'); - expect(request.data).toEqual(expectedData); - return sendDummyResponse(); - }); + mock.onGet(expectedUrl).reply(200, [{ + name: 'test', + }]); Api.groupProjects(groupId, query, (response) => { - expect(response).toBe(dummyResponse); + expect(response.length).toBe(1); + expect(response[0].name).toBe('test'); done(); }); }); @@ -197,14 +161,10 @@ describe('Api', () => { const licenseKey = "driver's license"; const data = { unused: 'option' }; const expectedUrl = `${dummyUrlRoot}/api/${dummyApiVersion}/templates/licenses/${licenseKey}`; - spyOn(jQuery, 'ajax').and.callFake((request) => { - expect(request.url).toEqual(expectedUrl); - expect(request.data).toEqual(data); - return sendDummyResponse(); - }); + mock.onGet(expectedUrl).reply(200, 'test'); Api.licenseText(licenseKey, data, (response) => { - expect(response).toBe(dummyResponse); + expect(response).toBe('test'); done(); }); }); @@ -214,13 +174,10 @@ describe('Api', () => { it('fetches a gitignore text', (done) => { const gitignoreKey = 'ignore git'; const expectedUrl = `${dummyUrlRoot}/api/${dummyApiVersion}/templates/gitignores/${gitignoreKey}`; - spyOn(jQuery, 'get').and.callFake((url, callback) => { - expect(url).toEqual(expectedUrl); - callback(dummyResponse); - }); + mock.onGet(expectedUrl).reply(200, 'test'); Api.gitignoreText(gitignoreKey, (response) => { - expect(response).toBe(dummyResponse); + expect(response).toBe('test'); done(); }); }); @@ -230,13 +187,10 @@ describe('Api', () => { it('fetches a .gitlab-ci.yml', (done) => { const gitlabCiYmlKey = 'Y CI ML'; const expectedUrl = `${dummyUrlRoot}/api/${dummyApiVersion}/templates/gitlab_ci_ymls/${gitlabCiYmlKey}`; - spyOn(jQuery, 'get').and.callFake((url, callback) => { - expect(url).toEqual(expectedUrl); - callback(dummyResponse); - }); + mock.onGet(expectedUrl).reply(200, 'test'); Api.gitlabCiYml(gitlabCiYmlKey, (response) => { - expect(response).toBe(dummyResponse); + expect(response).toBe('test'); done(); }); }); @@ -246,13 +200,10 @@ describe('Api', () => { it('fetches a Dockerfile', (done) => { const dockerfileYmlKey = 'a giant whale'; const expectedUrl = `${dummyUrlRoot}/api/${dummyApiVersion}/templates/dockerfiles/${dockerfileYmlKey}`; - spyOn(jQuery, 'get').and.callFake((url, callback) => { - expect(url).toEqual(expectedUrl); - callback(dummyResponse); - }); + mock.onGet(expectedUrl).reply(200, 'test'); Api.dockerfileYml(dockerfileYmlKey, (response) => { - expect(response).toBe(dummyResponse); + expect(response).toBe('test'); done(); }); }); @@ -265,14 +216,10 @@ describe('Api', () => { const templateKey = ' template #%?.key '; const templateType = 'template type'; const expectedUrl = `${dummyUrlRoot}/${namespace}/${project}/templates/${templateType}/${encodeURIComponent(templateKey)}`; - spyOn(jQuery, 'ajax').and.callFake((request) => { - expect(request.url).toEqual(expectedUrl); - return sendDummyResponse(); - }); + mock.onGet(expectedUrl).reply(200, 'test'); Api.issueTemplate(namespace, project, templateKey, templateType, (error, response) => { - expect(error).toBe(null); - expect(response).toBe(dummyResponse); + expect(response).toBe('test'); done(); }); }); @@ -283,20 +230,14 @@ describe('Api', () => { const query = 'dummy query'; const options = { unused: 'option' }; const expectedUrl = `${dummyUrlRoot}/api/${dummyApiVersion}/users.json`; - const expectedData = Object.assign({ - search: query, - per_page: 20, - }, options); - spyOn(jQuery, 'ajax').and.callFake((request) => { - expect(request.url).toEqual(expectedUrl); - expect(request.dataType).toEqual('json'); - expect(request.data).toEqual(expectedData); - return sendDummyResponse(); - }); + mock.onGet(expectedUrl).reply(200, [{ + name: 'test', + }]); Api.users(query, options) - .then((response) => { - expect(response).toBe(dummyResponse); + .then(({ data }) => { + expect(data.length).toBe(1); + expect(data[0].name).toBe('test'); }) .then(done) .catch(done.fail); diff --git a/spec/javascripts/blob/viewer/index_spec.js b/spec/javascripts/blob/viewer/index_spec.js index cfa6650d85f..892411a6a40 100644 --- a/spec/javascripts/blob/viewer/index_spec.js +++ b/spec/javascripts/blob/viewer/index_spec.js @@ -1,28 +1,35 @@ /* eslint-disable no-new */ +import MockAdapter from 'axios-mock-adapter'; import BlobViewer from '~/blob/viewer/index'; +import axios from '~/lib/utils/axios_utils'; describe('Blob viewer', () => { let blob; + let mock; + preloadFixtures('snippets/show.html.raw'); beforeEach(() => { + mock = new MockAdapter(axios); + loadFixtures('snippets/show.html.raw'); $('#modal-upload-blob').remove(); blob = new BlobViewer(); - spyOn($, 'ajax').and.callFake(() => { - const d = $.Deferred(); - - d.resolve({ - html: '<div>testing</div>', - }); + mock.onGet('http://test.host/snippets/1.json?viewer=rich').reply(200, { + html: '<div>testing</div>', + }); - return d.promise(); + mock.onGet('http://test.host/snippets/1.json?viewer=simple').reply(200, { + html: '<div>testing</div>', }); + + spyOn(axios, 'get').and.callThrough(); }); afterEach(() => { + mock.restore(); location.hash = ''; }); @@ -30,7 +37,6 @@ describe('Blob viewer', () => { document.querySelector('.js-blob-viewer-switch-btn[data-viewer="simple"]').click(); setTimeout(() => { - expect($.ajax).toHaveBeenCalled(); expect( document.querySelector('.js-blob-viewer-switch-btn[data-viewer="simple"]') .classList.contains('hidden'), @@ -46,7 +52,6 @@ describe('Blob viewer', () => { new BlobViewer(); setTimeout(() => { - expect($.ajax).toHaveBeenCalled(); expect( document.querySelector('.js-blob-viewer-switch-btn[data-viewer="simple"]') .classList.contains('hidden'), @@ -64,12 +69,8 @@ describe('Blob viewer', () => { }); asyncClick() + .then(() => asyncClick()) .then(() => { - expect($.ajax).toHaveBeenCalled(); - return asyncClick(); - }) - .then(() => { - expect($.ajax.calls.count()).toBe(1); expect( document.querySelector('.blob-viewer[data-type="simple"]').getAttribute('data-loaded'), ).toBe('true'); @@ -122,7 +123,6 @@ describe('Blob viewer', () => { document.querySelector('.js-blob-viewer-switch-btn[data-viewer="simple"]').click(); setTimeout(() => { - expect($.ajax).toHaveBeenCalled(); expect( copyButton.classList.contains('disabled'), ).toBeFalsy(); @@ -135,8 +135,6 @@ describe('Blob viewer', () => { document.querySelector('.js-blob-viewer-switch-btn[data-viewer="simple"]').click(); setTimeout(() => { - expect($.ajax).toHaveBeenCalled(); - expect( copyButton.getAttribute('data-original-title'), ).toBe('Copy source to clipboard'); @@ -171,14 +169,14 @@ describe('Blob viewer', () => { it('sends AJAX request when switching to simple view', () => { blob.switchToViewer('simple'); - expect($.ajax).toHaveBeenCalled(); + expect(axios.get).toHaveBeenCalled(); }); it('does not send AJAX request when switching to rich view', () => { blob.switchToViewer('simple'); blob.switchToViewer('rich'); - expect($.ajax.calls.count()).toBe(1); + expect(axios.get.calls.count()).toBe(1); }); }); }); diff --git a/spec/javascripts/commits_spec.js b/spec/javascripts/commits_spec.js index d0176520440..44ec9e4eabf 100644 --- a/spec/javascripts/commits_spec.js +++ b/spec/javascripts/commits_spec.js @@ -1,4 +1,6 @@ import 'vendor/jquery.endless-scroll'; +import MockAdapter from 'axios-mock-adapter'; +import axios from '~/lib/utils/axios_utils'; import CommitsList from '~/commits'; describe('Commits List', () => { @@ -43,30 +45,47 @@ describe('Commits List', () => { describe('on entering input', () => { let ajaxSpy; + let mock; beforeEach(() => { CommitsList.init(25); CommitsList.searchField.val(''); spyOn(history, 'replaceState').and.stub(); - ajaxSpy = spyOn(jQuery, 'ajax').and.callFake((req) => { - req.success({ - data: '<li>Result</li>', - }); + mock = new MockAdapter(axios); + + mock.onGet('/h5bp/html5-boilerplate/commits/master').reply(200, { + html: '<li>Result</li>', }); + + ajaxSpy = spyOn(axios, 'get').and.callThrough(); + }); + + afterEach(() => { + mock.restore(); }); - it('should save the last search string', () => { + it('should save the last search string', (done) => { CommitsList.searchField.val('GitLab'); - CommitsList.filterResults(); - expect(ajaxSpy).toHaveBeenCalled(); - expect(CommitsList.lastSearch).toEqual('GitLab'); + CommitsList.filterResults() + .then(() => { + expect(ajaxSpy).toHaveBeenCalled(); + expect(CommitsList.lastSearch).toEqual('GitLab'); + + done(); + }) + .catch(done.fail); }); - it('should not make ajax call if the input does not change', () => { - CommitsList.filterResults(); - expect(ajaxSpy).not.toHaveBeenCalled(); - expect(CommitsList.lastSearch).toEqual(''); + it('should not make ajax call if the input does not change', (done) => { + CommitsList.filterResults() + .then(() => { + expect(ajaxSpy).not.toHaveBeenCalled(); + expect(CommitsList.lastSearch).toEqual(''); + + done(); + }) + .catch(done.fail); }); }); }); diff --git a/spec/javascripts/create_item_dropdown_spec.js b/spec/javascripts/create_item_dropdown_spec.js index c8b00a4f553..143137c23ec 100644 --- a/spec/javascripts/create_item_dropdown_spec.js +++ b/spec/javascripts/create_item_dropdown_spec.js @@ -18,54 +18,67 @@ describe('CreateItemDropdown', () => { preloadFixtures('static/create_item_dropdown.html.raw'); let $wrapperEl; + let createItemDropdown; + + function createItemAndClearInput(text) { + // Filter for the new item + $wrapperEl.find('.dropdown-input-field') + .val(text) + .trigger('input'); + + // Create the new item + const $createButton = $wrapperEl.find('.js-dropdown-create-new-item'); + $createButton.click(); + + // Clear out the filter + $wrapperEl.find('.dropdown-input-field') + .val('') + .trigger('input'); + } beforeEach(() => { loadFixtures('static/create_item_dropdown.html.raw'); $wrapperEl = $('.js-create-item-dropdown-fixture-root'); - - // eslint-disable-next-line no-new - new CreateItemDropdown({ - $dropdown: $wrapperEl.find('.js-dropdown-menu-toggle'), - defaultToggleLabel: 'All variables', - fieldName: 'variable[environment]', - getData: (term, callback) => { - callback(DROPDOWN_ITEM_DATA); - }, - }); }); afterEach(() => { $wrapperEl.remove(); }); - it('should have a dropdown item for each piece of data', () => { - // Get the data in the dropdown - $('.js-dropdown-menu-toggle').click(); + describe('items', () => { + beforeEach(() => { + createItemDropdown = new CreateItemDropdown({ + $dropdown: $wrapperEl.find('.js-dropdown-menu-toggle'), + defaultToggleLabel: 'All variables', + fieldName: 'variable[environment]', + getData: (term, callback) => { + callback(DROPDOWN_ITEM_DATA); + }, + }); + }); + + it('should have a dropdown item for each piece of data', () => { + // Get the data in the dropdown + $('.js-dropdown-menu-toggle').click(); - const $itemEls = $wrapperEl.find('.js-dropdown-content a'); - expect($itemEls.length).toEqual(DROPDOWN_ITEM_DATA.length); + const $itemEls = $wrapperEl.find('.js-dropdown-content a'); + expect($itemEls.length).toEqual(DROPDOWN_ITEM_DATA.length); + }); }); describe('created items', () => { const NEW_ITEM_TEXT = 'foobarbaz'; - function createItemAndClearInput(text) { - // Filter for the new item - $wrapperEl.find('.dropdown-input-field') - .val(text) - .trigger('input'); - - // Create the new item - const $createButton = $wrapperEl.find('.js-dropdown-create-new-item'); - $createButton.click(); - - // Clear out the filter - $wrapperEl.find('.dropdown-input-field') - .val('') - .trigger('input'); - } - beforeEach(() => { + createItemDropdown = new CreateItemDropdown({ + $dropdown: $wrapperEl.find('.js-dropdown-menu-toggle'), + defaultToggleLabel: 'All variables', + fieldName: 'variable[environment]', + getData: (term, callback) => { + callback(DROPDOWN_ITEM_DATA); + }, + }); + // Open the dropdown $('.js-dropdown-menu-toggle').click(); @@ -103,4 +116,68 @@ describe('CreateItemDropdown', () => { expect($itemEls.length).toEqual(DROPDOWN_ITEM_DATA.length); }); }); + + describe('clearDropdown()', () => { + beforeEach(() => { + createItemDropdown = new CreateItemDropdown({ + $dropdown: $wrapperEl.find('.js-dropdown-menu-toggle'), + defaultToggleLabel: 'All variables', + fieldName: 'variable[environment]', + getData: (term, callback) => { + callback(DROPDOWN_ITEM_DATA); + }, + }); + }); + + it('should clear all data and filter input', () => { + const filterInput = $wrapperEl.find('.dropdown-input-field'); + + // Get the data in the dropdown + $('.js-dropdown-menu-toggle').click(); + + // Filter for an item + filterInput + .val('one') + .trigger('input'); + + const $itemElsAfterFilter = $wrapperEl.find('.js-dropdown-content a'); + expect($itemElsAfterFilter.length).toEqual(1); + + createItemDropdown.clearDropdown(); + + const $itemElsAfterClear = $wrapperEl.find('.js-dropdown-content a'); + expect($itemElsAfterClear.length).toEqual(0); + expect(filterInput.val()).toEqual(''); + }); + }); + + describe('createNewItemFromValue option', () => { + beforeEach(() => { + createItemDropdown = new CreateItemDropdown({ + $dropdown: $wrapperEl.find('.js-dropdown-menu-toggle'), + defaultToggleLabel: 'All variables', + fieldName: 'variable[environment]', + getData: (term, callback) => { + callback(DROPDOWN_ITEM_DATA); + }, + createNewItemFromValue: newValue => ({ + title: `${newValue}-title`, + id: `${newValue}-id`, + text: `${newValue}-text`, + }), + }); + }); + + it('all items go through createNewItemFromValue', () => { + // Get the data in the dropdown + $('.js-dropdown-menu-toggle').click(); + + createItemAndClearInput('new-item'); + + const $itemEls = $wrapperEl.find('.js-dropdown-content a'); + expect($itemEls.length).toEqual(1 + DROPDOWN_ITEM_DATA.length); + expect($($itemEls[3]).text()).toEqual('new-item-text'); + expect($wrapperEl.find('.dropdown-toggle-text').text()).toEqual('new-item-title'); + }); + }); }); diff --git a/spec/javascripts/integrations/integration_settings_form_spec.js b/spec/javascripts/integrations/integration_settings_form_spec.js index 9033eb9ce02..d0fba908e34 100644 --- a/spec/javascripts/integrations/integration_settings_form_spec.js +++ b/spec/javascripts/integrations/integration_settings_form_spec.js @@ -1,3 +1,5 @@ +import MockAdaptor from 'axios-mock-adapter'; +import axios from '~/lib/utils/axios_utils'; import IntegrationSettingsForm from '~/integrations/integration_settings_form'; describe('IntegrationSettingsForm', () => { @@ -109,91 +111,117 @@ describe('IntegrationSettingsForm', () => { describe('testSettings', () => { let integrationSettingsForm; let formData; + let mock; beforeEach(() => { + mock = new MockAdaptor(axios); + + spyOn(axios, 'put').and.callThrough(); + integrationSettingsForm = new IntegrationSettingsForm('.js-integration-settings-form'); formData = integrationSettingsForm.$form.serialize(); }); - it('should make an ajax request with provided `formData`', () => { - const deferred = $.Deferred(); - spyOn($, 'ajax').and.returnValue(deferred.promise()); + afterEach(() => { + mock.restore(); + }); - integrationSettingsForm.testSettings(formData); + it('should make an ajax request with provided `formData`', (done) => { + integrationSettingsForm.testSettings(formData) + .then(() => { + expect(axios.put).toHaveBeenCalledWith(integrationSettingsForm.testEndPoint, formData); - expect($.ajax).toHaveBeenCalledWith({ - type: 'PUT', - url: integrationSettingsForm.testEndPoint, - data: formData, - }); + done(); + }) + .catch(done.fail); }); - it('should show error Flash with `Save anyway` action if ajax request responds with error in test', () => { + it('should show error Flash with `Save anyway` action if ajax request responds with error in test', (done) => { const errorMessage = 'Test failed.'; - const deferred = $.Deferred(); - spyOn($, 'ajax').and.returnValue(deferred.promise()); - - integrationSettingsForm.testSettings(formData); + mock.onPut(integrationSettingsForm.testEndPoint).reply(200, { + error: true, + message: errorMessage, + service_response: 'some error', + }); - deferred.resolve({ error: true, message: errorMessage, service_response: 'some error' }); + integrationSettingsForm.testSettings(formData) + .then(() => { + const $flashContainer = $('.flash-container'); + expect($flashContainer.find('.flash-text').text().trim()).toEqual('Test failed. some error'); + expect($flashContainer.find('.flash-action')).toBeDefined(); + expect($flashContainer.find('.flash-action').text().trim()).toEqual('Save anyway'); - const $flashContainer = $('.flash-container'); - expect($flashContainer.find('.flash-text').text().trim()).toEqual('Test failed. some error'); - expect($flashContainer.find('.flash-action')).toBeDefined(); - expect($flashContainer.find('.flash-action').text().trim()).toEqual('Save anyway'); + done(); + }) + .catch(done.fail); }); - it('should submit form if ajax request responds without any error in test', () => { - const deferred = $.Deferred(); - spyOn($, 'ajax').and.returnValue(deferred.promise()); + it('should submit form if ajax request responds without any error in test', (done) => { + spyOn(integrationSettingsForm.$form, 'submit'); - integrationSettingsForm.testSettings(formData); + mock.onPut(integrationSettingsForm.testEndPoint).reply(200, { + error: false, + }); - spyOn(integrationSettingsForm.$form, 'submit'); - deferred.resolve({ error: false }); + integrationSettingsForm.testSettings(formData) + .then(() => { + expect(integrationSettingsForm.$form.submit).toHaveBeenCalled(); - expect(integrationSettingsForm.$form.submit).toHaveBeenCalled(); + done(); + }) + .catch(done.fail); }); - it('should submit form when clicked on `Save anyway` action of error Flash', () => { - const errorMessage = 'Test failed.'; - const deferred = $.Deferred(); - spyOn($, 'ajax').and.returnValue(deferred.promise()); + it('should submit form when clicked on `Save anyway` action of error Flash', (done) => { + spyOn(integrationSettingsForm.$form, 'submit'); - integrationSettingsForm.testSettings(formData); + const errorMessage = 'Test failed.'; + mock.onPut(integrationSettingsForm.testEndPoint).reply(200, { + error: true, + message: errorMessage, + }); - deferred.resolve({ error: true, message: errorMessage }); + integrationSettingsForm.testSettings(formData) + .then(() => { + const $flashAction = $('.flash-container .flash-action'); + expect($flashAction).toBeDefined(); - const $flashAction = $('.flash-container .flash-action'); - expect($flashAction).toBeDefined(); + $flashAction.get(0).click(); + }) + .then(() => { + expect(integrationSettingsForm.$form.submit).toHaveBeenCalled(); - spyOn(integrationSettingsForm.$form, 'submit'); - $flashAction.get(0).click(); - expect(integrationSettingsForm.$form.submit).toHaveBeenCalled(); + done(); + }) + .catch(done.fail); }); - it('should show error Flash if ajax request failed', () => { + it('should show error Flash if ajax request failed', (done) => { const errorMessage = 'Something went wrong on our end.'; - const deferred = $.Deferred(); - spyOn($, 'ajax').and.returnValue(deferred.promise()); - integrationSettingsForm.testSettings(formData); + mock.onPut(integrationSettingsForm.testEndPoint).networkError(); - deferred.reject(); + integrationSettingsForm.testSettings(formData) + .then(() => { + expect($('.flash-container .flash-text').text().trim()).toEqual(errorMessage); - expect($('.flash-container .flash-text').text().trim()).toEqual(errorMessage); + done(); + }) + .catch(done.fail); }); - it('should always call `toggleSubmitBtnState` with `false` once request is completed', () => { - const deferred = $.Deferred(); - spyOn($, 'ajax').and.returnValue(deferred.promise()); - - integrationSettingsForm.testSettings(formData); + it('should always call `toggleSubmitBtnState` with `false` once request is completed', (done) => { + mock.onPut(integrationSettingsForm.testEndPoint).networkError(); spyOn(integrationSettingsForm, 'toggleSubmitBtnState'); - deferred.reject(); - expect(integrationSettingsForm.toggleSubmitBtnState).toHaveBeenCalledWith(false); + integrationSettingsForm.testSettings(formData) + .then(() => { + expect(integrationSettingsForm.toggleSubmitBtnState).toHaveBeenCalledWith(false); + + done(); + }) + .catch(done.fail); }); }); }); diff --git a/spec/javascripts/issuable_spec.js b/spec/javascripts/issuable_spec.js index 5a9112716f4..d53ffecbd35 100644 --- a/spec/javascripts/issuable_spec.js +++ b/spec/javascripts/issuable_spec.js @@ -1,3 +1,5 @@ +import MockAdaptor from 'axios-mock-adapter'; +import axios from '~/lib/utils/axios_utils'; import IssuableIndex from '~/issuable_index'; describe('Issuable', () => { @@ -19,6 +21,8 @@ describe('Issuable', () => { }); describe('resetIncomingEmailToken', () => { + let mock; + beforeEach(() => { const element = document.createElement('a'); element.classList.add('incoming-email-token-reset'); @@ -30,14 +34,28 @@ describe('Issuable', () => { document.body.appendChild(input); Issuable = new IssuableIndex('issue_'); + + mock = new MockAdaptor(axios); + + mock.onPut('foo').reply(200, { + new_address: 'testing123', + }); }); - it('should send request to reset email token', () => { - spyOn(jQuery, 'ajax').and.callThrough(); + afterEach(() => { + mock.restore(); + }); + + it('should send request to reset email token', (done) => { + spyOn(axios, 'put').and.callThrough(); document.querySelector('.incoming-email-token-reset').click(); - expect(jQuery.ajax).toHaveBeenCalled(); - expect(jQuery.ajax.calls.argsFor(0)[0].url).toEqual('foo'); + setTimeout(() => { + expect(axios.put).toHaveBeenCalledWith('foo'); + expect($('#issuable_email').val()).toBe('testing123'); + + done(); + }); }); }); }); diff --git a/spec/javascripts/lib/utils/common_utils_spec.js b/spec/javascripts/lib/utils/common_utils_spec.js index bb71c181b3d..50dac3def60 100644 --- a/spec/javascripts/lib/utils/common_utils_spec.js +++ b/spec/javascripts/lib/utils/common_utils_spec.js @@ -1,6 +1,8 @@ /* eslint-disable promise/catch-or-return */ import axios from '~/lib/utils/axios_utils'; import * as commonUtils from '~/lib/utils/common_utils'; +import axios from '~/lib/utils/axios_utils'; +import MockAdapter from 'axios-mock-adapter'; describe('common_utils', () => { describe('parseUrl', () => { @@ -413,37 +415,48 @@ describe('common_utils', () => { describe('setCiStatusFavicon', () => { const BUILD_URL = `${gl.TEST_HOST}/frontend-fixtures/builds-project/-/jobs/1/status.json`; + let mock; beforeEach(() => { const favicon = document.createElement('link'); favicon.setAttribute('id', 'favicon'); document.body.appendChild(favicon); + mock = new MockAdapter(axios); }); afterEach(() => { + mock.restore(); document.body.removeChild(document.getElementById('favicon')); }); - it('should reset favicon in case of error', () => { - const favicon = document.getElementById('favicon'); - spyOn($, 'ajax').and.callFake(function (options) { - options.error(); - expect(favicon.getAttribute('href')).toEqual('null'); - }); + it('should reset favicon in case of error', (done) => { + mock.onGet(BUILD_URL).networkError(); - commonUtils.setCiStatusFavicon(BUILD_URL); + commonUtils.setCiStatusFavicon(BUILD_URL) + .then(() => { + const favicon = document.getElementById('favicon'); + expect(favicon.getAttribute('href')).toEqual('null'); + done(); + }) + // Error is already caught in catch() block of setCiStatusFavicon, + // It won't throw another error for us to catch + .catch(done.fail); }); - it('should set page favicon to CI status favicon based on provided status', () => { + it('should set page favicon to CI status favicon based on provided status', (done) => { const FAVICON_PATH = '//icon_status_success'; - const favicon = document.getElementById('favicon'); - spyOn($, 'ajax').and.callFake(function (options) { - options.success({ favicon: FAVICON_PATH }); - expect(favicon.getAttribute('href')).toEqual(FAVICON_PATH); + mock.onGet(BUILD_URL).reply(200, { + favicon: FAVICON_PATH, }); - commonUtils.setCiStatusFavicon(BUILD_URL); + commonUtils.setCiStatusFavicon(BUILD_URL) + .then(() => { + const favicon = document.getElementById('favicon'); + expect(favicon.getAttribute('href')).toEqual(FAVICON_PATH); + done(); + }) + .catch(done.fail); }); }); diff --git a/spec/javascripts/lib/utils/users_cache_spec.js b/spec/javascripts/lib/utils/users_cache_spec.js index ec6ea35952b..50371c8c5f6 100644 --- a/spec/javascripts/lib/utils/users_cache_spec.js +++ b/spec/javascripts/lib/utils/users_cache_spec.js @@ -92,7 +92,9 @@ describe('UsersCache', () => { apiSpy = (query, options) => { expect(query).toBe(''); expect(options).toEqual({ username: dummyUsername }); - return Promise.resolve([dummyUser]); + return Promise.resolve({ + data: [dummyUser], + }); }; UsersCache.retrieve(dummyUsername) diff --git a/spec/javascripts/repo/components/new_dropdown/modal_spec.js b/spec/javascripts/repo/components/new_dropdown/modal_spec.js index 233cca06ed0..8bbc3100357 100644 --- a/spec/javascripts/repo/components/new_dropdown/modal_spec.js +++ b/spec/javascripts/repo/components/new_dropdown/modal_spec.js @@ -18,8 +18,10 @@ describe('new file modal component', () => { })); spyOn(service, 'getBranchData').and.returnValue(Promise.resolve({ - commit: { - id: '123branch', + data: { + commit: { + id: '123branch', + }, }, })); diff --git a/spec/javascripts/repo/components/new_dropdown/upload_spec.js b/spec/javascripts/repo/components/new_dropdown/upload_spec.js index 788c08e5279..667112ab21a 100644 --- a/spec/javascripts/repo/components/new_dropdown/upload_spec.js +++ b/spec/javascripts/repo/components/new_dropdown/upload_spec.js @@ -17,8 +17,10 @@ describe('new dropdown upload', () => { })); spyOn(service, 'getBranchData').and.returnValue(Promise.resolve({ - commit: { - id: '123branch', + data: { + commit: { + id: '123branch', + }, }, })); diff --git a/spec/javascripts/repo/components/repo_commit_section_spec.js b/spec/javascripts/repo/components/repo_commit_section_spec.js index 676ac09f2c9..93e94b4f24c 100644 --- a/spec/javascripts/repo/components/repo_commit_section_spec.js +++ b/spec/javascripts/repo/components/repo_commit_section_spec.js @@ -87,8 +87,10 @@ describe('RepoCommitSection', () => { changedFiles = JSON.parse(JSON.stringify(vm.$store.getters.changedFiles)); spyOn(service, 'commit').and.returnValue(Promise.resolve({ - short_id: '1', - stats: {}, + data: { + short_id: '1', + stats: {}, + }, })); }); diff --git a/spec/javascripts/repo/stores/actions_spec.js b/spec/javascripts/repo/stores/actions_spec.js index 8d830c67290..f678967b092 100644 --- a/spec/javascripts/repo/stores/actions_spec.js +++ b/spec/javascripts/repo/stores/actions_spec.js @@ -178,7 +178,9 @@ describe('Multi-file store actions', () => { it('calls service', (done) => { spyOn(service, 'getBranchData').and.returnValue(Promise.resolve({ - commit: { id: '123' }, + data: { + commit: { id: '123' }, + }, })); store.dispatch('checkCommitStatus') @@ -192,7 +194,9 @@ describe('Multi-file store actions', () => { it('returns true if current ref does not equal returned ID', (done) => { spyOn(service, 'getBranchData').and.returnValue(Promise.resolve({ - commit: { id: '123' }, + data: { + commit: { id: '123' }, + }, })); store.dispatch('checkCommitStatus') @@ -206,7 +210,9 @@ describe('Multi-file store actions', () => { it('returns false if current ref equals returned ID', (done) => { spyOn(service, 'getBranchData').and.returnValue(Promise.resolve({ - commit: { id: '1' }, + data: { + commit: { id: '1' }, + }, })); store.dispatch('checkCommitStatus') @@ -250,13 +256,15 @@ describe('Multi-file store actions', () => { describe('success', () => { beforeEach(() => { spyOn(service, 'commit').and.returnValue(Promise.resolve({ - id: '123456', - short_id: '123', - message: 'test message', - committed_date: 'date', - stats: { - additions: '1', - deletions: '2', + data: { + id: '123456', + short_id: '123', + message: 'test message', + committed_date: 'date', + stats: { + additions: '1', + deletions: '2', + }, }, })); }); @@ -324,7 +332,9 @@ describe('Multi-file store actions', () => { describe('failed', () => { beforeEach(() => { spyOn(service, 'commit').and.returnValue(Promise.resolve({ - message: 'failed message', + data: { + message: 'failed message', + }, })); }); diff --git a/spec/lib/gitaly/server_spec.rb b/spec/lib/gitaly/server_spec.rb new file mode 100644 index 00000000000..ed5d56e91d4 --- /dev/null +++ b/spec/lib/gitaly/server_spec.rb @@ -0,0 +1,30 @@ +require 'spec_helper' + +describe Gitaly::Server do + describe '.all' do + let(:storages) { Gitlab.config.repositories.storages } + + it 'includes all storages' do + expect(storages.count).to eq(described_class.all.count) + expect(storages.keys).to eq(described_class.all.map(&:storage)) + end + end + + subject { described_class.all.first } + + it { is_expected.to respond_to(:server_version) } + it { is_expected.to respond_to(:git_binary_version) } + it { is_expected.to respond_to(:up_to_date?) } + it { is_expected.to respond_to(:address) } + + describe 'request memoization' do + context 'when requesting multiple properties', :request_store do + it 'uses memoization for the info request' do + expect do + subject.server_version + subject.up_to_date? + end.to change { Gitlab::GitalyClient.get_request_count }.by(1) + end + end + end +end diff --git a/spec/lib/gitlab/git/repository_spec.rb b/spec/lib/gitlab/git/repository_spec.rb index 935d1df6dad..bf01e6ef8e8 100644 --- a/spec/lib/gitlab/git/repository_spec.rb +++ b/spec/lib/gitlab/git/repository_spec.rb @@ -2,6 +2,7 @@ require "spec_helper" describe Gitlab::Git::Repository, seed_helper: true do include Gitlab::EncodingHelper + using RSpec::Parameterized::TableSyntax shared_examples 'wrapping gRPC errors' do |gitaly_client_class, gitaly_client_method| it 'wraps gRPC not found error' do @@ -442,6 +443,7 @@ describe Gitlab::Git::Repository, seed_helper: true do shared_examples 'simple commit counting' do it { expect(repository.commit_count("master")).to eq(25) } it { expect(repository.commit_count("feature")).to eq(9) } + it { expect(repository.commit_count("does-not-exist")).to eq(0) } end context 'when Gitaly commit_count feature is enabled' do @@ -560,35 +562,39 @@ describe Gitlab::Git::Repository, seed_helper: true do end describe '#delete_refs' do - before(:all) do - @repo = Gitlab::Git::Repository.new('default', TEST_MUTABLE_REPO_PATH, '') - end + shared_examples 'deleting refs' do + let(:repo) { Gitlab::Git::Repository.new('default', TEST_MUTABLE_REPO_PATH, '') } - it 'deletes the ref' do - @repo.delete_refs('refs/heads/feature') + after do + ensure_seeds + end - expect(@repo.rugged.references['refs/heads/feature']).to be_nil - end + it 'deletes the ref' do + repo.delete_refs('refs/heads/feature') - it 'deletes all refs' do - refs = %w[refs/heads/wip refs/tags/v1.1.0] - @repo.delete_refs(*refs) + expect(repo.rugged.references['refs/heads/feature']).to be_nil + end + + it 'deletes all refs' do + refs = %w[refs/heads/wip refs/tags/v1.1.0] + repo.delete_refs(*refs) - refs.each do |ref| - expect(@repo.rugged.references[ref]).to be_nil + refs.each do |ref| + expect(repo.rugged.references[ref]).to be_nil + end end - end - it 'raises an error if it failed' do - expect(@repo).to receive(:popen).and_return(['Error', 1]) + it 'raises an error if it failed' do + expect { repo.delete_refs('refs\heads\fix') }.to raise_error(Gitlab::Git::Repository::GitError) + end + end - expect do - @repo.delete_refs('refs/heads/fix') - end.to raise_error(Gitlab::Git::Repository::GitError) + context 'when Gitaly delete_refs feature is enabled' do + it_behaves_like 'deleting refs' end - after(:all) do - ensure_seeds + context 'when Gitaly delete_refs feature is disabled', :disable_gitaly do + it_behaves_like 'deleting refs' end end @@ -1032,6 +1038,29 @@ describe Gitlab::Git::Repository, seed_helper: true do it { is_expected.to eq(17) } end + describe '#merge_base' do + shared_examples '#merge_base' do + where(:from, :to, :result) do + '570e7b2abdd848b95f2f578043fc23bd6f6fd24d' | '40f4a7a617393735a95a0bb67b08385bc1e7c66d' | '570e7b2abdd848b95f2f578043fc23bd6f6fd24d' + '40f4a7a617393735a95a0bb67b08385bc1e7c66d' | '570e7b2abdd848b95f2f578043fc23bd6f6fd24d' | '570e7b2abdd848b95f2f578043fc23bd6f6fd24d' + '40f4a7a617393735a95a0bb67b08385bc1e7c66d' | 'foobar' | nil + 'foobar' | '40f4a7a617393735a95a0bb67b08385bc1e7c66d' | nil + end + + with_them do + it { expect(repository.merge_base(from, to)).to eq(result) } + end + end + + context 'with gitaly' do + it_behaves_like '#merge_base' + end + + context 'without gitaly', :skip_gitaly_mock do + it_behaves_like '#merge_base' + end + end + describe '#count_commits' do shared_examples 'extended commit counting' do context 'with after timestamp' do diff --git a/spec/lib/gitlab/git/tree_spec.rb b/spec/lib/gitlab/git/tree_spec.rb index 86f7bcb8e38..001e406a930 100644 --- a/spec/lib/gitlab/git/tree_spec.rb +++ b/spec/lib/gitlab/git/tree_spec.rb @@ -80,22 +80,18 @@ describe Gitlab::Git::Tree, seed_helper: true do end describe '#where' do - context 'with gitaly disabled' do - before do - allow(Gitlab::GitalyClient).to receive(:feature_enabled?).and_return(false) - end - - it 'calls #tree_entries_from_rugged' do - expect(described_class).to receive(:tree_entries_from_rugged) - - described_class.where(repository, SeedRepo::Commit::ID, '/') + shared_examples '#where' do + it 'returns an empty array when called with an invalid ref' do + expect(described_class.where(repository, 'foobar-does-not-exist')).to eq([]) end end - it 'gets the tree entries from GitalyClient' do - expect_any_instance_of(Gitlab::GitalyClient::CommitService).to receive(:tree_entries) + context 'with gitaly' do + it_behaves_like '#where' + end - described_class.where(repository, SeedRepo::Commit::ID, '/') + context 'without gitaly', :skip_gitaly_mock do + it_behaves_like '#where' end end end diff --git a/spec/lib/gitlab/gitaly_client/ref_service_spec.rb b/spec/lib/gitlab/gitaly_client/ref_service_spec.rb index 951e146a30a..257e4c50f2d 100644 --- a/spec/lib/gitlab/gitaly_client/ref_service_spec.rb +++ b/spec/lib/gitlab/gitaly_client/ref_service_spec.rb @@ -112,7 +112,7 @@ describe Gitlab::GitalyClient::RefService do expect_any_instance_of(Gitaly::RefService::Stub) .to receive(:delete_refs) .with(gitaly_request_with_params(except_with_prefix: prefixes), kind_of(Hash)) - .and_return(double('delete_refs_response')) + .and_return(double('delete_refs_response', git_error: "")) client.delete_refs(except_with_prefixes: prefixes) end diff --git a/spec/lib/gitlab/metrics/method_call_spec.rb b/spec/lib/gitlab/metrics/method_call_spec.rb index 41a9d1d9c90..d9379cfe674 100644 --- a/spec/lib/gitlab/metrics/method_call_spec.rb +++ b/spec/lib/gitlab/metrics/method_call_spec.rb @@ -5,6 +5,10 @@ describe Gitlab::Metrics::MethodCall do let(:method_call) { described_class.new('Foo#bar', :Foo, '#bar', transaction) } describe '#measure' do + after do + described_class.reload_metric!(:gitlab_method_call_duration_seconds) + end + it 'measures the performance of the supplied block' do method_call.measure { 'foo' } @@ -20,8 +24,6 @@ describe Gitlab::Metrics::MethodCall do context 'prometheus instrumentation is enabled' do before do - allow(Feature.get(:prometheus_metrics_method_instrumentation)).to receive(:enabled?).and_call_original - described_class.measurement_enabled_cache_expires_at.value = Time.now.to_i - 1 Feature.get(:prometheus_metrics_method_instrumentation).enable end @@ -31,30 +33,12 @@ describe Gitlab::Metrics::MethodCall do end end - it 'caches subsequent invocations of feature check' do - 10.times do - method_call.measure { 'foo' } - end - - expect(Feature.get(:prometheus_metrics_method_instrumentation)).to have_received(:enabled?).once - end - - it 'expires feature check cache after 1 minute' do - method_call.measure { 'foo' } - - Timecop.travel(1.minute.from_now) do - method_call.measure { 'foo' } - end - - Timecop.travel(1.minute.from_now + 1.second) do - method_call.measure { 'foo' } - end - - expect(Feature.get(:prometheus_metrics_method_instrumentation)).to have_received(:enabled?).twice + it 'metric is not a NullMetric' do + expect(described_class).not_to be_instance_of(Gitlab::Metrics::NullMetric) end it 'observes the performance of the supplied block' do - expect(described_class.call_duration_histogram) + expect(described_class.gitlab_method_call_duration_seconds) .to receive(:observe) .with({ module: :Foo, method: '#bar' }, be_a_kind_of(Numeric)) @@ -64,14 +48,12 @@ describe Gitlab::Metrics::MethodCall do context 'prometheus instrumentation is disabled' do before do - described_class.measurement_enabled_cache_expires_at.value = Time.now.to_i - 1 - Feature.get(:prometheus_metrics_method_instrumentation).disable end - it 'does not observe the performance' do - expect(described_class.call_duration_histogram) - .not_to receive(:observe) + it 'observes using NullMetric' do + expect(described_class.gitlab_method_call_duration_seconds).to be_instance_of(Gitlab::Metrics::NullMetric) + expect(described_class.gitlab_method_call_duration_seconds).to receive(:observe) method_call.measure { 'foo' } end @@ -81,12 +63,10 @@ describe Gitlab::Metrics::MethodCall do context 'when measurement is below threshold' do before do allow(method_call).to receive(:above_threshold?).and_return(false) - - Feature.get(:prometheus_metrics_method_instrumentation).enable end it 'does not observe the performance' do - expect(described_class.call_duration_histogram) + expect(described_class.gitlab_method_call_duration_seconds) .not_to receive(:observe) method_call.measure { 'foo' } @@ -96,7 +76,7 @@ describe Gitlab::Metrics::MethodCall do describe '#to_metric' do it 'returns a Metric instance' do - expect(method_call).to receive(:real_time).and_return(4.0001) + expect(method_call).to receive(:real_time).and_return(4.0001).twice expect(method_call).to receive(:cpu_time).and_return(3.0001) method_call.measure { 'foo' } diff --git a/spec/lib/gitlab/metrics/methods_spec.rb b/spec/lib/gitlab/metrics/methods_spec.rb new file mode 100644 index 00000000000..9d41ed2442b --- /dev/null +++ b/spec/lib/gitlab/metrics/methods_spec.rb @@ -0,0 +1,137 @@ +require 'spec_helper' + +describe Gitlab::Metrics::Methods do + subject { Class.new { include Gitlab::Metrics::Methods } } + + shared_context 'metric' do |metric_type, *args| + let(:docstring) { 'description' } + let(:metric_name) { :sample_metric } + + describe "#define_#{metric_type}" do + define_method(:call_define_metric_method) do |**args| + subject.__send__("define_#{metric_type}", metric_name, **args) + end + + context 'metrics access method not defined' do + it "defines metrics accessing method" do + expect(subject).not_to respond_to(metric_name) + + call_define_metric_method(docstring: docstring) + + expect(subject).to respond_to(metric_name) + end + end + + context 'metrics access method defined' do + before do + call_define_metric_method(docstring: docstring) + end + + it 'raises error when trying to redefine method' do + expect { call_define_metric_method(docstring: docstring) }.to raise_error(ArgumentError) + end + + context 'metric is not cached' do + it 'calls fetch_metric' do + expect(subject).to receive(:init_metric).with(metric_type, metric_name, docstring: docstring) + + subject.public_send(metric_name) + end + end + + context 'metric is cached' do + before do + subject.public_send(metric_name) + end + + it 'returns cached metric' do + expect(subject).not_to receive(:init_metric) + + subject.public_send(metric_name) + end + end + end + end + + describe "#fetch_#{metric_type}" do + let(:null_metric) { Gitlab::Metrics::NullMetric.instance } + + define_method(:call_fetch_metric_method) do |**args| + subject.__send__("fetch_#{metric_type}", metric_name, **args) + end + + context "when #{metric_type} is not cached" do + it 'initializes counter metric' do + allow(Gitlab::Metrics).to receive(metric_type).and_return(null_metric) + + call_fetch_metric_method(docstring: docstring) + + expect(Gitlab::Metrics).to have_received(metric_type).with(metric_name, docstring, *args) + end + end + + context "when #{metric_type} is cached" do + before do + call_fetch_metric_method(docstring: docstring) + end + + it 'uses class metric cache' do + expect(Gitlab::Metrics).not_to receive(metric_type) + + call_fetch_metric_method(docstring: docstring) + end + + context 'when metric is reloaded' do + before do + subject.reload_metric!(metric_name) + end + + it "initializes #{metric_type} metric" do + allow(Gitlab::Metrics).to receive(metric_type).and_return(null_metric) + + call_fetch_metric_method(docstring: docstring) + + expect(Gitlab::Metrics).to have_received(metric_type).with(metric_name, docstring, *args) + end + end + end + + context 'when metric is configured with feature' do + let(:feature_name) { :some_metric_feature } + let(:metric) { call_fetch_metric_method(docstring: docstring, with_feature: feature_name) } + + context 'when feature is enabled' do + before do + Feature.get(feature_name).enable + end + + it "initializes #{metric_type} metric" do + allow(Gitlab::Metrics).to receive(metric_type).and_return(null_metric) + + metric + + expect(Gitlab::Metrics).to have_received(metric_type).with(metric_name, docstring, *args) + end + end + + context 'when feature is disabled' do + before do + Feature.get(feature_name).disable + end + + it "returns NullMetric" do + allow(Gitlab::Metrics).to receive(metric_type) + + expect(metric).to be_instance_of(Gitlab::Metrics::NullMetric) + + expect(Gitlab::Metrics).not_to have_received(metric_type) + end + end + end + end + end + + include_examples 'metric', :counter, {} + include_examples 'metric', :gauge, {}, :all + include_examples 'metric', :histogram, {}, [0.005, 0.01, 0.1, 1, 10] +end diff --git a/spec/lib/gitlab/metrics/samplers/ruby_sampler_spec.rb b/spec/lib/gitlab/metrics/samplers/ruby_sampler_spec.rb index 375cbf8a9ca..54781dd52fc 100644 --- a/spec/lib/gitlab/metrics/samplers/ruby_sampler_spec.rb +++ b/spec/lib/gitlab/metrics/samplers/ruby_sampler_spec.rb @@ -2,6 +2,11 @@ require 'spec_helper' describe Gitlab::Metrics::Samplers::RubySampler do let(:sampler) { described_class.new(5) } + let(:null_metric) { double('null_metric', set: nil, observe: nil) } + + before do + allow(Gitlab::Metrics::NullMetric).to receive(:instance).and_return(null_metric) + end after do Allocations.stop if Gitlab::Metrics.mri? @@ -17,12 +22,9 @@ describe Gitlab::Metrics::Samplers::RubySampler do end it 'adds a metric containing the memory usage' do - expect(Gitlab::Metrics::System).to receive(:memory_usage) - .and_return(9000) + expect(Gitlab::Metrics::System).to receive(:memory_usage).and_return(9000) - expect(sampler.metrics[:memory_usage]).to receive(:set) - .with({}, 9000) - .and_call_original + expect(sampler.metrics[:memory_usage]).to receive(:set).with({}, 9000) sampler.sample end @@ -31,9 +33,7 @@ describe Gitlab::Metrics::Samplers::RubySampler do expect(Gitlab::Metrics::System).to receive(:file_descriptor_count) .and_return(4) - expect(sampler.metrics[:file_descriptors]).to receive(:set) - .with({}, 4) - .and_call_original + expect(sampler.metrics[:file_descriptors]).to receive(:set).with({}, 4) sampler.sample end @@ -49,16 +49,14 @@ describe Gitlab::Metrics::Samplers::RubySampler do it 'adds a metric containing garbage collection time statistics' do expect(GC::Profiler).to receive(:total_time).and_return(0.24) - expect(sampler.metrics[:total_time]).to receive(:set) - .with({}, 240) - .and_call_original + expect(sampler.metrics[:total_time]).to receive(:set).with({}, 240) sampler.sample end it 'adds a metric containing garbage collection statistics' do GC.stat.keys.each do |key| - expect(sampler.metrics[key]).to receive(:set).with({}, anything).and_call_original + expect(sampler.metrics[key]).to receive(:set).with({}, anything) end sampler.sample diff --git a/spec/lib/gitlab/metrics/subscribers/action_view_spec.rb b/spec/lib/gitlab/metrics/subscribers/action_view_spec.rb index eca75a4fac1..9f3af1acef7 100644 --- a/spec/lib/gitlab/metrics/subscribers/action_view_spec.rb +++ b/spec/lib/gitlab/metrics/subscribers/action_view_spec.rb @@ -32,7 +32,7 @@ describe Gitlab::Metrics::Subscribers::ActionView do end it 'observes view rendering time' do - expect(subscriber.send(:metric_view_rendering_duration_seconds)) + expect(described_class.gitlab_view_rendering_duration_seconds) .to receive(:observe) .with({ view: 'app/views/x.html.haml' }, 2.1) diff --git a/spec/lib/gitlab/metrics/subscribers/active_record_spec.rb b/spec/lib/gitlab/metrics/subscribers/active_record_spec.rb index 9b3698fb4a8..4e7bd433a9c 100644 --- a/spec/lib/gitlab/metrics/subscribers/active_record_spec.rb +++ b/spec/lib/gitlab/metrics/subscribers/active_record_spec.rb @@ -25,7 +25,7 @@ describe Gitlab::Metrics::Subscribers::ActiveRecord do expect(subscriber).to receive(:current_transaction) .at_least(:once) .and_return(transaction) - expect(subscriber.send(:metric_sql_duration_seconds)).to receive(:observe).with({}, 0.002) + expect(described_class.send(:gitlab_sql_duration_seconds)).to receive(:observe).with({}, 0.002) subscriber.sql(event) end diff --git a/spec/lib/gitlab/metrics/subscribers/rails_cache_spec.rb b/spec/lib/gitlab/metrics/subscribers/rails_cache_spec.rb index 58e28592cf9..6795c1ab56b 100644 --- a/spec/lib/gitlab/metrics/subscribers/rails_cache_spec.rb +++ b/spec/lib/gitlab/metrics/subscribers/rails_cache_spec.rb @@ -144,7 +144,10 @@ describe Gitlab::Metrics::Subscribers::RailsCache do end context 'with a transaction' do + let(:metric_cache_misses_total) { double('metric_cache_misses_total', increment: nil) } + before do + allow(subscriber).to receive(:metric_cache_misses_total).and_return(metric_cache_misses_total) allow(subscriber).to receive(:current_transaction) .and_return(transaction) end @@ -157,9 +160,9 @@ describe Gitlab::Metrics::Subscribers::RailsCache do end it 'increments the cache_read_miss total' do - expect(subscriber.send(:metric_cache_misses_total)).to receive(:increment).with({}) - subscriber.cache_generate(event) + + expect(metric_cache_misses_total).to have_received(:increment).with({}) end end end diff --git a/spec/lib/gitlab/metrics_spec.rb b/spec/lib/gitlab/metrics_spec.rb index 1619fbd88b1..9e405e9f736 100644 --- a/spec/lib/gitlab/metrics_spec.rb +++ b/spec/lib/gitlab/metrics_spec.rb @@ -20,7 +20,7 @@ describe Gitlab::Metrics do context 'prometheus metrics enabled in config' do before do - allow(described_class).to receive(:current_application_settings).and_return(prometheus_metrics_enabled: true) + allow(Gitlab::CurrentSettings).to receive(:current_application_settings).and_return(prometheus_metrics_enabled: true) end context 'when metrics folder is present' do diff --git a/spec/lib/gitlab/o_auth/user_spec.rb b/spec/lib/gitlab/o_auth/user_spec.rb index 45fff4c5787..03e0a9e2a03 100644 --- a/spec/lib/gitlab/o_auth/user_spec.rb +++ b/spec/lib/gitlab/o_auth/user_spec.rb @@ -44,6 +44,18 @@ describe Gitlab::OAuth::User do let(:provider) { 'twitter' } + describe 'when account exists on server' do + it 'does not mark the user as external' do + create(:omniauth_user, extern_uid: 'my-uid', provider: provider) + stub_omniauth_config(allow_single_sign_on: [provider], external_providers: [provider]) + + oauth_user.save + + expect(gl_user).to be_valid + expect(gl_user.external).to be_falsey + end + end + describe 'signup' do context 'when signup is disabled' do before do @@ -51,7 +63,7 @@ describe Gitlab::OAuth::User do end it 'creates the user' do - stub_omniauth_config(allow_single_sign_on: ['twitter']) + stub_omniauth_config(allow_single_sign_on: [provider]) oauth_user.save @@ -65,7 +77,7 @@ describe Gitlab::OAuth::User do end it 'creates and confirms the user anyway' do - stub_omniauth_config(allow_single_sign_on: ['twitter']) + stub_omniauth_config(allow_single_sign_on: [provider]) oauth_user.save @@ -75,7 +87,7 @@ describe Gitlab::OAuth::User do end it 'marks user as having password_automatically_set' do - stub_omniauth_config(allow_single_sign_on: ['twitter'], external_providers: ['twitter']) + stub_omniauth_config(allow_single_sign_on: [provider], external_providers: [provider]) oauth_user.save @@ -86,7 +98,7 @@ describe Gitlab::OAuth::User do shared_examples 'to verify compliance with allow_single_sign_on' do context 'provider is marked as external' do it 'marks user as external' do - stub_omniauth_config(allow_single_sign_on: ['twitter'], external_providers: ['twitter']) + stub_omniauth_config(allow_single_sign_on: [provider], external_providers: [provider]) oauth_user.save expect(gl_user).to be_valid expect(gl_user.external).to be_truthy @@ -95,8 +107,8 @@ describe Gitlab::OAuth::User do context 'provider was external, now has been removed' do it 'does not mark external user as internal' do - create(:omniauth_user, extern_uid: 'my-uid', provider: 'twitter', external: true) - stub_omniauth_config(allow_single_sign_on: ['twitter'], external_providers: ['facebook']) + create(:omniauth_user, extern_uid: 'my-uid', provider: provider, external: true) + stub_omniauth_config(allow_single_sign_on: [provider], external_providers: ['facebook']) oauth_user.save expect(gl_user).to be_valid expect(gl_user.external).to be_truthy @@ -118,7 +130,7 @@ describe Gitlab::OAuth::User do context 'with new allow_single_sign_on enabled syntax' do before do - stub_omniauth_config(allow_single_sign_on: ['twitter']) + stub_omniauth_config(allow_single_sign_on: [provider]) end it "creates a user from Omniauth" do @@ -127,7 +139,7 @@ describe Gitlab::OAuth::User do expect(gl_user).to be_valid identity = gl_user.identities.first expect(identity.extern_uid).to eql uid - expect(identity.provider).to eql 'twitter' + expect(identity.provider).to eql provider end end @@ -142,7 +154,7 @@ describe Gitlab::OAuth::User do expect(gl_user).to be_valid identity = gl_user.identities.first expect(identity.extern_uid).to eql uid - expect(identity.provider).to eql 'twitter' + expect(identity.provider).to eql provider end end diff --git a/spec/models/concerns/discussion_on_diff_spec.rb b/spec/models/concerns/discussion_on_diff_spec.rb index 2322eb206fb..30572ce9332 100644 --- a/spec/models/concerns/discussion_on_diff_spec.rb +++ b/spec/models/concerns/discussion_on_diff_spec.rb @@ -20,6 +20,16 @@ describe DiscussionOnDiff do expect(truncated_lines).not_to include(be_meta) end end + + context "when the diff line does not exist on a legacy diff note" do + it "returns an empty array" do + legacy_note = LegacyDiffNote.new + + allow(subject).to receive(:first_note).and_return(legacy_note) + + expect(truncated_lines).to eq([]) + end + end end describe '#line_code_in_diffs' do diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb index eb9690df313..243eeddc7a8 100644 --- a/spec/models/merge_request_spec.rb +++ b/spec/models/merge_request_spec.rb @@ -1339,6 +1339,10 @@ describe MergeRequest do it 'returns false' do expect(subject.mergeable_state?).to be_falsey end + + it 'returns true when skipping discussions check' do + expect(subject.mergeable_state?(skip_discussions_check: true)).to be(true) + end end end end diff --git a/spec/models/repository_spec.rb b/spec/models/repository_spec.rb index d4070b320ed..1102b1c9006 100644 --- a/spec/models/repository_spec.rb +++ b/spec/models/repository_spec.rb @@ -772,8 +772,7 @@ describe Repository do user, 'LICENSE', 'Copyright!', message: 'Add LICENSE', branch_name: 'master') - allow(repository).to receive(:file_on_head) - .and_raise(Rugged::ReferenceError) + allow(repository).to receive(:root_ref).and_raise(Gitlab::Git::Repository::NoRepository) expect(repository.license_blob).to be_nil end @@ -885,7 +884,7 @@ describe Repository do end it 'returns nil for empty repository' do - allow(repository).to receive(:file_on_head).and_raise(Rugged::ReferenceError) + allow(repository).to receive(:root_ref).and_raise(Gitlab::Git::Repository::NoRepository) expect(repository.gitlab_ci_yml).to be_nil end end @@ -1937,8 +1936,7 @@ describe Repository do describe '#avatar' do it 'returns nil if repo does not exist' do - expect(repository).to receive(:file_on_head) - .and_raise(Rugged::ReferenceError) + allow(repository).to receive(:root_ref).and_raise(Gitlab::Git::Repository::NoRepository) expect(repository.avatar).to eq(nil) end diff --git a/spec/models/wiki_page_spec.rb b/spec/models/wiki_page_spec.rb index cc9d79da708..9840afe6c4e 100644 --- a/spec/models/wiki_page_spec.rb +++ b/spec/models/wiki_page_spec.rb @@ -387,13 +387,23 @@ describe WikiPage do end describe '#formatted_content' do - it 'returns processed content of the page', :disable_gitaly do - subject.create({ title: "RDoc", content: "*bold*", format: "rdoc" }) - page = wiki.find_page('RDoc') + shared_examples 'fetching page formatted content' do + it 'returns processed content of the page' do + subject.create({ title: "RDoc", content: "*bold*", format: "rdoc" }) + page = wiki.find_page('RDoc') - expect(page.formatted_content).to eq("\n<p><strong>bold</strong></p>\n") + expect(page.formatted_content).to eq("\n<p><strong>bold</strong></p>\n") - destroy_page('RDoc') + destroy_page('RDoc') + end + end + + context 'when Gitaly wiki_page_formatted_data is enabled' do + it_behaves_like 'fetching page formatted content' + end + + context 'when Gitaly wiki_page_formatted_data is disabled', :disable_gitaly do + it_behaves_like 'fetching page formatted content' end end diff --git a/spec/requests/api/jobs_spec.rb b/spec/requests/api/jobs_spec.rb index 4dd8deb6404..f8d0b63afec 100644 --- a/spec/requests/api/jobs_spec.rb +++ b/spec/requests/api/jobs_spec.rb @@ -300,44 +300,53 @@ describe API::Jobs do end describe 'GET /projects/:id/jobs/:job_id/artifacts' do - before do - get api("/projects/#{project.id}/jobs/#{job.id}/artifacts", api_user) + shared_examples 'downloads artifact' do + let(:download_headers) do + { 'Content-Transfer-Encoding' => 'binary', + 'Content-Disposition' => 'attachment; filename=ci_build_artifacts.zip' } + end + + it 'returns specific job artifacts' do + expect(response).to have_gitlab_http_status(200) + expect(response.headers).to include(download_headers) + expect(response.body).to match_file(job.artifacts_file.file.file) + end end - context 'job with artifacts' do - let(:job) { create(:ci_build, :artifacts, pipeline: pipeline) } + context 'normal authentication' do + context 'job with artifacts' do + context 'when artifacts are stored locally' do + let(:job) { create(:ci_build, :artifacts, pipeline: pipeline) } - context 'authorized user' do - let(:download_headers) do - { 'Content-Transfer-Encoding' => 'binary', - 'Content-Disposition' => 'attachment; filename=ci_build_artifacts.zip' } - end + before do + get api("/projects/#{project.id}/jobs/#{job.id}/artifacts", api_user) + end - it 'returns specific job artifacts' do - expect(response).to have_gitlab_http_status(200) - expect(response.headers).to include(download_headers) - expect(response.body).to match_file(job.artifacts_file.file.file) + context 'authorized user' do + it_behaves_like 'downloads artifact' + end + + context 'unauthorized user' do + let(:api_user) { nil } + + it 'does not return specific job artifacts' do + expect(response).to have_gitlab_http_status(404) + end + end end - end - context 'when anonymous user is accessing private artifacts' do - let(:api_user) { nil } + it 'does not return job artifacts if not uploaded' do + get api("/projects/#{project.id}/jobs/#{job.id}/artifacts", api_user) - it 'hides artifacts and rejects request' do - expect(project).to be_private expect(response).to have_gitlab_http_status(404) end end end - - it 'does not return job artifacts if not uploaded' do - expect(response).to have_gitlab_http_status(404) - end end describe 'GET /projects/:id/artifacts/:ref_name/download?job=name' do let(:api_user) { reporter } - let(:job) { create(:ci_build, :artifacts, pipeline: pipeline) } + let(:job) { create(:ci_build, :artifacts, pipeline: pipeline, user: api_user) } before do job.success @@ -396,14 +405,16 @@ describe API::Jobs do context 'find proper job' do shared_examples 'a valid file' do - let(:download_headers) do - { 'Content-Transfer-Encoding' => 'binary', - 'Content-Disposition' => - "attachment; filename=#{job.artifacts_file.filename}" } - end + context 'when artifacts are stored locally' do + let(:download_headers) do + { 'Content-Transfer-Encoding' => 'binary', + 'Content-Disposition' => + "attachment; filename=#{job.artifacts_file.filename}" } + end - it { expect(response).to have_gitlab_http_status(200) } - it { expect(response.headers).to include(download_headers) } + it { expect(response).to have_gitlab_http_status(200) } + it { expect(response.headers).to include(download_headers) } + end end context 'with regular branch' do diff --git a/spec/requests/api/members_spec.rb b/spec/requests/api/members_spec.rb index 73bd4785b11..ec500838eb2 100644 --- a/spec/requests/api/members_spec.rb +++ b/spec/requests/api/members_spec.rb @@ -44,6 +44,21 @@ describe API::Members do end end + it 'avoids N+1 queries' do + # Establish baseline + get api("/#{source_type.pluralize}/#{source.id}/members", master) + + control = ActiveRecord::QueryRecorder.new do + get api("/#{source_type.pluralize}/#{source.id}/members", master) + end + + project.add_developer(create(:user)) + + expect do + get api("/#{source_type.pluralize}/#{source.id}/members", master) + end.not_to exceed_query_limit(control) + end + it 'does not return invitees' do create(:"#{source_type}_member", invite_token: '123', invite_email: 'test@abc.com', source: source, user: nil) diff --git a/spec/requests/api/v3/builds_spec.rb b/spec/requests/api/v3/builds_spec.rb index af9e36a3b29..3f92288fef0 100644 --- a/spec/requests/api/v3/builds_spec.rb +++ b/spec/requests/api/v3/builds_spec.rb @@ -4,16 +4,18 @@ describe API::V3::Builds do set(:user) { create(:user) } let(:api_user) { user } set(:project) { create(:project, :repository, creator: user, public_builds: false) } - set(:developer) { create(:project_member, :developer, user: user, project: project) } - set(:reporter) { create(:project_member, :reporter, project: project) } - set(:guest) { create(:project_member, :guest, project: project) } - set(:pipeline) { create(:ci_empty_pipeline, project: project, sha: project.commit.id, ref: project.default_branch) } - let!(:build) { create(:ci_build, pipeline: pipeline) } + let!(:developer) { create(:project_member, :developer, user: user, project: project) } + let(:reporter) { create(:project_member, :reporter, project: project) } + let(:guest) { create(:project_member, :guest, project: project) } + let(:pipeline) { create(:ci_empty_pipeline, project: project, sha: project.commit.id, ref: project.default_branch) } + let(:build) { create(:ci_build, pipeline: pipeline) } describe 'GET /projects/:id/builds ' do let(:query) { '' } before do |example| + build + create(:ci_build, :skipped, pipeline: pipeline) unless example.metadata[:skip_before_request] @@ -110,6 +112,10 @@ describe API::V3::Builds do end describe 'GET /projects/:id/repository/commits/:sha/builds' do + before do + build + end + context 'when commit does not exist in repository' do before do get v3_api("/projects/#{project.id}/repository/commits/1a271fd1/builds", api_user) @@ -214,18 +220,20 @@ describe API::V3::Builds do end context 'job with artifacts' do - let(:build) { create(:ci_build, :artifacts, pipeline: pipeline) } + context 'when artifacts are stored locally' do + let(:build) { create(:ci_build, :artifacts, pipeline: pipeline) } - context 'authorized user' do - let(:download_headers) do - { 'Content-Transfer-Encoding' => 'binary', - 'Content-Disposition' => 'attachment; filename=ci_build_artifacts.zip' } - end + context 'authorized user' do + let(:download_headers) do + { 'Content-Transfer-Encoding' => 'binary', + 'Content-Disposition' => 'attachment; filename=ci_build_artifacts.zip' } + end - it 'returns specific job artifacts' do - expect(response).to have_gitlab_http_status(200) - expect(response.headers).to include(download_headers) - expect(response.body).to match_file(build.artifacts_file.file.file) + it 'returns specific job artifacts' do + expect(response).to have_gitlab_http_status(200) + expect(response.headers).to include(download_headers) + expect(response.body).to match_file(build.artifacts_file.file.file) + end end end @@ -303,14 +311,16 @@ describe API::V3::Builds do context 'find proper job' do shared_examples 'a valid file' do - let(:download_headers) do - { 'Content-Transfer-Encoding' => 'binary', - 'Content-Disposition' => - "attachment; filename=#{build.artifacts_file.filename}" } - end + context 'when artifacts are stored locally' do + let(:download_headers) do + { 'Content-Transfer-Encoding' => 'binary', + 'Content-Disposition' => + "attachment; filename=#{build.artifacts_file.filename}" } + end - it { expect(response).to have_gitlab_http_status(200) } - it { expect(response.headers).to include(download_headers) } + it { expect(response).to have_gitlab_http_status(200) } + it { expect(response.headers).to include(download_headers) } + end end context 'with regular branch' do |