diff options
Diffstat (limited to 'spec')
103 files changed, 1651 insertions, 393 deletions
diff --git a/spec/controllers/ldap/omniauth_callbacks_controller_spec.rb b/spec/controllers/ldap/omniauth_callbacks_controller_spec.rb new file mode 100644 index 00000000000..87c10a86cdd --- /dev/null +++ b/spec/controllers/ldap/omniauth_callbacks_controller_spec.rb @@ -0,0 +1,58 @@ +require 'spec_helper' + +describe Ldap::OmniauthCallbacksController do + include_context 'Ldap::OmniauthCallbacksController' + + it 'allows sign in' do + post provider + + expect(request.env['warden']).to be_authenticated + end + + it 'respects remember me checkbox' do + expect do + post provider, remember_me: '1' + end.to change { user.reload.remember_created_at }.from(nil) + end + + context 'with 2FA' do + let(:user) { create(:omniauth_user, :two_factor_via_otp, extern_uid: uid, provider: provider) } + + it 'passes remember_me to the Devise view' do + post provider, remember_me: '1' + + expect(assigns[:user].remember_me).to eq '1' + end + end + + context 'access denied' do + let(:valid_login?) { false } + + it 'warns the user' do + post provider + + expect(flash[:alert]).to match(/Access denied for your LDAP account*/) + end + + it "doesn't authenticate user" do + post provider + + expect(request.env['warden']).not_to be_authenticated + expect(response).to redirect_to(new_user_session_path) + end + end + + context 'sign up' do + let(:user) { double(email: 'new@example.com') } + + before do + stub_omniauth_setting(block_auto_created_users: false) + end + + it 'is allowed' do + post provider + + expect(request.env['warden']).to be_authenticated + end + end +end diff --git a/spec/controllers/profiles_controller_spec.rb b/spec/controllers/profiles_controller_spec.rb index de6ef919221..c621eb69171 100644 --- a/spec/controllers/profiles_controller_spec.rb +++ b/spec/controllers/profiles_controller_spec.rb @@ -125,7 +125,7 @@ describe ProfilesController, :request_store do user.reload expect(response.status).to eq(302) - expect(gitlab_shell.exists?(project.repository_storage_path, "#{new_username}/#{project.path}.git")).to be_truthy + expect(gitlab_shell.exists?(project.repository_storage, "#{new_username}/#{project.path}.git")).to be_truthy end end @@ -143,7 +143,7 @@ describe ProfilesController, :request_store do user.reload expect(response.status).to eq(302) - expect(gitlab_shell.exists?(project.repository_storage_path, "#{project.disk_path}.git")).to be_truthy + expect(gitlab_shell.exists?(project.repository_storage, "#{project.disk_path}.git")).to be_truthy expect(before_disk_path).to eq(project.disk_path) end end diff --git a/spec/factories/deploy_tokens.rb b/spec/factories/deploy_tokens.rb index 5fea4a9d5a6..017e866e69c 100644 --- a/spec/factories/deploy_tokens.rb +++ b/spec/factories/deploy_tokens.rb @@ -10,5 +10,13 @@ FactoryBot.define do trait :revoked do revoked true end + + trait :gitlab_deploy_token do + name DeployToken::GITLAB_DEPLOY_TOKEN_NAME + end + + trait :expired do + expires_at { Date.today - 1.month } + end end end diff --git a/spec/factories/projects.rb b/spec/factories/projects.rb index 1ae6152a1f0..1904615778c 100644 --- a/spec/factories/projects.rb +++ b/spec/factories/projects.rb @@ -147,7 +147,15 @@ FactoryBot.define do # We delete hooks so that gitlab-shell will not try to authenticate with # an API that isn't running - FileUtils.rm_r(File.join(project.repository_storage_path, "#{project.disk_path}.git", 'hooks')) + project.gitlab_shell.rm_directory(project.repository_storage, + File.join("#{project.disk_path}.git", 'hooks')) + end + end + + trait :stubbed_repository do + after(:build) do |project| + allow(project).to receive(:empty_repo?).and_return(false) + allow(project.repository).to receive(:empty?).and_return(false) end end @@ -165,7 +173,8 @@ FactoryBot.define do after(:create) do |project| raise "Failed to create repository!" unless project.create_repository - FileUtils.rm_r(File.join(project.repository_storage_path, "#{project.disk_path}.git", 'refs')) + project.gitlab_shell.rm_directory(project.repository_storage, + File.join("#{project.disk_path}.git", 'refs')) end end diff --git a/spec/fast_spec_helper.rb b/spec/fast_spec_helper.rb index f28268b0754..978113a08a4 100644 --- a/spec/fast_spec_helper.rb +++ b/spec/fast_spec_helper.rb @@ -9,7 +9,8 @@ unless Object.respond_to?(:require_dependency) end end -# Defines Gitlab and Gitlab.config which are at the center of the app +# Defines Settings and Gitlab.config which are at the center of the app +require_relative '../config/settings' require_relative '../lib/gitlab' unless defined?(Gitlab.config) require_relative 'support/rspec' diff --git a/spec/features/labels_hierarchy_spec.rb b/spec/features/labels_hierarchy_spec.rb index 2c201629b44..4700ada1aae 100644 --- a/spec/features/labels_hierarchy_spec.rb +++ b/spec/features/labels_hierarchy_spec.rb @@ -170,6 +170,8 @@ feature 'Labels Hierarchy', :js, :nested_groups do context 'on issue sidebar' do before do + project_1.add_developer(user) + visit project_issue_path(project_1, issue) end @@ -180,6 +182,8 @@ feature 'Labels Hierarchy', :js, :nested_groups do let(:board) { create(:board, project: project_1) } before do + project_1.add_developer(user) + visit project_board_path(project_1, board) wait_for_requests @@ -194,6 +198,8 @@ feature 'Labels Hierarchy', :js, :nested_groups do let(:board) { create(:board, group: parent) } before do + parent.add_developer(user) + visit group_board_path(parent, board) wait_for_requests @@ -211,6 +217,8 @@ feature 'Labels Hierarchy', :js, :nested_groups do context 'on project issuable list' do before do + project_1.add_developer(user) + visit project_issues_path(project_1) end @@ -237,6 +245,8 @@ feature 'Labels Hierarchy', :js, :nested_groups do let(:board) { create(:board, project: project_1) } before do + project_1.add_developer(user) + visit project_board_path(project_1, board) end @@ -247,6 +257,8 @@ feature 'Labels Hierarchy', :js, :nested_groups do let(:board) { create(:board, group: parent) } before do + parent.add_developer(user) + visit group_board_path(parent, board) end @@ -259,6 +271,7 @@ feature 'Labels Hierarchy', :js, :nested_groups do let(:board) { create(:board, project: project_1) } before do + project_1.add_developer(user) visit project_board_path(project_1, board) find('.js-new-board-list').click wait_for_requests @@ -281,6 +294,7 @@ feature 'Labels Hierarchy', :js, :nested_groups do let(:board) { create(:board, group: parent) } before do + parent.add_developer(user) visit group_board_path(parent, board) find('.js-new-board-list').click wait_for_requests diff --git a/spec/features/oauth_login_spec.rb b/spec/features/oauth_login_spec.rb index a5e325ee2e3..013cdaa6479 100644 --- a/spec/features/oauth_login_spec.rb +++ b/spec/features/oauth_login_spec.rb @@ -28,35 +28,46 @@ feature 'OAuth Login', :js, :allow_forgery_protection do OmniAuth.config.full_host = @omniauth_config_full_host end + def login_with_provider(provider, enter_two_factor: false) + login_via(provider.to_s, user, uid, remember_me: remember_me) + enter_code(user.current_otp) if enter_two_factor + end + providers.each do |provider| context "when the user logs in using the #{provider} provider" do + let(:uid) { 'my-uid' } + let(:remember_me) { false } + let(:user) { create(:omniauth_user, extern_uid: uid, provider: provider.to_s) } + let(:two_factor_user) { create(:omniauth_user, :two_factor, extern_uid: uid, provider: provider.to_s) } + + before do + stub_omniauth_config(provider) + end + context 'when two-factor authentication is disabled' do it 'logs the user in' do - stub_omniauth_config(provider) - user = create(:omniauth_user, extern_uid: 'my-uid', provider: provider.to_s) - login_via(provider.to_s, user, 'my-uid') + login_with_provider(provider) expect(current_path).to eq root_path end end context 'when two-factor authentication is enabled' do + let(:user) { two_factor_user } + it 'logs the user in' do - stub_omniauth_config(provider) - user = create(:omniauth_user, :two_factor, extern_uid: 'my-uid', provider: provider.to_s) - login_via(provider.to_s, user, 'my-uid') + login_with_provider(provider, enter_two_factor: true) - enter_code(user.current_otp) expect(current_path).to eq root_path end end context 'when "remember me" is checked' do + let(:remember_me) { true } + context 'when two-factor authentication is disabled' do it 'remembers the user after a browser restart' do - stub_omniauth_config(provider) - user = create(:omniauth_user, extern_uid: 'my-uid', provider: provider.to_s) - login_via(provider.to_s, user, 'my-uid', remember_me: true) + login_with_provider(provider) clear_browser_session @@ -66,11 +77,10 @@ feature 'OAuth Login', :js, :allow_forgery_protection do end context 'when two-factor authentication is enabled' do + let(:user) { two_factor_user } + it 'remembers the user after a browser restart' do - stub_omniauth_config(provider) - user = create(:omniauth_user, :two_factor, extern_uid: 'my-uid', provider: provider.to_s) - login_via(provider.to_s, user, 'my-uid', remember_me: true) - enter_code(user.current_otp) + login_with_provider(provider, enter_two_factor: true) clear_browser_session @@ -83,9 +93,7 @@ feature 'OAuth Login', :js, :allow_forgery_protection do context 'when "remember me" is not checked' do context 'when two-factor authentication is disabled' do it 'does not remember the user after a browser restart' do - stub_omniauth_config(provider) - user = create(:omniauth_user, extern_uid: 'my-uid', provider: provider.to_s) - login_via(provider.to_s, user, 'my-uid', remember_me: false) + login_with_provider(provider) clear_browser_session @@ -95,11 +103,10 @@ feature 'OAuth Login', :js, :allow_forgery_protection do end context 'when two-factor authentication is enabled' do + let(:user) { two_factor_user } + it 'does not remember the user after a browser restart' do - stub_omniauth_config(provider) - user = create(:omniauth_user, :two_factor, extern_uid: 'my-uid', provider: provider.to_s) - login_via(provider.to_s, user, 'my-uid', remember_me: false) - enter_code(user.current_otp) + login_with_provider(provider, enter_two_factor: true) clear_browser_session diff --git a/spec/features/projects/blobs/user_creates_new_blob_in_new_project_spec.rb b/spec/features/projects/blobs/user_creates_new_blob_in_new_project_spec.rb new file mode 100644 index 00000000000..b7d063596c1 --- /dev/null +++ b/spec/features/projects/blobs/user_creates_new_blob_in_new_project_spec.rb @@ -0,0 +1,52 @@ +require 'spec_helper' + +feature 'User creates blob in new project', :js do + let(:user) { create(:user) } + let(:project) { create(:project, :empty_repo) } + + shared_examples 'creating a file' do + before do + sign_in(user) + visit project_path(project) + end + + it 'allows the user to add a new file' do + click_link 'New file' + + find('#editor') + execute_script('ace.edit("editor").setValue("Hello world")') + + fill_in(:file_name, with: 'dummy-file') + + click_button('Commit changes') + + expect(page).to have_content('The file has been successfully created') + end + end + + describe 'as a master' do + before do + project.add_master(user) + end + + it_behaves_like 'creating a file' + end + + describe 'as an admin' do + let(:user) { create(:user, :admin) } + + it_behaves_like 'creating a file' + end + + describe 'as a developer' do + before do + project.add_developer(user) + sign_in(user) + visit project_path(project) + end + + it 'does not allow pushing to the default branch' do + expect(page).not_to have_content('New file') + end + end +end diff --git a/spec/features/projects/files/user_find_file_spec.rb b/spec/features/projects/files/user_find_file_spec.rb new file mode 100644 index 00000000000..df405e70dd4 --- /dev/null +++ b/spec/features/projects/files/user_find_file_spec.rb @@ -0,0 +1,66 @@ +require 'spec_helper' + +describe 'User find project file' do + let(:user) { create :user } + let(:project) { create :project, :repository } + + before do + sign_in(user) + project.add_master(user) + + visit project_tree_path(project, project.repository.root_ref) + end + + def active_main_tab + find('.sidebar-top-level-items > li.active') + end + + def find_file(text) + fill_in 'file_find', with: text + end + + it 'navigates to find file by shortcut', :js do + find('body').native.send_key('t') + + expect(active_main_tab).to have_content('Repository') + expect(page).to have_selector('.file-finder-holder', count: 1) + end + + it 'navigates to find file' do + click_link 'Find file' + + expect(active_main_tab).to have_content('Repository') + expect(page).to have_selector('.file-finder-holder', count: 1) + end + + it 'searches CHANGELOG file', :js do + click_link 'Find file' + + find_file 'change' + + expect(page).to have_content('CHANGELOG') + expect(page).not_to have_content('.gitignore') + expect(page).not_to have_content('VERSION') + end + + it 'does not find file when search not exist file', :js do + click_link 'Find file' + + find_file 'asdfghjklqwertyuizxcvbnm' + + expect(page).not_to have_content('CHANGELOG') + expect(page).not_to have_content('.gitignore') + expect(page).not_to have_content('VERSION') + end + + it 'searches file by partially matches', :js do + click_link 'Find file' + + find_file 'git' + + expect(page).to have_content('.gitignore') + expect(page).to have_content('.gitmodules') + expect(page).not_to have_content('CHANGELOG') + expect(page).not_to have_content('VERSION') + end +end diff --git a/spec/features/projects/import_export/test_project_export.tar.gz b/spec/features/projects/import_export/test_project_export.tar.gz Binary files differindex ecb7651acad..72ab2d71f35 100644 --- a/spec/features/projects/import_export/test_project_export.tar.gz +++ b/spec/features/projects/import_export/test_project_export.tar.gz diff --git a/spec/features/projects/user_views_empty_project_spec.rb b/spec/features/projects/user_views_empty_project_spec.rb new file mode 100644 index 00000000000..7b982301ffc --- /dev/null +++ b/spec/features/projects/user_views_empty_project_spec.rb @@ -0,0 +1,43 @@ +require 'spec_helper' + +describe 'User views an empty project' do + let(:project) { create(:project, :empty_repo) } + let(:user) { create(:user) } + + shared_examples 'allowing push to default branch' do + before do + sign_in(user) + visit project_path(project) + end + + it 'shows push-to-master instructions' do + expect(page).to have_content('git push -u origin master') + end + end + + describe 'as a master' do + before do + project.add_master(user) + end + + it_behaves_like 'allowing push to default branch' + end + + describe 'as an admin' do + let(:user) { create(:user, :admin) } + + it_behaves_like 'allowing push to default branch' + end + + describe 'as a developer' do + before do + project.add_developer(user) + sign_in(user) + visit project_path(project) + end + + it 'does not show push-to-master instructions' do + expect(page).not_to have_content('git push -u origin master') + end + end +end diff --git a/spec/fixtures/exported-project.gz b/spec/fixtures/exported-project.gz Binary files differindex 352384f16c8..bef7e2ff8ee 100644 --- a/spec/fixtures/exported-project.gz +++ b/spec/fixtures/exported-project.gz diff --git a/spec/helpers/gitlab_routing_helper_spec.rb b/spec/helpers/gitlab_routing_helper_spec.rb index 6c4f7050ee0..143b28728a3 100644 --- a/spec/helpers/gitlab_routing_helper_spec.rb +++ b/spec/helpers/gitlab_routing_helper_spec.rb @@ -89,4 +89,19 @@ describe GitlabRoutingHelper do expect(preview_markdown_path(project)).to eq("/#{project.full_path}/preview_markdown") end end + + describe '#edit_milestone_path' do + it 'returns group milestone edit path when given entity parent is a Group' do + group = create(:group) + milestone = create(:milestone, group: group) + + expect(edit_milestone_path(milestone)).to eq("/groups/#{group.path}/-/milestones/#{milestone.iid}/edit") + end + + it 'returns project milestone edit path when given entity parent is not a Group' do + milestone = create(:milestone, group: nil) + + expect(edit_milestone_path(milestone)).to eq("/#{milestone.project.full_path}/milestones/#{milestone.iid}/edit") + end + end end diff --git a/spec/helpers/projects_helper_spec.rb b/spec/helpers/projects_helper_spec.rb index 46c55da24f8..8fcb175416f 100644 --- a/spec/helpers/projects_helper_spec.rb +++ b/spec/helpers/projects_helper_spec.rb @@ -274,16 +274,16 @@ describe ProjectsHelper do end end - describe '#sanitized_import_error' do + describe '#sanitizerepo_repo_path' do let(:project) { create(:project, :repository) } + let(:storage_path) { Gitlab.config.repositories.storages.default.legacy_disk_path } before do - allow(project).to receive(:repository_storage_path).and_return('/base/repo/path') allow(Settings.shared).to receive(:[]).with('path').and_return('/base/repo/export/path') end it 'removes the repo path' do - repo = '/base/repo/path/namespace/test.git' + repo = "#{storage_path}/namespace/test.git" import_error = "Could not clone #{repo}\n" expect(sanitize_repo_path(project, import_error)).to eq('Could not clone [REPOS PATH]/namespace/test.git') diff --git a/spec/javascripts/.eslintrc b/spec/javascripts/.eslintrc index 3d922021978..9eb0e732572 100644 --- a/spec/javascripts/.eslintrc +++ b/spec/javascripts/.eslintrc @@ -18,6 +18,7 @@ "sandbox": false, "setFixtures": false, "setStyleFixtures": false, + "spyOnDependency": false, "spyOnEvent": false, "ClassSpecHelper": false }, diff --git a/spec/javascripts/activities_spec.js b/spec/javascripts/activities_spec.js index 909a1bf76bc..5dbdcd24296 100644 --- a/spec/javascripts/activities_spec.js +++ b/spec/javascripts/activities_spec.js @@ -3,24 +3,30 @@ import $ from 'jquery'; import 'vendor/jquery.endless-scroll'; import Activities from '~/activities'; +import Pager from '~/pager'; -(() => { +describe('Activities', () => { window.gon || (window.gon = {}); const fixtureTemplate = 'static/event_filter.html.raw'; const filters = [ { id: 'all', - }, { + }, + { id: 'push', name: 'push events', - }, { + }, + { id: 'merged', name: 'merge events', - }, { + }, + { id: 'comments', - }, { + }, + { id: 'team', - }]; + }, + ]; function getEventName(index) { const filter = filters[index]; @@ -32,31 +38,34 @@ import Activities from '~/activities'; return `#${filter.id}_event_filter`; } - describe('Activities', () => { - beforeEach(() => { - loadFixtures(fixtureTemplate); - new Activities(); - }); - - for (let i = 0; i < filters.length; i += 1) { - ((i) => { - describe(`when selecting ${getEventName(i)}`, () => { - beforeEach(() => { - $(getSelector(i)).click(); - }); - - for (let x = 0; x < filters.length; x += 1) { - ((x) => { - const shouldHighlight = i === x; - const testName = shouldHighlight ? 'should highlight' : 'should not highlight'; - - it(`${testName} ${getEventName(x)}`, () => { - expect($(getSelector(x)).parent().hasClass('active')).toEqual(shouldHighlight); - }); - })(x); - } - }); - })(i); - } + beforeEach(() => { + loadFixtures(fixtureTemplate); + spyOn(Pager, 'init').and.stub(); + new Activities(); }); -})(); + + for (let i = 0; i < filters.length; i += 1) { + (i => { + describe(`when selecting ${getEventName(i)}`, () => { + beforeEach(() => { + $(getSelector(i)).click(); + }); + + for (let x = 0; x < filters.length; x += 1) { + (x => { + const shouldHighlight = i === x; + const testName = shouldHighlight ? 'should highlight' : 'should not highlight'; + + it(`${testName} ${getEventName(x)}`, () => { + expect( + $(getSelector(x)) + .parent() + .hasClass('active'), + ).toEqual(shouldHighlight); + }); + })(x); + } + }); + })(i); + } +}); diff --git a/spec/javascripts/behaviors/quick_submit_spec.js b/spec/javascripts/behaviors/quick_submit_spec.js index c37c62c63dd..d03836d10f9 100644 --- a/spec/javascripts/behaviors/quick_submit_spec.js +++ b/spec/javascripts/behaviors/quick_submit_spec.js @@ -1,7 +1,7 @@ import $ from 'jquery'; import '~/behaviors/quick_submit'; -describe('Quick Submit behavior', () => { +describe('Quick Submit behavior', function () { const keydownEvent = (options = { keyCode: 13, metaKey: true }) => $.Event('keydown', options); preloadFixtures('merge_requests/merge_request_with_task_list.html.raw'); diff --git a/spec/javascripts/blob/blob_file_dropzone_spec.js b/spec/javascripts/blob/blob_file_dropzone_spec.js index 0b1de504435..346f795c3f5 100644 --- a/spec/javascripts/blob/blob_file_dropzone_spec.js +++ b/spec/javascripts/blob/blob_file_dropzone_spec.js @@ -1,7 +1,7 @@ import $ from 'jquery'; import BlobFileDropzone from '~/blob/blob_file_dropzone'; -describe('BlobFileDropzone', () => { +describe('BlobFileDropzone', function () { preloadFixtures('blob/show.html.raw'); beforeEach(() => { diff --git a/spec/javascripts/comment_type_toggle_spec.js b/spec/javascripts/comment_type_toggle_spec.js index dfd0810d52e..0ba709298c5 100644 --- a/spec/javascripts/comment_type_toggle_spec.js +++ b/spec/javascripts/comment_type_toggle_spec.js @@ -1,5 +1,4 @@ import CommentTypeToggle from '~/comment_type_toggle'; -import * as dropLabSrc from '~/droplab/drop_lab'; import InputSetter from '~/droplab/plugins/input_setter'; describe('CommentTypeToggle', function () { @@ -59,14 +58,14 @@ describe('CommentTypeToggle', function () { this.droplab = jasmine.createSpyObj('droplab', ['init']); - spyOn(dropLabSrc, 'default').and.returnValue(this.droplab); + this.droplabConstructor = spyOnDependency(CommentTypeToggle, 'DropLab').and.returnValue(this.droplab); spyOn(this.commentTypeToggle, 'setConfig').and.returnValue(this.config); CommentTypeToggle.prototype.initDroplab.call(this.commentTypeToggle); }); it('should instantiate a DropLab instance', function () { - expect(dropLabSrc.default).toHaveBeenCalled(); + expect(this.droplabConstructor).toHaveBeenCalled(); }); it('should set .droplab', function () { diff --git a/spec/javascripts/commit/pipelines/pipelines_spec.js b/spec/javascripts/commit/pipelines/pipelines_spec.js index 53820770f3f..819ed7896ca 100644 --- a/spec/javascripts/commit/pipelines/pipelines_spec.js +++ b/spec/javascripts/commit/pipelines/pipelines_spec.js @@ -4,7 +4,7 @@ import axios from '~/lib/utils/axios_utils'; import pipelinesTable from '~/commit/pipelines/pipelines_table.vue'; import mountComponent from 'spec/helpers/vue_mount_component_helper'; -describe('Pipelines table in Commits and Merge requests', () => { +describe('Pipelines table in Commits and Merge requests', function () { const jsonFixtureName = 'pipelines/pipelines.json'; let pipeline; let PipelinesTable; diff --git a/spec/javascripts/commits_spec.js b/spec/javascripts/commits_spec.js index 977298b9221..60d100e8544 100644 --- a/spec/javascripts/commits_spec.js +++ b/spec/javascripts/commits_spec.js @@ -3,6 +3,7 @@ import 'vendor/jquery.endless-scroll'; import MockAdapter from 'axios-mock-adapter'; import axios from '~/lib/utils/axios_utils'; import CommitsList from '~/commits'; +import Pager from '~/pager'; describe('Commits List', () => { let commitsList; @@ -14,6 +15,7 @@ describe('Commits List', () => { </form> <ol id="commits-list"></ol> `); + spyOn(Pager, 'init').and.stub(); commitsList = new CommitsList(25); }); @@ -68,9 +70,10 @@ describe('Commits List', () => { mock.restore(); }); - it('should save the last search string', (done) => { + it('should save the last search string', done => { commitsList.searchField.val('GitLab'); - commitsList.filterResults() + commitsList + .filterResults() .then(() => { expect(ajaxSpy).toHaveBeenCalled(); expect(commitsList.lastSearch).toEqual('GitLab'); @@ -80,8 +83,9 @@ describe('Commits List', () => { .catch(done.fail); }); - it('should not make ajax call if the input does not change', (done) => { - commitsList.filterResults() + it('should not make ajax call if the input does not change', done => { + commitsList + .filterResults() .then(() => { expect(ajaxSpy).not.toHaveBeenCalled(); expect(commitsList.lastSearch).toEqual(''); diff --git a/spec/javascripts/droplab/hook_spec.js b/spec/javascripts/droplab/hook_spec.js index 3d39bd0812b..5eed1db2750 100644 --- a/spec/javascripts/droplab/hook_spec.js +++ b/spec/javascripts/droplab/hook_spec.js @@ -1,5 +1,4 @@ import Hook from '~/droplab/hook'; -import * as dropdownSrc from '~/droplab/drop_down'; describe('Hook', function () { describe('class constructor', function () { @@ -10,7 +9,7 @@ describe('Hook', function () { this.config = {}; this.dropdown = {}; - spyOn(dropdownSrc, 'default').and.returnValue(this.dropdown); + this.dropdownConstructor = spyOnDependency(Hook, 'DropDown').and.returnValue(this.dropdown); this.hook = new Hook(this.trigger, this.list, this.plugins, this.config); }); @@ -24,7 +23,7 @@ describe('Hook', function () { }); it('should call DropDown constructor', function () { - expect(dropdownSrc.default).toHaveBeenCalledWith(this.list, this.config); + expect(this.dropdownConstructor).toHaveBeenCalledWith(this.list, this.config); }); it('should set .type', function () { diff --git a/spec/javascripts/filtered_search/filtered_search_manager_spec.js b/spec/javascripts/filtered_search/filtered_search_manager_spec.js index 95d02974bdc..8fcee36beb8 100644 --- a/spec/javascripts/filtered_search/filtered_search_manager_spec.js +++ b/spec/javascripts/filtered_search/filtered_search_manager_spec.js @@ -1,5 +1,3 @@ -import * as urlUtils from '~/lib/utils/url_utility'; -import * as recentSearchesStoreSrc from '~/filtered_search/stores/recent_searches_store'; import RecentSearchesService from '~/filtered_search/services/recent_searches_service'; import RecentSearchesServiceError from '~/filtered_search/services/recent_searches_service_error'; import RecentSearchesRoot from '~/filtered_search/recent_searches_root'; @@ -11,7 +9,7 @@ import FilteredSearchDropdownManager from '~/filtered_search/filtered_search_dro import FilteredSearchManager from '~/filtered_search/filtered_search_manager'; import FilteredSearchSpecHelper from '../helpers/filtered_search_spec_helper'; -describe('Filtered Search Manager', () => { +describe('Filtered Search Manager', function () { let input; let manager; let tokensContainer; @@ -74,18 +72,19 @@ describe('Filtered Search Manager', () => { describe('class constructor', () => { const isLocalStorageAvailable = 'isLocalStorageAvailable'; + let RecentSearchesStoreSpy; beforeEach(() => { spyOn(RecentSearchesService, 'isAvailable').and.returnValue(isLocalStorageAvailable); - spyOn(recentSearchesStoreSrc, 'default'); spyOn(RecentSearchesRoot.prototype, 'render'); + RecentSearchesStoreSpy = spyOnDependency(FilteredSearchManager, 'RecentSearchesStore'); }); it('should instantiate RecentSearchesStore with isLocalStorageAvailable', () => { manager = new FilteredSearchManager({ page }); expect(RecentSearchesService.isAvailable).toHaveBeenCalled(); - expect(recentSearchesStoreSrc.default).toHaveBeenCalledWith({ + expect(RecentSearchesStoreSpy).toHaveBeenCalledWith({ isLocalStorageAvailable, allowedKeys: FilteredSearchTokenKeys.getKeys(), }); @@ -164,7 +163,7 @@ describe('Filtered Search Manager', () => { it('should search with a single word', (done) => { input.value = 'searchTerm'; - spyOn(urlUtils, 'visitUrl').and.callFake((url) => { + spyOnDependency(FilteredSearchManager, 'visitUrl').and.callFake((url) => { expect(url).toEqual(`${defaultParams}&search=searchTerm`); done(); }); @@ -175,7 +174,7 @@ describe('Filtered Search Manager', () => { it('should search with multiple words', (done) => { input.value = 'awesome search terms'; - spyOn(urlUtils, 'visitUrl').and.callFake((url) => { + spyOnDependency(FilteredSearchManager, 'visitUrl').and.callFake((url) => { expect(url).toEqual(`${defaultParams}&search=awesome+search+terms`); done(); }); @@ -186,7 +185,7 @@ describe('Filtered Search Manager', () => { it('should search with special characters', (done) => { input.value = '~!@#$%^&*()_+{}:<>,.?/'; - spyOn(urlUtils, 'visitUrl').and.callFake((url) => { + spyOnDependency(FilteredSearchManager, 'visitUrl').and.callFake((url) => { expect(url).toEqual(`${defaultParams}&search=~!%40%23%24%25%5E%26*()_%2B%7B%7D%3A%3C%3E%2C.%3F%2F`); done(); }); @@ -200,7 +199,7 @@ describe('Filtered Search Manager', () => { ${FilteredSearchSpecHelper.createFilterVisualTokenHTML('label', '~bug')} `); - spyOn(urlUtils, 'visitUrl').and.callFake((url) => { + spyOnDependency(FilteredSearchManager, 'visitUrl').and.callFake((url) => { expect(url).toEqual(`${defaultParams}&label_name[]=bug`); done(); }); diff --git a/spec/javascripts/filtered_search/recent_searches_root_spec.js b/spec/javascripts/filtered_search/recent_searches_root_spec.js index d8ba6de5f45..1e6272bad0b 100644 --- a/spec/javascripts/filtered_search/recent_searches_root_spec.js +++ b/spec/javascripts/filtered_search/recent_searches_root_spec.js @@ -1,11 +1,11 @@ import RecentSearchesRoot from '~/filtered_search/recent_searches_root'; -import * as vueSrc from 'vue'; describe('RecentSearchesRoot', () => { describe('render', () => { let recentSearchesRoot; let data; let template; + let VueSpy; beforeEach(() => { recentSearchesRoot = { @@ -14,7 +14,7 @@ describe('RecentSearchesRoot', () => { }, }; - spyOn(vueSrc, 'default').and.callFake((options) => { + VueSpy = spyOnDependency(RecentSearchesRoot, 'Vue').and.callFake((options) => { data = options.data; template = options.template; }); @@ -23,7 +23,7 @@ describe('RecentSearchesRoot', () => { }); it('should instantiate Vue', () => { - expect(vueSrc.default).toHaveBeenCalled(); + expect(VueSpy).toHaveBeenCalled(); expect(data()).toBe(recentSearchesRoot.store.state); expect(template).toContain(':is-local-storage-available="isLocalStorageAvailable"'); }); diff --git a/spec/javascripts/gl_dropdown_spec.js b/spec/javascripts/gl_dropdown_spec.js index 32efdb11379..175f386b60e 100644 --- a/spec/javascripts/gl_dropdown_spec.js +++ b/spec/javascripts/gl_dropdown_spec.js @@ -1,9 +1,8 @@ /* eslint-disable comma-dangle, no-param-reassign, no-unused-expressions, max-len */ import $ from 'jquery'; -import '~/gl_dropdown'; +import GLDropdown from '~/gl_dropdown'; import '~/lib/utils/common_utils'; -import * as urlUtils from '~/lib/utils/url_utility'; describe('glDropdown', function describeDropdown() { preloadFixtures('static/gl_dropdown.html.raw'); @@ -138,13 +137,13 @@ describe('glDropdown', function describeDropdown() { expect(this.dropdownContainerElement).toHaveClass('show'); const randomIndex = Math.floor(Math.random() * (this.projectsData.length - 1)) + 0; navigateWithKeys('down', randomIndex, () => { - spyOn(urlUtils, 'visitUrl').and.stub(); + const visitUrl = spyOnDependency(GLDropdown, 'visitUrl').and.stub(); navigateWithKeys('enter', null, () => { expect(this.dropdownContainerElement).not.toHaveClass('show'); const link = $(`${ITEM_SELECTOR}:eq(${randomIndex}) a`, this.$dropdownMenuElement); expect(link).toHaveClass('is-active'); const linkedLocation = link.attr('href'); - if (linkedLocation && linkedLocation !== '#') expect(urlUtils.visitUrl).toHaveBeenCalledWith(linkedLocation); + if (linkedLocation && linkedLocation !== '#') expect(visitUrl).toHaveBeenCalledWith(linkedLocation); }); }); }); diff --git a/spec/javascripts/groups/components/app_spec.js b/spec/javascripts/groups/components/app_spec.js index d8428bd0e08..2b92c485f41 100644 --- a/spec/javascripts/groups/components/app_spec.js +++ b/spec/javascripts/groups/components/app_spec.js @@ -1,7 +1,6 @@ import $ from 'jquery'; import Vue from 'vue'; -import * as utils from '~/lib/utils/url_utility'; import appComponent from '~/groups/components/app.vue'; import groupFolderComponent from '~/groups/components/group_folder.vue'; import groupItemComponent from '~/groups/components/group_item.vue'; @@ -177,7 +176,7 @@ describe('AppComponent', () => { it('should fetch groups for provided page details and update window state', (done) => { spyOn(vm, 'fetchGroups').and.returnValue(returnServicePromise(mockGroups)); spyOn(vm, 'updateGroups').and.callThrough(); - spyOn(utils, 'mergeUrlParams').and.callThrough(); + const mergeUrlParams = spyOnDependency(appComponent, 'mergeUrlParams').and.callThrough(); spyOn(window.history, 'replaceState'); spyOn($, 'scrollTo'); @@ -193,7 +192,7 @@ describe('AppComponent', () => { setTimeout(() => { expect(vm.isLoading).toBe(false); expect($.scrollTo).toHaveBeenCalledWith(0); - expect(utils.mergeUrlParams).toHaveBeenCalledWith({ page: 2 }, jasmine.any(String)); + expect(mergeUrlParams).toHaveBeenCalledWith({ page: 2 }, jasmine.any(String)); expect(window.history.replaceState).toHaveBeenCalledWith({ page: jasmine.any(String), }, jasmine.any(String), jasmine.any(String)); diff --git a/spec/javascripts/groups/components/group_item_spec.js b/spec/javascripts/groups/components/group_item_spec.js index e3c942597a3..49a139855c8 100644 --- a/spec/javascripts/groups/components/group_item_spec.js +++ b/spec/javascripts/groups/components/group_item_spec.js @@ -1,5 +1,4 @@ import Vue from 'vue'; -import * as urlUtils from '~/lib/utils/url_utility'; import groupItemComponent from '~/groups/components/group_item.vue'; import groupFolderComponent from '~/groups/components/group_folder.vue'; import eventHub from '~/groups/event_hub'; @@ -135,13 +134,13 @@ describe('GroupItemComponent', () => { const group = Object.assign({}, mockParentGroupItem); group.childrenCount = 0; const newVm = createComponent(group); - spyOn(urlUtils, 'visitUrl').and.stub(); + const visitUrl = spyOnDependency(groupItemComponent, 'visitUrl').and.stub(); spyOn(eventHub, '$emit'); newVm.onClickRowGroup(event); setTimeout(() => { expect(eventHub.$emit).not.toHaveBeenCalled(); - expect(urlUtils.visitUrl).toHaveBeenCalledWith(newVm.group.relativePath); + expect(visitUrl).toHaveBeenCalledWith(newVm.group.relativePath); done(); }, 0); }); diff --git a/spec/javascripts/helpers/class_spec_helper_spec.js b/spec/javascripts/helpers/class_spec_helper_spec.js index 1415ffb7eb3..fa104ae5bcd 100644 --- a/spec/javascripts/helpers/class_spec_helper_spec.js +++ b/spec/javascripts/helpers/class_spec_helper_spec.js @@ -2,7 +2,7 @@ import './class_spec_helper'; -describe('ClassSpecHelper', () => { +describe('ClassSpecHelper', function () { describe('itShouldBeAStaticMethod', () => { beforeEach(() => { class TestClass { diff --git a/spec/javascripts/ide/components/file_finder/index_spec.js b/spec/javascripts/ide/components/file_finder/index_spec.js new file mode 100644 index 00000000000..4f208e946d2 --- /dev/null +++ b/spec/javascripts/ide/components/file_finder/index_spec.js @@ -0,0 +1,308 @@ +import Vue from 'vue'; +import store from '~/ide/stores'; +import FindFileComponent from '~/ide/components/file_finder/index.vue'; +import { UP_KEY_CODE, DOWN_KEY_CODE, ENTER_KEY_CODE, ESC_KEY_CODE } from '~/lib/utils/keycodes'; +import router from '~/ide/ide_router'; +import { file, resetStore } from '../../helpers'; +import { mountComponentWithStore } from '../../../helpers/vue_mount_component_helper'; + +describe('IDE File finder item spec', () => { + const Component = Vue.extend(FindFileComponent); + let vm; + + beforeEach(done => { + setFixtures('<div id="app"></div>'); + + vm = mountComponentWithStore(Component, { + store, + el: '#app', + props: { + index: 0, + }, + }); + + setTimeout(done); + }); + + afterEach(() => { + vm.$destroy(); + + resetStore(vm.$store); + }); + + describe('with entries', () => { + beforeEach(done => { + Vue.set(vm.$store.state.entries, 'folder', { + ...file('folder'), + path: 'folder', + type: 'folder', + }); + + Vue.set(vm.$store.state.entries, 'index.js', { + ...file('index.js'), + path: 'index.js', + type: 'blob', + url: '/index.jsurl', + }); + + Vue.set(vm.$store.state.entries, 'component.js', { + ...file('component.js'), + path: 'component.js', + type: 'blob', + }); + + setTimeout(done); + }); + + it('renders list of blobs', () => { + expect(vm.$el.textContent).toContain('index.js'); + expect(vm.$el.textContent).toContain('component.js'); + expect(vm.$el.textContent).not.toContain('folder'); + }); + + it('filters entries', done => { + vm.searchText = 'index'; + + vm.$nextTick(() => { + expect(vm.$el.textContent).toContain('index.js'); + expect(vm.$el.textContent).not.toContain('component.js'); + + done(); + }); + }); + + it('shows clear button when searchText is not empty', done => { + vm.searchText = 'index'; + + vm.$nextTick(() => { + expect(vm.$el.querySelector('.dropdown-input-clear').classList).toContain('show'); + expect(vm.$el.querySelector('.dropdown-input-search').classList).toContain('hidden'); + + done(); + }); + }); + + it('clear button resets searchText', done => { + vm.searchText = 'index'; + + vm + .$nextTick() + .then(() => { + vm.$el.querySelector('.dropdown-input-clear').click(); + }) + .then(vm.$nextTick) + .then(() => { + expect(vm.searchText).toBe(''); + }) + .then(done) + .catch(done.fail); + }); + + it('clear button focues search input', done => { + spyOn(vm.$refs.searchInput, 'focus'); + vm.searchText = 'index'; + + vm + .$nextTick() + .then(() => { + vm.$el.querySelector('.dropdown-input-clear').click(); + }) + .then(vm.$nextTick) + .then(() => { + expect(vm.$refs.searchInput.focus).toHaveBeenCalled(); + }) + .then(done) + .catch(done.fail); + }); + + describe('listShowCount', () => { + it('returns 1 when no filtered entries exist', done => { + vm.searchText = 'testing 123'; + + vm.$nextTick(() => { + expect(vm.listShowCount).toBe(1); + + done(); + }); + }); + + it('returns entries length when not filtered', () => { + expect(vm.listShowCount).toBe(2); + }); + }); + + describe('listHeight', () => { + it('returns 55 when entries exist', () => { + expect(vm.listHeight).toBe(55); + }); + + it('returns 33 when entries dont exist', done => { + vm.searchText = 'testing 123'; + + vm.$nextTick(() => { + expect(vm.listHeight).toBe(33); + + done(); + }); + }); + }); + + describe('filteredBlobsLength', () => { + it('returns length of filtered blobs', done => { + vm.searchText = 'index'; + + vm.$nextTick(() => { + expect(vm.filteredBlobsLength).toBe(1); + + done(); + }); + }); + }); + + describe('watches', () => { + describe('searchText', () => { + it('resets focusedIndex when updated', done => { + vm.focusedIndex = 1; + vm.searchText = 'test'; + + vm.$nextTick(() => { + expect(vm.focusedIndex).toBe(0); + + done(); + }); + }); + }); + + describe('fileFindVisible', () => { + it('returns searchText when false', done => { + vm.searchText = 'test'; + vm.$store.state.fileFindVisible = true; + + vm + .$nextTick() + .then(() => { + vm.$store.state.fileFindVisible = false; + }) + .then(vm.$nextTick) + .then(() => { + expect(vm.searchText).toBe(''); + }) + .then(done) + .catch(done.fail); + }); + }); + }); + + describe('openFile', () => { + beforeEach(() => { + spyOn(router, 'push'); + spyOn(vm, 'toggleFileFinder'); + }); + + it('closes file finder', () => { + vm.openFile(vm.$store.state.entries['index.js']); + + expect(vm.toggleFileFinder).toHaveBeenCalled(); + }); + + it('pushes to router', () => { + vm.openFile(vm.$store.state.entries['index.js']); + + expect(router.push).toHaveBeenCalledWith('/project/index.jsurl'); + }); + }); + + describe('onKeyup', () => { + it('opens file on enter key', done => { + const event = new CustomEvent('keyup'); + event.keyCode = ENTER_KEY_CODE; + + spyOn(vm, 'openFile'); + + vm.$refs.searchInput.dispatchEvent(event); + + vm.$nextTick(() => { + expect(vm.openFile).toHaveBeenCalledWith(vm.$store.state.entries['index.js']); + + done(); + }); + }); + + it('closes file finder on esc key', done => { + const event = new CustomEvent('keyup'); + event.keyCode = ESC_KEY_CODE; + + spyOn(vm, 'toggleFileFinder'); + + vm.$refs.searchInput.dispatchEvent(event); + + vm.$nextTick(() => { + expect(vm.toggleFileFinder).toHaveBeenCalled(); + + done(); + }); + }); + }); + + describe('onKeyDown', () => { + let el; + + beforeEach(() => { + el = vm.$refs.searchInput; + }); + + describe('up key', () => { + const event = new CustomEvent('keydown'); + event.keyCode = UP_KEY_CODE; + + it('resets to last index when at top', () => { + el.dispatchEvent(event); + + expect(vm.focusedIndex).toBe(1); + }); + + it('minus 1 from focusedIndex', () => { + vm.focusedIndex = 1; + + el.dispatchEvent(event); + + expect(vm.focusedIndex).toBe(0); + }); + }); + + describe('down key', () => { + const event = new CustomEvent('keydown'); + event.keyCode = DOWN_KEY_CODE; + + it('resets to first index when at bottom', () => { + vm.focusedIndex = 1; + el.dispatchEvent(event); + + expect(vm.focusedIndex).toBe(0); + }); + + it('adds 1 to focusedIndex', () => { + el.dispatchEvent(event); + + expect(vm.focusedIndex).toBe(1); + }); + }); + }); + }); + + describe('without entries', () => { + it('renders loading text when loading', done => { + store.state.loading = true; + + vm.$nextTick(() => { + expect(vm.$el.textContent).toContain('Loading...'); + + done(); + }); + }); + + it('renders no files text', () => { + expect(vm.$el.textContent).toContain('No files found.'); + }); + }); +}); diff --git a/spec/javascripts/ide/components/file_finder/item_spec.js b/spec/javascripts/ide/components/file_finder/item_spec.js new file mode 100644 index 00000000000..0f1116c6912 --- /dev/null +++ b/spec/javascripts/ide/components/file_finder/item_spec.js @@ -0,0 +1,140 @@ +import Vue from 'vue'; +import ItemComponent from '~/ide/components/file_finder/item.vue'; +import { file } from '../../helpers'; +import createComponent from '../../../helpers/vue_mount_component_helper'; + +describe('IDE File finder item spec', () => { + const Component = Vue.extend(ItemComponent); + let vm; + let localFile; + + beforeEach(() => { + localFile = { + ...file(), + name: 'test file', + path: 'test/file', + }; + + vm = createComponent(Component, { + file: localFile, + focused: true, + searchText: '', + index: 0, + }); + }); + + afterEach(() => { + vm.$destroy(); + }); + + it('renders file name & path', () => { + expect(vm.$el.textContent).toContain('test file'); + expect(vm.$el.textContent).toContain('test/file'); + }); + + describe('focused', () => { + it('adds is-focused class', () => { + expect(vm.$el.classList).toContain('is-focused'); + }); + + it('does not have is-focused class when not focused', done => { + vm.focused = false; + + vm.$nextTick(() => { + expect(vm.$el.classList).not.toContain('is-focused'); + + done(); + }); + }); + }); + + describe('changed file icon', () => { + it('does not render when not a changed or temp file', () => { + expect(vm.$el.querySelector('.diff-changed-stats')).toBe(null); + }); + + it('renders when a changed file', done => { + vm.file.changed = true; + + vm.$nextTick(() => { + expect(vm.$el.querySelector('.diff-changed-stats')).not.toBe(null); + + done(); + }); + }); + + it('renders when a temp file', done => { + vm.file.tempFile = true; + + vm.$nextTick(() => { + expect(vm.$el.querySelector('.diff-changed-stats')).not.toBe(null); + + done(); + }); + }); + }); + + it('emits event when clicked', () => { + spyOn(vm, '$emit'); + + vm.$el.click(); + + expect(vm.$emit).toHaveBeenCalledWith('click', vm.file); + }); + + describe('path', () => { + let el; + + beforeEach(done => { + vm.searchText = 'file'; + + el = vm.$el.querySelector('.diff-changed-file-path'); + + vm.$nextTick(done); + }); + + it('highlights text', () => { + expect(el.querySelectorAll('.highlighted').length).toBe(4); + }); + + it('adds ellipsis to long text', done => { + vm.file.path = new Array(70) + .fill() + .map((_, i) => `${i}-`) + .join(''); + + vm.$nextTick(() => { + expect(el.textContent).toBe(`...${vm.file.path.substr(vm.file.path.length - 60)}`); + done(); + }); + }); + }); + + describe('name', () => { + let el; + + beforeEach(done => { + vm.searchText = 'file'; + + el = vm.$el.querySelector('.diff-changed-file-name'); + + vm.$nextTick(done); + }); + + it('highlights text', () => { + expect(el.querySelectorAll('.highlighted').length).toBe(4); + }); + + it('does not add ellipsis to long text', done => { + vm.file.name = new Array(70) + .fill() + .map((_, i) => `${i}-`) + .join(''); + + vm.$nextTick(() => { + expect(el.textContent).not.toBe(`...${vm.file.name.substr(vm.file.name.length - 60)}`); + done(); + }); + }); + }); +}); diff --git a/spec/javascripts/ide/components/ide_spec.js b/spec/javascripts/ide/components/ide_spec.js index 5bd890094cc..7bfcfc90572 100644 --- a/spec/javascripts/ide/components/ide_spec.js +++ b/spec/javascripts/ide/components/ide_spec.js @@ -1,4 +1,5 @@ import Vue from 'vue'; +import Mousetrap from 'mousetrap'; import store from '~/ide/stores'; import ide from '~/ide/components/ide.vue'; import { createComponentWithStore } from 'spec/helpers/vue_mount_component_helper'; @@ -38,4 +39,68 @@ describe('ide component', () => { done(); }); }); + + describe('file finder', () => { + beforeEach(done => { + spyOn(vm, 'toggleFileFinder'); + + vm.$store.state.fileFindVisible = true; + + vm.$nextTick(done); + }); + + it('calls toggleFileFinder on `t` key press', done => { + Mousetrap.trigger('t'); + + vm + .$nextTick() + .then(() => { + expect(vm.toggleFileFinder).toHaveBeenCalled(); + }) + .then(done) + .catch(done.fail); + }); + + it('calls toggleFileFinder on `command+p` key press', done => { + Mousetrap.trigger('command+p'); + + vm + .$nextTick() + .then(() => { + expect(vm.toggleFileFinder).toHaveBeenCalled(); + }) + .then(done) + .catch(done.fail); + }); + + it('calls toggleFileFinder on `ctrl+p` key press', done => { + Mousetrap.trigger('ctrl+p'); + + vm + .$nextTick() + .then(() => { + expect(vm.toggleFileFinder).toHaveBeenCalled(); + }) + .then(done) + .catch(done.fail); + }); + + it('always allows `command+p` to trigger toggleFileFinder', () => { + expect( + vm.mousetrapStopCallback(null, vm.$el.querySelector('.dropdown-input-field'), 'command+p'), + ).toBe(false); + }); + + it('always allows `ctrl+p` to trigger toggleFileFinder', () => { + expect( + vm.mousetrapStopCallback(null, vm.$el.querySelector('.dropdown-input-field'), 'ctrl+p'), + ).toBe(false); + }); + + it('onlys handles `t` when focused in input-field', () => { + expect( + vm.mousetrapStopCallback(null, vm.$el.querySelector('.dropdown-input-field'), 't'), + ).toBe(true); + }); + }); }); diff --git a/spec/javascripts/ide/stores/actions_spec.js b/spec/javascripts/ide/stores/actions_spec.js index 22a7441ba92..b6eadf56f9d 100644 --- a/spec/javascripts/ide/stores/actions_spec.js +++ b/spec/javascripts/ide/stores/actions_spec.js @@ -1,6 +1,5 @@ -import * as urlUtils from '~/lib/utils/url_utility'; +import actions, { stageAllChanges, unstageAllChanges, toggleFileFinder } from '~/ide/stores/actions'; import store from '~/ide/stores'; -import * as actions from '~/ide/stores/actions'; import * as types from '~/ide/stores/mutation_types'; import router from '~/ide/ide_router'; import { resetStore, file } from '../helpers'; @@ -17,12 +16,12 @@ describe('Multi-file store actions', () => { describe('redirectToUrl', () => { it('calls visitUrl', done => { - spyOn(urlUtils, 'visitUrl'); + const visitUrl = spyOnDependency(actions, 'visitUrl'); store .dispatch('redirectToUrl', 'test') .then(() => { - expect(urlUtils.visitUrl).toHaveBeenCalledWith('test'); + expect(visitUrl).toHaveBeenCalledWith('test'); done(); }) @@ -298,7 +297,7 @@ describe('Multi-file store actions', () => { store.state.changedFiles.push(file(), file('new')); testAction( - actions.stageAllChanges, + stageAllChanges, null, store.state, [ @@ -316,7 +315,7 @@ describe('Multi-file store actions', () => { store.state.stagedFiles.push(file(), file('new')); testAction( - actions.unstageAllChanges, + unstageAllChanges, null, store.state, [ @@ -340,4 +339,17 @@ describe('Multi-file store actions', () => { .catch(done.fail); }); }); + + describe('toggleFileFinder', () => { + it('commits TOGGLE_FILE_FINDER', done => { + testAction( + toggleFileFinder, + true, + null, + [{ type: 'TOGGLE_FILE_FINDER', payload: true }], + [], + done, + ); + }); + }); }); diff --git a/spec/javascripts/ide/stores/getters_spec.js b/spec/javascripts/ide/stores/getters_spec.js index 8d04b83928c..b6b4dd28729 100644 --- a/spec/javascripts/ide/stores/getters_spec.js +++ b/spec/javascripts/ide/stores/getters_spec.js @@ -64,4 +64,24 @@ describe('IDE store getters', () => { expect(getters.currentMergeRequest(localState)).toBeNull(); }); }); + + describe('allBlobs', () => { + beforeEach(() => { + Object.assign(localState.entries, { + index: { type: 'blob', name: 'index', lastOpenedAt: 0 }, + app: { type: 'blob', name: 'blob', lastOpenedAt: 0 }, + folder: { type: 'folder', name: 'folder', lastOpenedAt: 0 }, + }); + }); + + it('returns only blobs', () => { + expect(getters.allBlobs(localState).length).toBe(2); + }); + + it('returns list sorted by lastOpenedAt', () => { + localState.entries.app.lastOpenedAt = new Date().getTime(); + + expect(getters.allBlobs(localState)[0].name).toBe('blob'); + }); + }); }); diff --git a/spec/javascripts/ide/stores/modules/commit/actions_spec.js b/spec/javascripts/ide/stores/modules/commit/actions_spec.js index 116967208e0..b2b4b85ca42 100644 --- a/spec/javascripts/ide/stores/modules/commit/actions_spec.js +++ b/spec/javascripts/ide/stores/modules/commit/actions_spec.js @@ -1,7 +1,7 @@ +import actions from '~/ide/stores/actions'; import store from '~/ide/stores'; import service from '~/ide/services'; import router from '~/ide/ide_router'; -import * as urlUtils from '~/lib/utils/url_utility'; import eventHub from '~/ide/eventhub'; import * as consts from '~/ide/stores/modules/commit/constants'; import { resetStore, file } from 'spec/ide/helpers'; @@ -307,8 +307,10 @@ describe('IDE commit module actions', () => { }); describe('commitChanges', () => { + let visitUrl; + beforeEach(() => { - spyOn(urlUtils, 'visitUrl'); + visitUrl = spyOnDependency(actions, 'visitUrl'); document.body.innerHTML += '<div class="flash-container"></div>'; @@ -461,7 +463,7 @@ describe('IDE commit module actions', () => { store .dispatch('commit/commitChanges') .then(() => { - expect(urlUtils.visitUrl).toHaveBeenCalledWith( + expect(visitUrl).toHaveBeenCalledWith( `webUrl/merge_requests/new?merge_request[source_branch]=${ store.getters['commit/newBranchName'] }&merge_request[target_branch]=master`, diff --git a/spec/javascripts/ide/stores/mutations_spec.js b/spec/javascripts/ide/stores/mutations_spec.js index 26e7ed4535e..575039e755e 100644 --- a/spec/javascripts/ide/stores/mutations_spec.js +++ b/spec/javascripts/ide/stores/mutations_spec.js @@ -86,4 +86,12 @@ describe('Multi-file store mutations', () => { expect(localState.viewer).toBe('diff'); }); }); + + describe('TOGGLE_FILE_FINDER', () => { + it('updates fileFindVisible', () => { + mutations.TOGGLE_FILE_FINDER(localState, true); + + expect(localState.fileFindVisible).toBe(true); + }); + }); }); diff --git a/spec/javascripts/issue_show/components/app_spec.js b/spec/javascripts/issue_show/components/app_spec.js index d5a87b5ce20..bf1f0c822fe 100644 --- a/spec/javascripts/issue_show/components/app_spec.js +++ b/spec/javascripts/issue_show/components/app_spec.js @@ -2,7 +2,6 @@ import Vue from 'vue'; import MockAdapter from 'axios-mock-adapter'; import axios from '~/lib/utils/axios_utils'; import '~/behaviors/markdown/render_gfm'; -import * as urlUtils from '~/lib/utils/url_utility'; import issuableApp from '~/issue_show/components/app.vue'; import eventHub from '~/issue_show/event_hub'; import setTimeoutPromise from 'spec/helpers/set_timeout_promise_helper'; @@ -174,7 +173,7 @@ describe('Issuable output', () => { }); it('does not redirect if issue has not moved', (done) => { - spyOn(urlUtils, 'visitUrl'); + const visitUrl = spyOnDependency(issuableApp, 'visitUrl'); spyOn(vm.service, 'updateIssuable').and.callFake(() => new Promise((resolve) => { resolve({ data: { @@ -187,16 +186,13 @@ describe('Issuable output', () => { vm.updateIssuable(); setTimeout(() => { - expect( - urlUtils.visitUrl, - ).not.toHaveBeenCalled(); - + expect(visitUrl).not.toHaveBeenCalled(); done(); }); }); it('redirects if returned web_url has changed', (done) => { - spyOn(urlUtils, 'visitUrl'); + const visitUrl = spyOnDependency(issuableApp, 'visitUrl'); spyOn(vm.service, 'updateIssuable').and.callFake(() => new Promise((resolve) => { resolve({ data: { @@ -209,10 +205,7 @@ describe('Issuable output', () => { vm.updateIssuable(); setTimeout(() => { - expect( - urlUtils.visitUrl, - ).toHaveBeenCalledWith('/testing-issue-move'); - + expect(visitUrl).toHaveBeenCalledWith('/testing-issue-move'); done(); }); }); @@ -340,7 +333,7 @@ describe('Issuable output', () => { describe('deleteIssuable', () => { it('changes URL when deleted', (done) => { - spyOn(urlUtils, 'visitUrl'); + const visitUrl = spyOnDependency(issuableApp, 'visitUrl'); spyOn(vm.service, 'deleteIssuable').and.callFake(() => new Promise((resolve) => { resolve({ data: { @@ -352,16 +345,13 @@ describe('Issuable output', () => { vm.deleteIssuable(); setTimeout(() => { - expect( - urlUtils.visitUrl, - ).toHaveBeenCalledWith('/test'); - + expect(visitUrl).toHaveBeenCalledWith('/test'); done(); }); }); it('stops polling when deleting', (done) => { - spyOn(urlUtils, 'visitUrl'); + spyOnDependency(issuableApp, 'visitUrl'); spyOn(vm.poll, 'stop').and.callThrough(); spyOn(vm.service, 'deleteIssuable').and.callFake(() => new Promise((resolve) => { resolve({ @@ -377,7 +367,6 @@ describe('Issuable output', () => { expect( vm.poll.stop, ).toHaveBeenCalledWith(); - done(); }); }); diff --git a/spec/javascripts/issue_show/components/description_spec.js b/spec/javascripts/issue_show/components/description_spec.js index d96151a8a3a..889c8545faa 100644 --- a/spec/javascripts/issue_show/components/description_spec.js +++ b/spec/javascripts/issue_show/components/description_spec.js @@ -1,7 +1,6 @@ import $ from 'jquery'; import Vue from 'vue'; -import descriptionComponent from '~/issue_show/components/description.vue'; -import * as taskList from '~/task_list'; +import Description from '~/issue_show/components/description.vue'; import mountComponent from 'spec/helpers/vue_mount_component_helper'; describe('Description component', () => { @@ -17,7 +16,7 @@ describe('Description component', () => { }; beforeEach(() => { - DescriptionComponent = Vue.extend(descriptionComponent); + DescriptionComponent = Vue.extend(Description); if (!document.querySelector('.issuable-meta')) { const metaData = document.createElement('div'); @@ -82,18 +81,20 @@ describe('Description component', () => { }); describe('TaskList', () => { + let TaskList; + beforeEach(() => { vm = mountComponent(DescriptionComponent, Object.assign({}, props, { issuableType: 'issuableType', })); - spyOn(taskList, 'default'); + TaskList = spyOnDependency(Description, 'TaskList'); }); it('re-inits the TaskList when description changed', (done) => { vm.descriptionHtml = 'changed'; setTimeout(() => { - expect(taskList.default).toHaveBeenCalled(); + expect(TaskList).toHaveBeenCalled(); done(); }); }); @@ -103,7 +104,7 @@ describe('Description component', () => { vm.descriptionHtml = 'changed'; setTimeout(() => { - expect(taskList.default).not.toHaveBeenCalled(); + expect(TaskList).not.toHaveBeenCalled(); done(); }); }); @@ -112,7 +113,7 @@ describe('Description component', () => { vm.descriptionHtml = 'changed'; setTimeout(() => { - expect(taskList.default).toHaveBeenCalledWith({ + expect(TaskList).toHaveBeenCalledWith({ dataType: 'issuableType', fieldName: 'description', selector: '.detail-page-description', diff --git a/spec/javascripts/job_spec.js b/spec/javascripts/job_spec.js index c6bbacf237a..da00b615c9b 100644 --- a/spec/javascripts/job_spec.js +++ b/spec/javascripts/job_spec.js @@ -2,7 +2,6 @@ import $ from 'jquery'; import MockAdapter from 'axios-mock-adapter'; import axios from '~/lib/utils/axios_utils'; import { numberToHumanSize } from '~/lib/utils/number_utils'; -import * as urlUtils from '~/lib/utils/url_utility'; import '~/lib/utils/datetime_utility'; import Job from '~/job'; import '~/breakpoints'; @@ -22,7 +21,7 @@ describe('Job', () => { beforeEach(() => { loadFixtures('builds/build-with-artifacts.html.raw'); - spyOn(urlUtils, 'visitUrl'); + spyOnDependency(Job, 'visitUrl'); response = {}; diff --git a/spec/javascripts/jobs/sidebar_details_block_spec.js b/spec/javascripts/jobs/sidebar_details_block_spec.js index 6b397c22fb9..9c4454252ce 100644 --- a/spec/javascripts/jobs/sidebar_details_block_spec.js +++ b/spec/javascripts/jobs/sidebar_details_block_spec.js @@ -102,7 +102,7 @@ describe('Sidebar details block', () => { }); it('should render runner ID', () => { - expect(trimWhitespace(vm.$el.querySelector('.js-job-runner'))).toEqual('Runner: #1'); + expect(trimWhitespace(vm.$el.querySelector('.js-job-runner'))).toEqual('Runner: local ci runner (#1)'); }); it('should render timeout information', () => { diff --git a/spec/javascripts/lib/utils/csrf_token_spec.js b/spec/javascripts/lib/utils/csrf_token_spec.js index c484213df8e..81a39a97a84 100644 --- a/spec/javascripts/lib/utils/csrf_token_spec.js +++ b/spec/javascripts/lib/utils/csrf_token_spec.js @@ -1,6 +1,6 @@ import csrf from '~/lib/utils/csrf'; -describe('csrf', () => { +describe('csrf', function () { beforeEach(() => { this.tokenKey = 'X-CSRF-Token'; this.token = 'pH1cvjnP9grx2oKlhWEDvUZnJ8x2eXsIs1qzyHkF3DugSG5yTxR76CWeEZRhML2D1IeVB7NEW0t5l/axE4iJpQ=='; diff --git a/spec/javascripts/lib/utils/image_utility_spec.js b/spec/javascripts/lib/utils/image_utility_spec.js index 75addfcc833..a7eff419fba 100644 --- a/spec/javascripts/lib/utils/image_utility_spec.js +++ b/spec/javascripts/lib/utils/image_utility_spec.js @@ -1,4 +1,4 @@ -import * as imageUtility from '~/lib/utils/image_utility'; +import { isImageLoaded } from '~/lib/utils/image_utility'; describe('imageUtility', () => { describe('isImageLoaded', () => { @@ -8,7 +8,7 @@ describe('imageUtility', () => { naturalHeight: 100, }; - expect(imageUtility.isImageLoaded(element)).toEqual(false); + expect(isImageLoaded(element)).toEqual(false); }); it('should return false when naturalHeight = 0', () => { @@ -17,7 +17,7 @@ describe('imageUtility', () => { naturalHeight: 0, }; - expect(imageUtility.isImageLoaded(element)).toEqual(false); + expect(isImageLoaded(element)).toEqual(false); }); it('should return true when image.complete and naturalHeight != 0', () => { @@ -26,7 +26,7 @@ describe('imageUtility', () => { naturalHeight: 100, }; - expect(imageUtility.isImageLoaded(element)).toEqual(true); + expect(isImageLoaded(element)).toEqual(true); }); }); }); diff --git a/spec/javascripts/merge_request_tabs_spec.js b/spec/javascripts/merge_request_tabs_spec.js index 79c8cf0ba32..3dbd9756cd2 100644 --- a/spec/javascripts/merge_request_tabs_spec.js +++ b/spec/javascripts/merge_request_tabs_spec.js @@ -3,7 +3,6 @@ import $ from 'jquery'; import MockAdapter from 'axios-mock-adapter'; import axios from '~/lib/utils/axios_utils'; -import * as urlUtils from '~/lib/utils/url_utility'; import MergeRequestTabs from '~/merge_request_tabs'; import '~/commit/pipelines/pipelines_bundle'; import '~/breakpoints'; @@ -356,7 +355,7 @@ import 'vendor/jquery.scrollTo'; describe('with note fragment hash', () => { it('should expand and scroll to linked fragment hash #note_xxx', function (done) { - spyOn(urlUtils, 'getLocationHash').and.returnValue(noteId); + spyOnDependency(MergeRequestTabs, 'getLocationHash').and.returnValue(noteId); this.class.loadDiff('/foo/bar/merge_requests/1/diffs'); setTimeout(() => { @@ -372,7 +371,7 @@ import 'vendor/jquery.scrollTo'; }); it('should gracefully ignore non-existant fragment hash', function (done) { - spyOn(urlUtils, 'getLocationHash').and.returnValue('note_something-that-does-not-exist'); + spyOnDependency(MergeRequestTabs, 'getLocationHash').and.returnValue('note_something-that-does-not-exist'); this.class.loadDiff('/foo/bar/merge_requests/1/diffs'); setTimeout(() => { @@ -385,7 +384,7 @@ import 'vendor/jquery.scrollTo'; describe('with line number fragment hash', () => { it('should gracefully ignore line number fragment hash', function () { - spyOn(urlUtils, 'getLocationHash').and.returnValue(noteLineNumId); + spyOnDependency(MergeRequestTabs, 'getLocationHash').and.returnValue(noteLineNumId); this.class.loadDiff('/foo/bar/merge_requests/1/diffs'); expect(noteLineNumId.length).toBeGreaterThan(0); @@ -422,7 +421,7 @@ import 'vendor/jquery.scrollTo'; describe('with note fragment hash', () => { it('should expand and scroll to linked fragment hash #note_xxx', function (done) { - spyOn(urlUtils, 'getLocationHash').and.returnValue(noteId); + spyOnDependency(MergeRequestTabs, 'getLocationHash').and.returnValue(noteId); this.class.loadDiff('/foo/bar/merge_requests/1/diffs'); @@ -439,7 +438,7 @@ import 'vendor/jquery.scrollTo'; }); it('should gracefully ignore non-existant fragment hash', function (done) { - spyOn(urlUtils, 'getLocationHash').and.returnValue('note_something-that-does-not-exist'); + spyOnDependency(MergeRequestTabs, 'getLocationHash').and.returnValue('note_something-that-does-not-exist'); this.class.loadDiff('/foo/bar/merge_requests/1/diffs'); setTimeout(() => { @@ -451,7 +450,7 @@ import 'vendor/jquery.scrollTo'; describe('with line number fragment hash', () => { it('should gracefully ignore line number fragment hash', function () { - spyOn(urlUtils, 'getLocationHash').and.returnValue(noteLineNumId); + spyOnDependency(MergeRequestTabs, 'getLocationHash').and.returnValue(noteLineNumId); this.class.loadDiff('/foo/bar/merge_requests/1/diffs'); expect(noteLineNumId.length).toBeGreaterThan(0); diff --git a/spec/javascripts/monitoring/monitoring_store_spec.js b/spec/javascripts/monitoring/monitoring_store_spec.js index 88aa7659275..08d54946787 100644 --- a/spec/javascripts/monitoring/monitoring_store_spec.js +++ b/spec/javascripts/monitoring/monitoring_store_spec.js @@ -1,7 +1,7 @@ import MonitoringStore from '~/monitoring/stores/monitoring_store'; import MonitoringMock, { deploymentData } from './mock_data'; -describe('MonitoringStore', () => { +describe('MonitoringStore', function () { this.store = new MonitoringStore(); this.store.storeMetrics(MonitoringMock.data); diff --git a/spec/javascripts/notes_spec.js b/spec/javascripts/notes_spec.js index c709e7f13a3..648fb3e9bd3 100644 --- a/spec/javascripts/notes_spec.js +++ b/spec/javascripts/notes_spec.js @@ -3,7 +3,6 @@ import $ from 'jquery'; import _ from 'underscore'; import MockAdapter from 'axios-mock-adapter'; import axios from '~/lib/utils/axios_utils'; -import * as urlUtils from '~/lib/utils/url_utility'; import 'autosize'; import '~/gl_form'; import '~/lib/utils/text_utility'; @@ -222,7 +221,7 @@ import timeoutPromise from './helpers/set_timeout_promise_helper'; }); it('sets target when hash matches', () => { - spyOn(urlUtils, 'getLocationHash').and.returnValue(hash); + spyOnDependency(Notes, 'getLocationHash').and.returnValue(hash); Notes.updateNoteTargetSelector($note); @@ -231,7 +230,7 @@ import timeoutPromise from './helpers/set_timeout_promise_helper'; }); it('unsets target when hash does not match', () => { - spyOn(urlUtils, 'getLocationHash').and.returnValue('note_doesnotexist'); + spyOnDependency(Notes, 'getLocationHash').and.returnValue('note_doesnotexist'); Notes.updateNoteTargetSelector($note); @@ -239,7 +238,7 @@ import timeoutPromise from './helpers/set_timeout_promise_helper'; }); it('unsets target when there is not a hash fragment anymore', () => { - spyOn(urlUtils, 'getLocationHash').and.returnValue(null); + spyOnDependency(Notes, 'getLocationHash').and.returnValue(null); Notes.updateNoteTargetSelector($note); diff --git a/spec/javascripts/pager_spec.js b/spec/javascripts/pager_spec.js index b09494f0b77..04f2e7ef4f9 100644 --- a/spec/javascripts/pager_spec.js +++ b/spec/javascripts/pager_spec.js @@ -1,15 +1,25 @@ -/* global fixture */ +import $ from 'jquery'; import MockAdapter from 'axios-mock-adapter'; import axios from '~/lib/utils/axios_utils'; -import * as utils from '~/lib/utils/url_utility'; import Pager from '~/pager'; describe('pager', () => { + let axiosMock; + + beforeEach(() => { + axiosMock = new MockAdapter(axios); + }); + + afterEach(() => { + axiosMock.restore(); + }); + describe('init', () => { const originalHref = window.location.href; beforeEach(() => { setFixtures('<div class="content_list"></div><div class="loading"></div>'); + spyOn($.fn, 'endlessScroll').and.stub(); }); afterEach(() => { @@ -25,7 +35,7 @@ describe('pager', () => { it('should use current url if data-href attribute not provided', () => { const href = `${gl.TEST_HOST}/some_list`; - spyOn(utils, 'removeParams').and.returnValue(href); + spyOnDependency(Pager, 'removeParams').and.returnValue(href); Pager.init(); expect(Pager.url).toBe(href); }); @@ -39,42 +49,37 @@ describe('pager', () => { it('keeps extra query parameters from url', () => { window.history.replaceState({}, null, '?filter=test&offset=100'); const href = `${gl.TEST_HOST}/some_list?filter=test`; - spyOn(utils, 'removeParams').and.returnValue(href); + const removeParams = spyOnDependency(Pager, 'removeParams').and.returnValue(href); Pager.init(); - expect(utils.removeParams).toHaveBeenCalledWith(['limit', 'offset']); + expect(removeParams).toHaveBeenCalledWith(['limit', 'offset']); expect(Pager.url).toEqual(href); }); }); describe('getOld', () => { const urlRegex = /(.*)some_list(.*)$/; - let mock; function mockSuccess() { - mock.onGet(urlRegex).reply(200, { + axiosMock.onGet(urlRegex).reply(200, { count: 0, html: '', }); } function mockError() { - mock.onGet(urlRegex).networkError(); + axiosMock.onGet(urlRegex).networkError(); } beforeEach(() => { - setFixtures('<div class="content_list" data-href="/some_list"></div><div class="loading"></div>'); + setFixtures( + '<div class="content_list" data-href="/some_list"></div><div class="loading"></div>', + ); spyOn(axios, 'get').and.callThrough(); - mock = new MockAdapter(axios); - Pager.init(); }); - afterEach(() => { - mock.restore(); - }); - - it('shows loader while loading next page', (done) => { + it('shows loader while loading next page', done => { mockSuccess(); spyOn(Pager.loading, 'show'); @@ -87,7 +92,7 @@ describe('pager', () => { }); }); - it('hides loader on success', (done) => { + it('hides loader on success', done => { mockSuccess(); spyOn(Pager.loading, 'hide'); @@ -100,7 +105,7 @@ describe('pager', () => { }); }); - it('hides loader on error', (done) => { + it('hides loader on error', done => { mockError(); spyOn(Pager.loading, 'hide'); @@ -113,7 +118,7 @@ describe('pager', () => { }); }); - it('sends request to url with offset and limit params', (done) => { + it('sends request to url with offset and limit params', done => { Pager.offset = 100; Pager.limit = 20; Pager.getOld(); diff --git a/spec/javascripts/pages/admin/jobs/index/components/stop_jobs_modal_spec.js b/spec/javascripts/pages/admin/jobs/index/components/stop_jobs_modal_spec.js index a6fe9fb65e9..b69e5f9a3a0 100644 --- a/spec/javascripts/pages/admin/jobs/index/components/stop_jobs_modal_spec.js +++ b/spec/javascripts/pages/admin/jobs/index/components/stop_jobs_modal_spec.js @@ -2,7 +2,6 @@ import Vue from 'vue'; import axios from '~/lib/utils/axios_utils'; import stopJobsModal from '~/pages/admin/jobs/index/components/stop_jobs_modal.vue'; -import * as urlUtility from '~/lib/utils/url_utility'; import mountComponent from 'spec/helpers/vue_mount_component_helper'; @@ -24,7 +23,7 @@ describe('stop_jobs_modal.vue', () => { describe('onSubmit', () => { it('stops jobs and redirects to overview page', (done) => { const responseURL = `${gl.TEST_HOST}/stop_jobs_modal.vue/jobs`; - const redirectSpy = spyOn(urlUtility, 'redirectTo'); + const redirectSpy = spyOnDependency(stopJobsModal, 'redirectTo'); spyOn(axios, 'post').and.callFake((url) => { expect(url).toBe(props.url); return Promise.resolve({ @@ -44,7 +43,7 @@ describe('stop_jobs_modal.vue', () => { it('displays error if stopping jobs failed', (done) => { const dummyError = new Error('stopping jobs failed'); - const redirectSpy = spyOn(urlUtility, 'redirectTo'); + const redirectSpy = spyOnDependency(stopJobsModal, 'redirectTo'); spyOn(axios, 'post').and.callFake((url) => { expect(url).toBe(props.url); return Promise.reject(dummyError); diff --git a/spec/javascripts/pages/milestones/shared/components/delete_milestone_modal_spec.js b/spec/javascripts/pages/milestones/shared/components/delete_milestone_modal_spec.js index 6074e06fcec..94401beb5c9 100644 --- a/spec/javascripts/pages/milestones/shared/components/delete_milestone_modal_spec.js +++ b/spec/javascripts/pages/milestones/shared/components/delete_milestone_modal_spec.js @@ -3,7 +3,6 @@ import Vue from 'vue'; import axios from '~/lib/utils/axios_utils'; import deleteMilestoneModal from '~/pages/milestones/shared/components/delete_milestone_modal.vue'; import eventHub from '~/pages/milestones/shared/event_hub'; -import * as urlUtility from '~/lib/utils/url_utility'; import mountComponent from 'spec/helpers/vue_mount_component_helper'; @@ -40,7 +39,7 @@ describe('delete_milestone_modal.vue', () => { }, }); }); - const redirectSpy = spyOn(urlUtility, 'redirectTo'); + const redirectSpy = spyOnDependency(deleteMilestoneModal, 'redirectTo'); vm.onSubmit() .then(() => { @@ -60,7 +59,7 @@ describe('delete_milestone_modal.vue', () => { eventHub.$emit.calls.reset(); return Promise.reject(dummyError); }); - const redirectSpy = spyOn(urlUtility, 'redirectTo'); + const redirectSpy = spyOnDependency(deleteMilestoneModal, 'redirectTo'); vm.onSubmit() .catch((error) => { diff --git a/spec/javascripts/pages/projects/pipeline_schedules/shared/components/pipeline_schedule_callout_spec.js b/spec/javascripts/pages/projects/pipeline_schedules/shared/components/pipeline_schedule_callout_spec.js index f95a7cef18a..fb7d2763b49 100644 --- a/spec/javascripts/pages/projects/pipeline_schedules/shared/components/pipeline_schedule_callout_spec.js +++ b/spec/javascripts/pages/projects/pipeline_schedules/shared/components/pipeline_schedule_callout_spec.js @@ -6,7 +6,7 @@ const PipelineSchedulesCalloutComponent = Vue.extend(PipelineSchedulesCallout); const cookieKey = 'pipeline_schedules_callout_dismissed'; const docsUrl = 'help/ci/scheduled_pipelines'; -describe('Pipeline Schedule Callout', () => { +describe('Pipeline Schedule Callout', function () { beforeEach(() => { setFixtures(` <div id='pipeline-schedules-callout' data-docs-url=${docsUrl}></div> diff --git a/spec/javascripts/right_sidebar_spec.js b/spec/javascripts/right_sidebar_spec.js index 80770a61011..e264b16335f 100644 --- a/spec/javascripts/right_sidebar_spec.js +++ b/spec/javascripts/right_sidebar_spec.js @@ -9,8 +9,6 @@ import Sidebar from '~/right_sidebar'; (function() { var $aside, $icon, $labelsIcon, $page, $toggle, assertSidebarState; - this.sidebar = null; - $aside = null; $toggle = null; @@ -43,7 +41,7 @@ import Sidebar from '~/right_sidebar'; beforeEach(function() { loadFixtures(fixtureName); mock = new MockAdapter(axios); - this.sidebar = new Sidebar(); + new Sidebar(); // eslint-disable-line no-new $aside = $('.right-sidebar'); $page = $('.layout-page'); $icon = $aside.find('i'); diff --git a/spec/javascripts/search_autocomplete_spec.js b/spec/javascripts/search_autocomplete_spec.js index 1a27955983d..4f515f98a7e 100644 --- a/spec/javascripts/search_autocomplete_spec.js +++ b/spec/javascripts/search_autocomplete_spec.js @@ -4,7 +4,6 @@ import $ from 'jquery'; import '~/gl_dropdown'; import SearchAutocomplete from '~/search_autocomplete'; import '~/lib/utils/common_utils'; -import * as urlUtils from '~/lib/utils/url_utility'; describe('Search autocomplete dropdown', () => { var assertLinks, @@ -129,9 +128,6 @@ describe('Search autocomplete dropdown', () => { beforeEach(function() { loadFixtures('static/search_autocomplete.html.raw'); - // Prevent turbolinks from triggering within gl_dropdown - spyOn(urlUtils, 'visitUrl').and.returnValue(true); - window.gon = {}; window.gon.current_user_id = userId; window.gon.current_username = userName; diff --git a/spec/javascripts/shortcuts_dashboard_navigation_spec.js b/spec/javascripts/shortcuts_dashboard_navigation_spec.js index 888b49004bf..7cb201e01d8 100644 --- a/spec/javascripts/shortcuts_dashboard_navigation_spec.js +++ b/spec/javascripts/shortcuts_dashboard_navigation_spec.js @@ -1,24 +1,23 @@ import findAndFollowLink from '~/shortcuts_dashboard_navigation'; -import * as urlUtility from '~/lib/utils/url_utility'; describe('findAndFollowLink', () => { it('visits a link when the selector exists', () => { const href = '/some/path'; - const locationSpy = spyOn(urlUtility, 'visitUrl'); + const visitUrl = spyOnDependency(findAndFollowLink, 'visitUrl'); setFixtures(`<a class="my-shortcut" href="${href}">link</a>`); findAndFollowLink('.my-shortcut'); - expect(locationSpy).toHaveBeenCalledWith(href); + expect(visitUrl).toHaveBeenCalledWith(href); }); it('does not throw an exception when the selector does not exist', () => { - const locationSpy = spyOn(urlUtility, 'visitUrl'); + const visitUrl = spyOnDependency(findAndFollowLink, 'visitUrl'); // this should not throw an exception findAndFollowLink('.this-selector-does-not-exist'); - expect(locationSpy).not.toHaveBeenCalled(); + expect(visitUrl).not.toHaveBeenCalled(); }); }); diff --git a/spec/javascripts/shortcuts_issuable_spec.js b/spec/javascripts/shortcuts_issuable_spec.js index b0d714cbefb..d73608ed0ed 100644 --- a/spec/javascripts/shortcuts_issuable_spec.js +++ b/spec/javascripts/shortcuts_issuable_spec.js @@ -4,7 +4,7 @@ import ShortcutsIssuable from '~/shortcuts_issuable'; initCopyAsGFM(); -describe('ShortcutsIssuable', () => { +describe('ShortcutsIssuable', function () { const fixtureName = 'merge_requests/diff_comment.html.raw'; preloadFixtures(fixtureName); beforeEach(() => { diff --git a/spec/javascripts/sidebar/sidebar_mediator_spec.js b/spec/javascripts/sidebar/sidebar_mediator_spec.js index afa18cc127e..da950258a94 100644 --- a/spec/javascripts/sidebar/sidebar_mediator_spec.js +++ b/spec/javascripts/sidebar/sidebar_mediator_spec.js @@ -1,12 +1,11 @@ import _ from 'underscore'; import Vue from 'vue'; -import * as urlUtils from '~/lib/utils/url_utility'; import SidebarMediator from '~/sidebar/sidebar_mediator'; import SidebarStore from '~/sidebar/stores/sidebar_store'; import SidebarService from '~/sidebar/services/sidebar_service'; import Mock from './mock_data'; -describe('Sidebar mediator', () => { +describe('Sidebar mediator', function() { beforeEach(() => { Vue.http.interceptors.push(Mock.sidebarMockInterceptor); this.mediator = new SidebarMediator(Mock.mediator); @@ -87,12 +86,12 @@ describe('Sidebar mediator', () => { const moveToProjectId = 7; this.mediator.store.setMoveToProjectId(moveToProjectId); spyOn(this.mediator.service, 'moveIssue').and.callThrough(); - spyOn(urlUtils, 'visitUrl'); + const visitUrl = spyOnDependency(SidebarMediator, 'visitUrl'); this.mediator.moveIssue() .then(() => { expect(this.mediator.service.moveIssue).toHaveBeenCalledWith(moveToProjectId); - expect(urlUtils.visitUrl).toHaveBeenCalledWith('/root/some-project/issues/5'); + expect(visitUrl).toHaveBeenCalledWith('/root/some-project/issues/5'); }) .then(done) .catch(done.fail); diff --git a/spec/javascripts/sidebar/sidebar_move_issue_spec.js b/spec/javascripts/sidebar/sidebar_move_issue_spec.js index d8e636cbdf0..a3fb965fbab 100644 --- a/spec/javascripts/sidebar/sidebar_move_issue_spec.js +++ b/spec/javascripts/sidebar/sidebar_move_issue_spec.js @@ -7,7 +7,7 @@ import SidebarService from '~/sidebar/services/sidebar_service'; import SidebarMoveIssue from '~/sidebar/lib/sidebar_move_issue'; import Mock from './mock_data'; -describe('SidebarMoveIssue', () => { +describe('SidebarMoveIssue', function () { beforeEach(() => { Vue.http.interceptors.push(Mock.sidebarMockInterceptor); this.mediator = new SidebarMediator(Mock.mediator); diff --git a/spec/javascripts/sidebar/sidebar_store_spec.js b/spec/javascripts/sidebar/sidebar_store_spec.js index 3591f96ff87..08b112a54ba 100644 --- a/spec/javascripts/sidebar/sidebar_store_spec.js +++ b/spec/javascripts/sidebar/sidebar_store_spec.js @@ -31,7 +31,7 @@ const PARTICIPANT_LIST = [ { ...PARTICIPANT, id: 3 }, ]; -describe('Sidebar store', () => { +describe('Sidebar store', function () { beforeEach(() => { this.store = new SidebarStore({ currentUser: { diff --git a/spec/javascripts/test_bundle.js b/spec/javascripts/test_bundle.js index 14bff05e537..bcd15f5eae2 100644 --- a/spec/javascripts/test_bundle.js +++ b/spec/javascripts/test_bundle.js @@ -1,4 +1,5 @@ -/* eslint-disable jasmine/no-global-setup */ +/* eslint-disable jasmine/no-global-setup, jasmine/no-unsafe-spy, no-underscore-dangle */ + import $ from 'jquery'; import 'vendor/jasmine-jquery'; import '~/commons'; @@ -55,6 +56,17 @@ window.addEventListener('unhandledrejection', event => { console.error(event.reason.stack || event.reason); }); +// Add global function to spy on a module's dependencies via rewire +window.spyOnDependency = (module, name) => { + const dependency = module.__GetDependency__(name); + const spy = jasmine.createSpy(name, dependency); + module.__Rewire__(name, spy); + return spy; +}; + +// Reset any rewired modules after each test (see babel-plugin-rewire) +afterEach(__rewire_reset_all__); // eslint-disable-line + // HACK: Chrome 59 disconnects if there are too many synchronous tests in a row // because it appears to lock up the thread that communicates to Karma's socket // This async beforeEach gets called on every spec and releases the JS thread long diff --git a/spec/javascripts/todos_spec.js b/spec/javascripts/todos_spec.js index 898bbb3819b..e74f4bdef7e 100644 --- a/spec/javascripts/todos_spec.js +++ b/spec/javascripts/todos_spec.js @@ -1,5 +1,4 @@ import $ from 'jquery'; -import * as urlUtils from '~/lib/utils/url_utility'; import Todos from '~/pages/dashboard/todos/index/todos'; import '~/lib/utils/common_utils'; @@ -18,7 +17,7 @@ describe('Todos', () => { it('opens the todo url', (done) => { const todoLink = todoItem.dataset.url; - spyOn(urlUtils, 'visitUrl').and.callFake((url) => { + spyOnDependency(Todos, 'visitUrl').and.callFake((url) => { expect(url).toEqual(todoLink); done(); }); @@ -33,7 +32,7 @@ describe('Todos', () => { beforeEach(() => { metakeyEvent = $.Event('click', { keyCode: 91, ctrlKey: true }); - visitUrlSpy = spyOn(urlUtils, 'visitUrl').and.callFake(() => {}); + visitUrlSpy = spyOnDependency(Todos, 'visitUrl').and.callFake(() => {}); windowOpenSpy = spyOn(window, 'open').and.callFake(() => {}); }); diff --git a/spec/javascripts/u2f/authenticate_spec.js b/spec/javascripts/u2f/authenticate_spec.js index 39c47a5c06d..d84b13b07c4 100644 --- a/spec/javascripts/u2f/authenticate_spec.js +++ b/spec/javascripts/u2f/authenticate_spec.js @@ -3,7 +3,7 @@ import U2FAuthenticate from '~/u2f/authenticate'; import 'vendor/u2f'; import MockU2FDevice from './mock_u2f_device'; -describe('U2FAuthenticate', () => { +describe('U2FAuthenticate', function () { preloadFixtures('u2f/authenticate.html.raw'); beforeEach((done) => { diff --git a/spec/javascripts/u2f/register_spec.js b/spec/javascripts/u2f/register_spec.js index 136b4cad737..d9383314891 100644 --- a/spec/javascripts/u2f/register_spec.js +++ b/spec/javascripts/u2f/register_spec.js @@ -3,7 +3,7 @@ import U2FRegister from '~/u2f/register'; import 'vendor/u2f'; import MockU2FDevice from './mock_u2f_device'; -describe('U2FRegister', () => { +describe('U2FRegister', function () { preloadFixtures('u2f/register.html.raw'); beforeEach((done) => { diff --git a/spec/javascripts/vue_mr_widget/components/deployment_spec.js b/spec/javascripts/vue_mr_widget/components/deployment_spec.js index ff8d54c029f..c82ba61a5b1 100644 --- a/spec/javascripts/vue_mr_widget/components/deployment_spec.js +++ b/spec/javascripts/vue_mr_widget/components/deployment_spec.js @@ -1,5 +1,4 @@ import Vue from 'vue'; -import * as urlUtils from '~/lib/utils/url_utility'; import deploymentComponent from '~/vue_merge_request_widget/components/deployment.vue'; import MRWidgetService from '~/vue_merge_request_widget/services/mr_widget_service'; import { getTimeago } from '~/lib/utils/datetime_utility'; @@ -117,13 +116,13 @@ describe('Deployment component', () => { it('should show a confirm dialog and call service.stopEnvironment when confirmed', (done) => { spyOn(window, 'confirm').and.returnValue(true); spyOn(MRWidgetService, 'stopEnvironment').and.returnValue(returnPromise(true)); - spyOn(urlUtils, 'visitUrl').and.returnValue(true); + const visitUrl = spyOnDependency(deploymentComponent, 'visitUrl').and.returnValue(true); vm = mockStopEnvironment(); expect(window.confirm).toHaveBeenCalled(); expect(MRWidgetService.stopEnvironment).toHaveBeenCalledWith(deploymentMockData.stop_url); setTimeout(() => { - expect(urlUtils.visitUrl).toHaveBeenCalledWith(url); + expect(visitUrl).toHaveBeenCalledWith(url); done(); }, 333); }); diff --git a/spec/javascripts/vue_mr_widget/components/states/mr_widget_ready_to_merge_spec.js b/spec/javascripts/vue_mr_widget/components/states/mr_widget_ready_to_merge_spec.js index 300b7882d03..81c16593eb4 100644 --- a/spec/javascripts/vue_mr_widget/components/states/mr_widget_ready_to_merge_spec.js +++ b/spec/javascripts/vue_mr_widget/components/states/mr_widget_ready_to_merge_spec.js @@ -1,7 +1,6 @@ import Vue from 'vue'; import ReadyToMerge from '~/vue_merge_request_widget/components/states/ready_to_merge.vue'; import eventHub from '~/vue_merge_request_widget/event_hub'; -import * as simplePoll from '~/lib/utils/simple_poll'; const commitMessage = 'This is the commit message'; const commitMessageWithDescription = 'This is the commit message description'; @@ -355,9 +354,9 @@ describe('ReadyToMerge', () => { describe('initiateMergePolling', () => { it('should call simplePoll', () => { - spyOn(simplePoll, 'default'); + const simplePoll = spyOnDependency(ReadyToMerge, 'simplePoll'); vm.initiateMergePolling(); - expect(simplePoll.default).toHaveBeenCalled(); + expect(simplePoll).toHaveBeenCalled(); }); }); @@ -457,11 +456,11 @@ describe('ReadyToMerge', () => { describe('initiateRemoveSourceBranchPolling', () => { it('should emit event and call simplePoll', () => { spyOn(eventHub, '$emit'); - spyOn(simplePoll, 'default'); + const simplePoll = spyOnDependency(ReadyToMerge, 'simplePoll'); vm.initiateRemoveSourceBranchPolling(); expect(eventHub.$emit).toHaveBeenCalledWith('SetBranchRemoveFlag', [true]); - expect(simplePoll.default).toHaveBeenCalled(); + expect(simplePoll).toHaveBeenCalled(); }); }); @@ -524,18 +523,20 @@ describe('ReadyToMerge', () => { }); describe('when user can merge and can delete branch', () => { + let customVm; + beforeEach(() => { - this.customVm = createComponent({ + customVm = createComponent({ mr: { canRemoveSourceBranch: true }, }); }); it('isRemoveSourceBranchButtonDisabled should be false', () => { - expect(this.customVm.isRemoveSourceBranchButtonDisabled).toBe(false); + expect(customVm.isRemoveSourceBranchButtonDisabled).toBe(false); }); it('should be enabled in rendered output', () => { - const checkboxElement = this.customVm.$el.querySelector('#remove-source-branch-input'); + const checkboxElement = customVm.$el.querySelector('#remove-source-branch-input'); expect(checkboxElement).not.toBeNull(); }); }); diff --git a/spec/lib/backup/files_spec.rb b/spec/lib/backup/files_spec.rb index 14d055cbcc1..99872211a4e 100644 --- a/spec/lib/backup/files_spec.rb +++ b/spec/lib/backup/files_spec.rb @@ -62,5 +62,19 @@ describe Backup::Files do subject.restore end end + + describe 'folders that are a mountpoint' do + before do + allow(FileUtils).to receive(:mv).and_raise(Errno::EBUSY) + allow(subject).to receive(:run_pipeline!).and_return(true) + end + + it 'shows error message' do + expect(subject).to receive(:resource_busy_error).with("/var/gitlab-registry") + .and_call_original + + expect { subject.restore }.to raise_error(/is a mountpoint/) + end + end end end diff --git a/spec/lib/backup/repository_spec.rb b/spec/lib/backup/repository_spec.rb index e4c1c9bafc0..b3777be312b 100644 --- a/spec/lib/backup/repository_spec.rb +++ b/spec/lib/backup/repository_spec.rb @@ -81,6 +81,18 @@ describe Backup::Repository do subject.restore end end + + describe 'folder that is a mountpoint' do + before do + allow(FileUtils).to receive(:mv).and_raise(Errno::EBUSY) + end + + it 'shows error message' do + expect(subject).to receive(:resource_busy_error).and_call_original + + expect { subject.restore }.to raise_error(/is a mountpoint/) + end + end end describe '#empty_repo?' do diff --git a/spec/lib/gitlab/auth/ldap/user_spec.rb b/spec/lib/gitlab/auth/ldap/user_spec.rb index cab2169593a..653c19942ea 100644 --- a/spec/lib/gitlab/auth/ldap/user_spec.rb +++ b/spec/lib/gitlab/auth/ldap/user_spec.rb @@ -25,20 +25,20 @@ describe Gitlab::Auth::LDAP::User do OmniAuth::AuthHash.new(uid: 'uid=John Smith,ou=People,dc=example,dc=com', provider: 'ldapmain', info: info_upper_case) end - describe '#changed?' do + describe '#should_save?' do it "marks existing ldap user as changed" do create(:omniauth_user, extern_uid: 'uid=John Smith,ou=People,dc=example,dc=com', provider: 'ldapmain') - expect(ldap_user.changed?).to be_truthy + expect(ldap_user.should_save?).to be_truthy end it "marks existing non-ldap user if the email matches as changed" do create(:user, email: 'john@example.com') - expect(ldap_user.changed?).to be_truthy + expect(ldap_user.should_save?).to be_truthy end it "does not mark existing ldap user as changed" do create(:omniauth_user, email: 'john@example.com', extern_uid: 'uid=john smith,ou=people,dc=example,dc=com', provider: 'ldapmain') - expect(ldap_user.changed?).to be_falsey + expect(ldap_user.should_save?).to be_falsey end end diff --git a/spec/lib/gitlab/auth/o_auth/identity_linker_spec.rb b/spec/lib/gitlab/auth/o_auth/identity_linker_spec.rb new file mode 100644 index 00000000000..528f1b4ec57 --- /dev/null +++ b/spec/lib/gitlab/auth/o_auth/identity_linker_spec.rb @@ -0,0 +1,62 @@ +require 'spec_helper' + +describe Gitlab::Auth::OAuth::IdentityLinker do + let(:user) { create(:user) } + let(:provider) { 'twitter' } + let(:uid) { user.email } + let(:oauth) { { 'provider' => provider, 'uid' => uid } } + + subject { described_class.new(user, oauth) } + + context 'linked identity exists' do + let!(:identity) { user.identities.create!(provider: provider, extern_uid: uid) } + + it "doesn't create new identity" do + expect { subject.link }.not_to change { Identity.count } + end + + it "sets #changed? to false" do + subject.link + + expect(subject).not_to be_changed + end + end + + context 'identity already linked to different user' do + let!(:identity) { create(:identity, provider: provider, extern_uid: uid) } + + it "#changed? returns false" do + subject.link + + expect(subject).not_to be_changed + end + + it 'exposes error message' do + expect(subject.error_message).to eq 'Extern uid has already been taken' + end + end + + context 'identity needs to be created' do + it 'creates linked identity' do + expect { subject.link }.to change { user.identities.count } + end + + it 'sets identity provider' do + subject.link + + expect(user.identities.last.provider).to eq provider + end + + it 'sets identity extern_uid' do + subject.link + + expect(user.identities.last.extern_uid).to eq uid + end + + it 'sets #changed? to true' do + subject.link + + expect(subject).to be_changed + end + end +end diff --git a/spec/lib/gitlab/auth/saml/identity_linker_spec.rb b/spec/lib/gitlab/auth/saml/identity_linker_spec.rb new file mode 100644 index 00000000000..f3305d574cc --- /dev/null +++ b/spec/lib/gitlab/auth/saml/identity_linker_spec.rb @@ -0,0 +1,48 @@ +require 'spec_helper' + +describe Gitlab::Auth::Saml::IdentityLinker do + let(:user) { create(:user) } + let(:provider) { 'saml' } + let(:uid) { user.email } + let(:oauth) { { 'provider' => provider, 'uid' => uid } } + + subject { described_class.new(user, oauth) } + + context 'linked identity exists' do + let!(:identity) { user.identities.create!(provider: provider, extern_uid: uid) } + + it "doesn't create new identity" do + expect { subject.link }.not_to change { Identity.count } + end + + it "sets #changed? to false" do + subject.link + + expect(subject).not_to be_changed + end + end + + context 'identity needs to be created' do + it 'creates linked identity' do + expect { subject.link }.to change { user.identities.count } + end + + it 'sets identity provider' do + subject.link + + expect(user.identities.last.provider).to eq provider + end + + it 'sets identity extern_uid' do + subject.link + + expect(user.identities.last.extern_uid).to eq uid + end + + it 'sets #changed? to true' do + subject.link + + expect(subject).to be_changed + end + end +end diff --git a/spec/lib/gitlab/bare_repository_import/importer_spec.rb b/spec/lib/gitlab/bare_repository_import/importer_spec.rb index eb4b9d8b12f..5c8a19a53bc 100644 --- a/spec/lib/gitlab/bare_repository_import/importer_spec.rb +++ b/spec/lib/gitlab/bare_repository_import/importer_spec.rb @@ -4,6 +4,7 @@ describe Gitlab::BareRepositoryImport::Importer, repository: true do let!(:admin) { create(:admin) } let!(:base_dir) { Dir.mktmpdir + '/' } let(:bare_repository) { Gitlab::BareRepositoryImport::Repository.new(base_dir, File.join(base_dir, "#{project_path}.git")) } + let(:gitlab_shell) { Gitlab::Shell.new } subject(:importer) { described_class.new(admin, bare_repository) } @@ -84,12 +85,14 @@ describe Gitlab::BareRepositoryImport::Importer, repository: true do importer.create_project_if_needed project = Project.find_by_full_path(project_path) - repo_path = File.join(project.repository_storage_path, project.disk_path + '.git') + repo_path = "#{project.disk_path}.git" hook_path = File.join(repo_path, 'hooks') - expect(File).to exist(repo_path) - expect(File.symlink?(hook_path)).to be true - expect(File.readlink(hook_path)).to eq(Gitlab.config.gitlab_shell.hooks_path) + expect(gitlab_shell.exists?(project.repository_storage, repo_path)).to be(true) + expect(gitlab_shell.exists?(project.repository_storage, hook_path)).to be(true) + + full_hook_path = File.join(project.repository.path_to_repo, 'hooks') + expect(File.readlink(full_hook_path)).to eq(Gitlab.config.gitlab_shell.hooks_path) end context 'hashed storage enabled' do @@ -144,8 +147,8 @@ describe Gitlab::BareRepositoryImport::Importer, repository: true do project = Project.find_by_full_path("#{admin.full_path}/#{project_path}") - expect(File).to exist(File.join(project.repository_storage_path, project.disk_path + '.git')) - expect(File).to exist(File.join(project.repository_storage_path, project.disk_path + '.wiki.git')) + expect(gitlab_shell.exists?(project.repository_storage, project.disk_path + '.git')).to be(true) + expect(gitlab_shell.exists?(project.repository_storage, project.disk_path + '.wiki.git')).to be(true) end it 'moves an existing project to the correct path' do @@ -155,7 +158,9 @@ describe Gitlab::BareRepositoryImport::Importer, repository: true do project = build(:project, :legacy_storage, :repository) original_commit_count = project.repository.commit_count - bare_repo = Gitlab::BareRepositoryImport::Repository.new(project.repository_storage_path, project.repository.path) + legacy_path = Gitlab.config.repositories.storages[project.repository_storage].legacy_disk_path + + bare_repo = Gitlab::BareRepositoryImport::Repository.new(legacy_path, project.repository.path) gitlab_importer = described_class.new(admin, bare_repo) expect(gitlab_importer).to receive(:create_project).and_call_original @@ -183,7 +188,7 @@ describe Gitlab::BareRepositoryImport::Importer, repository: true do project = Project.find_by_full_path(project_path) - expect(File).to exist(File.join(project.repository_storage_path, project.disk_path + '.wiki.git')) + expect(gitlab_shell.exists?(project.repository_storage, project.disk_path + '.wiki.git')).to be(true) end end diff --git a/spec/lib/gitlab/bare_repository_import/repository_spec.rb b/spec/lib/gitlab/bare_repository_import/repository_spec.rb index 0dc3705825d..1504826c7a5 100644 --- a/spec/lib/gitlab/bare_repository_import/repository_spec.rb +++ b/spec/lib/gitlab/bare_repository_import/repository_spec.rb @@ -67,7 +67,7 @@ describe ::Gitlab::BareRepositoryImport::Repository do end after do - gitlab_shell.remove_repository(root_path, hashed_path) + gitlab_shell.remove_repository(repository_storage, hashed_path) end subject { described_class.new(root_path, repo_path) } diff --git a/spec/lib/gitlab/ci/status/build/play_spec.rb b/spec/lib/gitlab/ci/status/build/play_spec.rb index f128c1d4ca4..e2bb378f663 100644 --- a/spec/lib/gitlab/ci/status/build/play_spec.rb +++ b/spec/lib/gitlab/ci/status/build/play_spec.rb @@ -2,8 +2,8 @@ require 'spec_helper' describe Gitlab::Ci::Status::Build::Play do let(:user) { create(:user) } - let(:project) { build.project } - let(:build) { create(:ci_build, :manual) } + let(:project) { create(:project, :stubbed_repository) } + let(:build) { create(:ci_build, :manual, project: project) } let(:status) { Gitlab::Ci::Status::Core.new(build, user) } subject { described_class.new(status) } @@ -46,6 +46,8 @@ describe Gitlab::Ci::Status::Build::Play do context 'when user can not push to the branch' do before do build.project.add_developer(user) + create(:protected_branch, :masters_can_push, + name: build.ref, project: project) end it { is_expected.not_to have_action } diff --git a/spec/lib/gitlab/git/repository_spec.rb b/spec/lib/gitlab/git/repository_spec.rb index 5acf40ea5ce..da1a6229ccf 100644 --- a/spec/lib/gitlab/git/repository_spec.rb +++ b/spec/lib/gitlab/git/repository_spec.rb @@ -689,7 +689,7 @@ describe Gitlab::Git::Repository, seed_helper: true do end after do - Gitlab::Shell.new.remove_repository(storage_path, 'my_project') + Gitlab::Shell.new.remove_repository('default', 'my_project') end shared_examples 'repository mirror fecthing' do diff --git a/spec/lib/gitlab/import_export/all_models.yml b/spec/lib/gitlab/import_export/all_models.yml index 897a5984782..e7f20f81fe0 100644 --- a/spec/lib/gitlab/import_export/all_models.yml +++ b/spec/lib/gitlab/import_export/all_models.yml @@ -286,6 +286,7 @@ project: - internal_ids - project_deploy_tokens - deploy_tokens +- ci_cd_settings award_emoji: - awardable - user diff --git a/spec/lib/gitlab/import_export/safe_model_attributes.yml b/spec/lib/gitlab/import_export/safe_model_attributes.yml index 05790bb5fe1..31141807cb2 100644 --- a/spec/lib/gitlab/import_export/safe_model_attributes.yml +++ b/spec/lib/gitlab/import_export/safe_model_attributes.yml @@ -546,3 +546,5 @@ Badge: - created_at - updated_at - type +ProjectCiCdSetting: +- group_runners_enabled diff --git a/spec/lib/gitlab/import_export/wiki_restorer_spec.rb b/spec/lib/gitlab/import_export/wiki_restorer_spec.rb index 5c01ee0ebb8..f99f198da33 100644 --- a/spec/lib/gitlab/import_export/wiki_restorer_spec.rb +++ b/spec/lib/gitlab/import_export/wiki_restorer_spec.rb @@ -24,8 +24,8 @@ describe Gitlab::ImportExport::WikiRestorer do after do FileUtils.rm_rf(export_path) - Gitlab::Shell.new.remove_repository(project_with_wiki.wiki.repository_storage_path, project_with_wiki.wiki.disk_path) - Gitlab::Shell.new.remove_repository(project.wiki.repository_storage_path, project.wiki.disk_path) + Gitlab::Shell.new.remove_repository(project_with_wiki.wiki.repository_storage, project_with_wiki.wiki.disk_path) + Gitlab::Shell.new.remove_repository(project.wiki.repository_storage, project.wiki.disk_path) end it 'restores the wiki repo successfully' do diff --git a/spec/lib/gitlab/shell_spec.rb b/spec/lib/gitlab/shell_spec.rb index 7f579df1c36..bf6ee4b0b59 100644 --- a/spec/lib/gitlab/shell_spec.rb +++ b/spec/lib/gitlab/shell_spec.rb @@ -447,18 +447,18 @@ describe Gitlab::Shell do let(:disk_path) { "#{project.disk_path}.git" } it 'returns true when the command succeeds' do - expect(gitlab_shell.exists?(project.repository_storage_path, disk_path)).to be(true) + expect(gitlab_shell.exists?(project.repository_storage, disk_path)).to be(true) - expect(gitlab_shell.remove_repository(project.repository_storage_path, project.disk_path)).to be(true) + expect(gitlab_shell.remove_repository(project.repository_storage, project.disk_path)).to be(true) - expect(gitlab_shell.exists?(project.repository_storage_path, disk_path)).to be(false) + expect(gitlab_shell.exists?(project.repository_storage, disk_path)).to be(false) end it 'keeps the namespace directory' do - gitlab_shell.remove_repository(project.repository_storage_path, project.disk_path) + gitlab_shell.remove_repository(project.repository_storage, project.disk_path) - expect(gitlab_shell.exists?(project.repository_storage_path, disk_path)).to be(false) - expect(gitlab_shell.exists?(project.repository_storage_path, project.disk_path.gsub(project.name, ''))).to be(true) + expect(gitlab_shell.exists?(project.repository_storage, disk_path)).to be(false) + expect(gitlab_shell.exists?(project.repository_storage, project.disk_path.gsub(project.name, ''))).to be(true) end end @@ -469,18 +469,18 @@ describe Gitlab::Shell do old_path = project2.disk_path new_path = "project/new_path" - expect(gitlab_shell.exists?(project2.repository_storage_path, "#{old_path}.git")).to be(true) - expect(gitlab_shell.exists?(project2.repository_storage_path, "#{new_path}.git")).to be(false) + expect(gitlab_shell.exists?(project2.repository_storage, "#{old_path}.git")).to be(true) + expect(gitlab_shell.exists?(project2.repository_storage, "#{new_path}.git")).to be(false) - expect(gitlab_shell.mv_repository(project2.repository_storage_path, old_path, new_path)).to be_truthy + expect(gitlab_shell.mv_repository(project2.repository_storage, old_path, new_path)).to be_truthy - expect(gitlab_shell.exists?(project2.repository_storage_path, "#{old_path}.git")).to be(false) - expect(gitlab_shell.exists?(project2.repository_storage_path, "#{new_path}.git")).to be(true) + expect(gitlab_shell.exists?(project2.repository_storage, "#{old_path}.git")).to be(false) + expect(gitlab_shell.exists?(project2.repository_storage, "#{new_path}.git")).to be(true) end it 'returns false when the command fails' do - expect(gitlab_shell.mv_repository(project2.repository_storage_path, project2.disk_path, '')).to be_falsy - expect(gitlab_shell.exists?(project2.repository_storage_path, "#{project2.disk_path}.git")).to be(true) + expect(gitlab_shell.mv_repository(project2.repository_storage, project2.disk_path, '')).to be_falsy + expect(gitlab_shell.exists?(project2.repository_storage, "#{project2.disk_path}.git")).to be(true) end end @@ -679,48 +679,48 @@ describe Gitlab::Shell do describe 'namespace actions' do subject { described_class.new } - let(:storage_path) { Gitlab.config.repositories.storages.default.legacy_disk_path } + let(:storage) { Gitlab.config.repositories.storages.keys.first } describe '#add_namespace' do it 'creates a namespace' do - subject.add_namespace(storage_path, "mepmep") + subject.add_namespace(storage, "mepmep") - expect(subject.exists?(storage_path, "mepmep")).to be(true) + expect(subject.exists?(storage, "mepmep")).to be(true) end end describe '#exists?' do context 'when the namespace does not exist' do it 'returns false' do - expect(subject.exists?(storage_path, "non-existing")).to be(false) + expect(subject.exists?(storage, "non-existing")).to be(false) end end context 'when the namespace exists' do it 'returns true' do - subject.add_namespace(storage_path, "mepmep") + subject.add_namespace(storage, "mepmep") - expect(subject.exists?(storage_path, "mepmep")).to be(true) + expect(subject.exists?(storage, "mepmep")).to be(true) end end end describe '#remove' do it 'removes the namespace' do - subject.add_namespace(storage_path, "mepmep") - subject.rm_namespace(storage_path, "mepmep") + subject.add_namespace(storage, "mepmep") + subject.rm_namespace(storage, "mepmep") - expect(subject.exists?(storage_path, "mepmep")).to be(false) + expect(subject.exists?(storage, "mepmep")).to be(false) end end describe '#mv_namespace' do it 'renames the namespace' do - subject.add_namespace(storage_path, "mepmep") - subject.mv_namespace(storage_path, "mepmep", "2mep") + subject.add_namespace(storage, "mepmep") + subject.mv_namespace(storage, "mepmep", "2mep") - expect(subject.exists?(storage_path, "mepmep")).to be(false) - expect(subject.exists?(storage_path, "2mep")).to be(true) + expect(subject.exists?(storage, "mepmep")).to be(false) + expect(subject.exists?(storage, "2mep")).to be(true) end end end diff --git a/spec/lib/gitlab/user_access_spec.rb b/spec/lib/gitlab/user_access_spec.rb index 40c8286b1b9..97b6069f64d 100644 --- a/spec/lib/gitlab/user_access_spec.rb +++ b/spec/lib/gitlab/user_access_spec.rb @@ -32,6 +32,12 @@ describe Gitlab::UserAccess do let(:empty_project) { create(:project_empty_repo) } let(:project_access) { described_class.new(user, project: empty_project) } + it 'returns true for admins' do + user.update!(admin: true) + + expect(access.can_push_to_branch?('master')).to be_truthy + end + it 'returns true if user is master' do empty_project.add_master(user) @@ -71,6 +77,12 @@ describe Gitlab::UserAccess do let(:branch) { create :protected_branch, project: project, name: "test" } let(:not_existing_branch) { create :protected_branch, :developers_can_merge, project: project } + it 'returns true for admins' do + user.update!(admin: true) + + expect(access.can_push_to_branch?(branch.name)).to be_truthy + end + it 'returns true if user is a master' do project.add_master(user) diff --git a/spec/migrations/assure_commits_count_for_merge_request_diff_spec.rb b/spec/migrations/assure_commits_count_for_merge_request_diff_spec.rb new file mode 100644 index 00000000000..b8c3a3eda4e --- /dev/null +++ b/spec/migrations/assure_commits_count_for_merge_request_diff_spec.rb @@ -0,0 +1,32 @@ +require 'spec_helper' +require Rails.root.join('db', 'migrate', '20180425131009_assure_commits_count_for_merge_request_diff.rb') + +describe AssureCommitsCountForMergeRequestDiff, :migration, :sidekiq, :redis do + let(:migration) { spy('migration') } + + before do + allow(Gitlab::BackgroundMigration::AddMergeRequestDiffCommitsCount) + .to receive(:new).and_return(migration) + end + + context 'when there are still unmigrated commit_counts afterwards' do + let(:namespaces) { table('namespaces') } + let(:projects) { table('projects') } + let(:merge_requests) { table('merge_requests') } + let(:diffs) { table('merge_request_diffs') } + + before do + namespace = namespaces.create(name: 'foo', path: 'foo') + project = projects.create!(namespace_id: namespace.id) + merge_request = merge_requests.create!(source_branch: 'x', target_branch: 'y', target_project_id: project.id) + diffs.create!(commits_count: nil, merge_request_id: merge_request.id) + diffs.create!(commits_count: nil, merge_request_id: merge_request.id) + end + + it 'migrates commit_counts sequentially in batches' do + migrate! + + expect(migration).to have_received(:perform).once + end + end +end diff --git a/spec/models/ci/build_spec.rb b/spec/models/ci/build_spec.rb index fcdc31c8984..3158e006720 100644 --- a/spec/models/ci/build_spec.rb +++ b/spec/models/ci/build_spec.rb @@ -2035,6 +2035,34 @@ describe Ci::Build do expect(build).not_to be_persisted end end + + context 'for deploy tokens' do + let(:deploy_token) { create(:deploy_token, :gitlab_deploy_token) } + + let(:deploy_token_variables) do + [ + { key: 'CI_DEPLOY_USER', value: deploy_token.name, public: true }, + { key: 'CI_DEPLOY_PASSWORD', value: deploy_token.token, public: false } + ] + end + + context 'when gitlab-deploy-token exists' do + before do + project.deploy_tokens << deploy_token + end + + it 'should include deploy token variables' do + is_expected.to include(*deploy_token_variables) + end + end + + context 'when gitlab-deploy-token does not exist' do + it 'should not include deploy token variables' do + expect(subject.find { |v| v[:key] == 'CI_DEPLOY_USER'}).to be_nil + expect(subject.find { |v| v[:key] == 'CI_DEPLOY_PASSWORD'}).to be_nil + end + end + end end describe '#scoped_variables' do @@ -2083,7 +2111,9 @@ describe Ci::Build do CI_REGISTRY_USER CI_REGISTRY_PASSWORD CI_REPOSITORY_URL - CI_ENVIRONMENT_URL] + CI_ENVIRONMENT_URL + CI_DEPLOY_USER + CI_DEPLOY_PASSWORD] build.scoped_variables.map { |env| env[:key] }.tap do |names| expect(names).not_to include(*keys) diff --git a/spec/models/deploy_token_spec.rb b/spec/models/deploy_token_spec.rb index 780b200e837..f8d51a95833 100644 --- a/spec/models/deploy_token_spec.rb +++ b/spec/models/deploy_token_spec.rb @@ -142,4 +142,23 @@ describe DeployToken do end end end + + describe '.gitlab_deploy_token' do + let(:project) { create(:project ) } + + subject { project.deploy_tokens.gitlab_deploy_token } + + context 'with a gitlab deploy token associated' do + it 'should return the gitlab deploy token' do + deploy_token = create(:deploy_token, :gitlab_deploy_token, projects: [project]) + is_expected.to eq(deploy_token) + end + end + + context 'with no gitlab deploy token associated' do + it 'should return nil' do + is_expected.to be_nil + end + end + end end diff --git a/spec/models/environment_spec.rb b/spec/models/environment_spec.rb index 56161bfcc28..25d6597084c 100644 --- a/spec/models/environment_spec.rb +++ b/spec/models/environment_spec.rb @@ -1,7 +1,7 @@ require 'spec_helper' describe Environment do - let(:project) { create(:project) } + let(:project) { create(:project, :stubbed_repository) } subject(:environment) { create(:environment, project: project) } it { is_expected.to belong_to(:project) } @@ -201,7 +201,7 @@ describe Environment do end describe '#stop_with_action!' do - let(:user) { create(:admin) } + let(:user) { create(:user) } subject { environment.stop_with_action!(user) } diff --git a/spec/models/namespace_spec.rb b/spec/models/namespace_spec.rb index 62e95a622eb..506057dce87 100644 --- a/spec/models/namespace_spec.rb +++ b/spec/models/namespace_spec.rb @@ -5,6 +5,7 @@ describe Namespace do let!(:namespace) { create(:namespace) } let(:gitlab_shell) { Gitlab::Shell.new } + let(:repository_storage) { 'default' } describe 'associations' do it { is_expected.to have_many :projects } @@ -201,7 +202,7 @@ describe Namespace do it "moves dir if path changed" do namespace.update_attributes(path: namespace.full_path + '_new') - expect(gitlab_shell.exists?(project.repository_storage_path, "#{namespace.path}/#{project.path}.git")).to be_truthy + expect(gitlab_shell.exists?(project.repository_storage, "#{namespace.path}/#{project.path}.git")).to be_truthy end context 'with subgroups', :nested_groups do @@ -281,7 +282,7 @@ describe Namespace do namespace.update_attributes(path: namespace.full_path + '_new') expect(before_disk_path).to eq(project.disk_path) - expect(gitlab_shell.exists?(project.repository_storage_path, "#{project.disk_path}.git")).to be_truthy + expect(gitlab_shell.exists?(project.repository_storage, "#{project.disk_path}.git")).to be_truthy end end @@ -322,7 +323,7 @@ describe Namespace do end it 'schedules the namespace for deletion' do - expect(GitlabShellWorker).to receive(:perform_in).with(5.minutes, :rm_namespace, repository_storage_path, deleted_path) + expect(GitlabShellWorker).to receive(:perform_in).with(5.minutes, :rm_namespace, repository_storage, deleted_path) namespace.destroy end @@ -344,7 +345,7 @@ describe Namespace do end it 'schedules the namespace for deletion' do - expect(GitlabShellWorker).to receive(:perform_in).with(5.minutes, :rm_namespace, repository_storage_path, deleted_path) + expect(GitlabShellWorker).to receive(:perform_in).with(5.minutes, :rm_namespace, repository_storage, deleted_path) child.destroy end diff --git a/spec/models/project_ci_cd_setting_spec.rb b/spec/models/project_ci_cd_setting_spec.rb new file mode 100644 index 00000000000..4aa62028169 --- /dev/null +++ b/spec/models/project_ci_cd_setting_spec.rb @@ -0,0 +1,24 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe ProjectCiCdSetting do + describe '.available?' do + before do + described_class.reset_column_information + end + + it 'returns true' do + expect(described_class).to be_available + end + + it 'memoizes the schema version' do + expect(ActiveRecord::Migrator) + .to receive(:current_version) + .and_call_original + .once + + 2.times { described_class.available? } + end + end +end diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index 4002722e358..a9587b1005e 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -93,6 +93,15 @@ describe Project do end end + context 'when creating a new project' do + it 'automatically creates a CI/CD settings row' do + project = create(:project) + + expect(project.ci_cd_settings).to be_an_instance_of(ProjectCiCdSetting) + expect(project.ci_cd_settings).to be_persisted + end + end + describe '#members & #requesters' do let(:project) { create(:project, :public, :access_requestable) } let(:requester) { create(:user) } @@ -440,14 +449,6 @@ describe Project do end end - describe '#repository_storage_path' do - let(:project) { create(:project) } - - it 'returns the repository storage path' do - expect(Dir.exist?(project.repository_storage_path)).to be(true) - end - end - it 'returns valid url to repo' do project = described_class.new(path: 'somewhere') expect(project.url_to_repo).to eq(Gitlab.config.gitlab_shell.ssh_path_prefix + 'somewhere.git') @@ -1099,7 +1100,7 @@ describe Project do end context 'repository storage by default' do - let(:project) { create(:project) } + let(:project) { build(:project) } before do storages = { @@ -1452,7 +1453,7 @@ describe Project do .and_return(false) allow(shell).to receive(:create_repository) - .with(project.repository_storage_path, project.disk_path) + .with(project.repository_storage, project.disk_path) .and_return(true) expect(project).to receive(:create_repository).with(force: true) @@ -1483,52 +1484,6 @@ describe Project do end end - describe '#user_can_push_to_empty_repo?' do - let(:project) { create(:project) } - let(:user) { create(:user) } - - it 'returns false when default_branch_protection is in full protection and user is developer' do - project.add_developer(user) - stub_application_setting(default_branch_protection: Gitlab::Access::PROTECTION_FULL) - - expect(project.user_can_push_to_empty_repo?(user)).to be_falsey - end - - it 'returns false when default_branch_protection only lets devs merge and user is dev' do - project.add_developer(user) - stub_application_setting(default_branch_protection: Gitlab::Access::PROTECTION_DEV_CAN_MERGE) - - expect(project.user_can_push_to_empty_repo?(user)).to be_falsey - end - - it 'returns true when default_branch_protection lets devs push and user is developer' do - project.add_developer(user) - stub_application_setting(default_branch_protection: Gitlab::Access::PROTECTION_DEV_CAN_PUSH) - - expect(project.user_can_push_to_empty_repo?(user)).to be_truthy - end - - it 'returns true when default_branch_protection is unprotected and user is developer' do - project.add_developer(user) - stub_application_setting(default_branch_protection: Gitlab::Access::PROTECTION_NONE) - - expect(project.user_can_push_to_empty_repo?(user)).to be_truthy - end - - it 'returns true when user is master' do - project.add_master(user) - - expect(project.user_can_push_to_empty_repo?(user)).to be_truthy - end - - it 'returns false when the repo is not empty' do - project.add_master(user) - expect(project).to receive(:empty_repo?).and_return(false) - - expect(project.user_can_push_to_empty_repo?(user)).to be_falsey - end - end - describe '#container_registry_url' do let(:project) { create(:project) } @@ -2673,7 +2628,7 @@ describe Project do describe '#ensure_storage_path_exists' do it 'delegates to gitlab_shell to ensure namespace is created' do - expect(gitlab_shell).to receive(:add_namespace).with(project.repository_storage_path, project.base_dir) + expect(gitlab_shell).to receive(:add_namespace).with(project.repository_storage, project.base_dir) project.ensure_storage_path_exists end @@ -2712,12 +2667,12 @@ describe Project do expect(gitlab_shell).to receive(:mv_repository) .ordered - .with(project.repository_storage_path, "#{project.namespace.full_path}/foo", "#{project.full_path}") + .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_path, "#{project.namespace.full_path}/foo.wiki", "#{project.full_path}.wiki") + .with(project.repository_storage, "#{project.namespace.full_path}/foo.wiki", "#{project.full_path}.wiki") .and_return(true) expect_any_instance_of(SystemHooksService) @@ -2866,7 +2821,7 @@ describe Project do it 'delegates to gitlab_shell to ensure namespace is created' do allow(project).to receive(:gitlab_shell).and_return(gitlab_shell) - expect(gitlab_shell).to receive(:add_namespace).with(project.repository_storage_path, hashed_prefix) + expect(gitlab_shell).to receive(:add_namespace).with(project.repository_storage, hashed_prefix) project.ensure_storage_path_exists end @@ -3585,4 +3540,44 @@ describe Project do it { is_expected.not_to be_valid } end end + + describe '#gitlab_deploy_token' do + let(:project) { create(:project) } + + subject { project.gitlab_deploy_token } + + context 'when there is a gitlab deploy token associated' do + let!(:deploy_token) { create(:deploy_token, :gitlab_deploy_token, projects: [project]) } + + it { is_expected.to eq(deploy_token) } + end + + context 'when there is no a gitlab deploy token associated' do + it { is_expected.to be_nil } + end + + context 'when there is a gitlab deploy token associated but is has been revoked' do + let!(:deploy_token) { create(:deploy_token, :gitlab_deploy_token, :revoked, projects: [project]) } + it { is_expected.to be_nil } + end + + context 'when there is a gitlab deploy token associated but it is expired' do + let!(:deploy_token) { create(:deploy_token, :gitlab_deploy_token, :expired, projects: [project]) } + + it { is_expected.to be_nil } + end + + context 'when there is a deploy token associated with a different name' do + let!(:deploy_token) { create(:deploy_token, projects: [project]) } + + it { is_expected.to be_nil } + end + + context 'when there is a deploy token associated to a different project' do + let(:project_2) { create(:project) } + let!(:deploy_token) { create(:deploy_token, projects: [project_2]) } + + it { is_expected.to be_nil } + end + end end diff --git a/spec/models/project_wiki_spec.rb b/spec/models/project_wiki_spec.rb index 4e83f4353cf..cbe7d111fcd 100644 --- a/spec/models/project_wiki_spec.rb +++ b/spec/models/project_wiki_spec.rb @@ -11,7 +11,7 @@ describe ProjectWiki do subject { project_wiki } it { is_expected.to delegate_method(:empty?).to :pages } - it { is_expected.to delegate_method(:repository_storage_path).to :project } + it { is_expected.to delegate_method(:repository_storage).to :project } it { is_expected.to delegate_method(:hashed_storage?).to :project } describe "#full_path" do diff --git a/spec/policies/group_policy_spec.rb b/spec/policies/group_policy_spec.rb index b4d25e06d9a..9b5c290b9f9 100644 --- a/spec/policies/group_policy_spec.rb +++ b/spec/policies/group_policy_spec.rb @@ -7,9 +7,9 @@ describe GroupPolicy do let(:master) { create(:user) } let(:owner) { create(:user) } let(:admin) { create(:admin) } - let(:group) { create(:group) } + let(:group) { create(:group, :private) } - let(:guest_permissions) { [:read_group, :upload_file, :read_namespace] } + let(:guest_permissions) { [:read_label, :read_group, :upload_file, :read_namespace] } let(:reporter_permissions) { [:admin_label] } @@ -50,6 +50,7 @@ describe GroupPolicy do end context 'with no user' do + let(:group) { create(:group, :public) } let(:current_user) { nil } it do @@ -63,6 +64,28 @@ describe GroupPolicy do end end + context 'has projects' do + let(:current_user) { create(:user) } + let(:project) { create(:project, namespace: group) } + + before do + project.add_developer(current_user) + end + + it do + expect_allowed(:read_group, :read_label) + end + + context 'in subgroups', :nested_groups do + let(:subgroup) { create(:group, :private, parent: group) } + let(:project) { create(:project, namespace: subgroup) } + + it do + expect_allowed(:read_group, :read_label) + end + end + end + context 'guests' do let(:current_user) { guest } diff --git a/spec/presenters/project_presenter_spec.rb b/spec/presenters/project_presenter_spec.rb index 0a130c59037..830d2ee3b20 100644 --- a/spec/presenters/project_presenter_spec.rb +++ b/spec/presenters/project_presenter_spec.rb @@ -208,6 +208,17 @@ describe ProjectPresenter do it 'returns nil if user cannot push' do expect(presenter.new_file_anchor_data).to be_nil end + + context 'when the project is empty' do + let(:project) { create(:project, :empty_repo) } + + # Since we protect the default branch for empty repos + it 'is empty for a developer' do + project.add_developer(user) + + expect(presenter.new_file_anchor_data).to be_nil + end + end end describe '#readme_anchor_data' do diff --git a/spec/services/ci/retry_pipeline_service_spec.rb b/spec/services/ci/retry_pipeline_service_spec.rb index 6ce75c65c8c..f1acfc48468 100644 --- a/spec/services/ci/retry_pipeline_service_spec.rb +++ b/spec/services/ci/retry_pipeline_service_spec.rb @@ -235,6 +235,8 @@ describe Ci::RetryPipelineService, '#execute' do context 'when user is not allowed to trigger manual action' do before do project.add_developer(user) + create(:protected_branch, :masters_can_push, + name: pipeline.ref, project: project) end context 'when there is a failed manual action present' do diff --git a/spec/services/groups/destroy_service_spec.rb b/spec/services/groups/destroy_service_spec.rb index e8216abb08b..a9baccd061a 100644 --- a/spec/services/groups/destroy_service_spec.rb +++ b/spec/services/groups/destroy_service_spec.rb @@ -53,8 +53,8 @@ describe Groups::DestroyService do end it 'verifies that paths have been deleted' do - expect(gitlab_shell.exists?(project.repository_storage_path, group.path)).to be_falsey - expect(gitlab_shell.exists?(project.repository_storage_path, remove_path)).to be_falsey + expect(gitlab_shell.exists?(project.repository_storage, group.path)).to be_falsey + expect(gitlab_shell.exists?(project.repository_storage, remove_path)).to be_falsey end end end @@ -71,13 +71,13 @@ describe Groups::DestroyService do after do # Clean up stale directories - gitlab_shell.rm_namespace(project.repository_storage_path, group.path) - gitlab_shell.rm_namespace(project.repository_storage_path, remove_path) + gitlab_shell.rm_namespace(project.repository_storage, group.path) + gitlab_shell.rm_namespace(project.repository_storage, remove_path) end it 'verifies original paths and projects still exist' do - expect(gitlab_shell.exists?(project.repository_storage_path, group.path)).to be_truthy - expect(gitlab_shell.exists?(project.repository_storage_path, remove_path)).to be_falsey + expect(gitlab_shell.exists?(project.repository_storage, group.path)).to be_truthy + expect(gitlab_shell.exists?(project.repository_storage, remove_path)).to be_falsey expect(Project.unscoped.count).to eq(1) expect(Group.unscoped.count).to eq(2) end @@ -144,7 +144,7 @@ describe Groups::DestroyService do let!(:project) { create(:project, :legacy_storage, :empty_repo, namespace: group) } it 'removes repository' do - expect(gitlab_shell.exists?(project.repository_storage_path, "#{project.disk_path}.git")).to be_falsey + expect(gitlab_shell.exists?(project.repository_storage, "#{project.disk_path}.git")).to be_falsey end end @@ -152,7 +152,7 @@ describe Groups::DestroyService do let!(:project) { create(:project, :empty_repo, namespace: group) } it 'removes repository' do - expect(gitlab_shell.exists?(project.repository_storage_path, "#{project.disk_path}.git")).to be_falsey + expect(gitlab_shell.exists?(project.repository_storage, "#{project.disk_path}.git")).to be_falsey end end end diff --git a/spec/services/projects/create_service_spec.rb b/spec/services/projects/create_service_spec.rb index e35f0f6337a..a8f003b1073 100644 --- a/spec/services/projects/create_service_spec.rb +++ b/spec/services/projects/create_service_spec.rb @@ -171,7 +171,6 @@ describe Projects::CreateService, '#execute' do context 'when another repository already exists on disk' do let(:repository_storage) { 'default' } - let(:repository_storage_path) { Gitlab.config.repositories.storages[repository_storage].legacy_disk_path } let(:opts) do { @@ -186,7 +185,7 @@ describe Projects::CreateService, '#execute' do end after do - gitlab_shell.remove_repository(repository_storage_path, "#{user.namespace.full_path}/existing") + gitlab_shell.remove_repository(repository_storage, "#{user.namespace.full_path}/existing") end it 'does not allow to create a project when path matches existing repository on disk' do @@ -222,7 +221,7 @@ describe Projects::CreateService, '#execute' do end after do - gitlab_shell.remove_repository(repository_storage_path, hashed_path) + gitlab_shell.remove_repository(repository_storage, hashed_path) end it 'does not allow to create a project when path matches existing repository on disk' do diff --git a/spec/services/projects/destroy_service_spec.rb b/spec/services/projects/destroy_service_spec.rb index a66e3c5e995..b2c52214f48 100644 --- a/spec/services/projects/destroy_service_spec.rb +++ b/spec/services/projects/destroy_service_spec.rb @@ -18,8 +18,8 @@ describe Projects::DestroyService do it 'deletes the project' do expect(Project.unscoped.all).not_to include(project) - expect(project.gitlab_shell.exists?(project.repository_storage_path, path + '.git')).to be_falsey - expect(project.gitlab_shell.exists?(project.repository_storage_path, remove_path + '.git')).to be_falsey + expect(project.gitlab_shell.exists?(project.repository_storage, path + '.git')).to be_falsey + expect(project.gitlab_shell.exists?(project.repository_storage, remove_path + '.git')).to be_falsey end end @@ -252,21 +252,21 @@ describe Projects::DestroyService do let(:path) { project.disk_path + '.git' } before do - expect(project.gitlab_shell.exists?(project.repository_storage_path, path)).to be_truthy - expect(project.gitlab_shell.exists?(project.repository_storage_path, remove_path)).to be_falsey + expect(project.gitlab_shell.exists?(project.repository_storage, path)).to be_truthy + expect(project.gitlab_shell.exists?(project.repository_storage, remove_path)).to be_falsey # Dont run sidekiq to check if renamed repository exists Sidekiq::Testing.fake! { destroy_project(project, user, {}) } - expect(project.gitlab_shell.exists?(project.repository_storage_path, path)).to be_falsey - expect(project.gitlab_shell.exists?(project.repository_storage_path, remove_path)).to be_truthy + expect(project.gitlab_shell.exists?(project.repository_storage, path)).to be_falsey + expect(project.gitlab_shell.exists?(project.repository_storage, remove_path)).to be_truthy end it 'restores the repositories' do Sidekiq::Testing.fake! { described_class.new(project, user).attempt_repositories_rollback } - expect(project.gitlab_shell.exists?(project.repository_storage_path, path)).to be_truthy - expect(project.gitlab_shell.exists?(project.repository_storage_path, remove_path)).to be_falsey + expect(project.gitlab_shell.exists?(project.repository_storage, path)).to be_truthy + expect(project.gitlab_shell.exists?(project.repository_storage, remove_path)).to be_falsey end end diff --git a/spec/services/projects/fork_service_spec.rb b/spec/services/projects/fork_service_spec.rb index 0f7c46367d0..a93f6f1ddc2 100644 --- a/spec/services/projects/fork_service_spec.rb +++ b/spec/services/projects/fork_service_spec.rb @@ -112,7 +112,7 @@ describe Projects::ForkService do end after do - gitlab_shell.remove_repository(repository_storage_path, "#{@to_user.namespace.full_path}/#{@from_project.path}") + gitlab_shell.remove_repository(repository_storage, "#{@to_user.namespace.full_path}/#{@from_project.path}") end it 'does not allow creation' do diff --git a/spec/services/projects/hashed_storage/migrate_repository_service_spec.rb b/spec/services/projects/hashed_storage/migrate_repository_service_spec.rb index 747bd4529a0..7dca81eb59e 100644 --- a/spec/services/projects/hashed_storage/migrate_repository_service_spec.rb +++ b/spec/services/projects/hashed_storage/migrate_repository_service_spec.rb @@ -16,8 +16,8 @@ describe Projects::HashedStorage::MigrateRepositoryService do it 'renames project and wiki repositories' do service.execute - expect(gitlab_shell.exists?(project.repository_storage_path, "#{hashed_storage.disk_path}.git")).to be_truthy - expect(gitlab_shell.exists?(project.repository_storage_path, "#{hashed_storage.disk_path}.wiki.git")).to be_truthy + expect(gitlab_shell.exists?(project.repository_storage, "#{hashed_storage.disk_path}.git")).to be_truthy + expect(gitlab_shell.exists?(project.repository_storage, "#{hashed_storage.disk_path}.wiki.git")).to be_truthy end it 'updates project to be hashed and not read-only' do @@ -52,8 +52,8 @@ describe Projects::HashedStorage::MigrateRepositoryService do service.execute - expect(gitlab_shell.exists?(project.repository_storage_path, "#{hashed_storage.disk_path}.git")).to be_falsey - expect(gitlab_shell.exists?(project.repository_storage_path, "#{hashed_storage.disk_path}.wiki.git")).to be_falsey + expect(gitlab_shell.exists?(project.repository_storage, "#{hashed_storage.disk_path}.git")).to be_falsey + expect(gitlab_shell.exists?(project.repository_storage, "#{hashed_storage.disk_path}.wiki.git")).to be_falsey expect(project.repository_read_only?).to be_falsey end @@ -63,11 +63,11 @@ describe Projects::HashedStorage::MigrateRepositoryService do before do hashed_storage.ensure_storage_path_exists - gitlab_shell.mv_repository(project.repository_storage_path, from_name, to_name) + gitlab_shell.mv_repository(project.repository_storage, from_name, to_name) end it 'does not try to move nil repository over hashed' do - expect(gitlab_shell).not_to receive(:mv_repository).with(project.repository_storage_path, from_name, to_name) + expect(gitlab_shell).not_to receive(:mv_repository).with(project.repository_storage, from_name, to_name) expect_move_repository("#{project.disk_path}.wiki", "#{hashed_storage.disk_path}.wiki") service.execute @@ -76,7 +76,7 @@ describe Projects::HashedStorage::MigrateRepositoryService do end def expect_move_repository(from_name, to_name) - expect(gitlab_shell).to receive(:mv_repository).with(project.repository_storage_path, from_name, to_name).and_call_original + expect(gitlab_shell).to receive(:mv_repository).with(project.repository_storage, from_name, to_name).and_call_original end end end diff --git a/spec/services/projects/transfer_service_spec.rb b/spec/services/projects/transfer_service_spec.rb index ff9b2372a35..3e6483d7e28 100644 --- a/spec/services/projects/transfer_service_spec.rb +++ b/spec/services/projects/transfer_service_spec.rb @@ -84,7 +84,7 @@ describe Projects::TransferService do end def project_path(project) - File.join(project.repository_storage_path, "#{project.disk_path}.git") + project.repository.path_to_repo end def current_path @@ -94,7 +94,7 @@ describe Projects::TransferService do it 'rolls back repo location' do attempt_project_transfer - expect(Dir.exist?(original_path)).to be_truthy + expect(gitlab_shell.exists?(project.repository_storage, "#{project.disk_path}.git")).to be(true) expect(original_path).to eq current_path end @@ -165,7 +165,7 @@ describe Projects::TransferService do end after do - gitlab_shell.remove_repository(repository_storage_path, "#{group.full_path}/#{project.path}") + gitlab_shell.remove_repository(repository_storage, "#{group.full_path}/#{project.path}") end it { expect(@result).to eq false } diff --git a/spec/services/projects/update_service_spec.rb b/spec/services/projects/update_service_spec.rb index f48d466d263..3e6073b9861 100644 --- a/spec/services/projects/update_service_spec.rb +++ b/spec/services/projects/update_service_spec.rb @@ -200,7 +200,7 @@ describe Projects::UpdateService do end after do - gitlab_shell.remove_repository(repository_storage_path, "#{user.namespace.full_path}/existing") + gitlab_shell.remove_repository(repository_storage, "#{user.namespace.full_path}/existing") end it 'does not allow renaming when new path matches existing repository on disk' do diff --git a/spec/services/quick_actions/interpret_service_spec.rb b/spec/services/quick_actions/interpret_service_spec.rb index f793f55e51b..bd835a1fca6 100644 --- a/spec/services/quick_actions/interpret_service_spec.rb +++ b/spec/services/quick_actions/interpret_service_spec.rb @@ -306,6 +306,23 @@ describe QuickActions::InterpretService do end end + shared_examples 'copy_metadata command' do + it 'fetches issue or merge request and copies labels and milestone if content contains /copy_metadata reference' do + source_issuable # populate the issue + todo_label # populate this label + inreview_label # populate this label + _, updates = service.execute(content, issuable) + + expect(updates[:add_label_ids]).to match_array([inreview_label.id, todo_label.id]) + + if source_issuable.milestone + expect(updates[:milestone_id]).to eq(source_issuable.milestone.id) + else + expect(updates).not_to have_key(:milestone_id) + end + end + end + shared_examples 'shrug command' do it 'appends ¯\_(ツ)_/¯ to the comment' do new_content, _ = service.execute(content, issuable) @@ -757,6 +774,65 @@ describe QuickActions::InterpretService do let(:issuable) { issue } end + context '/copy_metadata command' do + let(:todo_label) { create(:label, project: project, title: 'To Do') } + let(:inreview_label) { create(:label, project: project, title: 'In Review') } + + it_behaves_like 'empty command' do + let(:content) { '/copy_metadata' } + let(:issuable) { issue } + end + + it_behaves_like 'copy_metadata command' do + let(:source_issuable) { create(:labeled_issue, project: project, labels: [inreview_label, todo_label]) } + + let(:content) { "/copy_metadata #{source_issuable.to_reference}" } + let(:issuable) { issue } + end + + context 'when the parent issuable has a milestone' do + it_behaves_like 'copy_metadata command' do + let(:source_issuable) { create(:labeled_issue, project: project, labels: [todo_label, inreview_label], milestone: milestone) } + + let(:content) { "/copy_metadata #{source_issuable.to_reference(project)}" } + let(:issuable) { issue } + end + end + + context 'when more than one issuable is passed' do + it_behaves_like 'copy_metadata command' do + let(:source_issuable) { create(:labeled_issue, project: project, labels: [inreview_label, todo_label]) } + let(:other_label) { create(:label, project: project, title: 'Other') } + let(:other_source_issuable) { create(:labeled_issue, project: project, labels: [other_label]) } + + let(:content) { "/copy_metadata #{source_issuable.to_reference} #{other_source_issuable.to_reference}" } + let(:issuable) { issue } + end + end + + context 'cross project references' do + it_behaves_like 'empty command' do + let(:other_project) { create(:project, :public) } + let(:source_issuable) { create(:labeled_issue, project: other_project, labels: [todo_label, inreview_label]) } + let(:content) { "/copy_metadata #{source_issuable.to_reference(project)}" } + let(:issuable) { issue } + end + + it_behaves_like 'empty command' do + let(:content) { "/copy_metadata imaginary#1234" } + let(:issuable) { issue } + end + + it_behaves_like 'empty command' do + let(:other_project) { create(:project, :private) } + let(:source_issuable) { create(:issue, project: other_project) } + + let(:content) { "/copy_metadata #{source_issuable.to_reference(project)}" } + let(:issuable) { issue } + end + end + end + context '/duplicate command' do it_behaves_like 'duplicate command' do let(:issue_duplicate) { create(:issue, project: project) } diff --git a/spec/services/users/destroy_service_spec.rb b/spec/services/users/destroy_service_spec.rb index 11c75ddfcf8..76f1e625fda 100644 --- a/spec/services/users/destroy_service_spec.rb +++ b/spec/services/users/destroy_service_spec.rb @@ -176,7 +176,7 @@ describe Users::DestroyService do let!(:project) { create(:project, :empty_repo, :legacy_storage, namespace: user.namespace) } it 'removes repository' do - expect(gitlab_shell.exists?(project.repository_storage_path, "#{project.disk_path}.git")).to be_falsey + expect(gitlab_shell.exists?(project.repository_storage, "#{project.disk_path}.git")).to be_falsey end end @@ -184,7 +184,7 @@ describe Users::DestroyService do let!(:project) { create(:project, :empty_repo, namespace: user.namespace) } it 'removes repository' do - expect(gitlab_shell.exists?(project.repository_storage_path, "#{project.disk_path}.git")).to be_falsey + expect(gitlab_shell.exists?(project.repository_storage, "#{project.disk_path}.git")).to be_falsey end end end diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 53045815a6a..cc61cd7d838 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -113,10 +113,10 @@ RSpec.configure do |config| m.call(*args) shard_name, repository_relative_path = args - shard_path = Gitlab.config.repositories.storages.fetch(shard_name).legacy_disk_path # We can't leave the hooks in place after a fork, as those would fail in tests # The "internal" API is not available - FileUtils.rm_rf(File.join(shard_path, repository_relative_path, 'hooks')) + Gitlab::Shell.new.rm_directory(shard_name, + File.join(repository_relative_path, 'hooks')) end # Enable all features by default for testing diff --git a/spec/support/controllers/ldap_omniauth_callbacks_controller_shared_context.rb b/spec/support/controllers/ldap_omniauth_callbacks_controller_shared_context.rb new file mode 100644 index 00000000000..72912ffb89d --- /dev/null +++ b/spec/support/controllers/ldap_omniauth_callbacks_controller_shared_context.rb @@ -0,0 +1,33 @@ +require 'spec_helper' + +shared_context 'Ldap::OmniauthCallbacksController' do + include LoginHelpers + include LdapHelpers + + let(:uid) { 'my-uid' } + let(:provider) { 'ldapmain' } + let(:valid_login?) { true } + let(:user) { create(:omniauth_user, extern_uid: uid, provider: provider) } + let(:ldap_server_config) do + { main: ldap_config_defaults(:main) } + end + + def ldap_config_defaults(key, hash = {}) + { + provider_name: "ldap#{key}", + attributes: {}, + encryption: 'plain' + }.merge(hash) + end + + before do + stub_ldap_setting(enabled: true, servers: ldap_server_config) + described_class.define_providers! + Rails.application.reload_routes! + + mock_auth_hash(provider.to_s, uid, user.email) + stub_omniauth_provider(provider, context: request) + + allow(Gitlab::Auth::LDAP::Access).to receive(:allowed?).and_return(valid_login?) + end +end diff --git a/spec/support/helpers/javascript_fixtures_helpers.rb b/spec/support/helpers/javascript_fixtures_helpers.rb index 2197bc9d853..086a345dca8 100644 --- a/spec/support/helpers/javascript_fixtures_helpers.rb +++ b/spec/support/helpers/javascript_fixtures_helpers.rb @@ -31,7 +31,7 @@ module JavaScriptFixturesHelpers end def remove_repository(project) - Gitlab::Shell.new.remove_repository(project.repository_storage_path, project.disk_path) + Gitlab::Shell.new.remove_repository(project.repository_storage, project.disk_path) end private diff --git a/spec/support/helpers/ldap_helpers.rb b/spec/support/helpers/ldap_helpers.rb index 0e87b3d359d..b90bbc4b106 100644 --- a/spec/support/helpers/ldap_helpers.rb +++ b/spec/support/helpers/ldap_helpers.rb @@ -18,6 +18,10 @@ module LdapHelpers allow_any_instance_of(::Gitlab::Auth::LDAP::Config).to receive_messages(messages) end + def stub_ldap_setting(messages) + allow(Gitlab.config.ldap).to receive_messages(to_settings(messages)) + end + # Stub an LDAP person search and provide the return entry. Specify `nil` for # `entry` to simulate when an LDAP person is not found # diff --git a/spec/support/helpers/login_helpers.rb b/spec/support/helpers/login_helpers.rb index db34090e971..72e5c2d66dd 100644 --- a/spec/support/helpers/login_helpers.rb +++ b/spec/support/helpers/login_helpers.rb @@ -112,7 +112,7 @@ module LoginHelpers } } }) - Rails.application.env_config['omniauth.auth'] = OmniAuth.config.mock_auth[:saml] + Rails.application.env_config['omniauth.auth'] = OmniAuth.config.mock_auth[provider.to_sym] end def mock_saml_config @@ -129,7 +129,7 @@ module LoginHelpers env = env_from_context(context) set_devise_mapping(context: context) - env['omniauth.auth'] = OmniAuth.config.mock_auth[provider] + env['omniauth.auth'] = OmniAuth.config.mock_auth[provider.to_sym] end def stub_omniauth_saml_config(messages) diff --git a/spec/support/helpers/test_env.rb b/spec/support/helpers/test_env.rb index d87f265cdf0..1dad39fdab3 100644 --- a/spec/support/helpers/test_env.rb +++ b/spec/support/helpers/test_env.rb @@ -218,7 +218,8 @@ module TestEnv end def copy_repo(project, bare_repo:, refs:) - target_repo_path = File.expand_path(project.repository_storage_path + "/#{project.disk_path}.git") + target_repo_path = File.expand_path(repos_path + "/#{project.disk_path}.git") + FileUtils.mkdir_p(target_repo_path) FileUtils.cp_r("#{File.expand_path(bare_repo)}/.", target_repo_path) FileUtils.chmod_R 0755, target_repo_path @@ -226,7 +227,7 @@ module TestEnv end def repos_path - Gitlab.config.repositories.storages[REPOS_STORAGE].legacy_disk_path + @repos_path ||= Gitlab.config.repositories.storages[REPOS_STORAGE].legacy_disk_path end def backup_path diff --git a/spec/support/shared_examples/issuables_list_metadata_shared_examples.rb b/spec/support/shared_examples/issuables_list_metadata_shared_examples.rb index e61983c60b4..f4bc6f8efa5 100644 --- a/spec/support/shared_examples/issuables_list_metadata_shared_examples.rb +++ b/spec/support/shared_examples/issuables_list_metadata_shared_examples.rb @@ -1,25 +1,30 @@ shared_examples 'issuables list meta-data' do |issuable_type, action = nil| - before do - @issuable_ids = [] - - %w[fix improve/awesome].each do |source_branch| - issuable = - if issuable_type == :issue - create(issuable_type, project: project, author: project.creator) - else - create(issuable_type, source_project: project, source_branch: source_branch, author: project.creator) - end - - @issuable_ids << issuable.id - end - end + include ProjectForksHelper - it "creates indexed meta-data object for issuable notes and votes count" do + def get_action(action, project) if action get action, author_id: project.creator.id else get :index, namespace_id: project.namespace, project_id: project end + end + + def create_issuable(issuable_type, project, source_branch:) + if issuable_type == :issue + create(issuable_type, project: project, author: project.creator) + else + create(issuable_type, source_project: project, source_branch: source_branch, author: project.creator) + end + end + + before do + @issuable_ids = %w[fix improve/awesome].map do |source_branch| + create_issuable(issuable_type, project, source_branch: source_branch).id + end + end + + it "creates indexed meta-data object for issuable notes and votes count" do + get_action(action, project) meta_data = assigns(:issuable_meta_data) @@ -29,18 +34,29 @@ shared_examples 'issuables list meta-data' do |issuable_type, action = nil| end end + it "avoids N+1 queries" do + control = ActiveRecord::QueryRecorder.new { get_action(action, project) } + issuable = create_issuable(issuable_type, project, source_branch: 'csv') + + if issuable_type == :merge_request + issuable.update!(source_project: fork_project(project)) + end + + expect { get_action(action, project) }.not_to exceed_query_limit(control.count) + end + describe "when given empty collection" do let(:project2) { create(:project, :public) } it "doesn't execute any queries with false conditions" do - get_action = + get_empty = if action proc { get action, author_id: project.creator.id } else proc { get :index, namespace_id: project2.namespace, project_id: project2 } end - expect(&get_action).not_to make_queries_matching(/WHERE (?:1=0|0=1)/) + expect(&get_empty).not_to make_queries_matching(/WHERE (?:1=0|0=1)/) end end end diff --git a/spec/tasks/gitlab/backup_rake_spec.rb b/spec/tasks/gitlab/backup_rake_spec.rb index 0d24782f317..a2e5642a72c 100644 --- a/spec/tasks/gitlab/backup_rake_spec.rb +++ b/spec/tasks/gitlab/backup_rake_spec.rb @@ -195,15 +195,12 @@ describe 'gitlab:app namespace rake task' do end context 'multiple repository storages' do - let(:storage_default) do - Gitlab::GitalyClient::StorageSettings.new(@default_storage_hash.merge('path' => 'tmp/tests/default_storage')) - end let(:test_second_storage) do Gitlab::GitalyClient::StorageSettings.new(@default_storage_hash.merge('path' => 'tmp/tests/custom_storage')) end let(:storages) do { - 'default' => storage_default, + 'default' => Gitlab.config.repositories.storages.default, 'test_second_storage' => test_second_storage } end @@ -215,8 +212,7 @@ describe 'gitlab:app namespace rake task' do before do # We only need a backup of the repositories for this test stub_env('SKIP', 'db,uploads,builds,artifacts,lfs,registry') - FileUtils.mkdir(Settings.absolute('tmp/tests/default_storage')) - FileUtils.mkdir(Settings.absolute('tmp/tests/custom_storage')) + allow(Gitlab.config.repositories).to receive(:storages).and_return(storages) # Avoid asking gitaly about the root ref (which will fail beacuse of the @@ -225,14 +221,23 @@ describe 'gitlab:app namespace rake task' do end after do - FileUtils.rm_rf(Settings.absolute('tmp/tests/default_storage')) FileUtils.rm_rf(Settings.absolute('tmp/tests/custom_storage')) end it 'includes repositories in all repository storages' do - project_a = create(:project, :repository, repository_storage: 'default') + project_a = create(:project, :repository) project_b = create(:project, :repository, repository_storage: 'test_second_storage') + b_storage_dir = File.join(Settings.absolute('tmp/tests/custom_storage'), File.dirname(project_b.disk_path)) + + FileUtils.mkdir_p(b_storage_dir) + + # Even when overriding the storage, we have to move it there, so it exists + FileUtils.mv( + File.join(Settings.absolute(storages['default'].legacy_disk_path), project_b.repository.disk_path + '.git'), + Rails.root.join(storages['test_second_storage'].legacy_disk_path, project_b.repository.disk_path + '.git') + ) + expect { run_rake_task('gitlab:backup:create') }.to output.to_stdout tar_contents, exit_status = Gitlab::Popen.popen( |