diff options
Diffstat (limited to 'spec')
119 files changed, 2019 insertions, 655 deletions
diff --git a/spec/controllers/projects/protected_tags_controller_spec.rb b/spec/controllers/projects/protected_tags_controller_spec.rb index 64658988b3f..b6de90039f3 100644 --- a/spec/controllers/projects/protected_tags_controller_spec.rb +++ b/spec/controllers/projects/protected_tags_controller_spec.rb @@ -8,4 +8,21 @@ describe Projects::ProtectedTagsController do get(:index, namespace_id: project.namespace.to_param, project_id: project) end end + + describe "DELETE #destroy" do + let(:project) { create(:project, :repository) } + let(:protected_tag) { create(:protected_tag, :developers_can_create, project: project) } + let(:user) { create(:user) } + + before do + project.add_master(user) + sign_in(user) + end + + it "deletes the protected tag" do + delete(:destroy, namespace_id: project.namespace.to_param, project_id: project, id: protected_tag.id) + + expect { ProtectedTag.find(protected_tag.id) }.to raise_error(ActiveRecord::RecordNotFound) + end + end end diff --git a/spec/controllers/projects/services_controller_spec.rb b/spec/controllers/projects/services_controller_spec.rb index 4e9b0c09ff2..efba9cc7306 100644 --- a/spec/controllers/projects/services_controller_spec.rb +++ b/spec/controllers/projects/services_controller_spec.rb @@ -10,9 +10,6 @@ describe Projects::ServicesController do before do sign_in(user) project.team << [user, :master] - - controller.instance_variable_set(:@project, project) - controller.instance_variable_set(:@service, service) end describe '#test' do @@ -20,7 +17,7 @@ describe Projects::ServicesController do it 'renders 404' do allow_any_instance_of(Service).to receive(:can_test?).and_return(false) - put :test, namespace_id: project.namespace.id, project_id: project.id, id: service.id + put :test, namespace_id: project.namespace, project_id: project, id: service.to_param expect(response).to have_http_status(404) end @@ -36,7 +33,7 @@ describe Projects::ServicesController do it 'returns success' do allow_any_instance_of(MicrosoftTeams::Notifier).to receive(:ping).and_return(true) - put :test, namespace_id: project.namespace.id, project_id: project.id, id: service.id + put :test, namespace_id: project.namespace, project_id: project, id: service.to_param expect(response.status).to eq(200) end @@ -45,7 +42,7 @@ describe Projects::ServicesController do it 'returns success' do expect(HipChat::Client).to receive(:new).with('hipchat_token_p', anything).and_return(hipchat_client) - put :test, namespace_id: project.namespace.id, project_id: project.id, id: service.id, service: service_params + put :test, namespace_id: project.namespace, project_id: project, id: service.to_param, service: service_params expect(response.status).to eq(200) end @@ -54,17 +51,42 @@ describe Projects::ServicesController do it 'returns success' do expect(HipChat::Client).to receive(:new).with('hipchat_token_p', anything).and_return(hipchat_client) - put :test, namespace_id: project.namespace.id, project_id: project.id, id: service.id, service: service_params + put :test, namespace_id: project.namespace, project_id: project, id: service.to_param, service: service_params expect(response.status).to eq(200) end + + context 'when service is configured for the first time' do + before do + allow_any_instance_of(ServiceHook).to receive(:execute).and_return(true) + end + + it 'persist the object' do + do_put + + expect(BuildkiteService.first).to be_present + end + + it 'creates the ServiceHook object' do + do_put + + expect(BuildkiteService.first.service_hook).to be_present + end + + def do_put + put :test, namespace_id: project.namespace, + project_id: project, + id: 'buildkite', + service: { 'active' => '1', 'push_events' => '1', token: 'token', 'project_url' => 'http://test.com' } + end + end end context 'failure' do it 'returns success status code and the error message' do expect(HipChat::Client).to receive(:new).with('hipchat_token_p', anything).and_raise('Bad test') - put :test, namespace_id: project.namespace.id, project_id: project.id, id: service.id, service: service_params + put :test, namespace_id: project.namespace, project_id: project, id: service.to_param, service: service_params expect(response.status).to eq(200) expect(JSON.parse(response.body)) @@ -77,7 +99,7 @@ describe Projects::ServicesController do context 'when param `active` is set to true' do it 'activates the service and redirects to integrations paths' do put :update, - namespace_id: project.namespace.id, project_id: project.id, id: service.id, service: { active: true } + namespace_id: project.namespace, project_id: project, id: service.to_param, service: { active: true } expect(response).to redirect_to(project_settings_integrations_path(project)) expect(flash[:notice]).to eq 'HipChat activated.' @@ -87,7 +109,7 @@ describe Projects::ServicesController do context 'when param `active` is set to false' do it 'does not activate the service but saves the settings' do put :update, - namespace_id: project.namespace.id, project_id: project.id, id: service.id, service: { active: false } + namespace_id: project.namespace, project_id: project, id: service.to_param, service: { active: false } expect(flash[:notice]).to eq 'HipChat settings saved, but not activated.' end diff --git a/spec/factories/ci/triggers.rb b/spec/factories/ci/triggers.rb index 40c4663c6d8..3734c7040c0 100644 --- a/spec/factories/ci/triggers.rb +++ b/spec/factories/ci/triggers.rb @@ -1,5 +1,7 @@ FactoryGirl.define do factory :ci_trigger_without_token, class: Ci::Trigger do + owner + factory :ci_trigger do sequence(:token) { |n| "token#{n}" } end diff --git a/spec/factories/projects.rb b/spec/factories/projects.rb index 4a2034b31b3..9ebda0ba03b 100644 --- a/spec/factories/projects.rb +++ b/spec/factories/projects.rb @@ -81,6 +81,10 @@ FactoryGirl.define do archived true end + trait :hashed do + storage_version Project::LATEST_STORAGE_VERSION + end + trait :access_requestable do request_access_enabled true end diff --git a/spec/features/admin/admin_projects_spec.rb b/spec/features/admin/admin_projects_spec.rb index 77710f80036..f4f2505d436 100644 --- a/spec/features/admin/admin_projects_spec.rb +++ b/spec/features/admin/admin_projects_spec.rb @@ -36,6 +36,14 @@ describe "Admin::Projects" do expect(page).to have_content(archived_project.name) expect(page).to have_xpath("//span[@class='label label-warning']", text: 'archived') end + + it 'renders only archived projects', js: true do + find(:css, '#sort-projects-dropdown').click + click_link 'Show archived projects only' + + expect(page).to have_content(archived_project.name) + expect(page).not_to have_content(project.name) + end end describe "GET /admin/projects/:namespace_id/:id" do diff --git a/spec/features/calendar_spec.rb b/spec/features/calendar_spec.rb index 9a597a2d690..4fc6956d111 100644 --- a/spec/features/calendar_spec.rb +++ b/spec/features/calendar_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' feature 'Contributions Calendar', :js do let(:user) { create(:user) } - let(:contributed_project) { create(:project, :public) } + let(:contributed_project) { create(:project, :public, :repository) } let(:issue_note) { create(:note, project: contributed_project) } # Ex/ Sunday Jan 1, 2016 diff --git a/spec/features/dashboard/activity_spec.rb b/spec/features/dashboard/activity_spec.rb index 582868bac1e..bd115785646 100644 --- a/spec/features/dashboard/activity_spec.rb +++ b/spec/features/dashboard/activity_spec.rb @@ -17,7 +17,7 @@ feature 'Dashboard > Activity' do end context 'event filters', :js do - let(:project) { create(:project) } + let(:project) { create(:project, :repository) } let(:merge_request) do create(:merge_request, author: user, source_project: project, target_project: project) diff --git a/spec/features/dashboard/archived_projects_spec.rb b/spec/features/dashboard/archived_projects_spec.rb index 814ec0e59c7..e8d699ff5e0 100644 --- a/spec/features/dashboard/archived_projects_spec.rb +++ b/spec/features/dashboard/archived_projects_spec.rb @@ -26,6 +26,13 @@ RSpec.describe 'Dashboard Archived Project' do expect(page).to have_link(archived_project.name) end + it 'renders only archived projects' do + click_link 'Show archived projects only' + + expect(page).to have_content(archived_project.name) + expect(page).not_to have_content(project.name) + end + it 'searchs archived projects', :js do click_button 'Last updated' click_link 'Show archived projects' diff --git a/spec/features/explore/new_menu_spec.rb b/spec/features/explore/new_menu_spec.rb index 2cd06258e22..e1c74a24890 100644 --- a/spec/features/explore/new_menu_spec.rb +++ b/spec/features/explore/new_menu_spec.rb @@ -74,7 +74,7 @@ feature 'Top Plus Menu', :js do expect(page).to have_content('Title') end - scenario 'Click on New subgroup shows new group page' do + scenario 'Click on New subgroup shows new group page', :nested_groups do visit group_path(group) click_topmenuitem("New subgroup") diff --git a/spec/features/groups/merge_requests_spec.rb b/spec/features/groups/merge_requests_spec.rb index c2241feb9f7..9ba9f5686f7 100644 --- a/spec/features/groups/merge_requests_spec.rb +++ b/spec/features/groups/merge_requests_spec.rb @@ -1,6 +1,8 @@ require 'spec_helper' feature 'Group merge requests page' do + include FilteredSearchHelpers + let(:path) { merge_requests_group_path(group) } let(:issuable) { create(:merge_request, source_project: project, target_project: project, title: 'this is my created issuable') } @@ -33,4 +35,17 @@ feature 'Group merge requests page' do expect(page.find('#state-all span.badge').text).to eq("1") end end + + context 'group filtered search', :js do + let(:access_level) { ProjectFeature::ENABLED } + let(:user) { user_in_group } + let(:user2) { user_outside_group } + + it 'filters by assignee only group users' do + filtered_search.set('assignee:') + + expect(find('#js-dropdown-assignee .filter-dropdown')).to have_content(user.name) + expect(find('#js-dropdown-assignee .filter-dropdown')).not_to have_content(user2.name) + end + end end diff --git a/spec/features/groups_spec.rb b/spec/features/groups_spec.rb index e59a484d992..20f9818b08b 100644 --- a/spec/features/groups_spec.rb +++ b/spec/features/groups_spec.rb @@ -104,18 +104,15 @@ feature 'Group' do end context 'as group owner' do - let(:user) { create(:user) } + it 'creates a nested group' do + user = create(:user) - before do group.add_owner(user) sign_out(:user) sign_in(user) visit subgroups_group_path(group) click_link 'New Subgroup' - end - - it 'creates a nested group' do fill_in 'Group path', with: 'bar' click_button 'Create group' @@ -123,6 +120,16 @@ feature 'Group' do expect(page).to have_content("Group 'bar' was successfully created.") end end + + context 'when nested group feature is disabled' do + it 'renders 404' do + allow(Group).to receive(:supports_nested_groups?).and_return(false) + + visit subgroups_group_path(group) + + expect(page.status_code).to eq(404) + end + end end it 'checks permissions to avoid exposing groups by parent_id' do diff --git a/spec/features/issues/filtered_search/filter_issues_spec.rb b/spec/features/issues/filtered_search/filter_issues_spec.rb index 2070043d842..a64c1cf220b 100644 --- a/spec/features/issues/filtered_search/filter_issues_spec.rb +++ b/spec/features/issues/filtered_search/filter_issues_spec.rb @@ -103,14 +103,6 @@ describe 'Filter issues', js: true do expect_issues_list_count(5) expect_filtered_search_input_empty end - - it 'filters issues by invalid author' do - skip('to be tested, issue #26546') - end - - it 'filters issues by multiple authors' do - skip('to be tested, issue #26546') - end end context 'author with other filters' do @@ -165,10 +157,6 @@ describe 'Filter issues', js: true do expect_filtered_search_input(search_term) end end - - it 'sorting' do - skip('to be tested, issue #26546') - end end describe 'filter issues by assignee' do @@ -190,14 +178,6 @@ describe 'Filter issues', js: true do expect_issues_list_count(8, 1) expect_filtered_search_input_empty end - - it 'filters issues by invalid assignee' do - skip('to be tested, issue #26546') - end - - it 'filters issues by multiple assignees' do - skip('to be tested, issue #26546') - end end context 'assignee with other filters' do @@ -250,12 +230,6 @@ describe 'Filter issues', js: true do expect_filtered_search_input(search_term) end end - - context 'sorting' do - it 'sorts' do - skip('to be tested, issue #26546') - end - end end describe 'filter issues by label' do @@ -278,10 +252,6 @@ describe 'Filter issues', js: true do expect_filtered_search_input_empty end - it 'filters issues by invalid label' do - skip('to be tested, issue #26546') - end - it 'filters issues by multiple labels' do input_filtered_search("label:~#{bug_label.title} label:~#{caps_sensitive_label.title}") @@ -493,12 +463,6 @@ describe 'Filter issues', js: true do expect_filtered_search_input_empty end end - - context 'sorting' do - it 'sorts' do - skip('to be tested, issue #26546') - end - end end describe 'filter issues by milestone' do @@ -535,14 +499,6 @@ describe 'Filter issues', js: true do expect_filtered_search_input_empty end - it 'filters issues by invalid milestones' do - skip('to be tested, issue #26546') - end - - it 'filters issues by multiple milestones' do - skip('to be tested, issue #26546') - end - it 'filters issues by milestone containing special characters' do special_milestone = create(:milestone, title: '!@\#{$%^&*()}', project: project) create(:issue, title: "Issue with special character milestone", project: project, milestone: special_milestone) @@ -618,12 +574,6 @@ describe 'Filter issues', js: true do expect_filtered_search_input(search_term) end end - - context 'sorting' do - it 'sorts' do - skip('to be tested, issue #26546') - end - end end describe 'filter issues by text' do diff --git a/spec/features/issues/issue_detail_spec.rb b/spec/features/issues/issue_detail_spec.rb index 28b636f9359..c470cb7c716 100644 --- a/spec/features/issues/issue_detail_spec.rb +++ b/spec/features/issues/issue_detail_spec.rb @@ -40,4 +40,18 @@ feature 'Issue Detail', :js do end end end + + context 'when authored by a user who is later deleted' do + before do + issue.update_attribute(:author_id, nil) + sign_in(user) + visit project_issue_path(project, issue) + end + + it 'shows the issue' do + page.within('.issuable-details') do + expect(find('h2')).to have_content(issue.title) + end + end + end end diff --git a/spec/features/merge_requests/merge_commit_message_toggle_spec.rb b/spec/features/merge_requests/merge_commit_message_toggle_spec.rb index 429bc277d73..08a3bb84aac 100644 --- a/spec/features/merge_requests/merge_commit_message_toggle_spec.rb +++ b/spec/features/merge_requests/merge_commit_message_toggle_spec.rb @@ -19,7 +19,7 @@ feature 'Clicking toggle commit message link', js: true do "Merge branch 'feature' into 'master'", merge_request.title, "Closes #{issue_1.to_reference} and #{issue_2.to_reference}", - "See merge request #{merge_request.to_reference}" + "See merge request #{merge_request.to_reference(full: true)}" ].join("\n\n") end let(:message_with_description) do @@ -27,7 +27,7 @@ feature 'Clicking toggle commit message link', js: true do "Merge branch 'feature' into 'master'", merge_request.title, merge_request.description, - "See merge request #{merge_request.to_reference}" + "See merge request #{merge_request.to_reference(full: true)}" ].join("\n\n") end diff --git a/spec/features/projects/commit/mini_pipeline_graph_spec.rb b/spec/features/projects/commit/mini_pipeline_graph_spec.rb index 2ef74e8857c..807a2189cc4 100644 --- a/spec/features/projects/commit/mini_pipeline_graph_spec.rb +++ b/spec/features/projects/commit/mini_pipeline_graph_spec.rb @@ -1,13 +1,8 @@ require 'rails_helper' feature 'Mini Pipeline Graph in Commit View', :js do - let(:user) { create(:user) } let(:project) { create(:project, :public, :repository) } - before do - sign_in(user) - end - context 'when commit has pipelines' do let(:pipeline) do create(:ci_empty_pipeline, @@ -15,21 +10,14 @@ feature 'Mini Pipeline Graph in Commit View', :js do ref: project.default_branch, sha: project.commit.sha) end + let(:build) { create(:ci_build, pipeline: pipeline) } - let(:build) do - create(:ci_build, pipeline: pipeline) - end - - before do + it 'displays a mini pipeline graph' do build.run visit project_commit_path(project, project.commit.id) - end - it 'should display a mini pipeline graph' do expect(page).to have_selector('.mr-widget-pipeline-graph') - end - it 'should show the builds list when stage is clicked' do first('.mini-pipeline-graph-dropdown-toggle').click wait_for_requests @@ -38,6 +26,8 @@ feature 'Mini Pipeline Graph in Commit View', :js do expect(page).to have_selector('.ci-status-icon-running') expect(page).to have_content(build.stage) end + + build.drop end end diff --git a/spec/features/projects/files/find_files_spec.rb b/spec/features/projects/files/find_files_spec.rb deleted file mode 100644 index 57d67b28920..00000000000 --- a/spec/features/projects/files/find_files_spec.rb +++ /dev/null @@ -1,23 +0,0 @@ -require 'spec_helper' - -feature 'Find files button in the tree header' do - given(:user) { create(:user) } - given(:project) { create(:project, :repository) } - - background do - sign_in(user) - project.team << [user, :developer] - end - - scenario 'project main screen' do - visit project_path(project) - - expect(page).to have_selector('.tree-controls .shortcuts-find-file') - end - - scenario 'project tree screen' do - visit project_tree_path(project, project.default_branch) - - expect(page).to have_selector('.tree-controls .shortcuts-find-file') - end -end diff --git a/spec/features/projects/files/user_searches_for_files_spec.rb b/spec/features/projects/files/user_searches_for_files_spec.rb new file mode 100644 index 00000000000..a105685bca7 --- /dev/null +++ b/spec/features/projects/files/user_searches_for_files_spec.rb @@ -0,0 +1,58 @@ +require 'spec_helper' + +describe 'User searches for files' do + let(:user) { create(:user) } + let(:project) { create(:project, :repository) } + + before do + sign_in(user) + end + + describe 'project main screen' do + context 'when project is empty' do + let(:empty_project) { create(:project) } + + before do + empty_project.add_developer(user) + visit project_path(empty_project) + end + + it 'does not show any result' do + fill_in('search', with: 'coffee') + click_button('Go') + + expect(page).to have_content("We couldn't find any") + end + end + + context 'when project is not empty' do + before do + project.add_developer(user) + visit project_path(project) + end + + it 'shows "Find file" button' do + expect(page).to have_selector('.tree-controls .shortcuts-find-file') + end + end + end + + describe 'project tree screen' do + before do + project.add_developer(user) + visit project_tree_path(project, project.default_branch) + end + + it 'shows "Find file" button' do + expect(page).to have_selector('.tree-controls .shortcuts-find-file') + end + + it 'shows found files' do + fill_in('search', with: 'coffee') + click_button('Go') + + expect(page).to have_content('coffee') + expect(page).to have_content('CONTRIBUTING.md') + end + end +end diff --git a/spec/features/projects/import_export/import_file_spec.rb b/spec/features/projects/import_export/import_file_spec.rb index 6a324d32ca7..2eb6fab129d 100644 --- a/spec/features/projects/import_export/import_file_spec.rb +++ b/spec/features/projects/import_export/import_file_spec.rb @@ -3,11 +3,13 @@ require 'spec_helper' feature 'Import/Export - project import integration test', js: true do include Select2Helper + let(:user) { create(:user) } let(:file) { File.join(Rails.root, 'spec', 'features', 'projects', 'import_export', 'test_project_export.tar.gz') } let(:export_path) { "#{Dir.tmpdir}/import_file_spec" } background do allow_any_instance_of(Gitlab::ImportExport).to receive(:storage_path).and_return(export_path) + gitlab_sign_in(user) end after do @@ -18,57 +20,67 @@ feature 'Import/Export - project import integration test', js: true do let(:user) { create(:admin) } let!(:namespace) { create(:namespace, name: "asd", owner: user) } - before do - gitlab_sign_in(user) - end + context 'prefilled the path' do + scenario 'user imports an exported project successfully' do + visit new_project_path - scenario 'user imports an exported project successfully' do - visit new_project_path + select2(namespace.id, from: '#project_namespace_id') + fill_in :project_path, with: 'test-project-path', visible: true + click_link 'GitLab export' - select2(namespace.id, from: '#project_namespace_id') - fill_in :project_path, with: 'test-project-path', visible: true - click_link 'GitLab export' + expect(page).to have_content('Import an exported GitLab project') + expect(URI.parse(current_url).query).to eq("namespace_id=#{namespace.id}&path=test-project-path") + expect(Gitlab::ImportExport).to receive(:import_upload_path).with(filename: /\A\h{32}_test-project-path\z/).and_call_original - expect(page).to have_content('Import an exported GitLab project') - expect(URI.parse(current_url).query).to eq("namespace_id=#{namespace.id}&path=test-project-path") - expect(Gitlab::ImportExport).to receive(:import_upload_path).with(filename: /\A\h{32}_test-project-path\z/).and_call_original + attach_file('file', file) - attach_file('file', file) + expect { click_on 'Import project' }.to change { Project.count }.by(1) - expect { click_on 'Import project' }.to change { Project.count }.from(0).to(1) - - project = Project.last - expect(project).not_to be_nil - expect(project.issues).not_to be_empty - expect(project.merge_requests).not_to be_empty - expect(project_hook_exists?(project)).to be true - expect(wiki_exists?(project)).to be true - expect(project.import_status).to eq('finished') + project = Project.last + expect(project).not_to be_nil + expect(project.issues).not_to be_empty + expect(project.merge_requests).not_to be_empty + expect(project_hook_exists?(project)).to be true + expect(wiki_exists?(project)).to be true + expect(project.import_status).to eq('finished') + end end - scenario 'invalid project' do - project = create(:project, namespace: namespace) + context 'path is not prefilled' do + scenario 'user imports an exported project successfully' do + visit new_project_path + click_link 'GitLab export' - visit new_project_path + fill_in :path, with: 'test-project-path', visible: true + attach_file('file', file) - select2(namespace.id, from: '#project_namespace_id') - fill_in :project_path, with: project.name, visible: true - click_link 'GitLab export' - attach_file('file', file) - click_on 'Import project' + expect { click_on 'Import project' }.to change { Project.count }.by(1) - page.within('.flash-container') do - expect(page).to have_content('Project could not be imported') + project = Project.last + expect(project).not_to be_nil + expect(page).to have_content("Project 'test-project-path' is being imported") end end end - context 'when limited to the default user namespace' do - let(:user) { create(:user) } - before do - gitlab_sign_in(user) + scenario 'invalid project' do + namespace = create(:namespace, name: "asd", owner: user) + project = create(:project, namespace: namespace) + + visit new_project_path + + select2(namespace.id, from: '#project_namespace_id') + fill_in :project_path, with: project.name, visible: true + click_link 'GitLab export' + attach_file('file', file) + click_on 'Import project' + + page.within('.flash-container') do + expect(page).to have_content('Project could not be imported') end + end + context 'when limited to the default user namespace' do scenario 'passes correct namespace ID in the URL' do visit new_project_path @@ -87,6 +99,6 @@ feature 'Import/Export - project import integration test', js: true do end def project_hook_exists?(project) - Gitlab::Git::Hook.new('post-receive', project).exists? + Gitlab::Git::Hook.new('post-receive', project.repository.raw_repository).exists? end end diff --git a/spec/features/projects/user_creates_files_spec.rb b/spec/features/projects/user_creates_files_spec.rb index 4b78cc4fc53..3d335687510 100644 --- a/spec/features/projects/user_creates_files_spec.rb +++ b/spec/features/projects/user_creates_files_spec.rb @@ -56,11 +56,10 @@ describe 'User creates files' do find('.add-to-tree').click click_link('New file') + expect(page).to have_selector('.file-editor') end it 'creates and commit a new file', js: true do - expect(page).to have_selector('.file-editor') - execute_script("ace.edit('editor').setValue('*.rbca')") fill_in(:file_name, with: 'not_a_file.md') fill_in(:commit_message, with: 'New commit message', visible: true) diff --git a/spec/features/runners_spec.rb b/spec/features/runners_spec.rb index cac31c34ad1..785cfeb34bd 100644 --- a/spec/features/runners_spec.rb +++ b/spec/features/runners_spec.rb @@ -1,149 +1,135 @@ require 'spec_helper' -describe "Runners" do - let(:user) { create(:user) } +feature 'Runners' do + given(:user) { create(:user) } - before do + background do sign_in(user) end - describe "specific runners" do - before do - @project = FactoryGirl.create :project, shared_runners_enabled: false - @project.team << [user, :master] + context 'when a project has enabled shared_runners' do + given(:project) { create(:project) } - @project2 = FactoryGirl.create :project - @project2.team << [user, :master] + background do + project.add_master(user) + end - @project3 = FactoryGirl.create :project - @project3.team << [user, :developer] + context 'when a specific runner is activated on the project' do + given(:specific_runner) { create(:ci_runner, :specific) } - @shared_runner = FactoryGirl.create :ci_runner, :shared - @specific_runner = FactoryGirl.create :ci_runner - @specific_runner2 = FactoryGirl.create :ci_runner - @specific_runner3 = FactoryGirl.create :ci_runner - @project.runners << @specific_runner - @project2.runners << @specific_runner2 - @project3.runners << @specific_runner3 + background do + project.runners << specific_runner + end - visit runners_path(@project) - end + scenario 'user sees the specific runner' do + visit runners_path(project) - before do - expect(page).not_to have_content(@specific_runner3.display_name) - expect(page).not_to have_content(@specific_runner3.display_name) - end + within '.activated-specific-runners' do + expect(page).to have_content(specific_runner.display_name) + end - it "places runners in right places" do - expect(page.find(".available-specific-runners")).to have_content(@specific_runner2.display_name) - expect(page.find(".activated-specific-runners")).to have_content(@specific_runner.display_name) - expect(page.find(".available-shared-runners")).to have_content(@shared_runner.display_name) - end + click_on specific_runner.short_sha - it "enables specific runner for project" do - within ".available-specific-runners" do - click_on "Enable for this project" + expect(page).to have_content(specific_runner.platform) end - expect(page.find(".activated-specific-runners")).to have_content(@specific_runner2.display_name) - end + scenario 'user removes an activated specific runner if this is last project for that runners' do + visit runners_path(project) - it "disables specific runner for project" do - @project2.runners << @specific_runner - visit runners_path(@project) + within '.activated-specific-runners' do + click_on 'Remove Runner' + end - within ".activated-specific-runners" do - click_on "Disable for this project" + expect(page).not_to have_content(specific_runner.display_name) end - expect(page.find(".available-specific-runners")).to have_content(@specific_runner.display_name) - end + context 'when a runner has a tag' do + background do + specific_runner.update(tag_list: ['tag']) + end - it "removes specific runner for project if this is last project for that runners" do - within ".activated-specific-runners" do - click_on "Remove Runner" - end + scenario 'user edits runner not to run untagged jobs' do + visit runners_path(project) - expect(Ci::Runner.exists?(id: @specific_runner)).to be_falsey - end - end + within '.activated-specific-runners' do + first('.edit-runner > a').click + end - describe "shared runners" do - before do - @project = FactoryGirl.create :project, shared_runners_enabled: false - @project.team << [user, :master] - visit runners_path(@project) - end + expect(page.find_field('runner[run_untagged]')).to be_checked - it "enables shared runners" do - click_on "Enable shared Runners" - expect(@project.reload.shared_runners_enabled).to be_truthy - end - end + uncheck 'runner_run_untagged' + click_button 'Save changes' - describe "shared runners description" do - let(:shared_runners_text) { 'custom **shared** runners description' } - let(:shared_runners_html) { 'custom shared runners description' } + expect(page).to have_content 'Can run untagged jobs No' + end + end - before do - stub_application_setting(shared_runners_text: shared_runners_text) - project = FactoryGirl.create :project, shared_runners_enabled: false - project.team << [user, :master] - visit runners_path(project) - end + context 'when a shared runner is activated on the project' do + given!(:shared_runner) { create(:ci_runner, :shared) } - it "sees shared runners description" do - expect(page.find(".shared-runners-description")).to have_content(shared_runners_html) - end - end + scenario 'user sees CI/CD setting page' do + visit runners_path(project) - describe "show page" do - before do - @project = FactoryGirl.create :project - @project.team << [user, :master] - @specific_runner = FactoryGirl.create :ci_runner - @project.runners << @specific_runner + expect(page.find('.available-shared-runners')).to have_content(shared_runner.display_name) + end + end end - it "shows runner information" do - visit runners_path(@project) - click_on @specific_runner.short_sha - expect(page).to have_content(@specific_runner.platform) - end - end + context 'when a specific runner exists in another project' do + given(:another_project) { create(:project) } + given(:specific_runner) { create(:ci_runner, :specific) } - feature 'configuring runners ability to picking untagged jobs' do - given(:project) { create(:project) } - given(:runner) { create(:ci_runner) } + background do + another_project.add_master(user) + another_project.runners << specific_runner + end - background do - project.team << [user, :master] - project.runners << runner - end + scenario 'user enables and disables a specific runner' do + visit runners_path(project) + + within '.available-specific-runners' do + click_on 'Enable for this project' + end - scenario 'user checks default configuration' do - visit project_runner_path(project, runner) + expect(page.find('.activated-specific-runners')).to have_content(specific_runner.display_name) - expect(page).to have_content 'Can run untagged jobs Yes' + within '.activated-specific-runners' do + click_on 'Disable for this project' + end + + expect(page.find('.available-specific-runners')).to have_content(specific_runner.display_name) + end end - context 'when runner has tags' do - before do - runner.update_attribute(:tag_list, ['tag']) + context 'when application settings have shared_runners_text' do + given(:shared_runners_text) { 'custom **shared** runners description' } + given(:shared_runners_html) { 'custom shared runners description' } + + background do + stub_application_setting(shared_runners_text: shared_runners_text) end - scenario 'user wants to prevent runner from running untagged job' do + scenario 'user sees shared runners description' do visit runners_path(project) - page.within('.activated-specific-runners') do - first('small > a').click - end - uncheck 'runner_run_untagged' - click_button 'Save changes' - - expect(page).to have_content 'Can run untagged jobs No' - expect(runner.reload.run_untagged?).to eq false + expect(page.find('.shared-runners-description')).to have_content(shared_runners_html) end end end + + context 'when a project has disabled shared_runners' do + given(:project) { create(:project, shared_runners_enabled: false) } + + background do + project.add_master(user) + end + + scenario 'user enables shared runners' do + visit runners_path(project) + + click_on 'Enable shared Runners' + + expect(page.find('.shared-runners-description')).to have_content('Disable shared Runners') + end + end end diff --git a/spec/features/search_spec.rb b/spec/features/search_spec.rb index 6742d77937f..31d509455ba 100644 --- a/spec/features/search_spec.rb +++ b/spec/features/search_spec.rb @@ -281,4 +281,30 @@ describe "Search" do expect(page).to have_selector('.commit-row-description', count: 9) end end + + context 'anonymous user' do + let(:project) { create(:project, :public) } + + before do + sign_out(user) + end + + it 'preserves the group being searched in' do + visit search_path(group_id: project.namespace.id) + + fill_in 'search', with: 'foo' + click_button 'Search' + + expect(find('#group_id').value).to eq(project.namespace.id.to_s) + end + + it 'preserves the project being searched in' do + visit search_path(project_id: project.id) + + fill_in 'search', with: 'foo' + click_button 'Search' + + expect(find('#project_id').value).to eq(project.id.to_s) + end + end end diff --git a/spec/features/snippets/notes_on_personal_snippets_spec.rb b/spec/features/snippets/notes_on_personal_snippets_spec.rb index c0c293dee78..bf79974b8c6 100644 --- a/spec/features/snippets/notes_on_personal_snippets_spec.rb +++ b/spec/features/snippets/notes_on_personal_snippets_spec.rb @@ -91,7 +91,7 @@ describe 'Comments on personal snippets', :js do context 'when editing a note' do it 'changes the text' do - find('.js-note-edit').click + find('.js-note-edit').trigger('click') page.within('.current-note-edit-form') do fill_in 'note[note]', with: 'new content' diff --git a/spec/features/tags/master_deletes_tag_spec.rb b/spec/features/tags/master_deletes_tag_spec.rb index 4d6fc13557f..d6a6b8fc7d5 100644 --- a/spec/features/tags/master_deletes_tag_spec.rb +++ b/spec/features/tags/master_deletes_tag_spec.rb @@ -36,8 +36,8 @@ feature 'Master deletes tag' do context 'when pre-receive hook fails', js: true do before do - allow_any_instance_of(GitHooksService).to receive(:execute) - .and_raise(GitHooksService::PreReceiveError, 'Do not delete tags') + allow_any_instance_of(Gitlab::Git::HooksService).to receive(:execute) + .and_raise(Gitlab::Git::HooksService::PreReceiveError, 'Do not delete tags') end scenario 'shows the error message' do diff --git a/spec/finders/admin/projects_finder_spec.rb b/spec/finders/admin/projects_finder_spec.rb index 28e36330029..4b67203a0df 100644 --- a/spec/finders/admin/projects_finder_spec.rb +++ b/spec/finders/admin/projects_finder_spec.rb @@ -118,6 +118,12 @@ describe Admin::ProjectsFinder do it { is_expected.to match_array([archived_project, shared_project, public_project, internal_project, private_project]) } end + + context 'archived=only' do + let(:params) { { archived: 'only' } } + + it { is_expected.to eq([archived_project]) } + end end context 'filter by personal' do diff --git a/spec/finders/projects_finder_spec.rb b/spec/finders/projects_finder_spec.rb index a5de586e869..0dfe6ba9c32 100644 --- a/spec/finders/projects_finder_spec.rb +++ b/spec/finders/projects_finder_spec.rb @@ -123,6 +123,12 @@ describe ProjectsFinder do it { is_expected.to match_array([public_project, internal_project, archived_project]) } end + describe 'filter by archived only' do + let(:params) { { archived: 'only' } } + + it { is_expected.to eq([archived_project]) } + end + describe 'filter by archived for backward compatibility' do let(:params) { { archived: false } } diff --git a/spec/fixtures/emails/ios_default.eml b/spec/fixtures/emails/ios_default.eml index 8d4d58feb16..fa19475104a 100644 --- a/spec/fixtures/emails/ios_default.eml +++ b/spec/fixtures/emails/ios_default.eml @@ -76,7 +76,7 @@ Content-Transfer-Encoding: 7bit <img src="https://meta-discourse.global.ssl.fastly.net/user_avatar/meta.discourse.org/techapj/45/3281.png" title="techAPJ" style="max-width:100%;" width="45" height="45"> </td> <td> - <a href="https://meta.discourse.org/users/techapj" target="_blank" style="text-decoration: none; font-weight: bold; color: #006699;; font-size:13px;font-family:'lucida grande',tahoma,verdana,arial,sans-serif;color:#3b5998;text-decoration:none;font-weight:bold">techAPJ</a><br> + <a href="https://meta.discourse.org/users/techapj" target="_blank" style="text-decoration: none; font-weight: 600; color: #006699;; font-size:13px;font-family:'lucida grande',tahoma,verdana,arial,sans-serif;color:#3b5998;text-decoration:none;font-weight:bold">techAPJ</a><br> <span style="text-align:right;color:#999999;padding-right:5px;font-family:'lucida grande',tahoma,verdana,arial,sans-serif;font-size:11px">November 28</span> </td> </tr> @@ -94,7 +94,7 @@ Content-Transfer-Encoding: 7bit <div style="color:#666;"> - <p>To respond, reply to this email or visit <a href="https://meta.discourse.org/t/testing-default-email-replies/22638/3" style="text-decoration: none; font-weight: bold; color: #006699;; color:#666;">https://meta.discourse.org/t/testing-default-email-replies/22638/3</a> in your browser.</p> + <p>To respond, reply to this email or visit <a href="https://meta.discourse.org/t/testing-default-email-replies/22638/3" style="text-decoration: none; font-weight: 600; color: #006699;; color:#666;">https://meta.discourse.org/t/testing-default-email-replies/22638/3</a> in your browser.</p> </div> <hr style="background-color: #ddd; height: 1px; border: 1px;; background-color: #ddd; height: 1px; border: 1px;"> <h4>Previous Replies</h4> @@ -106,7 +106,7 @@ Content-Transfer-Encoding: 7bit <img src="https://meta-discourse.global.ssl.fastly.net/user_avatar/meta.discourse.org/codinghorror/45/5297.png" title="codinghorror" style="max-width:100%;" width="45" height="45"> </td> <td> - <a href="https://meta.discourse.org/users/codinghorror" target="_blank" style="text-decoration: none; font-weight: bold; color: #006699;; font-size:13px;font-family:'lucida grande',tahoma,verdana,arial,sans-serif;color:#3b5998;text-decoration:none;font-weight:bold">codinghorror</a><br> + <a href="https://meta.discourse.org/users/codinghorror" target="_blank" style="text-decoration: none; font-weight: 600; color: #006699;; font-size:13px;font-family:'lucida grande',tahoma,verdana,arial,sans-serif;color:#3b5998;text-decoration:none;font-weight:bold">codinghorror</a><br> <span style="text-align:right;color:#999999;padding-right:5px;font-family:'lucida grande',tahoma,verdana,arial,sans-serif;font-size:11px">November 28</span> </td> </tr> @@ -114,7 +114,7 @@ Content-Transfer-Encoding: 7bit <td style="padding-top:5px;" colspan="2"> <p style="margin-top:0; border: 0;">We're testing the latest GitHub email processing library which we are integrating now.</p> -<p style="margin-top:0; border: 0;"><a href="https://github.com/github/email_reply_parser" target="_blank" style="text-decoration: none; font-weight: bold; color: #006699;">https://github.com/github/email_reply_parser</a></p> +<p style="margin-top:0; border: 0;"><a href="https://github.com/github/email_reply_parser" target="_blank" style="text-decoration: none; font-weight: 600; color: #006699;">https://github.com/github/email_reply_parser</a></p> <p style="margin-top:0; border: 0;">Go ahead and reply to this topic and I'll reply from various email clients for testing.</p> </td> @@ -126,10 +126,10 @@ Content-Transfer-Encoding: 7bit <hr style="background-color: #ddd; height: 1px; border: 1px;; background-color: #ddd; height: 1px; border: 1px;"> <div style="color:#666;"> -<p>To respond, reply to this email or visit <a href="https://meta.discourse.org/t/testing-default-email-replies/22638/3" style="text-decoration: none; font-weight: bold; color: #006699;; color:#666;">https://meta.discourse.org/t/testing-default-email-replies/22638/3</a> in your browser.</p> +<p>To respond, reply to this email or visit <a href="https://meta.discourse.org/t/testing-default-email-replies/22638/3" style="text-decoration: none; font-weight: 600; color: #006699;; color:#666;">https://meta.discourse.org/t/testing-default-email-replies/22638/3</a> in your browser.</p> </div> <div style="color:#666;"> -<p>To unsubscribe from these emails, visit your <a href="https://meta.discourse.org/my/preferences" style="text-decoration: none; font-weight: bold; color: #006699;; color:#666;">user preferences</a>.</p> +<p>To unsubscribe from these emails, visit your <a href="https://meta.discourse.org/my/preferences" style="text-decoration: none; font-weight: 600; color: #006699;; color:#666;">user preferences</a>.</p> </div> </div> </div></blockquote></body></html> diff --git a/spec/fixtures/emails/on_wrote.eml b/spec/fixtures/emails/on_wrote.eml index feb59bd27bb..af6a4e50a49 100644 --- a/spec/fixtures/emails/on_wrote.eml +++ b/spec/fixtures/emails/on_wrote.eml @@ -53,7 +53,7 @@ y > display: inline-block; > font-family: FontAwesome; > font-style: normal; -> font-weight: normal; +> font-weight: 400; > line-height: 1; > -webkit-font-smoothing: antialiased; > -moz-osx-font-smoothing: grayscale; @@ -227,7 +227,7 @@ ding:5px">.fa { display: inline-block; font-family: FontAwesome; font-style: normal; - font-weight: normal; + font-weight: 400; line-height: 1; -webkit-font-smoothing: antialiased; -moz-osx-font-smoothing: grayscale; @@ -274,4 +274,4 @@ ight:bold;color:#006699" target=3D"_blank">user preferences</a>.</p> </div> </blockquote></div><br></div></div> ---001a11c34c389e728f0502aa26a0--
\ No newline at end of file +--001a11c34c389e728f0502aa26a0-- diff --git a/spec/helpers/avatars_helper_spec.rb b/spec/helpers/avatars_helper_spec.rb index d16fcf21e45..4632c679972 100644 --- a/spec/helpers/avatars_helper_spec.rb +++ b/spec/helpers/avatars_helper_spec.rb @@ -28,7 +28,7 @@ describe AvatarsHelper do it 'displays user avatar' do is_expected.to eq image_tag( LazyImageTagHelper.placeholder_image, - class: 'avatar has-tooltip s16 lazy', + class: 'avatar s16 has-tooltip lazy', alt: "#{user.name}'s avatar", title: user.name, data: { container: 'body', src: avatar_icon(user, 16) } @@ -41,7 +41,7 @@ describe AvatarsHelper do it 'uses provided css_class' do is_expected.to eq image_tag( LazyImageTagHelper.placeholder_image, - class: "avatar has-tooltip s16 #{options[:css_class]} lazy", + class: "avatar s16 #{options[:css_class]} has-tooltip lazy", alt: "#{user.name}'s avatar", title: user.name, data: { container: 'body', src: avatar_icon(user, 16) } @@ -55,7 +55,7 @@ describe AvatarsHelper do it 'uses provided size' do is_expected.to eq image_tag( LazyImageTagHelper.placeholder_image, - class: "avatar has-tooltip s#{options[:size]} lazy", + class: "avatar s#{options[:size]} has-tooltip lazy", alt: "#{user.name}'s avatar", title: user.name, data: { container: 'body', src: avatar_icon(user, options[:size]) } @@ -69,7 +69,7 @@ describe AvatarsHelper do it 'uses provided url' do is_expected.to eq image_tag( LazyImageTagHelper.placeholder_image, - class: 'avatar has-tooltip s16 lazy', + class: 'avatar s16 has-tooltip lazy', alt: "#{user.name}'s avatar", title: user.name, data: { container: 'body', src: options[:url] } @@ -77,6 +77,36 @@ describe AvatarsHelper do end end + context 'with has_tooltip parameter' do + context 'with has_tooltip set to true' do + let(:options) { { user: user, has_tooltip: true } } + + it 'adds has-tooltip' do + is_expected.to eq image_tag( + LazyImageTagHelper.placeholder_image, + class: 'avatar s16 has-tooltip lazy', + alt: "#{user.name}'s avatar", + title: user.name, + data: { container: 'body', src: avatar_icon(user, 16) } + ) + end + end + + context 'with has_tooltip set to false' do + let(:options) { { user: user, has_tooltip: false } } + + it 'does not add has-tooltip or data container' do + is_expected.to eq image_tag( + LazyImageTagHelper.placeholder_image, + class: 'avatar s16 lazy', + alt: "#{user.name}'s avatar", + title: user.name, + data: { src: avatar_icon(user, 16) } + ) + end + end + end + context 'with user_name parameter' do let(:options) { { user_name: 'Tinky Winky', user_email: 'no@f.un' } } @@ -86,7 +116,7 @@ describe AvatarsHelper do it 'prefers user parameter' do is_expected.to eq image_tag( LazyImageTagHelper.placeholder_image, - class: 'avatar has-tooltip s16 lazy', + class: 'avatar s16 has-tooltip lazy', alt: "#{user.name}'s avatar", title: user.name, data: { container: 'body', src: avatar_icon(user, 16) } @@ -97,7 +127,7 @@ describe AvatarsHelper do it 'uses user_name and user_email parameter if user is not present' do is_expected.to eq image_tag( LazyImageTagHelper.placeholder_image, - class: 'avatar has-tooltip s16 lazy', + class: 'avatar s16 has-tooltip lazy', alt: "#{options[:user_name]}'s avatar", title: options[:user_name], data: { container: 'body', src: avatar_icon(options[:user_email], 16) } diff --git a/spec/helpers/button_helper_spec.rb b/spec/helpers/button_helper_spec.rb index 250ba239033..4423560ecaa 100644 --- a/spec/helpers/button_helper_spec.rb +++ b/spec/helpers/button_helper_spec.rb @@ -62,4 +62,67 @@ describe ButtonHelper do end end end + + describe 'clipboard_button' do + let(:user) { create(:user) } + let(:project) { build_stubbed(:project) } + + def element(data = {}) + element = helper.clipboard_button(data) + Nokogiri::HTML::DocumentFragment.parse(element).first_element_child + end + + before do + allow(helper).to receive(:current_user).and_return(user) + end + + context 'with default options' do + context 'when no `text` attribute is not provided' do + it 'shows copy to clipboard button with default configuration and no text set to copy' do + expect(element.attr('class')).to eq('btn btn-clipboard btn-transparent') + expect(element.attr('type')).to eq('button') + expect(element.attr('aria-label')).to eq('Copy to clipboard') + expect(element.attr('data-toggle')).to eq('tooltip') + expect(element.attr('data-placement')).to eq('bottom') + expect(element.attr('data-container')).to eq('body') + expect(element.attr('data-clipboard-text')).to eq(nil) + expect(element.inner_text).to eq("") + + expect(element).to have_selector('.fa.fa-clipboard') + end + end + + context 'when `text` attribute is provided' do + it 'shows copy to clipboard button with provided `text` to copy' do + expect(element(text: 'Hello World!').attr('data-clipboard-text')).to eq('Hello World!') + end + end + + context 'when `title` attribute is provided' do + it 'shows copy to clipboard button with provided `title` as tooltip' do + expect(element(title: 'Copy to my clipboard!').attr('aria-label')).to eq('Copy to my clipboard!') + end + end + end + + context 'with `button_text` attribute provided' do + it 'shows copy to clipboard button with provided `button_text` as button label' do + expect(element(button_text: 'Copy text').inner_text).to eq('Copy text') + end + end + + context 'with `hide_tooltip` attribute provided' do + it 'shows copy to clipboard button without tooltip support' do + expect(element(hide_tooltip: true).attr('data-placement')).to eq(nil) + expect(element(hide_tooltip: true).attr('data-toggle')).to eq(nil) + expect(element(hide_tooltip: true).attr('data-container')).to eq(nil) + end + end + + context 'with `hide_button_icon` attribute provided' do + it 'shows copy to clipboard button without tooltip support' do + expect(element(hide_button_icon: true)).not_to have_selector('.fa.fa-clipboard') + end + end + end end diff --git a/spec/helpers/events_helper_spec.rb b/spec/helpers/events_helper_spec.rb index 4b72dbb7964..d5536fcb22b 100644 --- a/spec/helpers/events_helper_spec.rb +++ b/spec/helpers/events_helper_spec.rb @@ -106,5 +106,9 @@ describe EventsHelper do it "handles empty strings" do expect(helper.event_commit_title("")).to eq("") end + + it 'handles nil values' do + expect(helper.event_commit_title(nil)).to eq('') + end end end diff --git a/spec/javascripts/api_spec.js b/spec/javascripts/api_spec.js index 867322ce8ae..8c68ceff914 100644 --- a/spec/javascripts/api_spec.js +++ b/spec/javascripts/api_spec.js @@ -17,7 +17,7 @@ describe('Api', () => { beforeEach(() => { originalGon = window.gon; - window.gon = dummyGon; + window.gon = Object.assign({}, dummyGon); }); afterEach(() => { @@ -98,10 +98,11 @@ describe('Api', () => { }); describe('projects', () => { - it('fetches projects', (done) => { + it('fetches projects with membership when logged in', (done) => { const query = 'dummy query'; const options = { unused: 'option' }; const expectedUrl = `${dummyUrlRoot}/api/${dummyApiVersion}/projects.json?simple=true`; + window.gon.current_user_id = 1; const expectedData = Object.assign({ search: query, per_page: 20, @@ -119,6 +120,27 @@ describe('Api', () => { done(); }); }); + + it('fetches projects without membership when not logged in', (done) => { + const query = 'dummy query'; + const options = { unused: 'option' }; + const expectedUrl = `${dummyUrlRoot}/api/${dummyApiVersion}/projects.json?simple=true`; + const expectedData = Object.assign({ + search: query, + per_page: 20, + }, options); + spyOn(jQuery, 'ajax').and.callFake((request) => { + expect(request.url).toEqual(expectedUrl); + expect(request.dataType).toEqual('json'); + expect(request.data).toEqual(expectedData); + return sendDummyResponse(); + }); + + Api.projects(query, options, (response) => { + expect(response).toBe(dummyResponse); + done(); + }); + }); }); describe('newLabel', () => { diff --git a/spec/javascripts/project_title_spec.js b/spec/javascripts/project_title_spec.js index cc336180ff7..3d36bb3e4d4 100644 --- a/spec/javascripts/project_title_spec.js +++ b/spec/javascripts/project_title_spec.js @@ -7,6 +7,7 @@ import '~/project_select'; import '~/project'; describe('Project Title', () => { + const dummyApiVersion = 'v3000'; preloadFixtures('issues/open-issue.html.raw'); loadJSONFixtures('projects.json'); @@ -14,7 +15,7 @@ describe('Project Title', () => { loadFixtures('issues/open-issue.html.raw'); window.gon = {}; - window.gon.api_version = 'v3'; + window.gon.api_version = dummyApiVersion; // eslint-disable-next-line no-new new Project(); @@ -37,9 +38,10 @@ describe('Project Title', () => { it('toggles dropdown', () => { const $menu = $('.js-dropdown-menu-projects'); + window.gon.current_user_id = 1; $('.js-projects-dropdown-toggle').click(); expect($menu).toHaveClass('open'); - expect(reqUrl).toBe('/api/v3/projects.json?simple=true'); + expect(reqUrl).toBe(`/api/${dummyApiVersion}/projects.json?simple=true`); expect(reqData).toEqual({ search: '', order_by: 'last_activity_at', diff --git a/spec/javascripts/repo/components/repo_commit_section_spec.js b/spec/javascripts/repo/components/repo_commit_section_spec.js index 249a2f36fcd..e604dcc152d 100644 --- a/spec/javascripts/repo/components/repo_commit_section_spec.js +++ b/spec/javascripts/repo/components/repo_commit_section_spec.js @@ -1,7 +1,7 @@ import Vue from 'vue'; import repoCommitSection from '~/repo/components/repo_commit_section.vue'; import RepoStore from '~/repo/stores/repo_store'; -import Api from '~/api'; +import RepoService from '~/repo/services/repo_service'; describe('RepoCommitSection', () => { const branch = 'master'; @@ -111,7 +111,7 @@ describe('RepoCommitSection', () => { expect(submitCommit.disabled).toBeFalsy(); spyOn(vm, 'makeCommit').and.callThrough(); - spyOn(Api, 'commitMultiple'); + spyOn(RepoService, 'commitFiles').and.callFake(() => Promise.resolve()); submitCommit.click(); @@ -119,10 +119,9 @@ describe('RepoCommitSection', () => { expect(vm.makeCommit).toHaveBeenCalled(); expect(submitCommit.querySelector('.fa-spinner.fa-spin')).toBeTruthy(); - const args = Api.commitMultiple.calls.allArgs()[0]; - const { commit_message, actions, branch: payloadBranch } = args[1]; + const args = RepoService.commitFiles.calls.allArgs()[0]; + const { commit_message, actions, branch: payloadBranch } = args[0]; - expect(args[0]).toBe(projectId); expect(commit_message).toBe(commitMessage); expect(actions.length).toEqual(2); expect(payloadBranch).toEqual(branch); diff --git a/spec/javascripts/repo/monaco_loader_spec.js b/spec/javascripts/repo/monaco_loader_spec.js index be6e779c50f..887a80160fc 100644 --- a/spec/javascripts/repo/monaco_loader_spec.js +++ b/spec/javascripts/repo/monaco_loader_spec.js @@ -1,17 +1,13 @@ -/* global __webpack_public_path__ */ import monacoContext from 'monaco-editor/dev/vs/loader'; +import monacoLoader from '~/repo/monaco_loader'; describe('MonacoLoader', () => { it('calls require.config and exports require', () => { - spyOn(monacoContext.require, 'config'); - - const monacoLoader = require('~/repo/monaco_loader'); // eslint-disable-line global-require - - expect(monacoContext.require.config).toHaveBeenCalledWith({ + expect(monacoContext.require.getConfig()).toEqual(jasmine.objectContaining({ paths: { vs: `${__webpack_public_path__}monaco-editor/vs`, // eslint-disable-line camelcase }, - }); - expect(monacoLoader.default).toBe(monacoContext.require); + })); + expect(monacoLoader).toBe(monacoContext.require); }); }); diff --git a/spec/javascripts/repo/services/repo_service_spec.js b/spec/javascripts/repo/services/repo_service_spec.js index d74e6a67b1e..6f530770525 100644 --- a/spec/javascripts/repo/services/repo_service_spec.js +++ b/spec/javascripts/repo/services/repo_service_spec.js @@ -1,5 +1,7 @@ import axios from 'axios'; import RepoService from '~/repo/services/repo_service'; +import RepoStore from '~/repo/stores/repo_store'; +import Api from '~/api'; describe('RepoService', () => { it('has default json format param', () => { @@ -118,4 +120,52 @@ describe('RepoService', () => { }).catch(done.fail); }); }); + + describe('commitFiles', () => { + it('calls commitMultiple and .then commitFlash', (done) => { + const projectId = 'projectId'; + const payload = {}; + RepoStore.projectId = projectId; + + spyOn(Api, 'commitMultiple').and.returnValue(Promise.resolve()); + spyOn(RepoService, 'commitFlash'); + + const apiPromise = RepoService.commitFiles(payload); + + expect(Api.commitMultiple).toHaveBeenCalledWith(projectId, payload); + + apiPromise.then(() => { + expect(RepoService.commitFlash).toHaveBeenCalled(); + done(); + }).catch(done.fail); + }); + }); + + describe('commitFlash', () => { + it('calls Flash with data.message', () => { + const data = { + message: 'message', + }; + spyOn(window, 'Flash'); + + RepoService.commitFlash(data); + + expect(window.Flash).toHaveBeenCalledWith(data.message); + }); + + it('calls Flash with success string if short_id and stats', () => { + const data = { + short_id: 'short_id', + stats: { + additions: '4', + deletions: '5', + }, + }; + spyOn(window, 'Flash'); + + RepoService.commitFlash(data); + + expect(window.Flash).toHaveBeenCalledWith(`Your changes have been committed. Commit ${data.short_id} with ${data.stats.additions} additions, ${data.stats.deletions} deletions.`, 'notice'); + }); + }); }); diff --git a/spec/lib/gitlab/bare_repository_importer_spec.rb b/spec/lib/gitlab/bare_repository_importer_spec.rb new file mode 100644 index 00000000000..36d1844b5b1 --- /dev/null +++ b/spec/lib/gitlab/bare_repository_importer_spec.rb @@ -0,0 +1,100 @@ +require 'spec_helper' + +describe Gitlab::BareRepositoryImporter, repository: true do + subject(:importer) { described_class.new('default', project_path) } + + let!(:admin) { create(:admin) } + + before do + allow(described_class).to receive(:log) + end + + shared_examples 'importing a repository' do + describe '.execute' do + it 'creates a project for a repository in storage' do + FileUtils.mkdir_p(File.join(TestEnv.repos_path, "#{project_path}.git")) + fake_importer = double + + expect(described_class).to receive(:new).with('default', project_path) + .and_return(fake_importer) + expect(fake_importer).to receive(:create_project_if_needed) + + described_class.execute + end + + it 'skips wiki repos' do + FileUtils.mkdir_p(File.join(TestEnv.repos_path, 'the-group', 'the-project.wiki.git')) + + expect(described_class).to receive(:log).with(' * Skipping wiki repo') + expect(described_class).not_to receive(:new) + + described_class.execute + end + end + + describe '#initialize' do + context 'without admin users' do + let(:admin) { nil } + + it 'raises an error' do + expect { importer }.to raise_error(Gitlab::BareRepositoryImporter::NoAdminError) + end + end + end + + describe '#create_project_if_needed' do + it 'starts an import for a project that did not exist' do + expect(importer).to receive(:create_project) + + importer.create_project_if_needed + end + + it 'skips importing when the project already exists' do + project = create(:project, path: 'a-project', namespace: existing_group) + + expect(importer).not_to receive(:create_project) + expect(importer).to receive(:log).with(" * #{project.name} (#{project_path}) exists") + + importer.create_project_if_needed + end + + it 'creates a project with the correct path in the database' do + importer.create_project_if_needed + + expect(Project.find_by_full_path(project_path)).not_to be_nil + end + end + end + + context 'with subgroups', :nested_groups do + let(:project_path) { 'a-group/a-sub-group/a-project' } + + let(:existing_group) do + group = create(:group, path: 'a-group') + create(:group, path: 'a-sub-group', parent: group) + end + + it_behaves_like 'importing a repository' + end + + context 'without subgroups' do + let(:project_path) { 'a-group/a-project' } + let(:existing_group) { create(:group, path: 'a-group') } + + it_behaves_like 'importing a repository' + end + + context 'when subgroups are not available' do + let(:project_path) { 'a-group/a-sub-group/a-project' } + + before do + expect(Group).to receive(:supports_nested_groups?) { false } + end + + describe '#create_project_if_needed' do + it 'raises an error' do + expect { importer.create_project_if_needed }.to raise_error('Nested groups are not supported on MySQL') + end + end + end +end diff --git a/spec/lib/gitlab/database/migration_helpers_spec.rb b/spec/lib/gitlab/database/migration_helpers_spec.rb index ec2274a70aa..c25fd459dd7 100644 --- a/spec/lib/gitlab/database/migration_helpers_spec.rb +++ b/spec/lib/gitlab/database/migration_helpers_spec.rb @@ -2,9 +2,7 @@ require 'spec_helper' describe Gitlab::Database::MigrationHelpers do let(:model) do - ActiveRecord::Migration.new.extend( - described_class - ) + ActiveRecord::Migration.new.extend(described_class) end before do @@ -845,4 +843,51 @@ describe Gitlab::Database::MigrationHelpers do end end end + + describe 'sidekiq migration helpers', :sidekiq, :redis do + let(:worker) do + Class.new do + include Sidekiq::Worker + sidekiq_options queue: 'test' + end + end + + describe '#sidekiq_queue_length' do + context 'when queue is empty' do + it 'returns zero' do + Sidekiq::Testing.disable! do + expect(model.sidekiq_queue_length('test')).to eq 0 + end + end + end + + context 'when queue contains jobs' do + it 'returns correct size of the queue' do + Sidekiq::Testing.disable! do + worker.perform_async('Something', [1]) + worker.perform_async('Something', [2]) + + expect(model.sidekiq_queue_length('test')).to eq 2 + end + end + end + end + + describe '#migrate_sidekiq_queue' do + it 'migrates jobs from one sidekiq queue to another' do + Sidekiq::Testing.disable! do + worker.perform_async('Something', [1]) + worker.perform_async('Something', [2]) + + expect(model.sidekiq_queue_length('test')).to eq 2 + expect(model.sidekiq_queue_length('new_test')).to eq 0 + + model.sidekiq_queue_migrate('test', to: 'new_test') + + expect(model.sidekiq_queue_length('test')).to eq 0 + expect(model.sidekiq_queue_length('new_test')).to eq 2 + end + end + end + end end diff --git a/spec/lib/gitlab/diff/file_spec.rb b/spec/lib/gitlab/diff/file_spec.rb index d3d841b0668..c91895cedc3 100644 --- a/spec/lib/gitlab/diff/file_spec.rb +++ b/spec/lib/gitlab/diff/file_spec.rb @@ -15,6 +15,17 @@ describe Gitlab::Diff::File do it { expect(diff_lines.first).to be_kind_of(Gitlab::Diff::Line) } end + describe '#highlighted_diff_lines' do + it 'highlights the diff and memoises the result' do + expect(Gitlab::Diff::Highlight).to receive(:new) + .with(diff_file, repository: project.repository) + .once + .and_call_original + + diff_file.highlighted_diff_lines + end + end + describe '#mode_changed?' do it { expect(diff_file.mode_changed?).to be_falsey } end @@ -122,8 +133,20 @@ describe Gitlab::Diff::File do let(:commit) { project.commit('2f63565e7aac07bcdadb654e253078b727143ec4') } let(:diff_file) { commit.diffs.diff_file_with_new_path('files/images/6049019_460s.jpg') } - it 'returns true' do - expect(diff_file.content_changed?).to be_truthy + context 'when the blobs are different' do + it 'returns true' do + expect(diff_file.content_changed?).to be_truthy + end + end + + context 'when there are no diff refs' do + before do + allow(diff_file).to receive(:diff_refs).and_return(nil) + end + + it 'returns false' do + expect(diff_file.content_changed?).to be_falsey + end end end @@ -131,8 +154,20 @@ describe Gitlab::Diff::File do let(:commit) { project.commit('570e7b2abdd848b95f2f578043fc23bd6f6fd24d') } let(:diff_file) { commit.diffs.diff_file_with_new_path('files/ruby/popen.rb') } - it 'returns true' do - expect(diff_file.content_changed?).to be_truthy + context 'when the blobs are different' do + it 'returns true' do + expect(diff_file.content_changed?).to be_truthy + end + end + + context 'when there are no diff refs' do + before do + allow(diff_file).to receive(:diff_refs).and_return(nil) + end + + it 'returns true' do + expect(diff_file.content_changed?).to be_truthy + end end end end @@ -270,6 +305,21 @@ describe Gitlab::Diff::File do expect(diff_file.simple_viewer).to be_a(DiffViewer::ModeChanged) end end + + context 'when no other conditions apply' do + before do + allow(diff_file).to receive(:content_changed?).and_return(false) + allow(diff_file).to receive(:new_file?).and_return(false) + allow(diff_file).to receive(:deleted_file?).and_return(false) + allow(diff_file).to receive(:renamed_file?).and_return(false) + allow(diff_file).to receive(:mode_changed?).and_return(false) + allow(diff_file).to receive(:raw_text?).and_return(false) + end + + it 'returns a No Preview viewer' do + expect(diff_file.simple_viewer).to be_a(DiffViewer::NoPreview) + end + end end describe '#rich_viewer' do diff --git a/spec/lib/gitlab/file_finder_spec.rb b/spec/lib/gitlab/file_finder_spec.rb index 3fb6315a39a..07cb10e563e 100644 --- a/spec/lib/gitlab/file_finder_spec.rb +++ b/spec/lib/gitlab/file_finder_spec.rb @@ -7,15 +7,23 @@ describe Gitlab::FileFinder do it 'finds by name' do results = finder.find('files') - expect(results.map(&:first)).to include('files/images/wm.svg') + + filename, blob = results.find { |_, blob| blob.filename == 'files/images/wm.svg' } + expect(filename).to eq('files/images/wm.svg') + expect(blob).to be_a(Gitlab::SearchResults::FoundBlob) + expect(blob.ref).to eq(finder.ref) + expect(blob.data).not_to be_empty end it 'finds by content' do results = finder.find('files') - blob = results.select { |result| result.first == "CHANGELOG" }.flatten.last + filename, blob = results.find { |_, blob| blob.filename == 'CHANGELOG' } - expect(blob.filename).to eq("CHANGELOG") + expect(filename).to eq('CHANGELOG') + expect(blob).to be_a(Gitlab::SearchResults::FoundBlob) + expect(blob.ref).to eq(finder.ref) + expect(blob.data).not_to be_empty end end end diff --git a/spec/lib/gitlab/git/blame_spec.rb b/spec/lib/gitlab/git/blame_spec.rb index 800c245b130..465c2012b05 100644 --- a/spec/lib/gitlab/git/blame_spec.rb +++ b/spec/lib/gitlab/git/blame_spec.rb @@ -2,7 +2,7 @@ require "spec_helper" describe Gitlab::Git::Blame, seed_helper: true do - let(:repository) { Gitlab::Git::Repository.new('default', TEST_REPO_PATH) } + let(:repository) { Gitlab::Git::Repository.new('default', TEST_REPO_PATH, '') } let(:blame) do Gitlab::Git::Blame.new(repository, SeedRepo::Commit::ID, "CONTRIBUTING.md") end diff --git a/spec/lib/gitlab/git/blob_spec.rb b/spec/lib/gitlab/git/blob_spec.rb index dfab0c2fe85..66ba00acb7d 100644 --- a/spec/lib/gitlab/git/blob_spec.rb +++ b/spec/lib/gitlab/git/blob_spec.rb @@ -3,7 +3,7 @@ require "spec_helper" describe Gitlab::Git::Blob, seed_helper: true do - let(:repository) { Gitlab::Git::Repository.new('default', TEST_REPO_PATH) } + let(:repository) { Gitlab::Git::Repository.new('default', TEST_REPO_PATH, '') } describe 'initialize' do let(:blob) { Gitlab::Git::Blob.new(name: 'test') } diff --git a/spec/lib/gitlab/git/branch_spec.rb b/spec/lib/gitlab/git/branch_spec.rb index cdf1b8beee3..318a7b7a332 100644 --- a/spec/lib/gitlab/git/branch_spec.rb +++ b/spec/lib/gitlab/git/branch_spec.rb @@ -1,7 +1,7 @@ require "spec_helper" describe Gitlab::Git::Branch, seed_helper: true do - let(:repository) { Gitlab::Git::Repository.new('default', TEST_REPO_PATH) } + let(:repository) { Gitlab::Git::Repository.new('default', TEST_REPO_PATH, '') } subject { repository.branches } diff --git a/spec/lib/gitlab/git/commit_spec.rb b/spec/lib/gitlab/git/commit_spec.rb index ac33cd8a2c9..14d64d8c4da 100644 --- a/spec/lib/gitlab/git/commit_spec.rb +++ b/spec/lib/gitlab/git/commit_spec.rb @@ -1,7 +1,7 @@ require "spec_helper" describe Gitlab::Git::Commit, seed_helper: true do - let(:repository) { Gitlab::Git::Repository.new('default', TEST_REPO_PATH) } + let(:repository) { Gitlab::Git::Repository.new('default', TEST_REPO_PATH, '') } let(:commit) { described_class.find(repository, SeedRepo::Commit::ID) } let(:rugged_commit) do repository.rugged.lookup(SeedRepo::Commit::ID) @@ -9,7 +9,7 @@ describe Gitlab::Git::Commit, seed_helper: true do describe "Commit info" do before do - repo = Gitlab::Git::Repository.new('default', TEST_REPO_PATH).rugged + repo = Gitlab::Git::Repository.new('default', TEST_REPO_PATH, '').rugged @committer = { email: 'mike@smith.com', @@ -59,7 +59,7 @@ describe Gitlab::Git::Commit, seed_helper: true do after do # Erase the new commit so other tests get the original repo - repo = Gitlab::Git::Repository.new('default', TEST_REPO_PATH).rugged + repo = Gitlab::Git::Repository.new('default', TEST_REPO_PATH, '').rugged repo.references.update("refs/heads/master", SeedRepo::LastCommit::ID) end end @@ -144,7 +144,7 @@ describe Gitlab::Git::Commit, seed_helper: true do end context 'with broken repo' do - let(:repository) { Gitlab::Git::Repository.new('default', TEST_BROKEN_REPO_PATH) } + let(:repository) { Gitlab::Git::Repository.new('default', TEST_BROKEN_REPO_PATH, '') } it 'returns nil' do expect(described_class.find(repository, SeedRepo::Commit::ID)).to be_nil diff --git a/spec/lib/gitlab/git/committer_spec.rb b/spec/lib/gitlab/git/committer_spec.rb new file mode 100644 index 00000000000..b0ddbb51449 --- /dev/null +++ b/spec/lib/gitlab/git/committer_spec.rb @@ -0,0 +1,22 @@ +require 'spec_helper' + +describe Gitlab::Git::Committer do + let(:name) { 'Jane Doe' } + let(:email) { 'janedoe@example.com' } + let(:gl_id) { 'user-123' } + + subject { described_class.new(name, email, gl_id) } + + describe '#==' do + def eq_other(name, email, gl_id) + eq(described_class.new(name, email, gl_id)) + end + + it { expect(subject).to eq_other(name, email, gl_id) } + + it { expect(subject).not_to eq_other(nil, nil, nil) } + it { expect(subject).not_to eq_other(name + 'x', email, gl_id) } + it { expect(subject).not_to eq_other(name, email + 'x', gl_id) } + it { expect(subject).not_to eq_other(name, email, gl_id + 'x') } + end +end diff --git a/spec/lib/gitlab/git/compare_spec.rb b/spec/lib/gitlab/git/compare_spec.rb index 4c9f4a28f32..b6a42e422b5 100644 --- a/spec/lib/gitlab/git/compare_spec.rb +++ b/spec/lib/gitlab/git/compare_spec.rb @@ -1,7 +1,7 @@ require "spec_helper" describe Gitlab::Git::Compare, seed_helper: true do - let(:repository) { Gitlab::Git::Repository.new('default', TEST_REPO_PATH) } + let(:repository) { Gitlab::Git::Repository.new('default', TEST_REPO_PATH, '') } let(:compare) { Gitlab::Git::Compare.new(repository, SeedRepo::BigCommit::ID, SeedRepo::Commit::ID, straight: false) } let(:compare_straight) { Gitlab::Git::Compare.new(repository, SeedRepo::BigCommit::ID, SeedRepo::Commit::ID, straight: true) } diff --git a/spec/lib/gitlab/git/diff_spec.rb b/spec/lib/gitlab/git/diff_spec.rb index 7ea3386ac2a..dfbdbee48f7 100644 --- a/spec/lib/gitlab/git/diff_spec.rb +++ b/spec/lib/gitlab/git/diff_spec.rb @@ -1,7 +1,7 @@ require "spec_helper" describe Gitlab::Git::Diff, seed_helper: true do - let(:repository) { Gitlab::Git::Repository.new('default', TEST_REPO_PATH) } + let(:repository) { Gitlab::Git::Repository.new('default', TEST_REPO_PATH, '') } before do @raw_diff_hash = { diff --git a/spec/lib/gitlab/git/hook_spec.rb b/spec/lib/gitlab/git/hook_spec.rb index 19391a70cf6..ea3e4680b1d 100644 --- a/spec/lib/gitlab/git/hook_spec.rb +++ b/spec/lib/gitlab/git/hook_spec.rb @@ -10,7 +10,8 @@ describe Gitlab::Git::Hook do describe "#trigger" do let(:project) { create(:project, :repository) } - let(:repo_path) { project.repository.path } + let(:repository) { project.repository.raw_repository } + let(:repo_path) { repository.path } let(:user) { create(:user) } let(:gl_id) { Gitlab::GlId.gl_id(user) } @@ -48,7 +49,7 @@ describe Gitlab::Git::Hook do it "returns success with no errors" do create_hook(hook_name) - hook = described_class.new(hook_name, project) + hook = described_class.new(hook_name, repository) blank = Gitlab::Git::BLANK_SHA ref = Gitlab::Git::BRANCH_REF_PREFIX + 'new_branch' @@ -66,7 +67,7 @@ describe Gitlab::Git::Hook do context "when the hook is unsuccessful" do it "returns failure with errors" do create_failing_hook(hook_name) - hook = described_class.new(hook_name, project) + hook = described_class.new(hook_name, repository) blank = Gitlab::Git::BLANK_SHA ref = Gitlab::Git::BRANCH_REF_PREFIX + 'new_branch' @@ -80,7 +81,7 @@ describe Gitlab::Git::Hook do context "when the hook doesn't exist" do it "returns success with no errors" do - hook = described_class.new('unknown_hook', project) + hook = described_class.new('unknown_hook', repository) blank = Gitlab::Git::BLANK_SHA ref = Gitlab::Git::BRANCH_REF_PREFIX + 'new_branch' diff --git a/spec/services/git_hooks_service_spec.rb b/spec/lib/gitlab/git/hooks_service_spec.rb index 3ce01a995b4..e9c0209fe3b 100644 --- a/spec/services/git_hooks_service_spec.rb +++ b/spec/lib/gitlab/git/hooks_service_spec.rb @@ -1,16 +1,14 @@ require 'spec_helper' -describe GitHooksService do - include RepoHelpers - - let(:user) { create(:user) } - let(:project) { create(:project, :repository) } +describe Gitlab::Git::HooksService, seed_helper: true do + let(:committer) { Gitlab::Git::Committer.new('Jane Doe', 'janedoe@example.com', 'user-456') } + let(:repository) { Gitlab::Git::Repository.new('default', TEST_REPO_PATH, 'project-123') } let(:service) { described_class.new } before do @blankrev = Gitlab::Git::BLANK_SHA - @oldrev = sample_commit.parent_id - @newrev = sample_commit.id + @oldrev = SeedRepo::Commit::PARENT_ID + @newrev = SeedRepo::Commit::ID @ref = 'refs/heads/feature' end @@ -20,7 +18,7 @@ describe GitHooksService do hook = double(trigger: [true, nil]) expect(Gitlab::Git::Hook).to receive(:new).exactly(3).times.and_return(hook) - service.execute(user, project, @blankrev, @newrev, @ref) { } + service.execute(committer, repository, @blankrev, @newrev, @ref) { } end end @@ -30,8 +28,8 @@ describe GitHooksService do expect(service).not_to receive(:run_hook).with('post-receive') expect do - service.execute(user, project, @blankrev, @newrev, @ref) - end.to raise_error(GitHooksService::PreReceiveError) + service.execute(committer, repository, @blankrev, @newrev, @ref) + end.to raise_error(Gitlab::Git::HooksService::PreReceiveError) end end @@ -42,8 +40,8 @@ describe GitHooksService do expect(service).not_to receive(:run_hook).with('post-receive') expect do - service.execute(user, project, @blankrev, @newrev, @ref) - end.to raise_error(GitHooksService::PreReceiveError) + service.execute(committer, repository, @blankrev, @newrev, @ref) + end.to raise_error(Gitlab::Git::HooksService::PreReceiveError) end end end diff --git a/spec/lib/gitlab/git/index_spec.rb b/spec/lib/gitlab/git/index_spec.rb index 21b71654251..73fbc6a6afa 100644 --- a/spec/lib/gitlab/git/index_spec.rb +++ b/spec/lib/gitlab/git/index_spec.rb @@ -1,7 +1,7 @@ require 'spec_helper' describe Gitlab::Git::Index, seed_helper: true do - let(:repository) { Gitlab::Git::Repository.new('default', TEST_REPO_PATH) } + let(:repository) { Gitlab::Git::Repository.new('default', TEST_REPO_PATH, '') } let(:index) { described_class.new(repository) } before do diff --git a/spec/lib/gitlab/git/repository_spec.rb b/spec/lib/gitlab/git/repository_spec.rb index 8ec8dfe6acf..6b9773c9b63 100644 --- a/spec/lib/gitlab/git/repository_spec.rb +++ b/spec/lib/gitlab/git/repository_spec.rb @@ -17,7 +17,7 @@ describe Gitlab::Git::Repository, seed_helper: true do end end - let(:repository) { Gitlab::Git::Repository.new('default', TEST_REPO_PATH) } + let(:repository) { Gitlab::Git::Repository.new('default', TEST_REPO_PATH, '') } describe "Respond to" do subject { repository } @@ -56,14 +56,14 @@ describe Gitlab::Git::Repository, seed_helper: true do describe "#rugged" do describe 'when storage is broken', broken_storage: true do it 'raises a storage exception when storage is not available' do - broken_repo = described_class.new('broken', 'a/path.git') + broken_repo = described_class.new('broken', 'a/path.git', '') expect { broken_repo.rugged }.to raise_error(Gitlab::Git::Storage::Inaccessible) end end it 'raises a no repository exception when there is no repo' do - broken_repo = described_class.new('default', 'a/path.git') + broken_repo = described_class.new('default', 'a/path.git', '') expect { broken_repo.rugged }.to raise_error(Gitlab::Git::Repository::NoRepository) end @@ -257,7 +257,7 @@ describe Gitlab::Git::Repository, seed_helper: true do end describe '#submodule_url_for' do - let(:repository) { Gitlab::Git::Repository.new('default', TEST_REPO_PATH) } + let(:repository) { Gitlab::Git::Repository.new('default', TEST_REPO_PATH, '') } let(:ref) { 'master' } def submodule_url(path) @@ -295,7 +295,7 @@ describe Gitlab::Git::Repository, seed_helper: true do end context '#submodules' do - let(:repository) { Gitlab::Git::Repository.new('default', TEST_REPO_PATH) } + let(:repository) { Gitlab::Git::Repository.new('default', TEST_REPO_PATH, '') } context 'where repo has submodules' do let(:submodules) { repository.send(:submodules, 'master') } @@ -391,7 +391,7 @@ describe Gitlab::Git::Repository, seed_helper: true do describe "#delete_branch" do before(:all) do - @repo = Gitlab::Git::Repository.new('default', TEST_MUTABLE_REPO_PATH) + @repo = Gitlab::Git::Repository.new('default', TEST_MUTABLE_REPO_PATH, '') @repo.delete_branch("feature") end @@ -407,7 +407,7 @@ describe Gitlab::Git::Repository, seed_helper: true do describe "#create_branch" do before(:all) do - @repo = Gitlab::Git::Repository.new('default', TEST_MUTABLE_REPO_PATH) + @repo = Gitlab::Git::Repository.new('default', TEST_MUTABLE_REPO_PATH, '') end it "should create a new branch" do @@ -445,7 +445,7 @@ describe Gitlab::Git::Repository, seed_helper: true do describe "#remote_delete" do before(:all) do - @repo = Gitlab::Git::Repository.new('default', TEST_MUTABLE_REPO_PATH) + @repo = Gitlab::Git::Repository.new('default', TEST_MUTABLE_REPO_PATH, '') @repo.remote_delete("expendable") end @@ -461,7 +461,7 @@ describe Gitlab::Git::Repository, seed_helper: true do describe "#remote_add" do before(:all) do - @repo = Gitlab::Git::Repository.new('default', TEST_MUTABLE_REPO_PATH) + @repo = Gitlab::Git::Repository.new('default', TEST_MUTABLE_REPO_PATH, '') @repo.remote_add("new_remote", SeedHelper::GITLAB_GIT_TEST_REPO_URL) end @@ -477,7 +477,7 @@ describe Gitlab::Git::Repository, seed_helper: true do describe "#remote_update" do before(:all) do - @repo = Gitlab::Git::Repository.new('default', TEST_MUTABLE_REPO_PATH) + @repo = Gitlab::Git::Repository.new('default', TEST_MUTABLE_REPO_PATH, '') @repo.remote_update("expendable", url: TEST_NORMAL_REPO_PATH) end @@ -506,7 +506,7 @@ describe Gitlab::Git::Repository, seed_helper: true do before(:context) do # Add new commits so that there's a renamed file in the commit history - repo = Gitlab::Git::Repository.new('default', TEST_REPO_PATH).rugged + repo = Gitlab::Git::Repository.new('default', TEST_REPO_PATH, '').rugged @commit_with_old_name_id = new_commit_edit_old_file(repo) @rename_commit_id = new_commit_move_file(repo) @commit_with_new_name_id = new_commit_edit_new_file(repo) @@ -514,7 +514,7 @@ describe Gitlab::Git::Repository, seed_helper: true do after(:context) do # Erase our commits so other tests get the original repo - repo = Gitlab::Git::Repository.new('default', TEST_REPO_PATH).rugged + repo = Gitlab::Git::Repository.new('default', TEST_REPO_PATH, '').rugged repo.references.update("refs/heads/master", SeedRepo::LastCommit::ID) end @@ -849,7 +849,7 @@ describe Gitlab::Git::Repository, seed_helper: true do describe '#autocrlf' do before(:all) do - @repo = Gitlab::Git::Repository.new('default', TEST_MUTABLE_REPO_PATH) + @repo = Gitlab::Git::Repository.new('default', TEST_MUTABLE_REPO_PATH, '') @repo.rugged.config['core.autocrlf'] = true end @@ -864,7 +864,7 @@ describe Gitlab::Git::Repository, seed_helper: true do describe '#autocrlf=' do before(:all) do - @repo = Gitlab::Git::Repository.new('default', TEST_MUTABLE_REPO_PATH) + @repo = Gitlab::Git::Repository.new('default', TEST_MUTABLE_REPO_PATH, '') @repo.rugged.config['core.autocrlf'] = false end @@ -933,7 +933,7 @@ describe Gitlab::Git::Repository, seed_helper: true do context 'with local and remote branches' do let(:repository) do - Gitlab::Git::Repository.new('default', File.join(TEST_MUTABLE_REPO_PATH, '.git')) + Gitlab::Git::Repository.new('default', File.join(TEST_MUTABLE_REPO_PATH, '.git'), '') end before do @@ -977,6 +977,36 @@ describe Gitlab::Git::Repository, seed_helper: true do it 'returns the number of branches' do expect(repository.branch_count).to eq(10) end + + context 'with local and remote branches' do + let(:repository) do + Gitlab::Git::Repository.new('default', File.join(TEST_MUTABLE_REPO_PATH, '.git'), '') + end + + before do + create_remote_branch(repository, 'joe', 'remote_branch', 'master') + repository.create_branch('local_branch', 'master') + end + + after do + FileUtils.rm_rf(TEST_MUTABLE_REPO_PATH) + ensure_seeds + end + + it 'returns the count of local branches' do + expect(repository.branch_count).to eq(repository.local_branches.count) + end + + context 'with Gitaly disabled' do + before do + allow(Gitlab::GitalyClient).to receive(:feature_enabled?).and_return(false) + end + + it 'returns the count of local branches' do + expect(repository.branch_count).to eq(repository.local_branches.count) + end + end + end end describe "#ls_files" do @@ -1080,6 +1110,34 @@ describe Gitlab::Git::Repository, seed_helper: true do end end + describe '#ref_exists?' do + shared_examples 'checks the existence of refs' do + it 'returns true for an existing tag' do + expect(repository.ref_exists?('refs/heads/master')).to eq(true) + end + + it 'returns false for a non-existing tag' do + expect(repository.ref_exists?('refs/tags/THIS_TAG_DOES_NOT_EXIST')).to eq(false) + end + + it 'raises an ArgumentError for an empty string' do + expect { repository.ref_exists?('') }.to raise_error(ArgumentError) + end + + it 'raises an ArgumentError for an invalid ref' do + expect { repository.ref_exists?('INVALID') }.to raise_error(ArgumentError) + end + end + + context 'when Gitaly ref_exists feature is enabled' do + it_behaves_like 'checks the existence of refs' + end + + context 'when Gitaly ref_exists feature is disabled', skip_gitaly_mock: true do + it_behaves_like 'checks the existence of refs' + end + end + describe '#tag_exists?' do shared_examples 'checks the existence of tags' do it 'returns true for an existing tag' do @@ -1128,7 +1186,7 @@ describe Gitlab::Git::Repository, seed_helper: true do describe '#local_branches' do before(:all) do - @repo = Gitlab::Git::Repository.new('default', File.join(TEST_MUTABLE_REPO_PATH, '.git')) + @repo = Gitlab::Git::Repository.new('default', File.join(TEST_MUTABLE_REPO_PATH, '.git'), '') end after(:all) do diff --git a/spec/lib/gitlab/git/storage/forked_storage_check_spec.rb b/spec/lib/gitlab/git/storage/forked_storage_check_spec.rb index 12366151f44..c708b15853a 100644 --- a/spec/lib/gitlab/git/storage/forked_storage_check_spec.rb +++ b/spec/lib/gitlab/git/storage/forked_storage_check_spec.rb @@ -1,6 +1,6 @@ require 'spec_helper' -describe Gitlab::Git::Storage::ForkedStorageCheck, skip_database_cleaner: true do +describe Gitlab::Git::Storage::ForkedStorageCheck, broken_storage: true, skip_database_cleaner: true do let(:existing_path) do existing_path = TestEnv.repos_path FileUtils.mkdir_p(existing_path) diff --git a/spec/lib/gitlab/git/tag_spec.rb b/spec/lib/gitlab/git/tag_spec.rb index 78d1e120013..cc10679ef1e 100644 --- a/spec/lib/gitlab/git/tag_spec.rb +++ b/spec/lib/gitlab/git/tag_spec.rb @@ -1,7 +1,7 @@ require "spec_helper" describe Gitlab::Git::Tag, seed_helper: true do - let(:repository) { Gitlab::Git::Repository.new('default', TEST_REPO_PATH) } + let(:repository) { Gitlab::Git::Repository.new('default', TEST_REPO_PATH, '') } shared_examples 'Gitlab::Git::Repository#tags' do describe 'first tag' do diff --git a/spec/lib/gitlab/git/tree_spec.rb b/spec/lib/gitlab/git/tree_spec.rb index 98ddd3c3664..c07a2d91768 100644 --- a/spec/lib/gitlab/git/tree_spec.rb +++ b/spec/lib/gitlab/git/tree_spec.rb @@ -1,7 +1,7 @@ require "spec_helper" describe Gitlab::Git::Tree, seed_helper: true do - let(:repository) { Gitlab::Git::Repository.new('default', TEST_REPO_PATH) } + let(:repository) { Gitlab::Git::Repository.new('default', TEST_REPO_PATH, '') } context :repo do let(:tree) { Gitlab::Git::Tree.where(repository, SeedRepo::Commit::ID) } diff --git a/spec/lib/gitlab/git_access_spec.rb b/spec/lib/gitlab/git_access_spec.rb index 80dc49e99cb..295a979da76 100644 --- a/spec/lib/gitlab/git_access_spec.rb +++ b/spec/lib/gitlab/git_access_spec.rb @@ -384,7 +384,7 @@ describe Gitlab::GitAccess do def stub_git_hooks # Running the `pre-receive` hook is expensive, and not necessary for this test. - allow_any_instance_of(GitHooksService).to receive(:execute) do |service, &block| + allow_any_instance_of(Gitlab::Git::HooksService).to receive(:execute) do |service, &block| block.call(service) end end diff --git a/spec/lib/gitlab/gitaly_client/commit_service_spec.rb b/spec/lib/gitlab/gitaly_client/commit_service_spec.rb index 2eaf4222964..f32fe5d8150 100644 --- a/spec/lib/gitlab/gitaly_client/commit_service_spec.rb +++ b/spec/lib/gitlab/gitaly_client/commit_service_spec.rb @@ -140,4 +140,29 @@ describe Gitlab::GitalyClient::CommitService do described_class.new(repository).find_commit(revision) end end + + describe '#patch' do + let(:request) do + Gitaly::CommitPatchRequest.new( + repository: repository_message, revision: revision + ) + end + let(:response) { [double(data: "my "), double(data: "diff")] } + + subject { described_class.new(repository).patch(revision) } + + it 'sends an RPC request' do + expect_any_instance_of(Gitaly::DiffService::Stub).to receive(:commit_patch) + .with(request, kind_of(Hash)).and_return([]) + + subject + end + + it 'concatenates the responses data' do + allow_any_instance_of(Gitaly::DiffService::Stub).to receive(:commit_patch) + .with(request, kind_of(Hash)).and_return(response) + + expect(subject).to eq("my diff") + end + end end diff --git a/spec/lib/gitlab/import_export/project.json b/spec/lib/gitlab/import_export/project.json index 4e631e13410..331b7cf2fea 100644 --- a/spec/lib/gitlab/import_export/project.json +++ b/spec/lib/gitlab/import_export/project.json @@ -2522,7 +2522,7 @@ "id": 27, "target_branch": "feature", "source_branch": "feature_conflict", - "source_project_id": 5, + "source_project_id": 999, "author_id": 1, "assignee_id": null, "title": "MR1", @@ -2536,6 +2536,9 @@ "position": 0, "updated_by_id": null, "merge_error": null, + "diff_head_sha": "HEAD", + "source_branch_sha": "ABCD", + "target_branch_sha": "DCBA", "merge_params": { "force_remove_source_branch": null }, diff --git a/spec/lib/gitlab/import_export/project_tree_restorer_spec.rb b/spec/lib/gitlab/import_export/project_tree_restorer_spec.rb index 956f1d56eb4..5b16fc5d084 100644 --- a/spec/lib/gitlab/import_export/project_tree_restorer_spec.rb +++ b/spec/lib/gitlab/import_export/project_tree_restorer_spec.rb @@ -10,6 +10,13 @@ describe Gitlab::ImportExport::ProjectTreeRestorer do @shared = Gitlab::ImportExport::Shared.new(relative_path: "", project_path: 'path') allow(@shared).to receive(:export_path).and_return('spec/lib/gitlab/import_export/') @project = create(:project, :builds_disabled, :issues_disabled, name: 'project', path: 'project') + + allow(@project.repository).to receive(:fetch_ref).and_return(true) + allow(@project.repository.raw).to receive(:rugged_branch_exists?).and_return(false) + + expect_any_instance_of(Gitlab::Git::Repository).to receive(:create_branch).with('feature', 'DCBA') + allow_any_instance_of(Gitlab::Git::Repository).to receive(:create_branch) + project_tree_restorer = described_class.new(user: @user, shared: @shared, project: @project) @restored_project_json = project_tree_restorer.restore end @@ -79,6 +86,10 @@ describe Gitlab::ImportExport::ProjectTreeRestorer do expect(event).not_to be_nil end + it 'has the action' do + expect(event.action).not_to be_nil + end + it 'event belongs to note, belongs to merge request, belongs to a project' do expect(event.note.noteable.project).not_to be_nil end diff --git a/spec/lib/gitlab/import_export/project_tree_saver_spec.rb b/spec/lib/gitlab/import_export/project_tree_saver_spec.rb index a278f89c1a1..065b0ec6658 100644 --- a/spec/lib/gitlab/import_export/project_tree_saver_spec.rb +++ b/spec/lib/gitlab/import_export/project_tree_saver_spec.rb @@ -11,6 +11,8 @@ describe Gitlab::ImportExport::ProjectTreeSaver do before do project.team << [user, :master] allow_any_instance_of(Gitlab::ImportExport).to receive(:storage_path).and_return(export_path) + allow_any_instance_of(MergeRequest).to receive(:source_branch_sha).and_return('ABCD') + allow_any_instance_of(MergeRequest).to receive(:target_branch_sha).and_return('DCBA') end after do @@ -43,6 +45,14 @@ describe Gitlab::ImportExport::ProjectTreeSaver do expect(saved_project_json['merge_requests'].first['milestone']).not_to be_empty end + it 'has merge request\'s source branch SHA' do + expect(saved_project_json['merge_requests'].first['source_branch_sha']).to eq('ABCD') + end + + it 'has merge request\'s target branch SHA' do + expect(saved_project_json['merge_requests'].first['target_branch_sha']).to eq('DCBA') + end + it 'has events' do expect(saved_project_json['merge_requests'].first['milestone']['events']).not_to be_empty end diff --git a/spec/lib/gitlab/import_export/repo_restorer_spec.rb b/spec/lib/gitlab/import_export/repo_restorer_spec.rb index 2786bc92fe5..c49af602a01 100644 --- a/spec/lib/gitlab/import_export/repo_restorer_spec.rb +++ b/spec/lib/gitlab/import_export/repo_restorer_spec.rb @@ -34,7 +34,7 @@ describe Gitlab::ImportExport::RepoRestorer do it 'has the webhooks' do restorer.restore - expect(Gitlab::Git::Hook.new('post-receive', project)).to exist + expect(Gitlab::Git::Hook.new('post-receive', project.repository.raw_repository)).to exist end end end diff --git a/spec/lib/gitlab/job_waiter_spec.rb b/spec/lib/gitlab/job_waiter_spec.rb index 6186cec2689..b0b4fdc09bc 100644 --- a/spec/lib/gitlab/job_waiter_spec.rb +++ b/spec/lib/gitlab/job_waiter_spec.rb @@ -1,30 +1,39 @@ require 'spec_helper' describe Gitlab::JobWaiter do - describe '#wait' do - let(:waiter) { described_class.new(%w(a)) } - it 'returns when all jobs have been completed' do - expect(Gitlab::SidekiqStatus).to receive(:all_completed?).with(%w(a)) - .and_return(true) + describe '.notify' do + it 'pushes the jid to the named queue' do + key = 'gitlab:job_waiter:foo' + jid = 1 - expect(waiter).not_to receive(:sleep) + redis = double('redis') + expect(Gitlab::Redis::SharedState).to receive(:with).and_yield(redis) + expect(redis).to receive(:lpush).with(key, jid) - waiter.wait + described_class.notify(key, jid) end + end + + describe '#wait' do + let(:waiter) { described_class.new(2) } - it 'sleeps between checking the job statuses' do - expect(Gitlab::SidekiqStatus).to receive(:all_completed?) - .with(%w(a)) - .and_return(false, true) + it 'returns when all jobs have been completed' do + described_class.notify(waiter.key, 'a') + described_class.notify(waiter.key, 'b') - expect(waiter).to receive(:sleep).with(described_class::INTERVAL) + result = nil + expect { Timeout.timeout(1) { result = waiter.wait(2) } }.not_to raise_error - waiter.wait + expect(result).to contain_exactly('a', 'b') end - it 'returns when timing out' do - expect(waiter).not_to receive(:sleep) - waiter.wait(0) + it 'times out if not all jobs complete' do + described_class.notify(waiter.key, 'a') + + result = nil + expect { Timeout.timeout(2) { result = waiter.wait(1) } }.not_to raise_error + + expect(result).to contain_exactly('a') end end end diff --git a/spec/lib/gitlab/ldap/adapter_spec.rb b/spec/lib/gitlab/ldap/adapter_spec.rb index d17d440d833..d9ddb4326be 100644 --- a/spec/lib/gitlab/ldap/adapter_spec.rb +++ b/spec/lib/gitlab/ldap/adapter_spec.rb @@ -16,7 +16,7 @@ describe Gitlab::LDAP::Adapter do expect(adapter).to receive(:ldap_search) do |arg| expect(arg[:filter].to_s).to eq('(uid=johndoe)') expect(arg[:base]).to eq('dc=example,dc=com') - expect(arg[:attributes]).to match(%w{uid cn dn uid userid sAMAccountName mail email userPrincipalName}) + expect(arg[:attributes]).to match(%w{dn uid cn mail email userPrincipalName}) end.and_return({}) adapter.users('uid', 'johndoe') @@ -26,7 +26,7 @@ describe Gitlab::LDAP::Adapter do expect(adapter).to receive(:ldap_search).with( base: 'uid=johndoe,ou=users,dc=example,dc=com', scope: Net::LDAP::SearchScope_BaseObject, - attributes: %w{uid cn dn uid userid sAMAccountName mail email userPrincipalName}, + attributes: %w{dn uid cn mail email userPrincipalName}, filter: nil ).and_return({}) @@ -63,7 +63,7 @@ describe Gitlab::LDAP::Adapter do it 'uses the right uid attribute when non-default' do stub_ldap_config(uid: 'sAMAccountName') expect(adapter).to receive(:ldap_search).with( - hash_including(attributes: %w{sAMAccountName cn dn uid userid sAMAccountName mail email userPrincipalName}) + hash_including(attributes: %w{dn sAMAccountName cn mail email userPrincipalName}) ).and_return({}) adapter.users('sAMAccountName', 'johndoe') diff --git a/spec/migrations/cleanup_namespaceless_pending_delete_projects_spec.rb b/spec/migrations/cleanup_namespaceless_pending_delete_projects_spec.rb index 12cac1d033d..b47f3314926 100644 --- a/spec/migrations/cleanup_namespaceless_pending_delete_projects_spec.rb +++ b/spec/migrations/cleanup_namespaceless_pending_delete_projects_spec.rb @@ -4,7 +4,7 @@ require Rails.root.join('db', 'post_migrate', '20170502101023_cleanup_namespacel describe CleanupNamespacelessPendingDeleteProjects do before do # Stub after_save callbacks that will fail when Project has no namespace - allow_any_instance_of(Project).to receive(:ensure_storage_path_exist).and_return(nil) + allow_any_instance_of(Project).to receive(:ensure_storage_path_exists).and_return(nil) allow_any_instance_of(Project).to receive(:update_project_statistics).and_return(nil) end diff --git a/spec/migrations/cleanup_nonexisting_namespace_pending_delete_projects_spec.rb b/spec/migrations/cleanup_nonexisting_namespace_pending_delete_projects_spec.rb new file mode 100644 index 00000000000..7879105a334 --- /dev/null +++ b/spec/migrations/cleanup_nonexisting_namespace_pending_delete_projects_spec.rb @@ -0,0 +1,32 @@ +require 'spec_helper' +require Rails.root.join('db', 'post_migrate', '20170816102555_cleanup_nonexisting_namespace_pending_delete_projects.rb') + +describe CleanupNonexistingNamespacePendingDeleteProjects do + before do + # Stub after_save callbacks that will fail when Project has invalid namespace + allow_any_instance_of(Project).to receive(:ensure_storage_path_exist).and_return(nil) + allow_any_instance_of(Project).to receive(:update_project_statistics).and_return(nil) + end + + describe '#up' do + set(:some_project) { create(:project) } + + it 'only cleans up when namespace does not exist' do + create(:project, pending_delete: true) + project = build(:project, pending_delete: true, namespace: nil, namespace_id: Namespace.maximum(:id).to_i.succ) + project.save(validate: false) + + expect(NamespacelessProjectDestroyWorker).to receive(:bulk_perform_async).with([[project.id]]) + + described_class.new.up + end + + it 'does nothing when no pending delete projects without namespace found' do + create(:project, pending_delete: true, namespace: create(:namespace)) + + expect(NamespacelessProjectDestroyWorker).not_to receive(:bulk_perform_async) + + described_class.new.up + end + end +end diff --git a/spec/migrations/migrate_pipeline_sidekiq_queues_spec.rb b/spec/migrations/migrate_pipeline_sidekiq_queues_spec.rb new file mode 100644 index 00000000000..e02bcd2f4da --- /dev/null +++ b/spec/migrations/migrate_pipeline_sidekiq_queues_spec.rb @@ -0,0 +1,55 @@ +require 'spec_helper' +require Rails.root.join('db', 'post_migrate', '20170822101017_migrate_pipeline_sidekiq_queues.rb') + +describe MigratePipelineSidekiqQueues, :sidekiq, :redis do + include Gitlab::Database::MigrationHelpers + + context 'when there are jobs in the queues' do + it 'correctly migrates queue when migrating up' do + Sidekiq::Testing.disable! do + stubbed_worker(queue: :pipeline).perform_async('Something', [1]) + stubbed_worker(queue: :build).perform_async('Something', [1]) + + described_class.new.up + + expect(sidekiq_queue_length('pipeline')).to eq 0 + expect(sidekiq_queue_length('build')).to eq 0 + expect(sidekiq_queue_length('pipeline_default')).to eq 2 + end + end + + it 'correctly migrates queue when migrating down' do + Sidekiq::Testing.disable! do + stubbed_worker(queue: :pipeline_default).perform_async('Class', [1]) + stubbed_worker(queue: :pipeline_processing).perform_async('Class', [2]) + stubbed_worker(queue: :pipeline_hooks).perform_async('Class', [3]) + stubbed_worker(queue: :pipeline_cache).perform_async('Class', [4]) + + described_class.new.down + + expect(sidekiq_queue_length('pipeline')).to eq 4 + expect(sidekiq_queue_length('pipeline_default')).to eq 0 + expect(sidekiq_queue_length('pipeline_processing')).to eq 0 + expect(sidekiq_queue_length('pipeline_hooks')).to eq 0 + expect(sidekiq_queue_length('pipeline_cache')).to eq 0 + end + end + end + + context 'when there are no jobs in the queues' do + it 'does not raise error when migrating up' do + expect { described_class.new.up }.not_to raise_error + end + + it 'does not raise error when migrating down' do + expect { described_class.new.down }.not_to raise_error + end + end + + def stubbed_worker(queue:) + Class.new do + include Sidekiq::Worker + sidekiq_options queue: queue + end + end +end diff --git a/spec/migrations/migrate_stages_statuses_spec.rb b/spec/migrations/migrate_stages_statuses_spec.rb index 4102d57e368..094c9bc604e 100644 --- a/spec/migrations/migrate_stages_statuses_spec.rb +++ b/spec/migrations/migrate_stages_statuses_spec.rb @@ -12,7 +12,7 @@ describe MigrateStagesStatuses, :migration do before do stub_const("#{described_class.name}::BATCH_SIZE", 2) - stub_const("#{described_class.name}::RANGE_SIZE", 2) + stub_const("#{described_class.name}::RANGE_SIZE", 1) projects.create!(id: 1, name: 'gitlab1', path: 'gitlab1') projects.create!(id: 2, name: 'gitlab2', path: 'gitlab2') @@ -50,9 +50,10 @@ describe MigrateStagesStatuses, :migration do Timecop.freeze do migrate! - expect(described_class::MIGRATION).to be_scheduled_migration(5.minutes, 1, 2) + expect(described_class::MIGRATION).to be_scheduled_migration(5.minutes, 1, 1) + expect(described_class::MIGRATION).to be_scheduled_migration(5.minutes, 2, 2) expect(described_class::MIGRATION).to be_scheduled_migration(10.minutes, 3, 3) - expect(BackgroundMigrationWorker.jobs.size).to eq 2 + expect(BackgroundMigrationWorker.jobs.size).to eq 3 end end end diff --git a/spec/models/broadcast_message_spec.rb b/spec/models/broadcast_message_spec.rb index 3369aef1d3e..461e754dc1f 100644 --- a/spec/models/broadcast_message_spec.rb +++ b/spec/models/broadcast_message_spec.rb @@ -53,6 +53,29 @@ describe BroadcastMessage do 2.times { described_class.current } end + + it 'includes messages that need to be displayed in the future' do + create(:broadcast_message) + + future = create( + :broadcast_message, + starts_at: Time.now + 10.minutes, + ends_at: Time.now + 20.minutes + ) + + expect(described_class.current.length).to eq(1) + + Timecop.travel(future.starts_at) do + expect(described_class.current.length).to eq(2) + end + end + + it 'does not clear the cache if only a future message should be displayed' do + create(:broadcast_message, :future) + + expect(Rails.cache).not_to receive(:delete) + expect(described_class.current.length).to eq(0) + end end describe '#active?' do diff --git a/spec/models/ci/build_spec.rb b/spec/models/ci/build_spec.rb index 767f0ad9e65..4f77f0d85cd 100644 --- a/spec/models/ci/build_spec.rb +++ b/spec/models/ci/build_spec.rb @@ -21,6 +21,16 @@ describe Ci::Build do it { is_expected.to respond_to(:has_trace?) } it { is_expected.to respond_to(:trace) } + describe 'callbacks' do + context 'when running after_create callback' do + it 'triggers asynchronous build hooks worker' do + expect(BuildHooksWorker).to receive(:perform_async) + + create(:ci_build) + end + end + end + describe '.manual_actions' do let!(:manual_but_created) { create(:ci_build, :manual, status: :created, pipeline: pipeline) } let!(:manual_but_succeeded) { create(:ci_build, :manual, status: :success, pipeline: pipeline) } diff --git a/spec/models/ci/pipeline_spec.rb b/spec/models/ci/pipeline_spec.rb index ac75c6501ee..b84e3ff18e8 100644 --- a/spec/models/ci/pipeline_spec.rb +++ b/spec/models/ci/pipeline_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' describe Ci::Pipeline, :mailer do let(:user) { create(:user) } - let(:project) { create(:project) } + set(:project) { create(:project) } let(:pipeline) do create(:ci_empty_pipeline, status: :created, project: project) @@ -159,6 +159,18 @@ describe Ci::Pipeline, :mailer do end end + describe '#predefined_variables' do + subject { pipeline.predefined_variables } + + it { is_expected.to be_an(Array) } + + it 'includes the defined keys' do + keys = subject.map { |v| v[:key] } + + expect(keys).to include('CI_PIPELINE_ID', 'CI_CONFIG_PATH', 'CI_PIPELINE_SOURCE') + end + end + describe '#auto_canceled?' do subject { pipeline.auto_canceled? } diff --git a/spec/models/ci/stage_spec.rb b/spec/models/ci/stage_spec.rb index 74c9d6145e2..586d073eb5e 100644 --- a/spec/models/ci/stage_spec.rb +++ b/spec/models/ci/stage_spec.rb @@ -38,6 +38,17 @@ describe Ci::Stage, :models do expect(stage.status).to eq 'success' end end + + context 'when stage status is not defined' do + before do + stage.update_column(:status, nil) + end + + it 'sets the default value' do + expect(described_class.find(stage.id).status) + .to eq 'created' + end + end end describe 'update_status' do diff --git a/spec/models/event_spec.rb b/spec/models/event_spec.rb index ff3224dd298..f55c161c821 100644 --- a/spec/models/event_spec.rb +++ b/spec/models/event_spec.rb @@ -304,6 +304,50 @@ describe Event do end end + describe '#body?' do + let(:push_event) do + event = build(:push_event) + + allow(event).to receive(:push?).and_return(true) + + event + end + + it 'returns true for a push event with commits' do + allow(push_event).to receive(:push_with_commits?).and_return(true) + + expect(push_event).to be_body + end + + it 'returns false for a push event without a valid commit range' do + allow(push_event).to receive(:push_with_commits?).and_return(false) + + expect(push_event).not_to be_body + end + + it 'returns true for a Note event' do + event = build(:event) + + allow(event).to receive(:note?).and_return(true) + + expect(event).to be_body + end + + it 'returns true if the target responds to #title' do + event = build(:event) + + allow(event).to receive(:target).and_return(double(:target, title: 'foo')) + + expect(event).to be_body + end + + it 'returns false for a regular event without a target' do + event = build(:event) + + expect(event).not_to be_body + end + end + def create_push_event(project, user) event = create(:push_event, project: project, author: user) diff --git a/spec/models/issue_spec.rb b/spec/models/issue_spec.rb index 9203f6562f2..de86788d142 100644 --- a/spec/models/issue_spec.rb +++ b/spec/models/issue_spec.rb @@ -751,4 +751,22 @@ describe Issue do end end end + + describe 'removing an issue' do + it 'refreshes the number of open issues of the project' do + project = subject.project + + expect { subject.destroy } + .to change { project.open_issues_count }.from(1).to(0) + end + end + + describe '.public_only' do + it 'only returns public issues' do + public_issue = create(:issue) + create(:issue, confidential: true) + + expect(described_class.public_only).to eq([public_issue]) + end + end end diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb index 026bdbd26d1..92cf15a5a51 100644 --- a/spec/models/merge_request_spec.rb +++ b/spec/models/merge_request_spec.rb @@ -604,7 +604,7 @@ describe MergeRequest do request = build_stubbed(:merge_request) expect(request.merge_commit_message) - .to match("See merge request #{request.to_reference}") + .to match("See merge request #{request.to_reference(full: true)}") end it 'excludes multiple linebreak runs when description is blank' do @@ -931,6 +931,23 @@ describe MergeRequest do end end + describe '#merge_async' do + it 'enqueues MergeWorker job and updates merge_jid' do + merge_request = create(:merge_request) + user_id = double(:user_id) + params = double(:params) + merge_jid = 'hash-123' + + expect(MergeWorker).to receive(:perform_async).with(merge_request.id, user_id, params) do + merge_jid + end + + merge_request.merge_async(user_id, params) + + expect(merge_request.reload.merge_jid).to eq(merge_jid) + end + end + describe '#check_if_can_be_merged' do let(:project) { create(:project, only_allow_merge_if_pipeline_succeeds: true) } @@ -1370,29 +1387,11 @@ describe MergeRequest do end describe '#merge_ongoing?' do - it 'returns true when merge process is ongoing for merge_jid' do - merge_request = create(:merge_request, merge_jid: 'foo') - - allow(Gitlab::SidekiqStatus).to receive(:num_running).with(['foo']).and_return(1) + it 'returns true when merge_id is present and MR is not merged' do + merge_request = build_stubbed(:merge_request, state: :open, merge_jid: 'foo') expect(merge_request.merge_ongoing?).to be(true) end - - it 'returns false when no merge process running for merge_jid' do - merge_request = build(:merge_request, merge_jid: 'foo') - - allow(Gitlab::SidekiqStatus).to receive(:num_running).with(['foo']).and_return(0) - - expect(merge_request.merge_ongoing?).to be(false) - end - - it 'returns false when merge_jid is nil' do - merge_request = build(:merge_request, merge_jid: nil) - - expect(Gitlab::SidekiqStatus).not_to receive(:num_running) - - expect(merge_request.merge_ongoing?).to be(false) - end end describe "#closed_without_fork?" do @@ -1692,4 +1691,13 @@ describe MergeRequest do expect(subject.ref_fetched?).to be_falsey end end + + describe 'removing a merge request' do + it 'refreshes the number of open merge requests of the target project' do + project = subject.target_project + + expect { subject.destroy } + .to change { project.open_merge_requests_count }.from(1).to(0) + end + end end diff --git a/spec/models/project_services/kubernetes_service_spec.rb b/spec/models/project_services/kubernetes_service_spec.rb index 55b96a0c12e..b1743cd608e 100644 --- a/spec/models/project_services/kubernetes_service_spec.rb +++ b/spec/models/project_services/kubernetes_service_spec.rb @@ -38,7 +38,8 @@ describe KubernetesService, :use_clean_rails_memory_store_caching do 'a' * 63 => true, 'a' * 64 => false, 'a.b' => false, - 'a*b' => false + 'a*b' => false, + 'FOO' => true }.each do |namespace, validity| it "validates #{namespace} as #{validity ? 'valid' : 'invalid'}" do subject.namespace = namespace diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index 5e60511f3a8..2e613c44357 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -1251,60 +1251,6 @@ describe Project do end end - describe '#rename_repo' do - let(:project) { create(:project, :repository) } - let(:gitlab_shell) { Gitlab::Shell.new } - - before do - # Project#gitlab_shell returns a new instance of Gitlab::Shell on every - # call. This makes testing a bit easier. - allow(project).to receive(:gitlab_shell).and_return(gitlab_shell) - allow(project).to receive(:previous_changes).and_return('path' => ['foo']) - end - - it 'renames a repository' do - stub_container_registry_config(enabled: false) - - expect(gitlab_shell).to receive(:mv_repository) - .ordered - .with(project.repository_storage_path, "#{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") - .and_return(true) - - expect_any_instance_of(SystemHooksService) - .to receive(:execute_hooks_for) - .with(project, :rename) - - expect_any_instance_of(Gitlab::UploadsTransfer) - .to receive(:rename_project) - .with('foo', project.path, project.namespace.full_path) - - expect(project).to receive(:expire_caches_before_rename) - - expect(project).to receive(:expires_full_path_cache) - - project.rename_repo - end - - context 'container registry with images' do - let(:container_repository) { create(:container_repository) } - - before do - stub_container_registry_config(enabled: true) - stub_container_registry_tags(repository: :any, tags: ['tag']) - project.container_repositories << container_repository - end - - subject { project.rename_repo } - - it { expect {subject}.to raise_error(StandardError) } - end - end - describe '#expire_caches_before_rename' do let(:project) { create(:project, :repository) } let(:repo) { double(:repo, exists?: true) } @@ -2367,4 +2313,181 @@ describe Project do expect(project.forks_count).to eq(1) end end + + context 'legacy storage' do + let(:project) { create(:project, :repository) } + let(:gitlab_shell) { Gitlab::Shell.new } + + before do + allow(project).to receive(:gitlab_shell).and_return(gitlab_shell) + end + + describe '#base_dir' do + it 'returns base_dir based on namespace only' do + expect(project.base_dir).to eq(project.namespace.full_path) + end + end + + describe '#disk_path' do + it 'returns disk_path based on namespace and project path' do + expect(project.disk_path).to eq("#{project.namespace.full_path}/#{project.path}") + end + end + + 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) + + project.ensure_storage_path_exists + end + end + + describe '#legacy_storage?' do + it 'returns true when storage_version is nil' do + project = build(:project) + + expect(project.legacy_storage?).to be_truthy + end + end + + describe '#rename_repo' do + before do + # Project#gitlab_shell returns a new instance of Gitlab::Shell on every + # call. This makes testing a bit easier. + allow(project).to receive(:gitlab_shell).and_return(gitlab_shell) + allow(project).to receive(:previous_changes).and_return('path' => ['foo']) + end + + it 'renames a repository' do + stub_container_registry_config(enabled: false) + + expect(gitlab_shell).to receive(:mv_repository) + .ordered + .with(project.repository_storage_path, "#{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") + .and_return(true) + + expect_any_instance_of(SystemHooksService) + .to receive(:execute_hooks_for) + .with(project, :rename) + + expect_any_instance_of(Gitlab::UploadsTransfer) + .to receive(:rename_project) + .with('foo', project.path, project.namespace.full_path) + + expect(project).to receive(:expire_caches_before_rename) + + expect(project).to receive(:expires_full_path_cache) + + project.rename_repo + end + + context 'container registry with images' do + let(:container_repository) { create(:container_repository) } + + before do + stub_container_registry_config(enabled: true) + stub_container_registry_tags(repository: :any, tags: ['tag']) + project.container_repositories << container_repository + end + + subject { project.rename_repo } + + it { expect { subject }.to raise_error(StandardError) } + end + end + + describe '#pages_path' do + it 'returns a path where pages are stored' do + expect(project.pages_path).to eq(File.join(Settings.pages.path, project.namespace.full_path, project.path)) + end + end + end + + context 'hashed storage' do + let(:project) { create(:project, :repository) } + let(:gitlab_shell) { Gitlab::Shell.new } + let(:hash) { '6b86b273ff34fce19d6b804eff5a3f5747ada4eaa22f1d49c01e52ddb7875b4b' } + + before do + stub_application_setting(hashed_storage_enabled: true) + allow(Digest::SHA2).to receive(:hexdigest) { hash } + allow(project).to receive(:gitlab_shell).and_return(gitlab_shell) + end + + describe '#base_dir' do + it 'returns base_dir based on hash of project id' do + expect(project.base_dir).to eq('@hashed/6b/86') + end + end + + describe '#disk_path' do + it 'returns disk_path based on hash of project id' do + hashed_path = '@hashed/6b/86/6b86b273ff34fce19d6b804eff5a3f5747ada4eaa22f1d49c01e52ddb7875b4b' + + expect(project.disk_path).to eq(hashed_path) + end + end + + 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, '@hashed/6b/86') + + project.ensure_storage_path_exists + end + end + + describe '#rename_repo' do + before do + # Project#gitlab_shell returns a new instance of Gitlab::Shell on every + # call. This makes testing a bit easier. + allow(project).to receive(:gitlab_shell).and_return(gitlab_shell) + allow(project).to receive(:previous_changes).and_return('path' => ['foo']) + end + + it 'renames a repository' do + stub_container_registry_config(enabled: false) + + expect(gitlab_shell).not_to receive(:mv_repository) + + expect_any_instance_of(SystemHooksService) + .to receive(:execute_hooks_for) + .with(project, :rename) + + expect_any_instance_of(Gitlab::UploadsTransfer) + .to receive(:rename_project) + .with('foo', project.path, project.namespace.full_path) + + expect(project).to receive(:expire_caches_before_rename) + + expect(project).to receive(:expires_full_path_cache) + + project.rename_repo + end + + context 'container registry with images' do + let(:container_repository) { create(:container_repository) } + + before do + stub_container_registry_config(enabled: true) + stub_container_registry_tags(repository: :any, tags: ['tag']) + project.container_repositories << container_repository + end + + subject { project.rename_repo } + + it { expect { subject }.to raise_error(StandardError) } + end + end + + describe '#pages_path' do + it 'returns a path where pages are stored' do + expect(project.pages_path).to eq(File.join(Settings.pages.path, project.namespace.full_path, project.path)) + end + end + end end diff --git a/spec/models/repository_spec.rb b/spec/models/repository_spec.rb index 4926d5d6c49..462e92b8b62 100644 --- a/spec/models/repository_spec.rb +++ b/spec/models/repository_spec.rb @@ -8,6 +8,7 @@ describe Repository, models: true do let(:repository) { project.repository } let(:broken_repository) { create(:project, :broken_storage).repository } let(:user) { create(:user) } + let(:committer) { Gitlab::Git::Committer.from_user(user) } let(:commit_options) do author = repository.user_to_committer(user) @@ -846,7 +847,7 @@ describe Repository, models: true do expect do repository.add_branch(user, 'new_feature', 'master') - end.to raise_error(GitHooksService::PreReceiveError) + end.to raise_error(Gitlab::Git::HooksService::PreReceiveError) end it 'does not create the branch' do @@ -854,7 +855,7 @@ describe Repository, models: true do expect do repository.add_branch(user, 'new_feature', 'master') - end.to raise_error(GitHooksService::PreReceiveError) + end.to raise_error(Gitlab::Git::HooksService::PreReceiveError) expect(repository.find_branch('new_feature')).to be_nil end end @@ -884,8 +885,8 @@ describe Repository, models: true do context 'when pre hooks were successful' do it 'runs without errors' do - expect_any_instance_of(GitHooksService).to receive(:execute) - .with(user, project, old_rev, blank_sha, 'refs/heads/feature') + expect_any_instance_of(Gitlab::Git::HooksService).to receive(:execute) + .with(committer, repository, old_rev, blank_sha, 'refs/heads/feature') expect { repository.rm_branch(user, 'feature') }.not_to raise_error end @@ -905,7 +906,7 @@ describe Repository, models: true do expect do repository.rm_branch(user, 'feature') - end.to raise_error(GitHooksService::PreReceiveError) + end.to raise_error(Gitlab::Git::HooksService::PreReceiveError) end it 'does not delete the branch' do @@ -913,7 +914,7 @@ describe Repository, models: true do expect do repository.rm_branch(user, 'feature') - end.to raise_error(GitHooksService::PreReceiveError) + end.to raise_error(Gitlab::Git::HooksService::PreReceiveError) expect(repository.find_branch('feature')).not_to be_nil end end @@ -925,23 +926,23 @@ describe Repository, models: true do context 'when pre hooks were successful' do before do - service = GitHooksService.new - expect(GitHooksService).to receive(:new).and_return(service) + service = Gitlab::Git::HooksService.new + expect(Gitlab::Git::HooksService).to receive(:new).and_return(service) expect(service).to receive(:execute) - .with(user, project, old_rev, new_rev, 'refs/heads/feature') + .with(committer, repository, old_rev, new_rev, 'refs/heads/feature') .and_yield(service).and_return(true) end it 'runs without errors' do expect do - GitOperationService.new(user, repository).with_branch('feature') do + GitOperationService.new(committer, repository).with_branch('feature') do new_rev end end.not_to raise_error end it 'ensures the autocrlf Git option is set to :input' do - service = GitOperationService.new(user, repository) + service = GitOperationService.new(committer, repository) expect(service).to receive(:update_autocrlf_option) @@ -952,7 +953,7 @@ describe Repository, models: true do it 'updates the head' do expect(repository.find_branch('feature').dereferenced_target.id).to eq(old_rev) - GitOperationService.new(user, repository).with_branch('feature') do + GitOperationService.new(committer, repository).with_branch('feature') do new_rev end @@ -974,7 +975,7 @@ describe Repository, models: true do end expect do - GitOperationService.new(user, target_project.repository) + GitOperationService.new(committer, target_project.repository) .with_branch('feature', start_project: project, &:itself) @@ -996,7 +997,7 @@ describe Repository, models: true do repository.add_branch(user, branch, old_rev) expect do - GitOperationService.new(user, repository).with_branch(branch) do + GitOperationService.new(committer, repository).with_branch(branch) do new_rev end end.not_to raise_error @@ -1014,7 +1015,7 @@ describe Repository, models: true do # Updating 'master' to new_rev would lose the commits on 'master' that # are not contained in new_rev. This should not be allowed. expect do - GitOperationService.new(user, repository).with_branch(branch) do + GitOperationService.new(committer, repository).with_branch(branch) do new_rev end end.to raise_error(Repository::CommitError) @@ -1026,10 +1027,10 @@ describe Repository, models: true do allow_any_instance_of(Gitlab::Git::Hook).to receive(:trigger).and_return([false, '']) expect do - GitOperationService.new(user, repository).with_branch('feature') do + GitOperationService.new(committer, repository).with_branch('feature') do new_rev end - end.to raise_error(GitHooksService::PreReceiveError) + end.to raise_error(Gitlab::Git::HooksService::PreReceiveError) end end @@ -1044,7 +1045,7 @@ describe Repository, models: true do expect(repository).not_to receive(:expire_emptiness_caches) expect(repository).to receive(:expire_branches_cache) - GitOperationService.new(user, repository) + GitOperationService.new(committer, repository) .with_branch('new-feature') do new_rev end diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 9a9e255f874..8e04eea56a7 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -1356,7 +1356,7 @@ describe User do end it "excludes push event if branch has been deleted" do - allow_any_instance_of(Repository).to receive(:branch_names).and_return(['foo']) + allow_any_instance_of(Repository).to receive(:branch_exists?).with('master').and_return(false) expect(subject.recent_push).to eq(nil) end diff --git a/spec/policies/group_policy_spec.rb b/spec/policies/group_policy_spec.rb index b17a93e3fbe..7f832bfa563 100644 --- a/spec/policies/group_policy_spec.rb +++ b/spec/policies/group_policy_spec.rb @@ -105,6 +105,8 @@ describe GroupPolicy do let(:current_user) { owner } it do + allow(Group).to receive(:supports_nested_groups?).and_return(true) + expect_allowed(:read_group) expect_allowed(*reporter_permissions) expect_allowed(*master_permissions) @@ -116,6 +118,8 @@ describe GroupPolicy do let(:current_user) { admin } it do + allow(Group).to receive(:supports_nested_groups?).and_return(true) + expect_allowed(:read_group) expect_allowed(*reporter_permissions) expect_allowed(*master_permissions) @@ -123,6 +127,36 @@ describe GroupPolicy do end end + describe 'when nested group support feature is disabled' do + before do + allow(Group).to receive(:supports_nested_groups?).and_return(false) + end + + context 'admin' do + let(:current_user) { admin } + + it 'allows every owner permission except creating subgroups' do + create_subgroup_permission = [:create_subgroup] + updated_owner_permissions = owner_permissions - create_subgroup_permission + + expect_disallowed(*create_subgroup_permission) + expect_allowed(*updated_owner_permissions) + end + end + + context 'owner' do + let(:current_user) { owner } + + it 'allows every owner permission except creating subgroups' do + create_subgroup_permission = [:create_subgroup] + updated_owner_permissions = owner_permissions - create_subgroup_permission + + expect_disallowed(*create_subgroup_permission) + expect_allowed(*updated_owner_permissions) + end + end + end + describe 'private nested group use the highest access level from the group and inherited permissions', :nested_groups do let(:nested_group) { create(:group, :private, parent: group) } @@ -199,6 +233,8 @@ describe GroupPolicy do let(:current_user) { owner } it do + allow(Group).to receive(:supports_nested_groups?).and_return(true) + expect_allowed(:read_group) expect_allowed(*reporter_permissions) expect_allowed(*master_permissions) diff --git a/spec/requests/api/groups_spec.rb b/spec/requests/api/groups_spec.rb index 313c38cd29c..a7557c7fb22 100644 --- a/spec/requests/api/groups_spec.rb +++ b/spec/requests/api/groups_spec.rb @@ -20,10 +20,15 @@ describe API::Groups do describe "GET /groups" do context "when unauthenticated" do - it "returns authentication error" do + it "returns public groups" do get api("/groups") - expect(response).to have_http_status(401) + expect(response).to have_http_status(200) + expect(response).to include_pagination_headers + expect(json_response).to be_an Array + expect(json_response.length).to eq(1) + expect(json_response) + .to satisfy_one { |group| group['name'] == group1.name } end end @@ -165,6 +170,18 @@ describe API::Groups do end describe "GET /groups/:id" do + context 'when unauthenticated' do + it 'returns 404 for a private group' do + get api("/groups/#{group2.id}") + expect(response).to have_http_status(404) + end + + it 'returns 200 for a public group' do + get api("/groups/#{group1.id}") + expect(response).to have_http_status(200) + end + end + context "when authenticated as user" do it "returns one of user1's groups" do project = create(:project, namespace: group2, path: 'Foo') diff --git a/spec/requests/api/triggers_spec.rb b/spec/requests/api/triggers_spec.rb index 572e9a0fd07..402d1040436 100644 --- a/spec/requests/api/triggers_spec.rb +++ b/spec/requests/api/triggers_spec.rb @@ -8,8 +8,8 @@ describe API::Triggers do let!(:project) { create(:project, :repository, creator: user) } let!(:master) { create(:project_member, :master, user: user, project: project) } let!(:developer) { create(:project_member, :developer, user: user2, project: project) } - let!(:trigger) { create(:ci_trigger, project: project, token: trigger_token) } - let!(:trigger2) { create(:ci_trigger, project: project, token: trigger_token_2) } + let!(:trigger) { create(:ci_trigger, project: project, token: trigger_token, owner: user) } + let!(:trigger2) { create(:ci_trigger, project: project, token: trigger_token_2, owner: user2) } let!(:trigger_request) { create(:ci_trigger_request, trigger: trigger, created_at: '2015-01-01 12:13:14') } describe 'POST /projects/:project_id/trigger/pipeline' do @@ -22,7 +22,6 @@ describe API::Triggers do before do stub_ci_pipeline_to_return_yaml_file - trigger.update(owner: user) end context 'Handles errors' do @@ -85,6 +84,22 @@ describe API::Triggers do expect(pipeline.variables.map { |v| { v.key => v.value } }.last).to eq(variables) end end + + context 'when legacy trigger' do + before do + trigger.update(owner: nil) + end + + it 'creates pipeline' do + post api("/projects/#{project.id}/trigger/pipeline"), options.merge(ref: 'master') + + expect(response).to have_http_status(201) + expect(json_response).to include('id' => pipeline.id) + pipeline.builds.reload + expect(pipeline.builds.pending.size).to eq(2) + expect(pipeline.builds.size).to eq(5) + end + end end context 'when triggering a pipeline from a trigger token' do @@ -254,8 +269,6 @@ describe API::Triggers do describe 'POST /projects/:id/triggers/:trigger_id/take_ownership' do context 'authenticated user with valid permissions' do it 'updates owner' do - expect(trigger.owner).to be_nil - post api("/projects/#{project.id}/triggers/#{trigger.id}/take_ownership", user) expect(response).to have_http_status(200) diff --git a/spec/requests/api/v3/triggers_spec.rb b/spec/requests/api/v3/triggers_spec.rb index 075de2c0cba..d4648136841 100644 --- a/spec/requests/api/v3/triggers_spec.rb +++ b/spec/requests/api/v3/triggers_spec.rb @@ -7,7 +7,10 @@ describe API::V3::Triggers do let!(:project) { create(:project, :repository, creator: user) } let!(:master) { create(:project_member, :master, user: user, project: project) } let!(:developer) { create(:project_member, :developer, user: user2, project: project) } - let!(:trigger) { create(:ci_trigger, project: project, token: trigger_token) } + + let!(:trigger) do + create(:ci_trigger, project: project, token: trigger_token, owner: user) + end describe 'POST /projects/:project_id/trigger' do let!(:project2) { create(:project) } diff --git a/spec/services/ci/create_pipeline_service_spec.rb b/spec/services/ci/create_pipeline_service_spec.rb index 8465a6f99bd..4ba3dada37c 100644 --- a/spec/services/ci/create_pipeline_service_spec.rb +++ b/spec/services/ci/create_pipeline_service_spec.rb @@ -413,14 +413,12 @@ describe Ci::CreatePipelineService do end context 'when trigger belongs to a developer' do - let(:user) {} + let(:user) { create(:user) } + let(:trigger) { create(:ci_trigger, owner: user) } + let(:trigger_request) { create(:ci_trigger_request, trigger: trigger) } - let(:trigger_request) do - create(:ci_trigger_request).tap do |request| - user = create(:user) - project.add_developer(user) - request.trigger.update(owner: user) - end + before do + project.add_developer(user) end it 'does not create a pipeline' do @@ -431,17 +429,15 @@ describe Ci::CreatePipelineService do end context 'when trigger belongs to a master' do - let(:user) {} + let(:user) { create(:user) } + let(:trigger) { create(:ci_trigger, owner: user) } + let(:trigger_request) { create(:ci_trigger_request, trigger: trigger) } - let(:trigger_request) do - create(:ci_trigger_request).tap do |request| - user = create(:user) - project.add_master(user) - request.trigger.update(owner: user) - end + before do + project.add_master(user) end - it 'does not create a pipeline' do + it 'creates a pipeline' do expect(execute_service(trigger_request: trigger_request)) .to be_persisted expect(Ci::Pipeline.count).to eq(1) @@ -470,7 +466,8 @@ describe Ci::CreatePipelineService do context 'when ref is not protected' do context 'when trigger belongs to no one' do let(:user) {} - let(:trigger_request) { create(:ci_trigger_request) } + let(:trigger) { create(:ci_trigger, owner: nil) } + let(:trigger_request) { create(:ci_trigger_request, trigger: trigger) } it 'creates a pipeline' do expect(execute_service(trigger_request: trigger_request)) diff --git a/spec/services/files/update_service_spec.rb b/spec/services/files/update_service_spec.rb index cc950ae6bb3..2b4f8cd42ee 100644 --- a/spec/services/files/update_service_spec.rb +++ b/spec/services/files/update_service_spec.rb @@ -76,7 +76,7 @@ describe Files::UpdateService do let(:branch_name) { "#{project.default_branch}-new" } it 'fires hooks only once' do - expect(GitHooksService).to receive(:new).once.and_call_original + expect(Gitlab::Git::HooksService).to receive(:new).once.and_call_original subject.execute end diff --git a/spec/services/groups/create_service_spec.rb b/spec/services/groups/create_service_spec.rb index b2175717a70..10dda45d2a1 100644 --- a/spec/services/groups/create_service_spec.rb +++ b/spec/services/groups/create_service_spec.rb @@ -22,7 +22,7 @@ describe Groups::CreateService, '#execute' do end end - describe 'creating subgroup' do + describe 'creating subgroup', :nested_groups do let!(:group) { create(:group) } let!(:service) { described_class.new(user, group_params.merge(parent_id: group.id)) } @@ -32,12 +32,24 @@ describe Groups::CreateService, '#execute' do end it { is_expected.to be_persisted } + + context 'when nested groups feature is disabled' do + it 'does not save group and returns an error' do + allow(Group).to receive(:supports_nested_groups?).and_return(false) + + is_expected.not_to be_persisted + expect(subject.errors[:parent_id]).to include('You don’t have permission to create a subgroup in this group.') + expect(subject.parent_id).to be_nil + end + end end context 'as guest' do it 'does not save group and returns an error' do + allow(Group).to receive(:supports_nested_groups?).and_return(true) + is_expected.not_to be_persisted - expect(subject.errors[:parent_id].first).to eq('manage access required to create subgroup') + expect(subject.errors[:parent_id].first).to eq('You don’t have permission to create a subgroup in this group.') expect(subject.parent_id).to be_nil end end diff --git a/spec/services/groups/destroy_service_spec.rb b/spec/services/groups/destroy_service_spec.rb index 1b2ce3cd03e..ac4b9c02ba7 100644 --- a/spec/services/groups/destroy_service_spec.rb +++ b/spec/services/groups/destroy_service_spec.rb @@ -8,8 +8,8 @@ describe Groups::DestroyService do let!(:nested_group) { create(:group, parent: group) } let!(:project) { create(:project, namespace: group) } let!(:notification_setting) { create(:notification_setting, source: group)} - let!(:gitlab_shell) { Gitlab::Shell.new } - let!(:remove_path) { group.path + "+#{group.id}+deleted" } + let(:gitlab_shell) { Gitlab::Shell.new } + let(:remove_path) { group.path + "+#{group.id}+deleted" } before do group.add_user(user, Gitlab::Access::OWNER) @@ -134,4 +134,26 @@ describe Groups::DestroyService do it_behaves_like 'group destruction', false end + + describe 'repository removal' do + before do + destroy_group(group, user, false) + end + + context 'legacy storage' 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 + end + end + + context 'hashed storage' do + let!(:project) { create(:project, :hashed, :empty_repo, namespace: group) } + + it 'removes repository' do + expect(gitlab_shell.exists?(project.repository_storage_path, "#{project.disk_path}.git")).to be_falsey + end + end + end end diff --git a/spec/services/groups/nested_create_service_spec.rb b/spec/services/groups/nested_create_service_spec.rb new file mode 100644 index 00000000000..6491fb34777 --- /dev/null +++ b/spec/services/groups/nested_create_service_spec.rb @@ -0,0 +1,88 @@ +require 'spec_helper' + +describe Groups::NestedCreateService do + let(:user) { create(:user) } + + subject(:service) { described_class.new(user, params) } + + shared_examples 'with a visibility level' do + it 'creates the group with correct visibility level' do + allow(Gitlab::CurrentSettings.current_application_settings) + .to receive(:default_group_visibility) { Gitlab::VisibilityLevel::INTERNAL } + + group = service.execute + + expect(group.visibility_level).to eq(Gitlab::VisibilityLevel::INTERNAL) + end + + context 'adding a visibility level ' do + it 'overwrites the visibility level' do + service = described_class.new(user, params.merge(visibility_level: Gitlab::VisibilityLevel::PRIVATE)) + + group = service.execute + + expect(group.visibility_level).to eq(Gitlab::VisibilityLevel::PRIVATE) + end + end + end + + describe 'without subgroups' do + let(:params) { { group_path: 'a-group' } } + + before do + allow(Group).to receive(:supports_nested_groups?) { false } + end + + it 'creates the group' do + group = service.execute + + expect(group).to be_persisted + end + + it 'returns the group if it already existed' do + existing_group = create(:group, path: 'a-group') + + expect(service.execute).to eq(existing_group) + end + + it 'raises an error when tring to create a subgroup' do + service = described_class.new(user, group_path: 'a-group/a-sub-group') + + expect { service.execute }.to raise_error('Nested groups are not supported on MySQL') + end + + it_behaves_like 'with a visibility level' + end + + describe 'with subgroups', :nested_groups do + let(:params) { { group_path: 'a-group/a-sub-group' } } + + describe "#execute" do + it 'returns the group if it already existed' do + parent = create(:group, path: 'a-group', owner: user) + child = create(:group, path: 'a-sub-group', parent: parent, owner: user) + + expect(service.execute).to eq(child) + end + + it 'reuses a parent if it already existed' do + parent = create(:group, path: 'a-group') + parent.add_owner(user) + + expect(service.execute.parent).to eq(parent) + end + + it 'creates group and subgroup in the database' do + service.execute + + parent = Group.find_by_full_path('a-group') + child = parent.children.find_by(path: 'a-sub-group') + + expect(parent).not_to be_nil + expect(child).not_to be_nil + end + + it_behaves_like 'with a visibility level' + end + end +end diff --git a/spec/services/issues/close_service_spec.rb b/spec/services/issues/close_service_spec.rb index a03f68434de..171f70c32a8 100644 --- a/spec/services/issues/close_service_spec.rb +++ b/spec/services/issues/close_service_spec.rb @@ -42,6 +42,11 @@ describe Issues::CloseService do service.execute(issue) end + it 'refreshes the number of open issues' do + expect { service.execute(issue) } + .to change { project.open_issues_count }.from(1).to(0) + end + it 'invalidates counter cache for assignees' do expect_any_instance_of(User).to receive(:invalidate_issue_cache_counts) diff --git a/spec/services/issues/create_service_spec.rb b/spec/services/issues/create_service_spec.rb index 9b2d9e79f4f..78b11cd7991 100644 --- a/spec/services/issues/create_service_spec.rb +++ b/spec/services/issues/create_service_spec.rb @@ -35,6 +35,10 @@ describe Issues::CreateService do expect(issue.due_date).to eq Date.tomorrow end + it 'refreshes the number of open issues' do + expect { issue }.to change { project.open_issues_count }.from(0).to(1) + end + context 'when current user cannot admin issues in the project' do let(:guest) { create(:user) } diff --git a/spec/services/issues/reopen_service_spec.rb b/spec/services/issues/reopen_service_spec.rb index 205e9ebd237..48fc98b3b2f 100644 --- a/spec/services/issues/reopen_service_spec.rb +++ b/spec/services/issues/reopen_service_spec.rb @@ -34,6 +34,13 @@ describe Issues::ReopenService do described_class.new(project, user).execute(issue) end + it 'refreshes the number of opened issues' do + service = described_class.new(project, user) + + expect { service.execute(issue) } + .to change { project.open_issues_count }.from(0).to(1) + end + context 'when issue is not confidential' do it 'executes issue hooks' do expect(project).to receive(:execute_hooks).with(an_instance_of(Hash), :issue_hooks) diff --git a/spec/services/merge_requests/close_service_spec.rb b/spec/services/merge_requests/close_service_spec.rb index 04bf267d167..7e65369762c 100644 --- a/spec/services/merge_requests/close_service_spec.rb +++ b/spec/services/merge_requests/close_service_spec.rb @@ -52,6 +52,13 @@ describe MergeRequests::CloseService do end end + it 'refreshes the number of open merge requests for a valid MR' do + service = described_class.new(project, user, {}) + + expect { service.execute(merge_request) } + .to change { project.open_merge_requests_count }.from(1).to(0) + end + context 'current user is not authorized to close merge request' do before do perform_enqueued_jobs do diff --git a/spec/services/merge_requests/create_service_spec.rb b/spec/services/merge_requests/create_service_spec.rb index a1f3bec42cc..d6409c0d625 100644 --- a/spec/services/merge_requests/create_service_spec.rb +++ b/spec/services/merge_requests/create_service_spec.rb @@ -18,31 +18,35 @@ describe MergeRequests::CreateService do end let(:service) { described_class.new(project, user, opts) } + let(:merge_request) { service.execute } before do project.team << [user, :master] project.team << [assignee, :developer] allow(service).to receive(:execute_hooks) - - @merge_request = service.execute end it 'creates an MR' do - expect(@merge_request).to be_valid - expect(@merge_request.title).to eq('Awesome merge_request') - expect(@merge_request.assignee).to be_nil - expect(@merge_request.merge_params['force_remove_source_branch']).to eq('1') + expect(merge_request).to be_valid + expect(merge_request.title).to eq('Awesome merge_request') + expect(merge_request.assignee).to be_nil + expect(merge_request.merge_params['force_remove_source_branch']).to eq('1') end it 'executes hooks with default action' do - expect(service).to have_received(:execute_hooks).with(@merge_request) + expect(service).to have_received(:execute_hooks).with(merge_request) + end + + it 'refreshes the number of open merge requests' do + expect { service.execute } + .to change { project.open_merge_requests_count }.from(0).to(1) end it 'does not creates todos' do attributes = { project: project, - target_id: @merge_request.id, - target_type: @merge_request.class.name + target_id: merge_request.id, + target_type: merge_request.class.name } expect(Todo.where(attributes).count).to be_zero @@ -51,8 +55,8 @@ describe MergeRequests::CreateService do it 'creates exactly 1 create MR event' do attributes = { action: Event::CREATED, - target_id: @merge_request.id, - target_type: @merge_request.class.name + target_id: merge_request.id, + target_type: merge_request.class.name } expect(Event.where(attributes).count).to eq(1) @@ -69,15 +73,15 @@ describe MergeRequests::CreateService do } end - it { expect(@merge_request.assignee).to eq assignee } + it { expect(merge_request.assignee).to eq assignee } it 'creates a todo for new assignee' do attributes = { project: project, author: user, user: assignee, - target_id: @merge_request.id, - target_type: @merge_request.class.name, + target_id: merge_request.id, + target_type: merge_request.class.name, action: Todo::ASSIGNED, state: :pending } diff --git a/spec/services/merge_requests/merge_service_spec.rb b/spec/services/merge_requests/merge_service_spec.rb index e593bfeeaf7..b60136064b7 100644 --- a/spec/services/merge_requests/merge_service_spec.rb +++ b/spec/services/merge_requests/merge_service_spec.rb @@ -12,6 +12,38 @@ describe MergeRequests::MergeService do end describe '#execute' do + context 'MergeRequest#merge_jid' do + before do + merge_request.update_column(:merge_jid, 'hash-123') + end + + it 'is cleaned when no error is raised' do + service = described_class.new(project, user, commit_message: 'Awesome message') + + service.execute(merge_request) + + expect(merge_request.reload.merge_jid).to be_nil + end + + it 'is cleaned when expected error is raised' do + service = described_class.new(project, user, commit_message: 'Awesome message') + allow(service).to receive(:commit).and_raise(described_class::MergeError) + + service.execute(merge_request) + + expect(merge_request.reload.merge_jid).to be_nil + end + + it 'is not cleaned when unexpected error is raised' do + service = described_class.new(project, user, commit_message: 'Awesome message') + allow(service).to receive(:commit).and_raise(StandardError) + + expect { service.execute(merge_request) }.to raise_error(StandardError) + + expect(merge_request.reload.merge_jid).to be_present + end + end + context 'valid params' do let(:service) { described_class.new(project, user, commit_message: 'Awesome message') } @@ -217,7 +249,7 @@ describe MergeRequests::MergeService do it 'logs and saves error if there is an PreReceiveError exception' do error_message = 'error message' - allow(service).to receive(:repository).and_raise(GitHooksService::PreReceiveError, error_message) + allow(service).to receive(:repository).and_raise(Gitlab::Git::HooksService::PreReceiveError, error_message) allow(service).to receive(:execute_hooks) service.execute(merge_request) diff --git a/spec/services/merge_requests/reopen_service_spec.rb b/spec/services/merge_requests/reopen_service_spec.rb index f02af0c582e..fa652611c6b 100644 --- a/spec/services/merge_requests/reopen_service_spec.rb +++ b/spec/services/merge_requests/reopen_service_spec.rb @@ -47,6 +47,13 @@ describe MergeRequests::ReopenService do end end + it 'refreshes the number of open merge requests for a valid MR' do + service = described_class.new(project, user, {}) + + expect { service.execute(merge_request) } + .to change { project.open_merge_requests_count }.from(0).to(1) + end + context 'current user is not authorized to reopen merge request' do before do perform_enqueued_jobs do diff --git a/spec/services/notification_service_spec.rb b/spec/services/notification_service_spec.rb index 44b2d28d1d4..5b349017c8b 100644 --- a/spec/services/notification_service_spec.rb +++ b/spec/services/notification_service_spec.rb @@ -130,7 +130,18 @@ describe NotificationService, :mailer do project.add_master(issue.author) project.add_master(assignee) project.add_master(note.author) - create(:note_on_issue, noteable: issue, project_id: issue.project_id, note: '@subscribed_participant cc this guy') + + @u_custom_off = create_user_with_notification(:custom, 'custom_off') + project.add_guest(@u_custom_off) + + create( + :note_on_issue, + author: @u_custom_off, + noteable: issue, + project_id: issue.project_id, + note: 'i think @subscribed_participant should see this' + ) + update_custom_notification(:new_note, @u_guest_custom, resource: project) update_custom_notification(:new_note, @u_custom_global) end @@ -140,8 +151,7 @@ describe NotificationService, :mailer do add_users_with_subscription(note.project, issue) reset_delivered_emails! - # Ensure create SentNotification by noteable = issue 6 times, not noteable = note - expect(SentNotification).to receive(:record).with(issue, any_args).exactly(8).times + expect(SentNotification).to receive(:record).with(issue, any_args).exactly(9).times notification.new_note(note) @@ -153,6 +163,7 @@ describe NotificationService, :mailer do should_email(@subscriber) should_email(@watcher_and_subscriber) should_email(@subscribed_participant) + should_email(@u_custom_off) should_not_email(@u_guest_custom) should_not_email(@u_guest_watcher) should_not_email(note.author) diff --git a/spec/services/projects/count_service_spec.rb b/spec/services/projects/count_service_spec.rb new file mode 100644 index 00000000000..79b01e7620e --- /dev/null +++ b/spec/services/projects/count_service_spec.rb @@ -0,0 +1,73 @@ +require 'spec_helper' + +describe Projects::CountService do + let(:project) { build(:project, id: 1) } + let(:service) { described_class.new(project) } + + describe '#relation_for_count' do + it 'raises NotImplementedError' do + expect { service.relation_for_count }.to raise_error(NotImplementedError) + end + end + + describe '#count' do + before do + allow(service).to receive(:cache_key_name).and_return('count_service') + end + + it 'returns the number of rows' do + allow(service).to receive(:uncached_count).and_return(1) + + expect(service.count).to eq(1) + end + + it 'caches the number of rows', :use_clean_rails_memory_store_caching do + expect(service).to receive(:uncached_count).once.and_return(1) + + 2.times do + expect(service.count).to eq(1) + end + end + end + + describe '#refresh_cache', :use_clean_rails_memory_store_caching do + before do + allow(service).to receive(:cache_key_name).and_return('count_service') + end + + it 'refreshes the cache' do + expect(service).to receive(:uncached_count).once.and_return(1) + + service.refresh_cache + + expect(service.count).to eq(1) + end + end + + describe '#delete_cache', :use_clean_rails_memory_store_caching do + before do + allow(service).to receive(:cache_key_name).and_return('count_service') + end + + it 'removes the cache' do + expect(service).to receive(:uncached_count).twice.and_return(1) + + service.count + service.delete_cache + service.count + end + end + + describe '#cache_key_name' do + it 'raises NotImplementedError' do + expect { service.cache_key_name }.to raise_error(NotImplementedError) + end + end + + describe '#cache_key' do + it 'returns the cache key as an Array' do + allow(service).to receive(:cache_key_name).and_return('count_service') + expect(service.cache_key).to eq(['projects', 1, 'count_service']) + end + end +end diff --git a/spec/services/projects/create_service_spec.rb b/spec/services/projects/create_service_spec.rb index b0dc7488b5f..088b7b4fc04 100644 --- a/spec/services/projects/create_service_spec.rb +++ b/spec/services/projects/create_service_spec.rb @@ -38,7 +38,7 @@ describe Projects::CreateService, '#execute' do expect(project).to be_persisted expect(project.owner).to eq(user) - expect(project.team.masters).to include(user, admin) + expect(project.team.masters).to contain_exactly(user) expect(project.namespace).to eq(user.namespace) end end diff --git a/spec/services/projects/forks_count_service_spec.rb b/spec/services/projects/forks_count_service_spec.rb index cf299c5d09b..9f8e7ee18a8 100644 --- a/spec/services/projects/forks_count_service_spec.rb +++ b/spec/services/projects/forks_count_service_spec.rb @@ -1,40 +1,14 @@ require 'spec_helper' describe Projects::ForksCountService do - let(:project) { build(:project, id: 42) } - let(:service) { described_class.new(project) } - describe '#count' do it 'returns the number of forks' do - allow(service).to receive(:uncached_count).and_return(1) - - expect(service.count).to eq(1) - end - - it 'caches the forks count', :use_clean_rails_memory_store_caching do - expect(service).to receive(:uncached_count).once.and_return(1) + project = build(:project, id: 42) + service = described_class.new(project) - 2.times { service.count } - end - end - - describe '#refresh_cache', :use_clean_rails_memory_store_caching do - it 'refreshes the cache' do - expect(service).to receive(:uncached_count).once.and_return(1) - - service.refresh_cache + allow(service).to receive(:uncached_count).and_return(1) expect(service.count).to eq(1) end end - - describe '#delete_cache', :use_clean_rails_memory_store_caching do - it 'removes the cache' do - expect(service).to receive(:uncached_count).twice.and_return(1) - - service.count - service.delete_cache - service.count - end - end end diff --git a/spec/services/projects/open_issues_count_service_spec.rb b/spec/services/projects/open_issues_count_service_spec.rb new file mode 100644 index 00000000000..f964f9972cd --- /dev/null +++ b/spec/services/projects/open_issues_count_service_spec.rb @@ -0,0 +1,21 @@ +require 'spec_helper' + +describe Projects::OpenIssuesCountService do + describe '#count' do + it 'returns the number of open issues' do + project = create(:project) + create(:issue, :opened, project: project) + + expect(described_class.new(project).count).to eq(1) + end + + it 'does not include confidential issues in the issue count' do + project = create(:project) + + create(:issue, :opened, project: project) + create(:issue, :opened, confidential: true, project: project) + + expect(described_class.new(project).count).to eq(1) + end + end +end diff --git a/spec/services/projects/open_merge_requests_count_service_spec.rb b/spec/services/projects/open_merge_requests_count_service_spec.rb new file mode 100644 index 00000000000..9f49b9ec6a2 --- /dev/null +++ b/spec/services/projects/open_merge_requests_count_service_spec.rb @@ -0,0 +1,15 @@ +require 'spec_helper' + +describe Projects::OpenMergeRequestsCountService do + describe '#count' do + it 'returns the number of open merge requests' do + project = create(:project) + create(:merge_request, + :opened, + source_project: project, + target_project: project) + + expect(described_class.new(project).count).to eq(1) + end + end +end diff --git a/spec/services/system_note_service_spec.rb b/spec/services/system_note_service_spec.rb index 6d36affa9dc..e6a18654651 100644 --- a/spec/services/system_note_service_spec.rb +++ b/spec/services/system_note_service_spec.rb @@ -13,37 +13,16 @@ describe SystemNoteService do let(:expected_noteable) { noteable } let(:commit_count) { nil } - it 'is valid' do + it 'has the correct attributes', :aggregate_failures do expect(subject).to be_valid - end + expect(subject).to be_system - it 'sets the noteable model' do expect(subject.noteable).to eq expected_noteable - end - - it 'sets the project' do expect(subject.project).to eq project - end - - it 'sets the author' do expect(subject.author).to eq author - end - it 'is a system note' do - expect(subject).to be_system - end - - context 'metadata' do - it 'creates a new system note metadata record' do - expect { subject }.to change { SystemNoteMetadata.count }.from(0).to(1) - end - - it 'creates a record correctly' do - metadata = subject.system_note_metadata - - expect(metadata.commit_count).to eq(commit_count) - expect(metadata.action).to eq(action) - end + expect(subject.system_note_metadata.action).to eq(action) + expect(subject.system_note_metadata.commit_count).to eq(commit_count) end end diff --git a/spec/services/tags/create_service_spec.rb b/spec/services/tags/create_service_spec.rb index 1b31ce29f7a..57013b54560 100644 --- a/spec/services/tags/create_service_spec.rb +++ b/spec/services/tags/create_service_spec.rb @@ -41,7 +41,7 @@ describe Tags::CreateService do it 'returns an error' do expect(repository).to receive(:add_tag) .with(user, 'v1.1.0', 'master', 'Foo') - .and_raise(GitHooksService::PreReceiveError, 'something went wrong') + .and_raise(Gitlab::Git::HooksService::PreReceiveError, 'something went wrong') response = service.execute('v1.1.0', 'master', 'Foo') diff --git a/spec/services/test_hooks/system_service_spec.rb b/spec/services/test_hooks/system_service_spec.rb index 00d89924766..a15708bf82f 100644 --- a/spec/services/test_hooks/system_service_spec.rb +++ b/spec/services/test_hooks/system_service_spec.rb @@ -7,7 +7,6 @@ describe TestHooks::SystemService do let(:project) { create(:project, :repository) } let(:hook) { create(:system_hook) } let(:service) { described_class.new(hook, current_user, trigger) } - let(:sample_data) { { data: 'sample' }} let(:success_result) { { status: :success, http_status: 200, message: 'ok' } } before do @@ -26,18 +25,11 @@ describe TestHooks::SystemService do context 'push_events' do let(:trigger) { 'push_events' } - it 'returns error message if not enough data' do - allow(project).to receive(:empty_repo?).and_return(true) - - expect(hook).not_to receive(:execute) - expect(service.execute).to include({ status: :error, message: "Ensure project \"#{project.human_name}\" has commits." }) - end - it 'executes hook' do allow(project).to receive(:empty_repo?).and_return(false) - allow(Gitlab::DataBuilder::Push).to receive(:build_sample).and_return(sample_data) + expect(Gitlab::DataBuilder::Push).to receive(:sample_data).and_call_original - expect(hook).to receive(:execute).with(sample_data, trigger).and_return(success_result) + expect(hook).to receive(:execute).with(Gitlab::DataBuilder::Push::SAMPLE_DATA, trigger).and_return(success_result) expect(service.execute).to include(success_result) end end @@ -45,18 +37,11 @@ describe TestHooks::SystemService do context 'tag_push_events' do let(:trigger) { 'tag_push_events' } - it 'returns error message if not enough data' do - allow(project.repository).to receive(:tags).and_return([]) - - expect(hook).not_to receive(:execute) - expect(service.execute).to include({ status: :error, message: "Ensure project \"#{project.human_name}\" has tags." }) - end - it 'executes hook' do allow(project.repository).to receive(:tags).and_return(['tag']) - allow(Gitlab::DataBuilder::Push).to receive(:build_sample).and_return(sample_data) + expect(Gitlab::DataBuilder::Push).to receive(:sample_data).and_call_original - expect(hook).to receive(:execute).with(sample_data, trigger).and_return(success_result) + expect(hook).to receive(:execute).with(Gitlab::DataBuilder::Push::SAMPLE_DATA, trigger).and_return(success_result) expect(service.execute).to include(success_result) end end @@ -64,17 +49,11 @@ describe TestHooks::SystemService do context 'repository_update_events' do let(:trigger) { 'repository_update_events' } - it 'returns error message if not enough data' do - allow(project).to receive(:commit).and_return(nil) - expect(hook).not_to receive(:execute) - expect(service.execute).to include({ status: :error, message: "Ensure project \"#{project.human_name}\" has commits." }) - end - it 'executes hook' do allow(project).to receive(:empty_repo?).and_return(false) - allow(Gitlab::DataBuilder::Repository).to receive(:update).and_return(sample_data) + expect(Gitlab::DataBuilder::Repository).to receive(:sample_data).and_call_original - expect(hook).to receive(:execute).with(sample_data, trigger).and_return(success_result) + expect(hook).to receive(:execute).with(Gitlab::DataBuilder::Repository::SAMPLE_DATA, trigger).and_return(success_result) expect(service.execute).to include(success_result) end end diff --git a/spec/services/user_project_access_changed_service_spec.rb b/spec/services/user_project_access_changed_service_spec.rb index 14a5e40350a..87a90378e2b 100644 --- a/spec/services/user_project_access_changed_service_spec.rb +++ b/spec/services/user_project_access_changed_service_spec.rb @@ -8,5 +8,12 @@ describe UserProjectAccessChangedService do described_class.new([1, 2]).execute end + + it 'permits non-blocking operation' do + expect(AuthorizedProjectsWorker).to receive(:bulk_perform_async) + .with([[1], [2]]) + + described_class.new([1, 2]).execute(blocking: false) + end end end diff --git a/spec/services/users/destroy_service_spec.rb b/spec/services/users/destroy_service_spec.rb index a82567f6f43..58a5bede3de 100644 --- a/spec/services/users/destroy_service_spec.rb +++ b/spec/services/users/destroy_service_spec.rb @@ -4,9 +4,10 @@ describe Users::DestroyService do describe "Deletes a user and all their personal projects" do let!(:user) { create(:user) } let!(:admin) { create(:admin) } - let!(:namespace) { create(:namespace, owner: user) } + let!(:namespace) { user.namespace } let!(:project) { create(:project, namespace: namespace) } let(:service) { described_class.new(admin) } + let(:gitlab_shell) { Gitlab::Shell.new } context 'no options are given' do it 'deletes the user' do @@ -14,7 +15,7 @@ describe Users::DestroyService do expect { user_data['email'].to eq(user.email) } expect { User.find(user.id) }.to raise_error(ActiveRecord::RecordNotFound) - expect { Namespace.with_deleted.find(user.namespace.id) }.to raise_error(ActiveRecord::RecordNotFound) + expect { Namespace.with_deleted.find(namespace.id) }.to raise_error(ActiveRecord::RecordNotFound) end it 'will delete the project' do @@ -165,5 +166,27 @@ describe Users::DestroyService do expect(Issue.exists?(issue.id)).to be_falsy end end + + describe "user personal's repository removal" do + before do + Sidekiq::Testing.inline! { service.execute(user) } + end + + context 'legacy storage' 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 + end + end + + context 'hashed storage' do + let!(:project) { create(:project, :empty_repo, :hashed, namespace: user.namespace) } + + it 'removes repository' do + expect(gitlab_shell.exists?(project.repository_storage_path, "#{project.disk_path}.git")).to be_falsey + end + end + end end end diff --git a/spec/services/users/update_service_spec.rb b/spec/services/users/update_service_spec.rb index 985f6d94876..6ee35a33b2d 100644 --- a/spec/services/users/update_service_spec.rb +++ b/spec/services/users/update_service_spec.rb @@ -37,7 +37,10 @@ describe Users::UpdateService do describe '#execute!' do it 'updates the name' do - result = update_user(user, name: 'New Name') + service = described_class.new(user, name: 'New Name') + expect(service).not_to receive(:notify_new_user) + + result = service.execute! expect(result).to be true expect(user.name).to eq('New Name') @@ -49,6 +52,18 @@ describe Users::UpdateService do end.to raise_error(ActiveRecord::RecordInvalid) end + it 'fires system hooks when a new user is saved' do + system_hook_service = spy(:system_hook_service) + user = build(:user) + service = described_class.new(user, name: 'John Doe') + expect(service).to receive(:notify_new_user).and_call_original + expect(service).to receive(:system_hook_service).and_return(system_hook_service) + + service.execute + + expect(system_hook_service).to have_received(:execute_hooks_for).with(user, :create) + end + def update_user(user, opts) described_class.new(user, opts).execute! end diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index c10197ff651..ff1754fbe7e 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -105,6 +105,18 @@ RSpec.configure do |config| reset_delivered_emails! end + # Stub the `ForkedStorageCheck.storage_available?` method unless + # `:broken_storage` metadata is defined + # + # This check can be slow and is unnecessary in a test environment where we + # know the storage is available, because we create it at runtime + config.before(:example) do |example| + unless example.metadata[:broken_storage] + allow(Gitlab::Git::Storage::ForkedStorageCheck) + .to receive(:storage_available?).and_return(true) + end + end + config.around(:each, :use_clean_rails_memory_store_caching) do |example| caching_store = Rails.cache Rails.cache = ActiveSupport::Cache::MemoryStore.new diff --git a/spec/support/api_helpers.rb b/spec/support/api_helpers.rb index ac0aaa524b7..01aca74274c 100644 --- a/spec/support/api_helpers.rb +++ b/spec/support/api_helpers.rb @@ -45,18 +45,4 @@ module ApiHelpers oauth_access_token: oauth_access_token ) end - - def ci_api(path, user = nil) - "/ci/api/v1/#{path}" + - - # Normalize query string - (path.index('?') ? '' : '?') + - - # Append private_token if given a User object - if user.respond_to?(:private_token) - "&private_token=#{user.private_token}" - else - '' - end - end end diff --git a/spec/support/cycle_analytics_helpers.rb b/spec/support/cycle_analytics_helpers.rb index 30911e7fa86..39586d37e93 100644 --- a/spec/support/cycle_analytics_helpers.rb +++ b/spec/support/cycle_analytics_helpers.rb @@ -78,6 +78,8 @@ module CycleAnalyticsHelpers @dummy_pipeline ||= Ci::Pipeline.new( sha: project.repository.commit('master').sha, + ref: 'master', + source: :push, project: project) end diff --git a/spec/support/db_cleaner.rb b/spec/support/db_cleaner.rb index b0f520d08e8..edaee03ea6c 100644 --- a/spec/support/db_cleaner.rb +++ b/spec/support/db_cleaner.rb @@ -4,18 +4,18 @@ RSpec.configure do |config| end config.append_after(:context) do - DatabaseCleaner.clean_with(:truncation) + DatabaseCleaner.clean_with(:truncation, cache_tables: false) end config.before(:each) do DatabaseCleaner.strategy = :transaction end - config.before(:each, js: true) do + config.before(:each, :js) do DatabaseCleaner.strategy = :truncation end - config.before(:each, truncate: true) do + config.before(:each, :truncate) do DatabaseCleaner.strategy = :truncation end diff --git a/spec/support/features/reportable_note_shared_examples.rb b/spec/support/features/reportable_note_shared_examples.rb index cb483ae9a5a..5a0e7c3d099 100644 --- a/spec/support/features/reportable_note_shared_examples.rb +++ b/spec/support/features/reportable_note_shared_examples.rb @@ -34,7 +34,7 @@ shared_examples 'reportable note' do end def open_dropdown(dropdown) - dropdown.click + dropdown.find('.more-actions-toggle').trigger('click') dropdown.find('.dropdown-menu li', match: :first) end end diff --git a/spec/support/helpers/note_interaction_helpers.rb b/spec/support/helpers/note_interaction_helpers.rb index 551c759133c..86008698692 100644 --- a/spec/support/helpers/note_interaction_helpers.rb +++ b/spec/support/helpers/note_interaction_helpers.rb @@ -2,7 +2,7 @@ module NoteInteractionHelpers def open_more_actions_dropdown(note) note_element = find("#note_#{note.id}") - note_element.find('.more-actions').click + note_element.find('.more-actions-toggle').trigger('click') note_element.find('.more-actions .dropdown-menu li', match: :first) end end diff --git a/spec/support/import_export/export_file_helper.rb b/spec/support/import_export/export_file_helper.rb index 2011408be93..562423afc2a 100644 --- a/spec/support/import_export/export_file_helper.rb +++ b/spec/support/import_export/export_file_helper.rb @@ -35,7 +35,8 @@ module ExportFileHelper project: project, commit_id: ci_pipeline.sha) - create(:event, :created, target: milestone, project: project, author: user) + event = create(:event, :created, target: milestone, project: project, author: user, action: 5) + create(:push_event_payload, event: event) create(:project_member, :master, user: user, project: project) create(:ci_variable, project: project) create(:ci_trigger, project: project) diff --git a/spec/support/migrations_helpers.rb b/spec/support/migrations_helpers.rb index 255b3d96a62..4ca019c1b05 100644 --- a/spec/support/migrations_helpers.rb +++ b/spec/support/migrations_helpers.rb @@ -16,6 +16,8 @@ module MigrationsHelpers end def reset_column_in_migration_models + ActiveRecord::Base.clear_cache! + described_class.constants.sort.each do |name| const = described_class.const_get(name) diff --git a/spec/workers/authorized_projects_worker_spec.rb b/spec/workers/authorized_projects_worker_spec.rb index 03b9b99e263..90ed1309d4a 100644 --- a/spec/workers/authorized_projects_worker_spec.rb +++ b/spec/workers/authorized_projects_worker_spec.rb @@ -3,47 +3,100 @@ require 'spec_helper' describe AuthorizedProjectsWorker do let(:project) { create(:project) } + def build_args_list(*ids, multiply: 1) + args_list = ids.map { |id| [id] } + args_list * multiply + end + describe '.bulk_perform_and_wait' do it 'schedules the ids and waits for the jobs to complete' do + args_list = build_args_list(project.owner.id) + project.owner.project_authorizations.delete_all + described_class.bulk_perform_and_wait(args_list) + + expect(project.owner.project_authorizations.count).to eq(1) + end - described_class.bulk_perform_and_wait([[project.owner.id]]) + it 'inlines workloads <= 3 jobs' do + args_list = build_args_list(project.owner.id, multiply: 3) + expect(described_class).to receive(:bulk_perform_inline).with(args_list) + + described_class.bulk_perform_and_wait(args_list) + end + + it 'runs > 3 jobs using sidekiq' do + project.owner.project_authorizations.delete_all + + expect(described_class).to receive(:bulk_perform_async).and_call_original + + args_list = build_args_list(project.owner.id, multiply: 4) + described_class.bulk_perform_and_wait(args_list) expect(project.owner.project_authorizations.count).to eq(1) end end + describe '.bulk_perform_inline' do + it 'refreshes the authorizations inline' do + project.owner.project_authorizations.delete_all + + expect_any_instance_of(described_class).to receive(:perform).and_call_original + + described_class.bulk_perform_inline(build_args_list(project.owner.id)) + + expect(project.owner.project_authorizations.count).to eq(1) + end + + it 'enqueues jobs if an error is raised' do + invalid_id = -1 + args_list = build_args_list(project.owner.id, invalid_id) + + allow_any_instance_of(described_class).to receive(:perform).with(project.owner.id) + allow_any_instance_of(described_class).to receive(:perform).with(invalid_id).and_raise(ArgumentError) + expect(described_class).to receive(:bulk_perform_async).with(build_args_list(invalid_id)) + + described_class.bulk_perform_inline(args_list) + end + end + describe '.bulk_perform_async' do it "uses it's respective sidekiq queue" do - args = [[project.owner.id]] + args_list = build_args_list(project.owner.id) push_bulk_args = { 'class' => described_class, 'queue' => described_class.sidekiq_options['queue'], - 'args' => args + 'args' => args_list } expect(Sidekiq::Client).to receive(:push_bulk).with(push_bulk_args).once - described_class.bulk_perform_async(args) + described_class.bulk_perform_async(args_list) end end describe '#perform' do - subject { described_class.new } + let(:user) { create(:user) } - it "refreshes user's authorized projects" do - user = create(:user) + subject(:job) { described_class.new } + it "refreshes user's authorized projects" do expect_any_instance_of(User).to receive(:refresh_authorized_projects) - subject.perform(user.id) + job.perform(user.id) + end + + it 'notifies the JobWaiter when done if the key is provided' do + expect(Gitlab::JobWaiter).to receive(:notify).with('notify-key', job.jid) + + job.perform(user.id, 'notify-key') end context "when the user is not found" do it "does nothing" do expect_any_instance_of(User).not_to receive(:refresh_authorized_projects) - subject.perform(-1) + job.perform(-1) end end end diff --git a/spec/workers/build_finished_worker_spec.rb b/spec/workers/build_finished_worker_spec.rb index 2868167c7d4..8cc3f37ebe8 100644 --- a/spec/workers/build_finished_worker_spec.rb +++ b/spec/workers/build_finished_worker_spec.rb @@ -3,7 +3,7 @@ require 'spec_helper' describe BuildFinishedWorker do describe '#perform' do context 'when build exists' do - let(:build) { create(:ci_build) } + let!(:build) { create(:ci_build) } it 'calculates coverage and calls hooks' do expect(BuildCoverageWorker) diff --git a/spec/workers/concerns/build_queue_spec.rb b/spec/workers/concerns/build_queue_spec.rb deleted file mode 100644 index 6bf955e0be2..00000000000 --- a/spec/workers/concerns/build_queue_spec.rb +++ /dev/null @@ -1,14 +0,0 @@ -require 'spec_helper' - -describe BuildQueue do - let(:worker) do - Class.new do - include Sidekiq::Worker - include BuildQueue - end - end - - it 'sets the queue name of a worker' do - expect(worker.sidekiq_options['queue'].to_s).to eq('build') - end -end diff --git a/spec/workers/concerns/pipeline_queue_spec.rb b/spec/workers/concerns/pipeline_queue_spec.rb index 40794d0e42a..eac5a770e5f 100644 --- a/spec/workers/concerns/pipeline_queue_spec.rb +++ b/spec/workers/concerns/pipeline_queue_spec.rb @@ -8,7 +8,17 @@ describe PipelineQueue do end end - it 'sets the queue name of a worker' do - expect(worker.sidekiq_options['queue'].to_s).to eq('pipeline') + it 'sets a default pipelines queue automatically' do + expect(worker.sidekiq_options['queue']) + .to eq 'pipeline_default' + end + + describe '.enqueue_in' do + it 'sets a custom sidekiq queue with prefix and group' do + worker.enqueue_in(group: :processing) + + expect(worker.sidekiq_options['queue']) + .to eq 'pipeline_processing' + end end end diff --git a/spec/workers/merge_worker_spec.rb b/spec/workers/merge_worker_spec.rb index ee51000161a..303193bab9b 100644 --- a/spec/workers/merge_worker_spec.rb +++ b/spec/workers/merge_worker_spec.rb @@ -27,15 +27,4 @@ describe MergeWorker do expect(source_project.repository.branch_names).not_to include('markdown') end end - - it 'persists merge_jid' do - merge_request = create(:merge_request, merge_jid: nil) - user = create(:user) - worker = described_class.new - - allow(worker).to receive(:jid) { '999' } - - expect { worker.perform(merge_request.id, user.id, {}) } - .to change { merge_request.reload.merge_jid }.from(nil).to('999') - end end diff --git a/spec/workers/namespaceless_project_destroy_worker_spec.rb b/spec/workers/namespaceless_project_destroy_worker_spec.rb index f2706254284..20cf580af8a 100644 --- a/spec/workers/namespaceless_project_destroy_worker_spec.rb +++ b/spec/workers/namespaceless_project_destroy_worker_spec.rb @@ -5,7 +5,7 @@ describe NamespacelessProjectDestroyWorker do before do # Stub after_save callbacks that will fail when Project has no namespace - allow_any_instance_of(Project).to receive(:ensure_storage_path_exist).and_return(nil) + allow_any_instance_of(Project).to receive(:ensure_storage_path_exists).and_return(nil) allow_any_instance_of(Project).to receive(:update_project_statistics).and_return(nil) end @@ -75,5 +75,19 @@ describe NamespacelessProjectDestroyWorker do end end end + + context 'project has non-existing namespace' do + let!(:project) do + project = build(:project, namespace_id: Namespace.maximum(:id).to_i.succ) + project.save(validate: false) + project + end + + it 'deletes the project' do + subject.perform(project.id) + + expect(Project.unscoped.all).not_to include(project) + end + end end end diff --git a/spec/workers/pipeline_metrics_worker_spec.rb b/spec/workers/pipeline_metrics_worker_spec.rb index ef71125c0b6..896f9e6e7f2 100644 --- a/spec/workers/pipeline_metrics_worker_spec.rb +++ b/spec/workers/pipeline_metrics_worker_spec.rb @@ -2,7 +2,12 @@ require 'spec_helper' describe PipelineMetricsWorker do let(:project) { create(:project, :repository) } - let!(:merge_request) { create(:merge_request, source_project: project, source_branch: pipeline.ref, head_pipeline: pipeline) } + + let!(:merge_request) do + create(:merge_request, source_project: project, + source_branch: pipeline.ref, + head_pipeline: pipeline) + end let(:pipeline) do create(:ci_empty_pipeline, @@ -14,6 +19,8 @@ describe PipelineMetricsWorker do finished_at: Time.now) end + let(:status) { 'pending' } + describe '#perform' do before do described_class.new.perform(pipeline.id) |