diff options
Diffstat (limited to 'spec')
83 files changed, 2223 insertions, 582 deletions
diff --git a/spec/controllers/admin/dashboard_controller_spec.rb b/spec/controllers/admin/dashboard_controller_spec.rb new file mode 100644 index 00000000000..6eb9f7867d5 --- /dev/null +++ b/spec/controllers/admin/dashboard_controller_spec.rb @@ -0,0 +1,21 @@ +require 'spec_helper' + +describe Admin::DashboardController do + describe '#index' do + context 'with pending_delete projects' do + render_views + + it 'does not retrieve projects that are pending deletion' do + sign_in(create(:admin)) + + project = create(:project) + pending_delete_project = create(:project, pending_delete: true) + + get :index + + expect(response.body).to match(project.name) + expect(response.body).not_to match(pending_delete_project.name) + end + end + end +end diff --git a/spec/controllers/projects/jobs_controller_spec.rb b/spec/controllers/projects/jobs_controller_spec.rb index 472e5fc51a0..5a295ae47a6 100644 --- a/spec/controllers/projects/jobs_controller_spec.rb +++ b/spec/controllers/projects/jobs_controller_spec.rb @@ -7,6 +7,10 @@ describe Projects::JobsController do let(:pipeline) { create(:ci_pipeline, project: project) } let(:user) { create(:user) } + before do + stub_not_protect_default_branch + end + describe 'GET index' do context 'when scope is pending' do before do diff --git a/spec/controllers/projects/pipelines_controller_spec.rb b/spec/controllers/projects/pipelines_controller_spec.rb index 734532668d3..c8de275ca3e 100644 --- a/spec/controllers/projects/pipelines_controller_spec.rb +++ b/spec/controllers/projects/pipelines_controller_spec.rb @@ -8,6 +8,7 @@ describe Projects::PipelinesController do let(:feature) { ProjectFeature::DISABLED } before do + stub_not_protect_default_branch project.add_developer(user) project.project_feature.update( builds_access_level: feature) @@ -158,7 +159,7 @@ describe Projects::PipelinesController do context 'when builds are enabled' do let(:feature) { ProjectFeature::ENABLED } - + it 'retries a pipeline without returning any content' do expect(response).to have_http_status(:no_content) expect(build.reload).to be_retried @@ -175,7 +176,7 @@ describe Projects::PipelinesController do describe 'POST cancel.json' do let!(:pipeline) { create(:ci_pipeline, project: project) } let!(:build) { create(:ci_build, :running, pipeline: pipeline) } - + before do post :cancel, namespace_id: project.namespace, project_id: project, @@ -185,7 +186,7 @@ describe Projects::PipelinesController do context 'when builds are enabled' do let(:feature) { ProjectFeature::ENABLED } - + it 'cancels a pipeline without returning any content' do expect(response).to have_http_status(:no_content) expect(pipeline.reload).to be_canceled diff --git a/spec/factories/projects.rb b/spec/factories/projects.rb index 1bb2db11e7f..485ed48d2de 100644 --- a/spec/factories/projects.rb +++ b/spec/factories/projects.rb @@ -171,10 +171,6 @@ FactoryGirl.define do end after :create do |project, evaluator| - TestEnv.copy_repo(project, - bare_repo: TestEnv.factory_repo_path_bare, - refs: TestEnv::BRANCH_SHA) - if evaluator.create_template args = evaluator.create_template diff --git a/spec/features/dashboard/issues_spec.rb b/spec/features/dashboard/issues_spec.rb index 69c1a2ed89a..ea7a9efc326 100644 --- a/spec/features/dashboard/issues_spec.rb +++ b/spec/features/dashboard/issues_spec.rb @@ -78,5 +78,27 @@ RSpec.describe 'Dashboard Issues', feature: true do expect(page).not_to have_content(project_with_issues_disabled.name_with_namespace) end end + + it 'shows the new issue page', js: true do + original_defaults = Gitlab::Application.routes.default_url_options + + Gitlab::Application.routes.default_url_options = { + host: Capybara.current_session.server.host, + port: Capybara.current_session.server.port, + protocol: 'http' + } + + find('.new-project-item-select-button').trigger('click') + wait_for_requests + find('.select2-results li').click + + expect(page).to have_current_path("/#{project.path_with_namespace}/issues/new") + + page.within('#content-body') do + expect(page).to have_selector('.issue-form') + end + + Gitlab::Application.routes.default_url_options = original_defaults + end end end diff --git a/spec/features/issues/user_uses_slash_commands_spec.rb b/spec/features/issues/user_uses_slash_commands_spec.rb index 4740402dc01..0c3c27e3e45 100644 --- a/spec/features/issues/user_uses_slash_commands_spec.rb +++ b/spec/features/issues/user_uses_slash_commands_spec.rb @@ -118,5 +118,42 @@ feature 'Issues > User uses quick actions', feature: true, js: true do expect(page).not_to have_content '/wip' end end + + describe 'mark issue as duplicate' do + let(:issue) { create(:issue, project: project) } + let(:original_issue) { create(:issue, project: project) } + + context 'when the current user can update issues' do + it 'does not create a note, and marks the issue as a duplicate' do + write_note("/duplicate ##{original_issue.to_reference}") + + expect(page).not_to have_content "/duplicate #{original_issue.to_reference}" + expect(page).to have_content 'Commands applied' + expect(page).to have_content "marked this issue as a duplicate of #{original_issue.to_reference}" + + expect(issue.reload).to be_closed + end + end + + context 'when the current user cannot update the issue' do + let(:guest) { create(:user) } + before do + project.team << [guest, :guest] + gitlab_sign_out + sign_in(guest) + visit project_issue_path(project, issue) + end + + it 'does not create a note, and does not mark the issue as a duplicate' do + write_note("/duplicate ##{original_issue.to_reference}") + + expect(page).to have_content "/duplicate ##{original_issue.to_reference}" + expect(page).not_to have_content 'Commands applied' + expect(page).not_to have_content "marked this issue as a duplicate of #{original_issue.to_reference}" + + expect(issue.reload).to be_open + end + end + end end end diff --git a/spec/features/oauth_login_spec.rb b/spec/features/oauth_login_spec.rb index 0064c9ef25e..49d8e52f861 100644 --- a/spec/features/oauth_login_spec.rb +++ b/spec/features/oauth_login_spec.rb @@ -1,6 +1,6 @@ require 'spec_helper' -feature 'OAuth Login', js: true do +feature 'OAuth Login', :js, :allow_forgery_protection do include DeviseHelpers def enter_code(code) diff --git a/spec/features/projects/blobs/edit_spec.rb b/spec/features/projects/blobs/edit_spec.rb index c9384a09ccd..ddd27083147 100644 --- a/spec/features/projects/blobs/edit_spec.rb +++ b/spec/features/projects/blobs/edit_spec.rb @@ -3,7 +3,7 @@ require 'spec_helper' feature 'Editing file blob', feature: true, js: true do include TreeHelper - let(:project) { create(:project, :public, :test_repo) } + let(:project) { create(:project, :public) } let(:merge_request) { create(:merge_request, source_project: project, source_branch: 'feature', target_branch: 'master') } let(:branch) { 'master' } let(:file_path) { project.repository.ls_files(project.repository.root_ref)[1] } diff --git a/spec/features/projects/labels/update_prioritization_spec.rb b/spec/features/projects/labels/update_prioritization_spec.rb index 61f6d734ed3..9b51b427845 100644 --- a/spec/features/projects/labels/update_prioritization_spec.rb +++ b/spec/features/projects/labels/update_prioritization_spec.rb @@ -114,6 +114,12 @@ feature 'Prioritize labels', feature: true do expect(page.all('li').last).to have_content('bug') end end + + it 'shows a help message about prioritized labels' do + visit project_labels_path(project) + + expect(page).to have_content 'Star a label' + end end context 'as a guest' do @@ -128,6 +134,7 @@ feature 'Prioritize labels', feature: true do expect(page).to have_content 'wontfix' expect(page).to have_content 'feature' expect(page).not_to have_css('.prioritized-labels') + expect(page).not_to have_content 'Star a label' end end @@ -139,6 +146,7 @@ feature 'Prioritize labels', feature: true do expect(page).to have_content 'wontfix' expect(page).to have_content 'feature' expect(page).not_to have_css('.prioritized-labels') + expect(page).not_to have_content 'Star a label' end end end diff --git a/spec/features/projects/ref_switcher_spec.rb b/spec/features/projects/ref_switcher_spec.rb index 31c7b492ab7..9f5544ac43e 100644 --- a/spec/features/projects/ref_switcher_spec.rb +++ b/spec/features/projects/ref_switcher_spec.rb @@ -19,14 +19,14 @@ feature 'Ref switcher', feature: true, js: true do input.set 'binary' wait_for_requests - expect(find('.dropdown-content ul')).to have_selector('li', count: 6) + expect(find('.dropdown-content ul')).to have_selector('li', count: 7) page.within '.dropdown-content ul' do input.native.send_keys :enter end end - expect(page).to have_title 'binary-encoding' + expect(page).to have_title 'add-pdf-text-binary' end it "user selects ref with special characters" do diff --git a/spec/features/projects/services/jira_service_spec.rb b/spec/features/projects/services/jira_service_spec.rb index 7c29af247d6..b71eec0ecfd 100644 --- a/spec/features/projects/services/jira_service_spec.rb +++ b/spec/features/projects/services/jira_service_spec.rb @@ -6,17 +6,12 @@ feature 'Setup Jira service', :feature, :js do let(:service) { project.create_jira_service } let(:url) { 'http://jira.example.com' } - - def stub_project_url - WebMock.stub_request(:get, 'http://jira.example.com/rest/api/2/project/GitLabProject') - .with(basic_auth: %w(username password)) - end + let(:test_url) { 'http://jira.example.com/rest/api/2/serverInfo' } def fill_form(active = true) check 'Active' if active fill_in 'service_url', with: url - fill_in 'service_project_key', with: 'GitLabProject' fill_in 'service_username', with: 'username' fill_in 'service_password', with: 'password' fill_in 'service_jira_issue_transition_id', with: '25' @@ -31,11 +26,10 @@ feature 'Setup Jira service', :feature, :js do describe 'user sets and activates Jira Service' do context 'when Jira connection test succeeds' do - before do - stub_project_url - end - it 'activates the JIRA service' do + server_info = { key: 'value' }.to_json + WebMock.stub_request(:get, test_url).with(basic_auth: %w(username password)).to_return(body: server_info) + click_link('JIRA') fill_form click_button('Test settings and save changes') @@ -47,10 +41,6 @@ feature 'Setup Jira service', :feature, :js do end context 'when Jira connection test fails' do - before do - stub_project_url.to_return(status: 401) - end - it 'shows errors when some required fields are not filled in' do click_link('JIRA') @@ -64,6 +54,9 @@ feature 'Setup Jira service', :feature, :js do end it 'activates the JIRA service' do + WebMock.stub_request(:get, test_url).with(basic_auth: %w(username password)) + .to_raise(JIRA::HTTPError.new(double(message: 'message'))) + click_link('JIRA') fill_form click_button('Test settings and save changes') diff --git a/spec/features/projects/show_project_spec.rb b/spec/features/projects/show_project_spec.rb new file mode 100644 index 00000000000..1bc6fae9e7f --- /dev/null +++ b/spec/features/projects/show_project_spec.rb @@ -0,0 +1,20 @@ +require 'spec_helper' + +describe 'Project show page', feature: true do + context 'when project pending delete' do + let(:project) { create(:project, :empty_repo, pending_delete: true) } + + before do + sign_in(project.owner) + end + + it 'shows error message if deletion for project fails' do + project.update_attributes(delete_error: "Something went wrong", pending_delete: false) + + visit project_path(project) + + expect(page).to have_selector('.project-deletion-failed-message') + expect(page).to have_content("This project was scheduled for deletion, but failed with the following message: #{project.delete_error}") + end + end +end diff --git a/spec/fixtures/api/schemas/public_api/v4/branch.json b/spec/fixtures/api/schemas/public_api/v4/branch.json new file mode 100644 index 00000000000..a3581178974 --- /dev/null +++ b/spec/fixtures/api/schemas/public_api/v4/branch.json @@ -0,0 +1,20 @@ +{ + "type": "object", + "required" : [ + "name", + "commit", + "merged", + "protected", + "developers_can_push", + "developers_can_merge" + ], + "properties" : { + "name": { "type": "string" }, + "commit": { "$ref": "commit/basic.json" }, + "merged": { "type": "boolean" }, + "protected": { "type": "boolean" }, + "developers_can_push": { "type": "boolean" }, + "developers_can_merge": { "type": "boolean" } + }, + "additionalProperties": false +} diff --git a/spec/fixtures/api/schemas/public_api/v4/branches.json b/spec/fixtures/api/schemas/public_api/v4/branches.json new file mode 100644 index 00000000000..854c902b485 --- /dev/null +++ b/spec/fixtures/api/schemas/public_api/v4/branches.json @@ -0,0 +1,4 @@ +{ + "type": "array", + "items": { "$ref": "branch.json" } +} diff --git a/spec/fixtures/api/schemas/public_api/v4/commit/basic.json b/spec/fixtures/api/schemas/public_api/v4/commit/basic.json new file mode 100644 index 00000000000..9d99628a286 --- /dev/null +++ b/spec/fixtures/api/schemas/public_api/v4/commit/basic.json @@ -0,0 +1,37 @@ +{ + "type": "object", + "required" : [ + "id", + "short_id", + "title", + "created_at", + "parent_ids", + "message", + "author_name", + "author_email", + "authored_date", + "committer_name", + "committer_email", + "committed_date" + ], + "properties" : { + "id": { "type": ["string", "null"] }, + "short_id": { "type": ["string", "null"] }, + "title": { "type": "string" }, + "created_at": { "type": "date" }, + "parent_ids": { + "type": ["array", "null"], + "items": { + "type": "string", + "additionalProperties": false + } + }, + "message": { "type": "string" }, + "author_name": { "type": "string" }, + "author_email": { "type": "string" }, + "authored_date": { "type": "date" }, + "committer_name": { "type": "string" }, + "committer_email": { "type": "string" }, + "committed_date": { "type": "date" } + } +} diff --git a/spec/javascripts/vue_mr_widget/components/mr_widget_deployment_spec.js b/spec/javascripts/vue_mr_widget/components/mr_widget_deployment_spec.js index d4b200875df..ab8a3f6c64c 100644 --- a/spec/javascripts/vue_mr_widget/components/mr_widget_deployment_spec.js +++ b/spec/javascripts/vue_mr_widget/components/mr_widget_deployment_spec.js @@ -10,6 +10,7 @@ const deploymentMockData = [ url: '/root/acets-review-apps/environments/15', stop_url: '/root/acets-review-apps/environments/15/stop', metrics_url: '/root/acets-review-apps/environments/15/deployments/1/metrics', + metrics_monitoring_url: '/root/acets-review-apps/environments/15/metrics', external_url: 'http://diplo.', external_url_formatted: 'diplo.', deployed_at: '2017-03-22T22:44:42.258Z', diff --git a/spec/javascripts/vue_mr_widget/components/mr_widget_memory_usage_spec.js b/spec/javascripts/vue_mr_widget/components/mr_widget_memory_usage_spec.js index 2c3d0ddff28..6adcbc73ed7 100644 --- a/spec/javascripts/vue_mr_widget/components/mr_widget_memory_usage_spec.js +++ b/spec/javascripts/vue_mr_widget/components/mr_widget_memory_usage_spec.js @@ -3,6 +3,7 @@ import memoryUsageComponent from '~/vue_merge_request_widget/components/mr_widge import MRWidgetService from '~/vue_merge_request_widget/services/mr_widget_service'; const url = '/root/acets-review-apps/environments/15/deployments/1/metrics'; +const monitoringUrl = '/root/acets-review-apps/environments/15/metrics'; const metricsMockData = { success: true, @@ -39,6 +40,7 @@ const createComponent = () => { el: document.createElement('div'), propsData: { metricsUrl: url, + metricsMonitoringUrl: monitoringUrl, memoryMetrics: [], deploymentTime: 0, hasMetrics: false, diff --git a/spec/lib/gitlab/backup/manager_spec.rb b/spec/lib/gitlab/backup/manager_spec.rb index 1c3d2547fec..8536d152272 100644 --- a/spec/lib/gitlab/backup/manager_spec.rb +++ b/spec/lib/gitlab/backup/manager_spec.rb @@ -214,4 +214,56 @@ describe Backup::Manager, lib: true do end end end + + describe '#upload' do + let(:backup_file) { Tempfile.new('backup', Gitlab.config.backup.path) } + let(:backup_filename) { File.basename(backup_file.path) } + + before do + allow(subject).to receive(:tar_file).and_return(backup_filename) + + stub_backup_setting( + upload: { + connection: { + provider: 'AWS', + aws_access_key_id: 'id', + aws_secret_access_key: 'secret' + }, + remote_directory: 'directory', + multipart_chunk_size: 104857600, + encryption: nil, + storage_class: nil + } + ) + + # the Fog mock only knows about directories we create explicitly + Fog.mock! + connection = ::Fog::Storage.new(Gitlab.config.backup.upload.connection.symbolize_keys) + connection.directories.create(key: Gitlab.config.backup.upload.remote_directory) + end + + context 'target path' do + it 'uses the tar filename by default' do + expect_any_instance_of(Fog::Collection).to receive(:create) + .with(hash_including(key: backup_filename)) + .and_return(true) + + Dir.chdir(Gitlab.config.backup.path) do + subject.upload + end + end + + it 'adds the DIRECTORY environment variable if present' do + stub_env('DIRECTORY', 'daily') + + expect_any_instance_of(Fog::Collection).to receive(:create) + .with(hash_including(key: "daily/#{backup_filename}")) + .and_return(true) + + Dir.chdir(Gitlab.config.backup.path) do + subject.upload + end + end + end + end end diff --git a/spec/lib/gitlab/ci/status/build/cancelable_spec.rb b/spec/lib/gitlab/ci/status/build/cancelable_spec.rb index 114d2490490..5a7a42d84c0 100644 --- a/spec/lib/gitlab/ci/status/build/cancelable_spec.rb +++ b/spec/lib/gitlab/ci/status/build/cancelable_spec.rb @@ -48,7 +48,9 @@ describe Gitlab::Ci::Status::Build::Cancelable do describe '#has_action?' do context 'when user is allowed to update build' do before do - build.project.team << [user, :developer] + stub_not_protect_default_branch + + build.project.add_developer(user) end it { is_expected.to have_action } diff --git a/spec/lib/gitlab/ci/status/build/factory_spec.rb b/spec/lib/gitlab/ci/status/build/factory_spec.rb index c8a97016f20..8768302eda1 100644 --- a/spec/lib/gitlab/ci/status/build/factory_spec.rb +++ b/spec/lib/gitlab/ci/status/build/factory_spec.rb @@ -7,7 +7,9 @@ describe Gitlab::Ci::Status::Build::Factory do let(:factory) { described_class.new(build, user) } before do - project.team << [user, :developer] + stub_not_protect_default_branch + + project.add_developer(user) end context 'when build is successful' do @@ -225,19 +227,19 @@ describe Gitlab::Ci::Status::Build::Factory do end context 'when user has ability to play action' do - before do - project.add_developer(user) - - create(:protected_branch, :developers_can_merge, - name: build.ref, project: project) - end - it 'fabricates status that has action' do expect(status).to have_action end end context 'when user does not have ability to play action' do + before do + allow(build.project).to receive(:empty_repo?).and_return(false) + + create(:protected_branch, :no_one_can_push, + name: build.ref, project: build.project) + end + it 'fabricates status that has no action' do expect(status).not_to have_action end @@ -262,6 +264,13 @@ describe Gitlab::Ci::Status::Build::Factory do end context 'when user is not allowed to execute manual action' do + before do + allow(build.project).to receive(:empty_repo?).and_return(false) + + create(:protected_branch, :no_one_can_push, + name: build.ref, project: build.project) + end + it 'fabricates status with correct details' do expect(status.text).to eq 'manual' expect(status.group).to eq 'manual' diff --git a/spec/lib/gitlab/ci/status/build/retryable_spec.rb b/spec/lib/gitlab/ci/status/build/retryable_spec.rb index 099d873fc01..21026f2c968 100644 --- a/spec/lib/gitlab/ci/status/build/retryable_spec.rb +++ b/spec/lib/gitlab/ci/status/build/retryable_spec.rb @@ -48,7 +48,9 @@ describe Gitlab::Ci::Status::Build::Retryable do describe '#has_action?' do context 'when user is allowed to update build' do before do - build.project.team << [user, :developer] + stub_not_protect_default_branch + + build.project.add_developer(user) end it { is_expected.to have_action } diff --git a/spec/lib/gitlab/ci/status/build/stop_spec.rb b/spec/lib/gitlab/ci/status/build/stop_spec.rb index 23902f26b1a..e0425103f41 100644 --- a/spec/lib/gitlab/ci/status/build/stop_spec.rb +++ b/spec/lib/gitlab/ci/status/build/stop_spec.rb @@ -20,7 +20,9 @@ describe Gitlab::Ci::Status::Build::Stop do describe '#has_action?' do context 'when user is allowed to update build' do before do - build.project.team << [user, :developer] + stub_not_protect_default_branch + + build.project.add_developer(user) end it { is_expected.to have_action } diff --git a/spec/lib/gitlab/gitaly_client/repository_service_spec.rb b/spec/lib/gitlab/gitaly_client/repository_service_spec.rb new file mode 100644 index 00000000000..5a9f3fc130c --- /dev/null +++ b/spec/lib/gitlab/gitaly_client/repository_service_spec.rb @@ -0,0 +1,19 @@ +require 'spec_helper' + +describe Gitlab::GitalyClient::RepositoryService do + set(:project) { create(:empty_project) } + let(:storage_name) { project.repository_storage } + let(:relative_path) { project.path_with_namespace + '.git' } + let(:client) { described_class.new(project.repository) } + + describe '#exists?' do + it 'sends an exists message' do + expect_any_instance_of(Gitaly::RepositoryService::Stub) + .to receive(:exists) + .with(gitaly_request_with_path(storage_name, relative_path), kind_of(Hash)) + .and_call_original + + client.exists? + end + end +end diff --git a/spec/lib/gitlab/health_checks/fs_shards_check_spec.rb b/spec/lib/gitlab/health_checks/fs_shards_check_spec.rb index 3de73a9ff65..8abc4320c59 100644 --- a/spec/lib/gitlab/health_checks/fs_shards_check_spec.rb +++ b/spec/lib/gitlab/health_checks/fs_shards_check_spec.rb @@ -3,7 +3,7 @@ require 'spec_helper' describe Gitlab::HealthChecks::FsShardsCheck do def command_exists?(command) _, status = Gitlab::Popen.popen(%W{ #{command} 1 echo }) - status == 0 + status.zero? rescue Errno::ENOENT false end @@ -64,9 +64,7 @@ describe Gitlab::HealthChecks::FsShardsCheck do it 'cleans up files used for testing' do expect(described_class).to receive(:storage_write_test).with(any_args).and_call_original - subject - - expect(Dir.entries(tmp_dir).count).to eq(2) + expect { subject }.not_to change(Dir.entries(tmp_dir), :count) end context 'read test fails' do @@ -88,8 +86,6 @@ describe Gitlab::HealthChecks::FsShardsCheck do end describe '#metrics' do - subject { described_class.metrics } - context 'storage points to not existing folder' do let(:storages_paths) do { @@ -104,14 +100,15 @@ describe Gitlab::HealthChecks::FsShardsCheck do end it 'provides metrics' do - expect(subject).to all(have_attributes(labels: { shard: :default })) - expect(subject).to include(an_object_having_attributes(name: :filesystem_accessible, value: 0)) - expect(subject).to include(an_object_having_attributes(name: :filesystem_readable, value: 0)) - expect(subject).to include(an_object_having_attributes(name: :filesystem_writable, value: 0)) - - expect(subject).to include(an_object_having_attributes(name: :filesystem_access_latency_seconds, value: be >= 0)) - expect(subject).to include(an_object_having_attributes(name: :filesystem_read_latency_seconds, value: be >= 0)) - expect(subject).to include(an_object_having_attributes(name: :filesystem_write_latency_seconds, value: be >= 0)) + metrics = described_class.metrics + + expect(metrics).to all(have_attributes(labels: { shard: :default })) + expect(metrics).to include(an_object_having_attributes(name: :filesystem_accessible, value: 0)) + expect(metrics).to include(an_object_having_attributes(name: :filesystem_readable, value: 0)) + expect(metrics).to include(an_object_having_attributes(name: :filesystem_writable, value: 0)) + expect(metrics).to include(an_object_having_attributes(name: :filesystem_access_latency_seconds, value: be >= 0)) + expect(metrics).to include(an_object_having_attributes(name: :filesystem_read_latency_seconds, value: be >= 0)) + expect(metrics).to include(an_object_having_attributes(name: :filesystem_write_latency_seconds, value: be >= 0)) end end @@ -121,15 +118,19 @@ describe Gitlab::HealthChecks::FsShardsCheck do end it 'provides metrics' do - expect(subject).to all(have_attributes(labels: { shard: :default })) - - expect(subject).to include(an_object_having_attributes(name: :filesystem_accessible, value: 1)) - expect(subject).to include(an_object_having_attributes(name: :filesystem_readable, value: 1)) - expect(subject).to include(an_object_having_attributes(name: :filesystem_writable, value: 1)) + metrics = described_class.metrics + + expect(metrics).to all(have_attributes(labels: { shard: :default })) + expect(metrics).to include(an_object_having_attributes(name: :filesystem_accessible, value: 1)) + expect(metrics).to include(an_object_having_attributes(name: :filesystem_readable, value: 1)) + expect(metrics).to include(an_object_having_attributes(name: :filesystem_writable, value: 1)) + expect(metrics).to include(an_object_having_attributes(name: :filesystem_access_latency_seconds, value: be >= 0)) + expect(metrics).to include(an_object_having_attributes(name: :filesystem_read_latency_seconds, value: be >= 0)) + expect(metrics).to include(an_object_having_attributes(name: :filesystem_write_latency_seconds, value: be >= 0)) + end - expect(subject).to include(an_object_having_attributes(name: :filesystem_access_latency_seconds, value: be >= 0)) - expect(subject).to include(an_object_having_attributes(name: :filesystem_read_latency_seconds, value: be >= 0)) - expect(subject).to include(an_object_having_attributes(name: :filesystem_write_latency_seconds, value: be >= 0)) + it 'cleans up files used for metrics' do + expect { described_class.metrics }.not_to change(Dir.entries(tmp_dir), :count) end end end @@ -150,18 +151,16 @@ describe Gitlab::HealthChecks::FsShardsCheck do end describe '#metrics' do - subject { described_class.metrics } - it 'provides metrics' do - expect(subject).to all(have_attributes(labels: { shard: :default })) - - expect(subject).to include(an_object_having_attributes(name: :filesystem_accessible, value: 0)) - expect(subject).to include(an_object_having_attributes(name: :filesystem_readable, value: 0)) - expect(subject).to include(an_object_having_attributes(name: :filesystem_writable, value: 0)) - - expect(subject).to include(an_object_having_attributes(name: :filesystem_access_latency_seconds, value: be >= 0)) - expect(subject).to include(an_object_having_attributes(name: :filesystem_read_latency_seconds, value: be >= 0)) - expect(subject).to include(an_object_having_attributes(name: :filesystem_write_latency_seconds, value: be >= 0)) + metrics = described_class.metrics + + expect(metrics).to all(have_attributes(labels: { shard: :default })) + expect(metrics).to include(an_object_having_attributes(name: :filesystem_accessible, value: 0)) + expect(metrics).to include(an_object_having_attributes(name: :filesystem_readable, value: 0)) + expect(metrics).to include(an_object_having_attributes(name: :filesystem_writable, value: 0)) + expect(metrics).to include(an_object_having_attributes(name: :filesystem_access_latency_seconds, value: be >= 0)) + expect(metrics).to include(an_object_having_attributes(name: :filesystem_read_latency_seconds, value: be >= 0)) + expect(metrics).to include(an_object_having_attributes(name: :filesystem_write_latency_seconds, value: be >= 0)) end end end diff --git a/spec/lib/gitlab/import_export/fork_spec.rb b/spec/lib/gitlab/import_export/fork_spec.rb index 70796781532..e8eb7e4f8f4 100644 --- a/spec/lib/gitlab/import_export/fork_spec.rb +++ b/spec/lib/gitlab/import_export/fork_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' describe 'forked project import', services: true do let(:user) { create(:user) } - let!(:project_with_repo) { create(:project, :test_repo, name: 'test-repo-restorer', path: 'test-repo-restorer') } + let!(:project_with_repo) { create(:project, name: 'test-repo-restorer', path: 'test-repo-restorer') } let!(:project) { create(:empty_project, name: 'test-repo-restorer-no-repo', path: 'test-repo-restorer-no-repo') } let(:export_path) { "#{Dir.tmpdir}/project_tree_saver_spec" } let(:shared) { Gitlab::ImportExport::Shared.new(relative_path: project.path_with_namespace) } diff --git a/spec/lib/gitlab/import_export/merge_request_parser_spec.rb b/spec/lib/gitlab/import_export/merge_request_parser_spec.rb index 349be4596b6..f2b66c4421c 100644 --- a/spec/lib/gitlab/import_export/merge_request_parser_spec.rb +++ b/spec/lib/gitlab/import_export/merge_request_parser_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' describe Gitlab::ImportExport::MergeRequestParser do let(:user) { create(:user) } - let!(:project) { create(:project, :test_repo, name: 'test-repo-restorer', path: 'test-repo-restorer') } + let!(:project) { create(:project, name: 'test-repo-restorer', path: 'test-repo-restorer') } let(:forked_from_project) { create(:project) } let(:fork_link) { create(:forked_project_link, forked_from_project: project) } diff --git a/spec/lib/gitlab/import_export/repo_restorer_spec.rb b/spec/lib/gitlab/import_export/repo_restorer_spec.rb index 30b6a0d8845..09bfaa8fb75 100644 --- a/spec/lib/gitlab/import_export/repo_restorer_spec.rb +++ b/spec/lib/gitlab/import_export/repo_restorer_spec.rb @@ -3,7 +3,7 @@ require 'spec_helper' describe Gitlab::ImportExport::RepoRestorer, services: true do describe 'bundle a project Git repo' do let(:user) { create(:user) } - let!(:project_with_repo) { create(:project, :test_repo, name: 'test-repo-restorer', path: 'test-repo-restorer') } + let!(:project_with_repo) { create(:project, name: 'test-repo-restorer', path: 'test-repo-restorer') } let!(:project) { create(:empty_project) } let(:export_path) { "#{Dir.tmpdir}/project_tree_saver_spec" } let(:shared) { Gitlab::ImportExport::Shared.new(relative_path: project.path_with_namespace) } diff --git a/spec/lib/gitlab/import_export/safe_model_attributes.yml b/spec/lib/gitlab/import_export/safe_model_attributes.yml index 4ef3db3721f..11f4c16ff96 100644 --- a/spec/lib/gitlab/import_export/safe_model_attributes.yml +++ b/spec/lib/gitlab/import_export/safe_model_attributes.yml @@ -195,6 +195,7 @@ MergeRequestDiffFile: - a_mode - b_mode - too_large +- binary Ci::Pipeline: - id - project_id @@ -396,6 +397,7 @@ Project: - build_allow_git_fetch - last_repository_updated_at - ci_config_path +- delete_error Author: - name ProjectFeature: diff --git a/spec/lib/gitlab/ldap/adapter_spec.rb b/spec/lib/gitlab/ldap/adapter_spec.rb index 9454878b057..0f4b8dbf7b7 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, lib: true 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 mail dn}) + expect(arg[:attributes]).to match(%w{uid cn dn uid userid sAMAccountName mail email userPrincipalName}) end.and_return({}) adapter.users('uid', 'johndoe') @@ -26,7 +26,7 @@ describe Gitlab::LDAP::Adapter, lib: true 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 mail dn}, + attributes: %w{uid cn dn uid userid sAMAccountName mail email userPrincipalName}, filter: nil ).and_return({}) @@ -63,7 +63,7 @@ describe Gitlab::LDAP::Adapter, lib: true 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 mail dn}) + hash_including(attributes: %w{sAMAccountName cn dn uid userid sAMAccountName mail email userPrincipalName}) ).and_return({}) adapter.users('sAMAccountName', 'johndoe') diff --git a/spec/lib/gitlab/ldap/config_spec.rb b/spec/lib/gitlab/ldap/config_spec.rb index cab2e9908ff..3a56797d68b 100644 --- a/spec/lib/gitlab/ldap/config_spec.rb +++ b/spec/lib/gitlab/ldap/config_spec.rb @@ -5,7 +5,7 @@ describe Gitlab::LDAP::Config, lib: true do let(:config) { Gitlab::LDAP::Config.new('ldapmain') } - describe '#initalize' do + describe '#initialize' do it 'requires a provider' do expect{ Gitlab::LDAP::Config.new }.to raise_error ArgumentError end @@ -23,9 +23,9 @@ describe Gitlab::LDAP::Config, lib: true do it 'constructs basic options' do stub_ldap_config( options: { - 'host' => 'ldap.example.com', - 'port' => 386, - 'method' => 'plain' + 'host' => 'ldap.example.com', + 'port' => 386, + 'encryption' => 'plain' } ) @@ -39,24 +39,140 @@ describe Gitlab::LDAP::Config, lib: true do it 'includes authentication options when auth is configured' do stub_ldap_config( options: { - 'host' => 'ldap.example.com', - 'port' => 686, - 'method' => 'ssl', - 'bind_dn' => 'uid=admin,dc=example,dc=com', - 'password' => 'super_secret' + 'host' => 'ldap.example.com', + 'port' => 686, + 'encryption' => 'simple_tls', + 'verify_certificates' => true, + 'bind_dn' => 'uid=admin,dc=example,dc=com', + 'password' => 'super_secret' } ) - expect(config.adapter_options).to eq( - host: 'ldap.example.com', - port: 686, - encryption: :simple_tls, + expect(config.adapter_options).to include({ auth: { method: :simple, username: 'uid=admin,dc=example,dc=com', password: 'super_secret' } + }) + end + + it 'sets encryption method to simple_tls when configured as simple_tls' do + stub_ldap_config( + options: { + 'host' => 'ldap.example.com', + 'port' => 686, + 'encryption' => 'simple_tls' + } ) + + expect(config.adapter_options[:encryption]).to include({ method: :simple_tls }) + end + + it 'sets encryption method to start_tls when configured as start_tls' do + stub_ldap_config( + options: { + 'host' => 'ldap.example.com', + 'port' => 686, + 'encryption' => 'start_tls' + } + ) + + expect(config.adapter_options[:encryption]).to include({ method: :start_tls }) + end + + context 'when verify_certificates is enabled' do + it 'sets tls_options to OpenSSL defaults' do + stub_ldap_config( + options: { + 'host' => 'ldap.example.com', + 'port' => 686, + 'encryption' => 'simple_tls', + 'verify_certificates' => true + } + ) + + expect(config.adapter_options[:encryption]).to include({ tls_options: OpenSSL::SSL::SSLContext::DEFAULT_PARAMS }) + end + end + + context 'when verify_certificates is disabled' do + it 'sets verify_mode to OpenSSL VERIFY_NONE' do + stub_ldap_config( + options: { + 'host' => 'ldap.example.com', + 'port' => 686, + 'encryption' => 'simple_tls', + 'verify_certificates' => false + } + ) + + expect(config.adapter_options[:encryption]).to include({ + tls_options: { + verify_mode: OpenSSL::SSL::VERIFY_NONE + } + }) + end + end + + context 'when ca_file is specified' do + it 'passes it through in tls_options' do + stub_ldap_config( + options: { + 'host' => 'ldap.example.com', + 'port' => 686, + 'encryption' => 'simple_tls', + 'ca_file' => '/etc/ca.pem' + } + ) + + expect(config.adapter_options[:encryption][:tls_options]).to include({ ca_file: '/etc/ca.pem' }) + end + end + + context 'when ca_file is a blank string' do + it 'does not add the ca_file key to tls_options' do + stub_ldap_config( + options: { + 'host' => 'ldap.example.com', + 'port' => 686, + 'encryption' => 'simple_tls', + 'ca_file' => ' ' + } + ) + + expect(config.adapter_options[:encryption][:tls_options]).not_to have_key(:ca_file) + end + end + + context 'when ssl_version is specified' do + it 'passes it through in tls_options' do + stub_ldap_config( + options: { + 'host' => 'ldap.example.com', + 'port' => 686, + 'encryption' => 'simple_tls', + 'ssl_version' => 'TLSv1_2' + } + ) + + expect(config.adapter_options[:encryption][:tls_options]).to include({ ssl_version: 'TLSv1_2' }) + end + end + + context 'when ssl_version is a blank string' do + it 'does not add the ssl_version key to tls_options' do + stub_ldap_config( + options: { + 'host' => 'ldap.example.com', + 'port' => 686, + 'encryption' => 'simple_tls', + 'ssl_version' => ' ' + } + ) + + expect(config.adapter_options[:encryption][:tls_options]).not_to have_key(:ssl_version) + end end end @@ -64,11 +180,11 @@ describe Gitlab::LDAP::Config, lib: true do it 'constructs basic options' do stub_ldap_config( options: { - 'host' => 'ldap.example.com', - 'port' => 386, - 'base' => 'ou=users,dc=example,dc=com', - 'method' => 'plain', - 'uid' => 'uid' + 'host' => 'ldap.example.com', + 'port' => 386, + 'base' => 'ou=users,dc=example,dc=com', + 'encryption' => 'plain', + 'uid' => 'uid' } ) @@ -76,7 +192,7 @@ describe Gitlab::LDAP::Config, lib: true do host: 'ldap.example.com', port: 386, base: 'ou=users,dc=example,dc=com', - method: 'plain', + encryption: 'plain', filter: '(uid=%{username})' ) expect(config.omniauth_options.keys).not_to include(:bind_dn, :password) @@ -98,6 +214,100 @@ describe Gitlab::LDAP::Config, lib: true do password: 'super_secret' ) end + + context 'when verify_certificates is enabled' do + it 'specifies disable_verify_certificates as false' do + stub_ldap_config( + options: { + 'host' => 'ldap.example.com', + 'port' => 686, + 'encryption' => 'simple_tls', + 'verify_certificates' => true + } + ) + + expect(config.omniauth_options).to include({ disable_verify_certificates: false }) + end + end + + context 'when verify_certificates is disabled' do + it 'specifies disable_verify_certificates as true' do + stub_ldap_config( + options: { + 'host' => 'ldap.example.com', + 'port' => 686, + 'encryption' => 'simple_tls', + 'verify_certificates' => false + } + ) + + expect(config.omniauth_options).to include({ disable_verify_certificates: true }) + end + end + + context 'when ca_file is present' do + it 'passes it through' do + stub_ldap_config( + options: { + 'host' => 'ldap.example.com', + 'port' => 686, + 'encryption' => 'simple_tls', + 'verify_certificates' => true, + 'ca_file' => '/etc/ca.pem' + } + ) + + expect(config.omniauth_options).to include({ ca_file: '/etc/ca.pem' }) + end + end + + context 'when ca_file is blank' do + it 'does not include the ca_file option' do + stub_ldap_config( + options: { + 'host' => 'ldap.example.com', + 'port' => 686, + 'encryption' => 'simple_tls', + 'verify_certificates' => true, + 'ca_file' => ' ' + } + ) + + expect(config.omniauth_options).not_to have_key(:ca_file) + end + end + + context 'when ssl_version is present' do + it 'passes it through' do + stub_ldap_config( + options: { + 'host' => 'ldap.example.com', + 'port' => 686, + 'encryption' => 'simple_tls', + 'verify_certificates' => true, + 'ssl_version' => 'TLSv1_2' + } + ) + + expect(config.omniauth_options).to include({ ssl_version: 'TLSv1_2' }) + end + end + + context 'when ssl_version is blank' do + it 'does not include the ssl_version option' do + stub_ldap_config( + options: { + 'host' => 'ldap.example.com', + 'port' => 686, + 'encryption' => 'simple_tls', + 'verify_certificates' => true, + 'ssl_version' => ' ' + } + ) + + expect(config.omniauth_options).not_to have_key(:ssl_version) + end + end end describe '#has_auth?' do diff --git a/spec/lib/gitlab/path_regex_spec.rb b/spec/lib/gitlab/path_regex_spec.rb index 1eea710c80b..20be743d224 100644 --- a/spec/lib/gitlab/path_regex_spec.rb +++ b/spec/lib/gitlab/path_regex_spec.rb @@ -36,9 +36,12 @@ describe Gitlab::PathRegex, lib: true do described_class::PROJECT_WILDCARD_ROUTES.include?(path.split('/').first) end - def failure_message(missing_words, constant_name, migration_helper) + def failure_message(constant_name, migration_helper, missing_words: [], additional_words: []) missing_words = Array(missing_words) - <<-MSG + additional_words = Array(additional_words) + message = "" + if missing_words.any? + message += <<-MISSING Found new routes that could cause conflicts with existing namespaced routes for groups or projects. @@ -51,7 +54,18 @@ describe Gitlab::PathRegex, lib: true do Make sure to make a note of the renamed records in the release blog post. - MSG + MISSING + end + + if additional_words.any? + message += <<-ADDITIONAL + Why are <#{additional_words.join(', ')}> in `#{constant_name}`? + If they are really required, update these specs to reflect that. + + ADDITIONAL + end + + message end let(:all_routes) do @@ -68,9 +82,23 @@ describe Gitlab::PathRegex, lib: true do let(:routes_not_starting_in_wildcard) { routes_without_format.select { |p| p !~ %r{^/[:*]} } } let(:top_level_words) do - routes_not_starting_in_wildcard.map do |route| + words = routes_not_starting_in_wildcard.map do |route| route.split('/')[1] end.compact.uniq + + words + ee_top_level_words + files_in_public + Array(API::API.prefix.to_s) + end + + let(:ee_top_level_words) do + ['unsubscribes'] + end + + let(:files_in_public) do + git = Gitlab.config.git.bin_path + `cd #{Rails.root} && #{git} ls-files public` + .split("\n") + .map { |entry| entry.gsub('public/', '') } + .uniq end # All routes that start with a namespaced path, that have 1 or more @@ -115,18 +143,29 @@ describe Gitlab::PathRegex, lib: true do let(:paths_after_group_id) do group_routes.map do |route| route.gsub(STARTING_WITH_GROUP, '').split('/').first - end.uniq + end.uniq + ee_paths_after_group_id + end + + let(:ee_paths_after_group_id) do + %w(analytics + ldap + ldap_group_links + notification_setting + audit_events + pipeline_quota hooks) end describe 'TOP_LEVEL_ROUTES' do it 'includes all the top level namespaces' do failure_block = lambda do missing_words = top_level_words - described_class::TOP_LEVEL_ROUTES - failure_message(missing_words, 'TOP_LEVEL_ROUTES', 'rename_root_paths') + additional_words = described_class::TOP_LEVEL_ROUTES - top_level_words + failure_message('TOP_LEVEL_ROUTES', 'rename_root_paths', + missing_words: missing_words, additional_words: additional_words) end expect(described_class::TOP_LEVEL_ROUTES) - .to include(*top_level_words), failure_block + .to contain_exactly(*top_level_words), failure_block end end @@ -134,11 +173,13 @@ describe Gitlab::PathRegex, lib: true do it "don't contain a second wildcard" do failure_block = lambda do missing_words = paths_after_group_id - described_class::GROUP_ROUTES - failure_message(missing_words, 'GROUP_ROUTES', 'rename_child_paths') + additional_words = described_class::GROUP_ROUTES - paths_after_group_id + failure_message('GROUP_ROUTES', 'rename_child_paths', + missing_words: missing_words, additional_words: additional_words) end expect(described_class::GROUP_ROUTES) - .to include(*paths_after_group_id), failure_block + .to contain_exactly(*paths_after_group_id), failure_block end end @@ -147,7 +188,7 @@ describe Gitlab::PathRegex, lib: true do aggregate_failures do all_wildcard_paths.each do |path| expect(wildcards_include?(path)) - .to be(true), failure_message(path, 'PROJECT_WILDCARD_ROUTES', 'rename_wildcard_paths') + .to be(true), failure_message('PROJECT_WILDCARD_ROUTES', 'rename_wildcard_paths', missing_words: path) end end end diff --git a/spec/lib/gitlab/request_forgery_protection_spec.rb b/spec/lib/gitlab/request_forgery_protection_spec.rb new file mode 100644 index 00000000000..305de613866 --- /dev/null +++ b/spec/lib/gitlab/request_forgery_protection_spec.rb @@ -0,0 +1,89 @@ +require 'spec_helper' + +describe Gitlab::RequestForgeryProtection, :allow_forgery_protection do + let(:csrf_token) { SecureRandom.base64(ActionController::RequestForgeryProtection::AUTHENTICITY_TOKEN_LENGTH) } + let(:env) do + { + 'rack.input' => '', + 'rack.session' => { + _csrf_token: csrf_token + } + } + end + + describe '.call' do + context 'when the request method is GET' do + before do + env['REQUEST_METHOD'] = 'GET' + end + + it 'does not raise an exception' do + expect { described_class.call(env) }.not_to raise_exception + end + end + + context 'when the request method is POST' do + before do + env['REQUEST_METHOD'] = 'POST' + end + + context 'when the CSRF token is valid' do + before do + env['HTTP_X_CSRF_TOKEN'] = csrf_token + end + + it 'does not raise an exception' do + expect { described_class.call(env) }.not_to raise_exception + end + end + + context 'when the CSRF token is invalid' do + before do + env['HTTP_X_CSRF_TOKEN'] = 'foo' + end + + it 'raises an ActionController::InvalidAuthenticityToken exception' do + expect { described_class.call(env) }.to raise_exception(ActionController::InvalidAuthenticityToken) + end + end + end + end + + describe '.verified?' do + context 'when the request method is GET' do + before do + env['REQUEST_METHOD'] = 'GET' + end + + it 'returns true' do + expect(described_class.verified?(env)).to be_truthy + end + end + + context 'when the request method is POST' do + before do + env['REQUEST_METHOD'] = 'POST' + end + + context 'when the CSRF token is valid' do + before do + env['HTTP_X_CSRF_TOKEN'] = csrf_token + end + + it 'returns true' do + expect(described_class.verified?(env)).to be_truthy + end + end + + context 'when the CSRF token is invalid' do + before do + env['HTTP_X_CSRF_TOKEN'] = 'foo' + end + + it 'returns false' do + expect(described_class.verified?(env)).to be_falsey + end + end + end + end +end diff --git a/spec/lib/gitlab/workhorse_spec.rb b/spec/lib/gitlab/workhorse_spec.rb index 7b39441e76e..6ca1edb01b9 100644 --- a/spec/lib/gitlab/workhorse_spec.rb +++ b/spec/lib/gitlab/workhorse_spec.rb @@ -237,7 +237,8 @@ describe Gitlab::Workhorse, lib: true do context 'when action is not enabled by feature flag' do it 'does not include Gitaly params in the returned value' do - allow(Gitlab::GitalyClient).to receive(:feature_enabled?).with(feature_flag).and_return(false) + status_opt_out = Gitlab::GitalyClient::MigrationStatus::OPT_OUT + allow(Gitlab::GitalyClient).to receive(:feature_enabled?).with(feature_flag, status: status_opt_out).and_return(false) expect(subject).not_to include(gitaly_params) end diff --git a/spec/models/ci/pipeline_spec.rb b/spec/models/ci/pipeline_spec.rb index ba0696fa210..bbd45f10b1b 100644 --- a/spec/models/ci/pipeline_spec.rb +++ b/spec/models/ci/pipeline_spec.rb @@ -734,6 +734,8 @@ describe Ci::Pipeline, models: true do context 'on failure and build retry' do before do + stub_not_protect_default_branch + build.drop project.add_developer(user) @@ -999,6 +1001,8 @@ describe Ci::Pipeline, models: true do let(:latest_status) { pipeline.statuses.latest.pluck(:status) } before do + stub_not_protect_default_branch + project.add_developer(user) end diff --git a/spec/models/merge_request_diff_file_spec.rb b/spec/models/merge_request_diff_file_spec.rb index 7276f5b5061..239620ef4fc 100644 --- a/spec/models/merge_request_diff_file_spec.rb +++ b/spec/models/merge_request_diff_file_spec.rb @@ -1,8 +1,33 @@ require 'rails_helper' describe MergeRequestDiffFile, type: :model do + describe '#diff' do + let(:unpacked) { 'unpacked' } + let(:packed) { [unpacked].pack('m0') } + + before do + subject.diff = packed + end + + context 'when the diff is marked as binary' do + before do + subject.binary = true + end + + it 'unpacks from base 64' do + expect(subject.diff).to eq(unpacked) + end + end + + context 'when the diff is not marked as binary' do + it 'returns the raw diff' do + expect(subject.diff).to eq(packed) + end + end + end + describe '#utf8_diff' do - it 'does not raise error when a hash value is in binary' do + it 'does not raise error when the diff is binary' do subject.diff = "\x05\x00\x68\x65\x6c\x6c\x6f" expect { subject.utf8_diff }.not_to raise_error diff --git a/spec/models/merge_request_diff_spec.rb b/spec/models/merge_request_diff_spec.rb index edc2f4bb9f0..0e77752bccc 100644 --- a/spec/models/merge_request_diff_spec.rb +++ b/spec/models/merge_request_diff_spec.rb @@ -105,6 +105,15 @@ describe MergeRequestDiff, models: true do expect(mr_diff.empty?).to be_truthy end + + it 'saves binary diffs correctly' do + path = 'files/images/icn-time-tracking.pdf' + mr_diff = create(:merge_request, source_branch: 'add-pdf-text-binary', target_branch: 'master').merge_request_diff + diff_file = mr_diff.merge_request_diff_files.find_by(new_path: path) + + expect(diff_file).to be_binary + expect(diff_file.diff).to eq(mr_diff.compare.diffs(paths: [path]).to_a.first.diff) + end end describe '#commit_shas' do diff --git a/spec/models/project_services/jira_service_spec.rb b/spec/models/project_services/jira_service_spec.rb index 105afed1337..d7d09808a98 100644 --- a/spec/models/project_services/jira_service_spec.rb +++ b/spec/models/project_services/jira_service_spec.rb @@ -15,7 +15,6 @@ describe JiraService, models: true do end it { is_expected.to validate_presence_of(:url) } - it { is_expected.to validate_presence_of(:project_key) } it_behaves_like 'issue tracker service URL attribute', :url end @@ -34,7 +33,6 @@ describe JiraService, models: true do active: true, username: 'username', password: 'test', - project_key: 'TEST', jira_issue_transition_id: 24, url: 'http://jira.test.com' ) @@ -88,7 +86,6 @@ describe JiraService, models: true do url: 'http://jira.example.com', username: 'gitlab_jira_username', password: 'gitlab_jira_password', - project_key: 'GitLabProject', jira_issue_transition_id: "custom-id" ) @@ -196,15 +193,14 @@ describe JiraService, models: true do project: create(:project), url: 'http://jira.example.com', username: 'jira_username', - password: 'jira_password', - project_key: 'GitLabProject' + password: 'jira_password' ) end def test_settings(api_url) - project_url = "http://#{api_url}/rest/api/2/project/GitLabProject" + test_url = "http://#{api_url}/rest/api/2/serverInfo" - WebMock.stub_request(:get, project_url).with(basic_auth: %w(jira_username jira_password)) + WebMock.stub_request(:get, test_url).with(basic_auth: %w(jira_username jira_password)).to_return(body: { url: 'http://url' }.to_json ) jira_service.test_settings end diff --git a/spec/models/project_wiki_spec.rb b/spec/models/project_wiki_spec.rb index 1f314791479..79ab50c1234 100644 --- a/spec/models/project_wiki_spec.rb +++ b/spec/models/project_wiki_spec.rb @@ -21,7 +21,7 @@ describe ProjectWiki, models: true do describe '#web_url' do it 'returns the full web URL to the wiki' do - expect(subject.web_url).to eq("#{Gitlab.config.gitlab.url}/#{project.path_with_namespace}/wikis/home") + expect(subject.web_url).to match("https?://[^\/]+/#{project.path_with_namespace}/wikis/home") end end diff --git a/spec/models/repository_spec.rb b/spec/models/repository_spec.rb index 7635b0868e7..fcda4248446 100644 --- a/spec/models/repository_spec.rb +++ b/spec/models/repository_spec.rb @@ -956,21 +956,25 @@ describe Repository, models: true do end end - describe '#exists?' do + shared_examples 'repo exists check' do it 'returns true when a repository exists' do expect(repository.exists?).to eq(true) end - it 'returns false when a repository does not exist' do - allow(repository).to receive(:refs_directory_exists?).and_return(false) + it 'returns false if no full path can be constructed' do + allow(repository).to receive(:path_with_namespace).and_return(nil) expect(repository.exists?).to eq(false) end + end - it 'returns false when there is no namespace' do - allow(repository).to receive(:path_with_namespace).and_return(nil) + describe '#exists?' do + context 'when repository_exists is disabled' do + it_behaves_like 'repo exists check' + end - expect(repository.exists?).to eq(false) + context 'when repository_exists is enabled', skip_gitaly_mock: true do + it_behaves_like 'repo exists check' end end diff --git a/spec/policies/ci/build_policy_spec.rb b/spec/policies/ci/build_policy_spec.rb index 9f3212b1a63..e3ea3c960a4 100644 --- a/spec/policies/ci/build_policy_spec.rb +++ b/spec/policies/ci/build_policy_spec.rb @@ -96,87 +96,57 @@ describe Ci::BuildPolicy, :models do end end - describe 'rules for manual actions' do + describe 'rules for protected ref' do let(:project) { create(:project) } + let(:build) { create(:ci_build, ref: 'some-ref', pipeline: pipeline) } before do project.add_developer(user) end - shared_examples 'protected ref' do - context 'when build is a manual action' do - let(:build) do - create(:ci_build, :manual, ref: 'some-ref', pipeline: pipeline) - end - - it 'does not include ability to update build' do - expect(policy).to be_disallowed :update_build - end + context 'when no one can push or merge to the branch' do + before do + create(:protected_branch, :no_one_can_push, + name: build.ref, project: project) end - context 'when build is not a manual action' do - let(:build) do - create(:ci_build, ref: 'some-ref', pipeline: pipeline) - end - - it 'includes ability to update build' do - expect(policy).to be_allowed :update_build - end + it 'does not include ability to update build' do + expect(policy).to be_disallowed :update_build end end - context 'when build is against a protected branch' do + context 'when developers can push to the branch' do before do - create(:protected_branch, :no_one_can_push, - name: 'some-ref', project: project) + create(:protected_branch, :developers_can_merge, + name: build.ref, project: project) end - it_behaves_like 'protected ref' + it 'includes ability to update build' do + expect(policy).to be_allowed :update_build + end end - context 'when build is against a protected tag' do + context 'when no one can create the tag' do before do create(:protected_tag, :no_one_can_create, - name: 'some-ref', project: project) + name: build.ref, project: project) build.update(tag: true) end - it_behaves_like 'protected ref' + it 'does not include ability to update build' do + expect(policy).to be_disallowed :update_build + end end - context 'when build is against a protected tag but it is not a tag' do + context 'when no one can create the tag but it is not a tag' do before do create(:protected_tag, :no_one_can_create, - name: 'some-ref', project: project) + name: build.ref, project: project) end - context 'when build is a manual action' do - let(:build) do - create(:ci_build, :manual, ref: 'some-ref', pipeline: pipeline) - end - - it 'includes ability to update build' do - expect(policy).to be_allowed :update_build - end - end - end - - context 'when branch build is assigned to is not protected' do - context 'when build is a manual action' do - let(:build) { create(:ci_build, :manual, pipeline: pipeline) } - - it 'includes ability to update build' do - expect(policy).to be_allowed :update_build - end - end - - context 'when build is not a manual action' do - let(:build) { create(:ci_build, pipeline: pipeline) } - - it 'includes ability to update build' do - expect(policy).to be_allowed :update_build - end + it 'includes ability to update build' do + expect(policy).to be_allowed :update_build end end end diff --git a/spec/policies/ci/pipeline_policy_spec.rb b/spec/policies/ci/pipeline_policy_spec.rb new file mode 100644 index 00000000000..b11b06d301f --- /dev/null +++ b/spec/policies/ci/pipeline_policy_spec.rb @@ -0,0 +1,66 @@ +require 'spec_helper' + +describe Ci::PipelinePolicy, :models do + let(:user) { create(:user) } + let(:pipeline) { create(:ci_empty_pipeline, project: project) } + + let(:policy) do + described_class.new(user, pipeline) + end + + describe 'rules' do + describe 'rules for protected ref' do + let(:project) { create(:project) } + + before do + project.add_developer(user) + end + + context 'when no one can push or merge to the branch' do + before do + create(:protected_branch, :no_one_can_push, + name: pipeline.ref, project: project) + end + + it 'does not include ability to update pipeline' do + expect(policy).to be_disallowed :update_pipeline + end + end + + context 'when developers can push to the branch' do + before do + create(:protected_branch, :developers_can_merge, + name: pipeline.ref, project: project) + end + + it 'includes ability to update pipeline' do + expect(policy).to be_allowed :update_pipeline + end + end + + context 'when no one can create the tag' do + before do + create(:protected_tag, :no_one_can_create, + name: pipeline.ref, project: project) + + pipeline.update(tag: true) + end + + it 'does not include ability to update pipeline' do + expect(policy).to be_disallowed :update_pipeline + end + end + + context 'when no one can create the tag but it is not a tag' do + before do + create(:protected_tag, :no_one_can_create, + name: pipeline.ref, project: project) + end + + it 'includes ability to update pipeline' do + expect(policy).to be_allowed :update_pipeline + end + end + end + end +end diff --git a/spec/policies/global_policy_spec.rb b/spec/policies/global_policy_spec.rb index bb0fa0c0e9c..c3e2b603c4b 100644 --- a/spec/policies/global_policy_spec.rb +++ b/spec/policies/global_policy_spec.rb @@ -30,5 +30,25 @@ describe GlobalPolicy, models: true do it { is_expected.to be_allowed(:read_users_list) } end end + + context "for an admin" do + let(:current_user) { create(:admin) } + + context "when the public level is restricted" do + before do + stub_application_setting(restricted_visibility_levels: [Gitlab::VisibilityLevel::PUBLIC]) + end + + it { is_expected.to be_allowed(:read_users_list) } + end + + context "when the public level is not restricted" do + before do + stub_application_setting(restricted_visibility_levels: []) + end + + it { is_expected.to be_allowed(:read_users_list) } + end + end end end diff --git a/spec/requests/api/branches_spec.rb b/spec/requests/api/branches_spec.rb index c64499fc8c0..5a2e1b2cf2d 100644 --- a/spec/requests/api/branches_spec.rb +++ b/spec/requests/api/branches_spec.rb @@ -1,25 +1,31 @@ require 'spec_helper' -require 'mime/types' describe API::Branches do let(:user) { create(:user) } - let!(:project) { create(:project, :repository, creator: user) } - let!(:master) { create(:project_member, :master, user: user, project: project) } - let(:guest) { create(:user).tap { |u| create(:project_member, :guest, user: u, project: project) } } - let!(:branch_name) { 'feature' } - let!(:branch_sha) { '0b4bc9a49b562e85de7cc9e834518ea6828729b9' } - let(:branch_with_dot) { CreateBranchService.new(project, user).execute("with.1.2.3", "master")[:branch] } + let(:guest) { create(:user).tap { |u| project.add_guest(u) } } + let(:project) { create(:project, :repository, creator: user, path: 'my.project') } + let(:branch_name) { 'feature' } + let(:branch_sha) { '0b4bc9a49b562e85de7cc9e834518ea6828729b9' } + let(:branch_with_dot) { project.repository.find_branch('ends-with.json') } + let(:branch_with_slash) { project.repository.find_branch('improve/awesome') } + + let(:project_id) { project.id } + let(:current_user) { nil } + + before do + project.add_master(user) + end describe "GET /projects/:id/repository/branches" do - let(:route) { "/projects/#{project.id}/repository/branches" } + let(:route) { "/projects/#{project_id}/repository/branches" } shared_examples_for 'repository branches' do it 'returns the repository branches' do get api(route, current_user), per_page: 100 - expect(response).to have_http_status(200) + expect(response).to have_gitlab_http_status(200) + expect(response).to match_response_schema('public_api/v4/branches') expect(response).to include_pagination_headers - expect(json_response).to be_an Array branch_names = json_response.map { |x| x['name'] } expect(branch_names).to match_array(project.repository.branch_names) end @@ -34,10 +40,9 @@ describe API::Branches do end context 'when unauthenticated', 'and project is public' do - it_behaves_like 'repository branches' do - let(:project) { create(:project, :public, :repository) } - let(:current_user) { nil } - end + let(:project) { create(:project, :public, :repository) } + + it_behaves_like 'repository branches' end context 'when unauthenticated', 'and project is private' do @@ -47,9 +52,15 @@ describe API::Branches do end end - context 'when authenticated', 'as a developer' do - it_behaves_like 'repository branches' do - let(:current_user) { user } + context 'when authenticated', 'as a master' do + let(:current_user) { user } + + it_behaves_like 'repository branches' + + context 'requesting with the escaped project full path' do + let(:project_id) { CGI.escape(project.full_path) } + + it_behaves_like 'repository branches' end end @@ -61,31 +72,15 @@ describe API::Branches do end describe "GET /projects/:id/repository/branches/:branch" do - let(:route) { "/projects/#{project.id}/repository/branches/#{branch_name}" } + let(:route) { "/projects/#{project_id}/repository/branches/#{branch_name}" } - shared_examples_for 'repository branch' do |merged: false| + shared_examples_for 'repository branch' do it 'returns the repository branch' do get api(route, current_user) - expect(response).to have_http_status(200) - expect(json_response['name']).to eq(branch_name) - expect(json_response['merged']).to eq(merged) - expect(json_response['protected']).to eq(false) - expect(json_response['developers_can_push']).to eq(false) - expect(json_response['developers_can_merge']).to eq(false) - - json_commit = json_response['commit'] - expect(json_commit['id']).to eq(branch_sha) - expect(json_commit).to have_key('short_id') - expect(json_commit).to have_key('title') - expect(json_commit).to have_key('message') - expect(json_commit).to have_key('author_name') - expect(json_commit).to have_key('author_email') - expect(json_commit).to have_key('authored_date') - expect(json_commit).to have_key('committer_name') - expect(json_commit).to have_key('committer_email') - expect(json_commit).to have_key('committed_date') - expect(json_commit).to have_key('parent_ids') + expect(response).to have_gitlab_http_status(200) + expect(response).to match_response_schema('public_api/v4/branch') + expect(json_response['name']).to eq(CGI.unescape(branch_name)) end context 'when branch does not exist' do @@ -107,10 +102,9 @@ describe API::Branches do end context 'when unauthenticated', 'and project is public' do - it_behaves_like 'repository branch' do - let(:project) { create(:project, :public, :repository) } - let(:current_user) { nil } - end + let(:project) { create(:project, :public, :repository) } + + it_behaves_like 'repository branch' end context 'when unauthenticated', 'and project is private' do @@ -120,22 +114,41 @@ describe API::Branches do end end - context 'when authenticated', 'as a developer' do + context 'when authenticated', 'as a master' do let(:current_user) { user } + it_behaves_like 'repository branch' context 'when branch contains a dot' do let(:branch_name) { branch_with_dot.name } - let(:branch_sha) { project.commit('master').sha } it_behaves_like 'repository branch' end - context 'when branch is merged' do - let(:branch_name) { 'merge-test' } - let(:branch_sha) { project.commit('merge-test').sha } + context 'when branch contains a slash' do + let(:branch_name) { branch_with_slash.name } + + it_behaves_like '404 response' do + let(:request) { get api(route, current_user) } + end + end + + context 'when branch contains an escaped slash' do + let(:branch_name) { CGI.escape(branch_with_slash.name) } + + it_behaves_like 'repository branch' + end + + context 'requesting with the escaped project full path' do + let(:project_id) { CGI.escape(project.full_path) } + + it_behaves_like 'repository branch' - it_behaves_like 'repository branch', merged: true + context 'when branch contains a dot' do + let(:branch_name) { branch_with_dot.name } + + it_behaves_like 'repository branch' + end end end @@ -147,268 +160,348 @@ describe API::Branches do end describe 'PUT /projects/:id/repository/branches/:branch/protect' do - context "when a protected branch doesn't already exist" do - it 'protects a single branch' do - put api("/projects/#{project.id}/repository/branches/#{branch_name}/protect", user) + let(:route) { "/projects/#{project_id}/repository/branches/#{branch_name}/protect" } - expect(response).to have_http_status(200) - expect(json_response['name']).to eq(branch_name) - expect(json_response['commit']['id']).to eq(branch_sha) - expect(json_response['protected']).to eq(true) - expect(json_response['developers_can_push']).to eq(false) - expect(json_response['developers_can_merge']).to eq(false) - end - - it "protects a single branch with dots in the name" do - put api("/projects/#{project.id}/repository/branches/#{branch_with_dot.name}/protect", user) + shared_examples_for 'repository new protected branch' do + it 'protects a single branch' do + put api(route, current_user) - expect(response).to have_http_status(200) - expect(json_response['name']).to eq(branch_with_dot.name) + expect(response).to have_gitlab_http_status(200) + expect(response).to match_response_schema('public_api/v4/branch') + expect(json_response['name']).to eq(CGI.unescape(branch_name)) expect(json_response['protected']).to eq(true) end it 'protects a single branch and developers can push' do - put api("/projects/#{project.id}/repository/branches/#{branch_name}/protect", user), - developers_can_push: true + put api(route, current_user), developers_can_push: true - expect(response).to have_http_status(200) - expect(json_response['name']).to eq(branch_name) - expect(json_response['commit']['id']).to eq(branch_sha) + expect(response).to have_gitlab_http_status(200) + expect(response).to match_response_schema('public_api/v4/branch') + expect(json_response['name']).to eq(CGI.unescape(branch_name)) expect(json_response['protected']).to eq(true) expect(json_response['developers_can_push']).to eq(true) expect(json_response['developers_can_merge']).to eq(false) end it 'protects a single branch and developers can merge' do - put api("/projects/#{project.id}/repository/branches/#{branch_name}/protect", user), - developers_can_merge: true + put api(route, current_user), developers_can_merge: true - expect(response).to have_http_status(200) - expect(json_response['name']).to eq(branch_name) - expect(json_response['commit']['id']).to eq(branch_sha) + expect(response).to have_gitlab_http_status(200) + expect(response).to match_response_schema('public_api/v4/branch') + expect(json_response['name']).to eq(CGI.unescape(branch_name)) expect(json_response['protected']).to eq(true) expect(json_response['developers_can_push']).to eq(false) expect(json_response['developers_can_merge']).to eq(true) end it 'protects a single branch and developers can push and merge' do - put api("/projects/#{project.id}/repository/branches/#{branch_name}/protect", user), - developers_can_push: true, developers_can_merge: true + put api(route, current_user), developers_can_push: true, developers_can_merge: true - expect(response).to have_http_status(200) - expect(json_response['name']).to eq(branch_name) - expect(json_response['commit']['id']).to eq(branch_sha) + expect(response).to have_gitlab_http_status(200) + expect(response).to match_response_schema('public_api/v4/branch') + expect(json_response['name']).to eq(CGI.unescape(branch_name)) expect(json_response['protected']).to eq(true) expect(json_response['developers_can_push']).to eq(true) expect(json_response['developers_can_merge']).to eq(true) end + + context 'when branch does not exist' do + let(:branch_name) { 'unknown' } + + it_behaves_like '404 response' do + let(:request) { put api(route, current_user) } + let(:message) { '404 Branch Not Found' } + end + end + + context 'when repository is disabled' do + include_context 'disabled repository' + + it_behaves_like '403 response' do + let(:request) { put api(route, current_user) } + end + end end - context 'for an existing protected branch' do - before do - project.repository.add_branch(user, protected_branch.name, 'master') + context 'when unauthenticated', 'and project is private' do + it_behaves_like '404 response' do + let(:request) { put api(route) } + let(:message) { '404 Project Not Found' } end + end + + context 'when authenticated', 'as a guest' do + it_behaves_like '403 response' do + let(:request) { put api(route, guest) } + end + end + + context 'when authenticated', 'as a master' do + let(:current_user) { user } - context "when developers can push and merge" do - let(:protected_branch) { create(:protected_branch, :developers_can_push, :developers_can_merge, project: project, name: 'protected_branch') } + context "when a protected branch doesn't already exist" do + it_behaves_like 'repository new protected branch' - it 'updates that a developer cannot push or merge' do - put api("/projects/#{project.id}/repository/branches/#{protected_branch.name}/protect", user), - developers_can_push: false, developers_can_merge: false + context 'when branch contains a dot' do + let(:branch_name) { branch_with_dot.name } - expect(response).to have_http_status(200) - expect(json_response['name']).to eq(protected_branch.name) - expect(json_response['protected']).to eq(true) - expect(json_response['developers_can_push']).to eq(false) - expect(json_response['developers_can_merge']).to eq(false) + it_behaves_like 'repository new protected branch' end - it "doesn't result in 0 access levels when 'developers_can_push' is switched off" do - put api("/projects/#{project.id}/repository/branches/#{protected_branch.name}/protect", user), - developers_can_push: false + context 'when branch contains a slash' do + let(:branch_name) { branch_with_slash.name } - expect(response).to have_http_status(200) - expect(json_response['name']).to eq(protected_branch.name) - expect(protected_branch.reload.push_access_levels.first).to be_present - expect(protected_branch.reload.push_access_levels.first.access_level).to eq(Gitlab::Access::MASTER) + it_behaves_like '404 response' do + let(:request) { put api(route, current_user) } + end end - it "doesn't result in 0 access levels when 'developers_can_merge' is switched off" do - put api("/projects/#{project.id}/repository/branches/#{protected_branch.name}/protect", user), - developers_can_merge: false + context 'when branch contains an escaped slash' do + let(:branch_name) { CGI.escape(branch_with_slash.name) } - expect(response).to have_http_status(200) - expect(json_response['name']).to eq(protected_branch.name) - expect(protected_branch.reload.merge_access_levels.first).to be_present - expect(protected_branch.reload.merge_access_levels.first.access_level).to eq(Gitlab::Access::MASTER) + it_behaves_like 'repository new protected branch' + end + + context 'requesting with the escaped project full path' do + let(:project_id) { CGI.escape(project.full_path) } + + it_behaves_like 'repository new protected branch' + + context 'when branch contains a dot' do + let(:branch_name) { branch_with_dot.name } + + it_behaves_like 'repository new protected branch' + end end end - context "when developers cannot push or merge" do - let(:protected_branch) { create(:protected_branch, project: project, name: 'protected_branch') } + context 'when protected branch already exists' do + before do + project.repository.add_branch(user, protected_branch.name, 'master') + end + + context 'when developers can push and merge' do + let(:protected_branch) { create(:protected_branch, :developers_can_push, :developers_can_merge, project: project, name: 'protected_branch') } + + it 'updates that a developer cannot push or merge' do + put api("/projects/#{project.id}/repository/branches/#{protected_branch.name}/protect", user), + developers_can_push: false, developers_can_merge: false + + expect(response).to have_gitlab_http_status(200) + expect(response).to match_response_schema('public_api/v4/branch') + expect(json_response['name']).to eq(protected_branch.name) + expect(json_response['protected']).to eq(true) + expect(json_response['developers_can_push']).to eq(false) + expect(json_response['developers_can_merge']).to eq(false) + expect(protected_branch.reload.push_access_levels.first.access_level).to eq(Gitlab::Access::MASTER) + expect(protected_branch.reload.merge_access_levels.first.access_level).to eq(Gitlab::Access::MASTER) + end + end + + context 'when developers cannot push or merge' do + let(:protected_branch) { create(:protected_branch, project: project, name: 'protected_branch') } - it 'updates that a developer can push and merge' do - put api("/projects/#{project.id}/repository/branches/#{protected_branch.name}/protect", user), - developers_can_push: true, developers_can_merge: true + it 'updates that a developer can push and merge' do + put api("/projects/#{project.id}/repository/branches/#{protected_branch.name}/protect", user), + developers_can_push: true, developers_can_merge: true - expect(response).to have_http_status(200) - expect(json_response['name']).to eq(protected_branch.name) - expect(json_response['protected']).to eq(true) - expect(json_response['developers_can_push']).to eq(true) - expect(json_response['developers_can_merge']).to eq(true) + expect(response).to have_gitlab_http_status(200) + expect(response).to match_response_schema('public_api/v4/branch') + expect(json_response['name']).to eq(protected_branch.name) + expect(json_response['protected']).to eq(true) + expect(json_response['developers_can_push']).to eq(true) + expect(json_response['developers_can_merge']).to eq(true) + end end end end + end - context "multiple API calls" do - it "returns success when `protect` is called twice" do - put api("/projects/#{project.id}/repository/branches/#{branch_name}/protect", user) - put api("/projects/#{project.id}/repository/branches/#{branch_name}/protect", user) + describe 'PUT /projects/:id/repository/branches/:branch/unprotect' do + let(:route) { "/projects/#{project_id}/repository/branches/#{branch_name}/unprotect" } - expect(response).to have_http_status(200) - expect(json_response['name']).to eq(branch_name) - expect(json_response['protected']).to eq(true) - expect(json_response['developers_can_push']).to eq(false) - expect(json_response['developers_can_merge']).to eq(false) + shared_examples_for 'repository unprotected branch' do + it 'unprotects a single branch' do + put api(route, current_user) + + expect(response).to have_gitlab_http_status(200) + expect(response).to match_response_schema('public_api/v4/branch') + expect(json_response['name']).to eq(CGI.unescape(branch_name)) + expect(json_response['protected']).to eq(false) end - it "returns success when `protect` is called twice with `developers_can_push` turned on" do - put api("/projects/#{project.id}/repository/branches/#{branch_name}/protect", user), developers_can_push: true - put api("/projects/#{project.id}/repository/branches/#{branch_name}/protect", user), developers_can_push: true + context 'when branch does not exist' do + let(:branch_name) { 'unknown' } - expect(response).to have_http_status(200) - expect(json_response['name']).to eq(branch_name) - expect(json_response['protected']).to eq(true) - expect(json_response['developers_can_push']).to eq(true) - expect(json_response['developers_can_merge']).to eq(false) + it_behaves_like '404 response' do + let(:request) { put api(route, current_user) } + let(:message) { '404 Branch Not Found' } + end end - it "returns success when `protect` is called twice with `developers_can_merge` turned on" do - put api("/projects/#{project.id}/repository/branches/#{branch_name}/protect", user), developers_can_merge: true - put api("/projects/#{project.id}/repository/branches/#{branch_name}/protect", user), developers_can_merge: true + context 'when repository is disabled' do + include_context 'disabled repository' - expect(response).to have_http_status(200) - expect(json_response['name']).to eq(branch_name) - expect(json_response['protected']).to eq(true) - expect(json_response['developers_can_push']).to eq(false) - expect(json_response['developers_can_merge']).to eq(true) + it_behaves_like '403 response' do + let(:request) { put api(route, current_user) } + end end end - it "returns a 404 error if branch not found" do - put api("/projects/#{project.id}/repository/branches/unknown/protect", user) - expect(response).to have_http_status(404) + context 'when unauthenticated', 'and project is private' do + it_behaves_like '404 response' do + let(:request) { put api(route) } + let(:message) { '404 Project Not Found' } + end end - it "returns a 403 error if guest" do - put api("/projects/#{project.id}/repository/branches/#{branch_name}/protect", guest) - expect(response).to have_http_status(403) + context 'when authenticated', 'as a guest' do + it_behaves_like '403 response' do + let(:request) { put api(route, guest) } + end end - end - describe "PUT /projects/:id/repository/branches/:branch/unprotect" do - it "unprotects a single branch" do - put api("/projects/#{project.id}/repository/branches/#{branch_name}/unprotect", user) - expect(response).to have_http_status(200) + context 'when authenticated', 'as a master' do + let(:current_user) { user } + + context "when a protected branch doesn't already exist" do + it_behaves_like 'repository unprotected branch' + + context 'when branch contains a dot' do + let(:branch_name) { branch_with_dot.name } + + it_behaves_like 'repository unprotected branch' + end + + context 'when branch contains a slash' do + let(:branch_name) { branch_with_slash.name } + + it_behaves_like '404 response' do + let(:request) { put api(route, current_user) } + end + end + + context 'when branch contains an escaped slash' do + let(:branch_name) { CGI.escape(branch_with_slash.name) } - expect(json_response['name']).to eq(branch_name) - expect(json_response['commit']['id']).to eq(branch_sha) - expect(json_response['protected']).to eq(false) + it_behaves_like 'repository unprotected branch' + end + + context 'requesting with the escaped project full path' do + let(:project_id) { CGI.escape(project.full_path) } + + it_behaves_like 'repository unprotected branch' + + context 'when branch contains a dot' do + let(:branch_name) { branch_with_dot.name } + + it_behaves_like 'repository unprotected branch' + end + end + end end + end - it "update branches with dots in branch name" do - put api("/projects/#{project.id}/repository/branches/#{branch_with_dot.name}/unprotect", user) + describe 'POST /projects/:id/repository/branches' do + let(:route) { "/projects/#{project_id}/repository/branches" } - expect(response).to have_http_status(200) - expect(json_response['name']).to eq(branch_with_dot.name) - expect(json_response['protected']).to eq(false) + shared_examples_for 'repository new branch' do + it 'creates a new branch' do + post api(route, current_user), branch: 'feature1', ref: branch_sha + + expect(response).to have_gitlab_http_status(201) + expect(response).to match_response_schema('public_api/v4/branch') + expect(json_response['name']).to eq('feature1') + expect(json_response['commit']['id']).to eq(branch_sha) + end + + context 'when repository is disabled' do + include_context 'disabled repository' + + it_behaves_like '403 response' do + let(:request) { post api(route, current_user) } + end + end end - it "returns success when unprotect branch" do - put api("/projects/#{project.id}/repository/branches/unknown/unprotect", user) - expect(response).to have_http_status(404) + context 'when unauthenticated', 'and project is private' do + it_behaves_like '404 response' do + let(:request) { post api(route) } + let(:message) { '404 Project Not Found' } + end end - it "returns success when unprotect branch again" do - put api("/projects/#{project.id}/repository/branches/#{branch_name}/unprotect", user) - put api("/projects/#{project.id}/repository/branches/#{branch_name}/unprotect", user) - expect(response).to have_http_status(200) + context 'when authenticated', 'as a guest' do + it_behaves_like '403 response' do + let(:request) { post api(route, guest) } + end end - end - describe "POST /projects/:id/repository/branches" do - it "creates a new branch" do - post api("/projects/#{project.id}/repository/branches", user), - branch: 'feature1', - ref: branch_sha + context 'when authenticated', 'as a master' do + let(:current_user) { user } - expect(response).to have_http_status(201) + context "when a protected branch doesn't already exist" do + it_behaves_like 'repository new branch' - expect(json_response['name']).to eq('feature1') - expect(json_response['commit']['id']).to eq(branch_sha) - end + context 'requesting with the escaped project full path' do + let(:project_id) { CGI.escape(project.full_path) } - it "denies for user without push access" do - post api("/projects/#{project.id}/repository/branches", guest), - branch: branch_name, - ref: branch_sha - expect(response).to have_http_status(403) + it_behaves_like 'repository new branch' + end + end end it 'returns 400 if branch name is invalid' do - post api("/projects/#{project.id}/repository/branches", user), - branch: 'new design', - ref: branch_sha - expect(response).to have_http_status(400) + post api(route, user), branch: 'new design', ref: branch_sha + + expect(response).to have_gitlab_http_status(400) expect(json_response['message']).to eq('Branch name is invalid') end it 'returns 400 if branch already exists' do - post api("/projects/#{project.id}/repository/branches", user), - branch: 'new_design1', - ref: branch_sha - expect(response).to have_http_status(201) - - post api("/projects/#{project.id}/repository/branches", user), - branch: 'new_design1', - ref: branch_sha - expect(response).to have_http_status(400) + post api(route, user), branch: 'new_design1', ref: branch_sha + + expect(response).to have_gitlab_http_status(201) + + post api(route, user), branch: 'new_design1', ref: branch_sha + + expect(response).to have_gitlab_http_status(400) expect(json_response['message']).to eq('Branch already exists') end it 'returns 400 if ref name is invalid' do - post api("/projects/#{project.id}/repository/branches", user), - branch: 'new_design3', - ref: 'foo' - expect(response).to have_http_status(400) + post api(route, user), branch: 'new_design3', ref: 'foo' + + expect(response).to have_gitlab_http_status(400) expect(json_response['message']).to eq('Invalid reference name') end end - describe "DELETE /projects/:id/repository/branches/:branch" do + describe 'DELETE /projects/:id/repository/branches/:branch' do before do allow_any_instance_of(Repository).to receive(:rm_branch).and_return(true) end - it "removes branch" do + it 'removes branch' do delete api("/projects/#{project.id}/repository/branches/#{branch_name}", user) - expect(response).to have_http_status(204) + expect(response).to have_gitlab_http_status(204) end - it "removes a branch with dots in the branch name" do + it 'removes a branch with dots in the branch name' do delete api("/projects/#{project.id}/repository/branches/#{branch_with_dot.name}", user) - expect(response).to have_http_status(204) + expect(response).to have_gitlab_http_status(204) end it 'returns 404 if branch not exists' do delete api("/projects/#{project.id}/repository/branches/foobar", user) - expect(response).to have_http_status(404) + + expect(response).to have_gitlab_http_status(404) end end - describe "DELETE /projects/:id/repository/merged_branches" do + describe 'DELETE /projects/:id/repository/merged_branches' do before do allow_any_instance_of(Repository).to receive(:rm_branch).and_return(true) end @@ -416,13 +509,14 @@ describe API::Branches do it 'returns 202 with json body' do delete api("/projects/#{project.id}/repository/merged_branches", user) - expect(response).to have_http_status(202) + expect(response).to have_gitlab_http_status(202) expect(json_response['message']).to eql('202 Accepted') end it 'returns a 403 error if guest' do delete api("/projects/#{project.id}/repository/merged_branches", guest) - expect(response).to have_http_status(403) + + expect(response).to have_gitlab_http_status(403) end end end diff --git a/spec/requests/api/group_milestones_spec.rb b/spec/requests/api/group_milestones_spec.rb new file mode 100644 index 00000000000..9b24658771f --- /dev/null +++ b/spec/requests/api/group_milestones_spec.rb @@ -0,0 +1,21 @@ +require 'spec_helper' + +describe API::GroupMilestones do + let(:user) { create(:user) } + let(:group) { create(:group, :private) } + let(:project) { create(:empty_project, namespace: group) } + let!(:group_member) { create(:group_member, group: group, user: user) } + let!(:closed_milestone) { create(:closed_milestone, group: group, title: 'version1', description: 'closed milestone') } + let!(:milestone) { create(:milestone, group: group, title: 'version2', description: 'open milestone') } + + it_behaves_like 'group and project milestones', "/groups/:id/milestones" do + let(:route) { "/groups/#{group.id}/milestones" } + end + + def setup_for_group + context_group.update(visibility_level: Gitlab::VisibilityLevel::PUBLIC) + context_group.add_developer(user) + public_project.update(namespace: context_group) + context_group.reload + end +end diff --git a/spec/requests/api/groups_spec.rb b/spec/requests/api/groups_spec.rb index 656f098aea8..1d7adc6ac45 100644 --- a/spec/requests/api/groups_spec.rb +++ b/spec/requests/api/groups_spec.rb @@ -510,7 +510,7 @@ describe API::Groups do describe "POST /groups/:id/projects/:project_id" do let(:project) { create(:empty_project) } - let(:project_path) { project.full_path.gsub('/', '%2F') } + let(:project_path) { CGI.escape(project.full_path) } before(:each) do allow_any_instance_of(Projects::TransferService) diff --git a/spec/requests/api/helpers_spec.rb b/spec/requests/api/helpers_spec.rb index 25ec44fa036..7a1bd76af7a 100644 --- a/spec/requests/api/helpers_spec.rb +++ b/spec/requests/api/helpers_spec.rb @@ -10,8 +10,16 @@ describe API::Helpers do let(:key) { create(:key, user: user) } let(:params) { {} } - let(:env) { { 'REQUEST_METHOD' => 'GET' } } - let(:request) { Rack::Request.new(env) } + let(:csrf_token) { SecureRandom.base64(ActionController::RequestForgeryProtection::AUTHENTICITY_TOKEN_LENGTH) } + let(:env) do + { + 'rack.input' => '', + 'rack.session' => { + _csrf_token: csrf_token + }, + 'REQUEST_METHOD' => 'GET' + } + end let(:header) { } before do @@ -58,7 +66,7 @@ describe API::Helpers do describe ".current_user" do subject { current_user } - describe "Warden authentication" do + describe "Warden authentication", :allow_forgery_protection do before do doorkeeper_guard_returns false end @@ -99,7 +107,17 @@ describe API::Helpers do env['REQUEST_METHOD'] = 'PUT' end - it { is_expected.to be_nil } + context 'without CSRF token' do + it { is_expected.to be_nil } + end + + context 'with CSRF token' do + before do + env['HTTP_X_CSRF_TOKEN'] = csrf_token + end + + it { is_expected.to eq(user) } + end end context "POST request" do @@ -107,7 +125,17 @@ describe API::Helpers do env['REQUEST_METHOD'] = 'POST' end - it { is_expected.to be_nil } + context 'without CSRF token' do + it { is_expected.to be_nil } + end + + context 'with CSRF token' do + before do + env['HTTP_X_CSRF_TOKEN'] = csrf_token + end + + it { is_expected.to eq(user) } + end end context "DELETE request" do @@ -115,7 +143,17 @@ describe API::Helpers do env['REQUEST_METHOD'] = 'DELETE' end - it { is_expected.to be_nil } + context 'without CSRF token' do + it { is_expected.to be_nil } + end + + context 'with CSRF token' do + before do + env['HTTP_X_CSRF_TOKEN'] = csrf_token + end + + it { is_expected.to eq(user) } + end end end end diff --git a/spec/requests/api/issues_spec.rb b/spec/requests/api/issues_spec.rb index 9837fedb522..33cea02153e 100644 --- a/spec/requests/api/issues_spec.rb +++ b/spec/requests/api/issues_spec.rb @@ -71,7 +71,6 @@ describe API::Issues do expect(response).to have_http_status(401) end end - context "when authenticated" do let(:first_issue) { json_response.first } @@ -105,6 +104,42 @@ describe API::Issues do expect(json_response.second['id']).to eq(closed_issue.id) end + it 'returns issues assigned to me' do + issue2 = create(:issue, assignees: [user2], project: project) + + get api('/issues', user2), scope: 'assigned-to-me' + + expect_paginated_array_response(size: 1) + expect(first_issue['id']).to eq(issue2.id) + end + + it 'returns issues authored by the given author id' do + issue2 = create(:issue, author: user2, project: project) + + get api('/issues', user), author_id: user2.id, scope: 'all' + + expect_paginated_array_response(size: 1) + expect(first_issue['id']).to eq(issue2.id) + end + + it 'returns issues assigned to the given assignee id' do + issue2 = create(:issue, assignees: [user2], project: project) + + get api('/issues', user), assignee_id: user2.id, scope: 'all' + + expect_paginated_array_response(size: 1) + expect(first_issue['id']).to eq(issue2.id) + end + + it 'returns issues authored by the given author id and assigned to the given assignee id' do + issue2 = create(:issue, author: user2, assignees: [user2], project: project) + + get api('/issues', user), author_id: user2.id, assignee_id: user2.id, scope: 'all' + + expect_paginated_array_response(size: 1) + expect(first_issue['id']).to eq(issue2.id) + end + it 'returns issues matching given search string for title' do get api("/issues", user), search: issue.title @@ -693,6 +728,19 @@ describe API::Issues do expect(json_response['confidential']).to be_falsy end + context 'links exposure' do + it 'exposes related resources full URIs' do + get api("/projects/#{project.id}/issues/#{issue.iid}", user) + + links = json_response['_links'] + + expect(links['self']).to end_with("/api/v4/projects/#{project.id}/issues/#{issue.iid}") + expect(links['notes']).to end_with("/api/v4/projects/#{project.id}/issues/#{issue.iid}/notes") + expect(links['award_emoji']).to end_with("/api/v4/projects/#{project.id}/issues/#{issue.iid}/award_emoji") + expect(links['project']).to end_with("/api/v4/projects/#{project.id}") + end + end + it "returns a project issue by internal id" do get api("/projects/#{project.id}/issues/#{issue.iid}", user) diff --git a/spec/requests/api/project_milestones_spec.rb b/spec/requests/api/project_milestones_spec.rb new file mode 100644 index 00000000000..fe8fdbfd7e4 --- /dev/null +++ b/spec/requests/api/project_milestones_spec.rb @@ -0,0 +1,25 @@ +require 'spec_helper' + +describe API::ProjectMilestones do + let(:user) { create(:user) } + let!(:project) { create(:empty_project, namespace: user.namespace ) } + let!(:closed_milestone) { create(:closed_milestone, project: project, title: 'version1', description: 'closed milestone') } + let!(:milestone) { create(:milestone, project: project, title: 'version2', description: 'open milestone') } + + before do + project.team << [user, :developer] + end + + it_behaves_like 'group and project milestones', "/projects/:id/milestones" do + let(:route) { "/projects/#{project.id}/milestones" } + end + + describe 'PUT /projects/:id/milestones/:milestone_id to test observer on close' do + it 'creates an activity event when an milestone is closed' do + expect(Event).to receive(:create) + + put api("/projects/#{project.id}/milestones/#{milestone.id}", user), + state_event: 'close' + end + end +end diff --git a/spec/requests/api/projects_spec.rb b/spec/requests/api/projects_spec.rb index 457f64cc88c..6ed68fcff09 100644 --- a/spec/requests/api/projects_spec.rb +++ b/spec/requests/api/projects_spec.rb @@ -768,7 +768,7 @@ describe API::Projects do dot_user = create(:user, username: 'dot.user') project = create(:empty_project, creator_id: dot_user.id, namespace: dot_user.namespace) - get api("/projects/#{dot_user.namespace.name}%2F#{project.path}", dot_user) + get api("/projects/#{CGI.escape(project.full_path)}", dot_user) expect(response).to have_http_status(200) expect(json_response['name']).to eq(project.name) end @@ -815,6 +815,38 @@ describe API::Projects do expect(json_response).not_to include("import_error") end + context 'links exposure' do + it 'exposes related resources full URIs' do + get api("/projects/#{project.id}", user) + + links = json_response['_links'] + + expect(links['self']).to end_with("/api/v4/projects/#{project.id}") + expect(links['issues']).to end_with("/api/v4/projects/#{project.id}/issues") + expect(links['merge_requests']).to end_with("/api/v4/projects/#{project.id}/merge_requests") + expect(links['repo_branches']).to end_with("/api/v4/projects/#{project.id}/repository/branches") + expect(links['labels']).to end_with("/api/v4/projects/#{project.id}/labels") + expect(links['events']).to end_with("/api/v4/projects/#{project.id}/events") + expect(links['members']).to end_with("/api/v4/projects/#{project.id}/members") + end + + it 'filters related URIs when their feature is not enabled' do + project = create(:empty_project, :public, + :merge_requests_disabled, + :issues_disabled, + creator_id: user.id, + namespace: user.namespace) + + get api("/projects/#{project.id}", user) + + links = json_response['_links'] + + expect(links.has_key?('merge_requests')).to be_falsy + expect(links.has_key?('issues')).to be_falsy + expect(links['self']).to end_with("/api/v4/projects/#{project.id}") + end + end + describe 'permissions' do context 'all projects' do before do diff --git a/spec/requests/api/todos_spec.rb b/spec/requests/api/todos_spec.rb index 92533f4dfea..9fc73c6e092 100644 --- a/spec/requests/api/todos_spec.rb +++ b/spec/requests/api/todos_spec.rb @@ -1,7 +1,7 @@ require 'spec_helper' describe API::Todos do - let(:project_1) { create(:empty_project, :test_repo) } + let(:project_1) { create(:project) } let(:project_2) { create(:empty_project) } let(:author_1) { create(:user) } let(:author_2) { create(:user) } diff --git a/spec/requests/api/triggers_spec.rb b/spec/requests/api/triggers_spec.rb index 16ddade27d9..c2636b6614e 100644 --- a/spec/requests/api/triggers_spec.rb +++ b/spec/requests/api/triggers_spec.rb @@ -61,7 +61,8 @@ describe API::Triggers do post api("/projects/#{project.id}/trigger/pipeline"), options.merge(ref: 'other-branch') expect(response).to have_http_status(400) - expect(json_response['message']).to eq('No pipeline created') + expect(json_response['message']['base']) + .to contain_exactly('Reference not found') end context 'Validates variables' do diff --git a/spec/requests/api/users_spec.rb b/spec/requests/api/users_spec.rb index 877bde3b9a6..66b165b438b 100644 --- a/spec/requests/api/users_spec.rb +++ b/spec/requests/api/users_spec.rb @@ -55,17 +55,22 @@ describe API::Users do context "when public level is restricted" do before do stub_application_setting(restricted_visibility_levels: [Gitlab::VisibilityLevel::PUBLIC]) - allow_any_instance_of(API::Helpers).to receive(:authenticate!).and_return(true) end - it "renders 403" do - get api("/users") - expect(response).to have_http_status(403) + context 'when authenticate as a regular user' do + it "renders 403" do + get api("/users", user) + + expect(response).to have_gitlab_http_status(403) + end end - it "renders 404" do - get api("/users/#{user.id}") - expect(response).to have_http_status(404) + context 'when authenticate as an admin' do + it "renders 200" do + get api("/users", admin) + + expect(response).to have_gitlab_http_status(200) + end end end diff --git a/spec/requests/api/v3/groups_spec.rb b/spec/requests/api/v3/groups_spec.rb index 63c5707b2e4..5cdc528e190 100644 --- a/spec/requests/api/v3/groups_spec.rb +++ b/spec/requests/api/v3/groups_spec.rb @@ -502,7 +502,7 @@ describe API::V3::Groups do describe "POST /groups/:id/projects/:project_id" do let(:project) { create(:empty_project) } - let(:project_path) { "#{project.namespace.path}%2F#{project.path}" } + let(:project_path) { CGI.escape(project.full_path) } before(:each) do allow_any_instance_of(Projects::TransferService) diff --git a/spec/requests/api/v3/projects_spec.rb b/spec/requests/api/v3/projects_spec.rb index af44ffa2331..bbfcaab1ea1 100644 --- a/spec/requests/api/v3/projects_spec.rb +++ b/spec/requests/api/v3/projects_spec.rb @@ -720,7 +720,7 @@ describe API::V3::Projects do dot_user = create(:user, username: 'dot.user') project = create(:empty_project, creator_id: dot_user.id, namespace: dot_user.namespace) - get v3_api("/projects/#{dot_user.namespace.name}%2F#{project.path}", dot_user) + get v3_api("/projects/#{CGI.escape(project.full_path)}", dot_user) expect(response).to have_http_status(200) expect(json_response['name']).to eq(project.name) end diff --git a/spec/requests/api/v3/triggers_spec.rb b/spec/requests/api/v3/triggers_spec.rb index d3de6bf13bc..60212660fb6 100644 --- a/spec/requests/api/v3/triggers_spec.rb +++ b/spec/requests/api/v3/triggers_spec.rb @@ -52,7 +52,8 @@ describe API::V3::Triggers do it 'returns bad request with no builds created if there\'s no commit for that ref' do post v3_api("/projects/#{project.id}/trigger/builds"), options.merge(ref: 'other-branch') expect(response).to have_http_status(400) - expect(json_response['message']).to eq('No builds created') + expect(json_response['message']['base']) + .to contain_exactly('Reference not found') end context 'Validates variables' do diff --git a/spec/requests/ci/api/triggers_spec.rb b/spec/requests/ci/api/triggers_spec.rb index 26b03c0f148..e481ca916ab 100644 --- a/spec/requests/ci/api/triggers_spec.rb +++ b/spec/requests/ci/api/triggers_spec.rb @@ -5,7 +5,14 @@ describe Ci::API::Triggers do let!(:trigger_token) { 'secure token' } let!(:project) { create(:project, :repository, ci_id: 10) } let!(:project2) { create(:empty_project, ci_id: 11) } - let!(:trigger) { create(:ci_trigger, project: project, token: trigger_token) } + + let!(:trigger) do + create(:ci_trigger, + project: project, + token: trigger_token, + owner: create(:user)) + end + let(:options) do { token: trigger_token @@ -14,6 +21,8 @@ describe Ci::API::Triggers do before do stub_ci_pipeline_to_return_yaml_file + + project.add_developer(trigger.owner) end context 'Handles errors' do @@ -47,7 +56,8 @@ describe Ci::API::Triggers do it 'returns bad request with no builds created if there\'s no commit for that ref' do post ci_api("/projects/#{project.ci_id}/refs/other-branch/trigger"), options expect(response).to have_http_status(400) - expect(json_response['message']).to eq('No builds created') + expect(json_response['message']['base']) + .to contain_exactly('Reference not found') end context 'Validates variables' do diff --git a/spec/serializers/build_details_entity_spec.rb b/spec/serializers/build_details_entity_spec.rb index b92c1c28ba8..1332572fffc 100644 --- a/spec/serializers/build_details_entity_spec.rb +++ b/spec/serializers/build_details_entity_spec.rb @@ -9,47 +9,96 @@ describe BuildDetailsEntity do describe '#as_json' do let(:project) { create(:project, :repository) } - let!(:build) { create(:ci_build, :failed, project: project) } + let(:pipeline) { create(:ci_pipeline, project: project) } + let(:build) { create(:ci_build, :failed, pipeline: pipeline) } let(:request) { double('request') } - let(:entity) { described_class.new(build, request: request, current_user: user, project: project) } + + let(:entity) do + described_class.new(build, request: request, + current_user: user, + project: project) + end + subject { entity.as_json } before do allow(request).to receive(:current_user).and_return(user) end + it 'contains the needed key value pairs' do + expect(subject).to include(:coverage, :erased_at, :duration) + expect(subject).to include(:runner, :pipeline) + expect(subject).to include(:raw_path, :new_issue_path) + end + context 'when the user has access to issues and merge requests' do - let!(:merge_request) do - create(:merge_request, source_project: project, source_branch: build.ref) - end + context 'when merge request orginates from the same project' do + let(:merge_request) do + create(:merge_request, source_project: project, source_branch: build.ref) + end - before do - allow(build).to receive(:merge_request).and_return(merge_request) - end + before do + allow(build).to receive(:merge_request).and_return(merge_request) + end + + it 'contains the needed key value pairs' do + expect(subject).to include(:merge_request) + expect(subject).to include(:new_issue_path) + end - it 'contains the needed key value pairs' do - expect(subject).to include(:coverage, :erased_at, :duration) - expect(subject).to include(:runner, :pipeline) - expect(subject).to include(:raw_path, :merge_request) - expect(subject).to include(:new_issue_path) + it 'exposes correct details of the merge request' do + expect(subject[:merge_request][:iid]).to eq merge_request.iid + end + + it 'has a correct merge request path' do + expect(subject[:merge_request][:path]).to include project.full_path + end end - it 'exposes details of the merge request' do - expect(subject[:merge_request]).to include(:iid, :path) + context 'when merge request is from a fork' do + let(:fork_project) do + create(:empty_project, forked_from_project: project) + end + + let(:pipeline) { create(:ci_pipeline, project: fork_project) } + + before do + allow(build).to receive(:merge_request).and_return(merge_request) + end + + let(:merge_request) do + create(:merge_request, source_project: fork_project, + target_project: project, + source_branch: build.ref) + end + + it 'contains the needed key value pairs' do + expect(subject).to include(:merge_request) + expect(subject).to include(:new_issue_path) + end + + it 'exposes details of the merge request' do + expect(subject[:merge_request][:iid]).to eq merge_request.iid + end + + it 'has a merge request path to a target project' do + expect(subject[:merge_request][:path]) + .to include project.full_path + end end - context 'when the build has been erased' do - let!(:build) { create(:ci_build, :erasable, project: project) } + context 'when the build has not been erased' do + let(:build) { create(:ci_build, :erasable, project: project) } - it 'exposes the user whom erased the build' do + it 'exposes a build erase path' do expect(subject).to include(:erase_path) end end context 'when the build has been erased' do - let!(:build) { create(:ci_build, erased_at: Time.now, project: project, erased_by: user) } + let(:build) { create(:ci_build, :erased, project: project) } - it 'exposes the user whom erased the build' do + it 'exposes the user who erased the build' do expect(subject).to include(:erased_by) end end diff --git a/spec/serializers/deploy_key_entity_spec.rb b/spec/serializers/deploy_key_entity_spec.rb index 9620f9665cf..8149de869f1 100644 --- a/spec/serializers/deploy_key_entity_spec.rb +++ b/spec/serializers/deploy_key_entity_spec.rb @@ -2,13 +2,15 @@ require 'spec_helper' describe DeployKeyEntity do include RequestAwareEntity - + let(:user) { create(:user) } let(:project) { create(:empty_project, :internal)} let(:project_private) { create(:empty_project, :private)} + let!(:project_pending_delete) { create(:empty_project, :internal, pending_delete: true) } let(:deploy_key) { create(:deploy_key) } let!(:deploy_key_internal) { create(:deploy_keys_project, project: project, deploy_key: deploy_key) } let!(:deploy_key_private) { create(:deploy_keys_project, project: project_private, deploy_key: deploy_key) } + let!(:deploy_key_pending_delete) { create(:deploy_keys_project, project: project_pending_delete, deploy_key: deploy_key) } let(:entity) { described_class.new(deploy_key, user: user) } diff --git a/spec/serializers/job_entity_spec.rb b/spec/serializers/job_entity_spec.rb index 5ca7bf2fcaf..026360e91a3 100644 --- a/spec/serializers/job_entity_spec.rb +++ b/spec/serializers/job_entity_spec.rb @@ -7,7 +7,9 @@ describe JobEntity do let(:request) { double('request') } before do + stub_not_protect_default_branch allow(request).to receive(:current_user).and_return(user) + project.add_developer(user) end @@ -77,7 +79,7 @@ describe JobEntity do project.add_developer(user) create(:protected_branch, :developers_can_merge, - name: 'master', project: project) + name: job.ref, project: job.project) end it 'contains path to play action' do @@ -90,6 +92,13 @@ describe JobEntity do end context 'when user is not allowed to trigger action' do + before do + allow(job.project).to receive(:empty_repo?).and_return(false) + + create(:protected_branch, :no_one_can_push, + name: job.ref, project: job.project) + end + it 'does not contain path to play action' do expect(subject).not_to include(:play_path) end diff --git a/spec/serializers/pipeline_details_entity_spec.rb b/spec/serializers/pipeline_details_entity_spec.rb index d28dec9592a..b990370a271 100644 --- a/spec/serializers/pipeline_details_entity_spec.rb +++ b/spec/serializers/pipeline_details_entity_spec.rb @@ -9,6 +9,8 @@ describe PipelineDetailsEntity do end before do + stub_not_protect_default_branch + allow(request).to receive(:current_user).and_return(user) end @@ -52,7 +54,7 @@ describe PipelineDetailsEntity do context 'user has ability to retry pipeline' do before do - project.team << [user, :developer] + project.add_developer(user) end it 'retryable flag is true' do @@ -97,7 +99,7 @@ describe PipelineDetailsEntity do context 'when pipeline has commit statuses' do let(:pipeline) { create(:ci_empty_pipeline) } - + before do create(:generic_commit_status, pipeline: pipeline) end diff --git a/spec/serializers/pipeline_entity_spec.rb b/spec/serializers/pipeline_entity_spec.rb index 46650f3a80d..5b01cc4fc9e 100644 --- a/spec/serializers/pipeline_entity_spec.rb +++ b/spec/serializers/pipeline_entity_spec.rb @@ -5,6 +5,8 @@ describe PipelineEntity do let(:request) { double('request') } before do + stub_not_protect_default_branch + allow(request).to receive(:current_user).and_return(user) end @@ -52,7 +54,7 @@ describe PipelineEntity do context 'user has ability to retry pipeline' do before do - project.team << [user, :developer] + project.add_developer(user) end it 'contains retry path' do diff --git a/spec/serializers/pipeline_serializer_spec.rb b/spec/serializers/pipeline_serializer_spec.rb index 44813656aff..262bc4acb69 100644 --- a/spec/serializers/pipeline_serializer_spec.rb +++ b/spec/serializers/pipeline_serializer_spec.rb @@ -108,14 +108,35 @@ describe PipelineSerializer do end end - it 'verifies number of queries', :request_store do - recorded = ActiveRecord::QueryRecorder.new { subject } - expect(recorded.count).to be_within(1).of(57) - expect(recorded.cached_count).to eq(0) + shared_examples 'no N+1 queries' do + it 'verifies number of queries', :request_store do + recorded = ActiveRecord::QueryRecorder.new { subject } + expect(recorded.count).to be_within(1).of(59) + expect(recorded.cached_count).to eq(0) + end + end + + context 'with the same ref' do + let(:ref) { 'feature' } + + it_behaves_like 'no N+1 queries' + end + + context 'with different refs' do + def ref + @sequence ||= 0 + @sequence += 1 + "feature-#{@sequence}" + end + + it_behaves_like 'no N+1 queries' end def create_pipeline(status) - create(:ci_empty_pipeline, project: project, status: status).tap do |pipeline| + create(:ci_empty_pipeline, + project: project, + status: status, + ref: ref).tap do |pipeline| Ci::Build::AVAILABLE_STATUSES.each do |status| create_build(pipeline, status, status) end @@ -125,7 +146,7 @@ describe PipelineSerializer do def create_build(pipeline, stage, status) create(:ci_build, :tags, :triggered, :artifacts, pipeline: pipeline, stage: stage, - name: stage, status: status) + name: stage, status: status, ref: pipeline.ref) end end end diff --git a/spec/services/ci/create_pipeline_service_spec.rb b/spec/services/ci/create_pipeline_service_spec.rb index ba07c01d43f..146d25daba3 100644 --- a/spec/services/ci/create_pipeline_service_spec.rb +++ b/spec/services/ci/create_pipeline_service_spec.rb @@ -3,19 +3,26 @@ require 'spec_helper' describe Ci::CreatePipelineService, :services do let(:project) { create(:project, :repository) } let(:user) { create(:admin) } + let(:ref_name) { 'refs/heads/master' } before do stub_ci_pipeline_to_return_yaml_file end describe '#execute' do - def execute_service(source: :push, after: project.commit.id, message: 'Message', ref: 'refs/heads/master') + def execute_service( + source: :push, + after: project.commit.id, + message: 'Message', + ref: ref_name, + trigger_request: nil) params = { ref: ref, before: '00000000', after: after, commits: [{ message: message }] } - described_class.new(project, user, params).execute(source) + described_class.new(project, user, params).execute( + source, trigger_request: trigger_request) end context 'valid params' do @@ -334,5 +341,209 @@ describe Ci::CreatePipelineService, :services do expect(pipeline.builds.find_by(name: 'rspec').retries_max).to eq 2 end end + + shared_examples 'when ref is protected' do + let(:user) { create(:user) } + + context 'when user is developer' do + before do + project.add_developer(user) + end + + it 'does not create a pipeline' do + expect(execute_service).not_to be_persisted + expect(Ci::Pipeline.count).to eq(0) + end + end + + context 'when user is master' do + before do + project.add_master(user) + end + + it 'creates a pipeline' do + expect(execute_service).to be_persisted + expect(Ci::Pipeline.count).to eq(1) + end + end + + context 'when trigger belongs to no one' do + let(:user) {} + let(:trigger_request) { create(:ci_trigger_request) } + + it 'does not create a pipeline' do + expect(execute_service(trigger_request: trigger_request)) + .not_to be_persisted + expect(Ci::Pipeline.count).to eq(0) + end + end + + context 'when trigger belongs to a developer' do + let(:user) {} + + let(:trigger_request) do + create(:ci_trigger_request).tap do |request| + user = create(:user) + project.add_developer(user) + request.trigger.update(owner: user) + end + end + + it 'does not create a pipeline' do + expect(execute_service(trigger_request: trigger_request)) + .not_to be_persisted + expect(Ci::Pipeline.count).to eq(0) + end + end + + context 'when trigger belongs to a master' do + let(:user) {} + + let(:trigger_request) do + create(:ci_trigger_request).tap do |request| + user = create(:user) + project.add_master(user) + request.trigger.update(owner: user) + end + end + + it 'does not create a pipeline' do + expect(execute_service(trigger_request: trigger_request)) + .to be_persisted + expect(Ci::Pipeline.count).to eq(1) + end + end + end + + context 'when ref is a protected branch' do + before do + create(:protected_branch, project: project, name: 'master') + end + + it_behaves_like 'when ref is protected' + end + + context 'when ref is a protected tag' do + let(:ref_name) { 'refs/tags/v1.0.0' } + + before do + create(:protected_tag, project: project, name: '*') + end + + it_behaves_like 'when ref is protected' + end + + context 'when ref is not protected' do + context 'when trigger belongs to no one' do + let(:user) {} + let(:trigger_request) { create(:ci_trigger_request) } + + it 'creates a pipeline' do + expect(execute_service(trigger_request: trigger_request)) + .to be_persisted + expect(Ci::Pipeline.count).to eq(1) + end + end + end + end + + describe '#allowed_to_create?' do + let(:user) { create(:user) } + let(:project) { create(:project, :repository) } + let(:ref) { 'master' } + + subject do + described_class.new(project, user, ref: ref) + .send(:allowed_to_create?, user) + end + + context 'when user is a developer' do + before do + project.add_developer(user) + end + + it { is_expected.to be_truthy } + + context 'when the branch is protected' do + let!(:protected_branch) do + create(:protected_branch, project: project, name: ref) + end + + it { is_expected.to be_falsey } + + context 'when developers are allowed to merge' do + let!(:protected_branch) do + create(:protected_branch, + :developers_can_merge, + project: project, + name: ref) + end + + it { is_expected.to be_truthy } + end + end + + context 'when the tag is protected' do + let(:ref) { 'v1.0.0' } + + let!(:protected_tag) do + create(:protected_tag, project: project, name: ref) + end + + it { is_expected.to be_falsey } + + context 'when developers are allowed to create the tag' do + let!(:protected_tag) do + create(:protected_tag, + :developers_can_create, + project: project, + name: ref) + end + + it { is_expected.to be_truthy } + end + end + end + + context 'when user is a master' do + before do + project.add_master(user) + end + + it { is_expected.to be_truthy } + + context 'when the branch is protected' do + let!(:protected_branch) do + create(:protected_branch, project: project, name: ref) + end + + it { is_expected.to be_truthy } + end + + context 'when the tag is protected' do + let(:ref) { 'v1.0.0' } + + let!(:protected_tag) do + create(:protected_tag, project: project, name: ref) + end + + it { is_expected.to be_truthy } + + context 'when no one can create the tag' do + let!(:protected_tag) do + create(:protected_tag, + :no_one_can_create, + project: project, + name: ref) + end + + it { is_expected.to be_falsey } + end + end + end + + context 'when owner cannot create pipeline' do + it { is_expected.to be_falsey } + end end end diff --git a/spec/services/ci/create_trigger_request_service_spec.rb b/spec/services/ci/create_trigger_request_service_spec.rb index f2956262f4b..37ca9804f56 100644 --- a/spec/services/ci/create_trigger_request_service_spec.rb +++ b/spec/services/ci/create_trigger_request_service_spec.rb @@ -1,12 +1,15 @@ require 'spec_helper' describe Ci::CreateTriggerRequestService, services: true do - let(:service) { described_class.new } + let(:service) { described_class } let(:project) { create(:project, :repository) } - let(:trigger) { create(:ci_trigger, project: project) } + let(:trigger) { create(:ci_trigger, project: project, owner: owner) } + let(:owner) { create(:user) } before do stub_ci_pipeline_to_return_yaml_file + + project.add_developer(owner) end describe '#execute' do @@ -14,29 +17,26 @@ describe Ci::CreateTriggerRequestService, services: true do subject { service.execute(project, trigger, 'master') } context 'without owner' do - it { expect(subject).to be_kind_of(Ci::TriggerRequest) } + it { expect(subject.trigger_request).to be_kind_of(Ci::TriggerRequest) } + it { expect(subject.trigger_request.builds.first).to be_kind_of(Ci::Build) } it { expect(subject.pipeline).to be_kind_of(Ci::Pipeline) } it { expect(subject.pipeline).to be_trigger } - it { expect(subject.builds.first).to be_kind_of(Ci::Build) } end context 'with owner' do - let(:owner) { create(:user) } - let(:trigger) { create(:ci_trigger, project: project, owner: owner) } - - it { expect(subject).to be_kind_of(Ci::TriggerRequest) } + it { expect(subject.trigger_request).to be_kind_of(Ci::TriggerRequest) } + it { expect(subject.trigger_request.builds.first).to be_kind_of(Ci::Build) } + it { expect(subject.trigger_request.builds.first.user).to eq(owner) } it { expect(subject.pipeline).to be_kind_of(Ci::Pipeline) } it { expect(subject.pipeline).to be_trigger } it { expect(subject.pipeline.user).to eq(owner) } - it { expect(subject.builds.first).to be_kind_of(Ci::Build) } - it { expect(subject.builds.first.user).to eq(owner) } end end context 'no commit for ref' do subject { service.execute(project, trigger, 'other-branch') } - it { expect(subject).to be_nil } + it { expect(subject.pipeline).not_to be_persisted } end context 'no builds created' do @@ -46,7 +46,7 @@ describe Ci::CreateTriggerRequestService, services: true do stub_ci_pipeline_yaml_file('script: { only: [develop], script: hello World }') end - it { expect(subject).to be_nil } + it { expect(subject.pipeline).not_to be_persisted } end end end diff --git a/spec/services/ci/process_pipeline_service_spec.rb b/spec/services/ci/process_pipeline_service_spec.rb index 0934833a4fa..6346f311696 100644 --- a/spec/services/ci/process_pipeline_service_spec.rb +++ b/spec/services/ci/process_pipeline_service_spec.rb @@ -9,6 +9,8 @@ describe Ci::ProcessPipelineService, '#execute', :services do end before do + stub_not_protect_default_branch + project.add_developer(user) end diff --git a/spec/services/ci/retry_build_service_spec.rb b/spec/services/ci/retry_build_service_spec.rb index ef9927c5969..2cf62b54666 100644 --- a/spec/services/ci/retry_build_service_spec.rb +++ b/spec/services/ci/retry_build_service_spec.rb @@ -85,6 +85,8 @@ describe Ci::RetryBuildService, :services do context 'when user has ability to execute build' do before do + stub_not_protect_default_branch + project.add_developer(user) end @@ -131,6 +133,8 @@ describe Ci::RetryBuildService, :services do context 'when user has ability to execute build' do before do + stub_not_protect_default_branch + project.add_developer(user) end diff --git a/spec/services/ci/retry_pipeline_service_spec.rb b/spec/services/ci/retry_pipeline_service_spec.rb index 3e860203063..7798db3f3b9 100644 --- a/spec/services/ci/retry_pipeline_service_spec.rb +++ b/spec/services/ci/retry_pipeline_service_spec.rb @@ -244,13 +244,9 @@ describe Ci::RetryPipelineService, '#execute', :services do create_build('verify', :canceled, 1) end - it 'does not reprocess manual action' do - service.execute(pipeline) - - expect(build('test')).to be_pending - expect(build('deploy')).to be_failed - expect(build('verify')).to be_created - expect(pipeline.reload).to be_running + it 'raises an error' do + expect { service.execute(pipeline) } + .to raise_error Gitlab::Access::AccessDeniedError end end @@ -261,13 +257,9 @@ describe Ci::RetryPipelineService, '#execute', :services do create_build('verify', :canceled, 2) end - it 'does not reprocess manual action' do - service.execute(pipeline) - - expect(build('test')).to be_pending - expect(build('deploy')).to be_failed - expect(build('verify')).to be_created - expect(pipeline.reload).to be_running + it 'raises an error' do + expect { service.execute(pipeline) } + .to raise_error Gitlab::Access::AccessDeniedError end end end diff --git a/spec/services/create_deployment_service_spec.rb b/spec/services/create_deployment_service_spec.rb index dfab6ebf372..2794721e157 100644 --- a/spec/services/create_deployment_service_spec.rb +++ b/spec/services/create_deployment_service_spec.rb @@ -244,6 +244,8 @@ describe CreateDeploymentService, services: true do context 'when job is retried' do it_behaves_like 'creates deployment' do before do + stub_not_protect_default_branch + project.add_developer(user) end diff --git a/spec/services/issues/duplicate_service_spec.rb b/spec/services/issues/duplicate_service_spec.rb new file mode 100644 index 00000000000..82daf53b173 --- /dev/null +++ b/spec/services/issues/duplicate_service_spec.rb @@ -0,0 +1,80 @@ +require 'spec_helper' + +describe Issues::DuplicateService, services: true do + let(:user) { create(:user) } + let(:canonical_project) { create(:empty_project) } + let(:duplicate_project) { create(:empty_project) } + + let(:canonical_issue) { create(:issue, project: canonical_project) } + let(:duplicate_issue) { create(:issue, project: duplicate_project) } + + subject { described_class.new(duplicate_project, user, {}) } + + describe '#execute' do + context 'when the issues passed are the same' do + it 'does nothing' do + expect(subject).not_to receive(:close_service) + expect(SystemNoteService).not_to receive(:mark_duplicate_issue) + expect(SystemNoteService).not_to receive(:mark_canonical_issue_of_duplicate) + + subject.execute(duplicate_issue, duplicate_issue) + end + end + + context 'when the user cannot update the duplicate issue' do + before do + canonical_project.add_reporter(user) + end + + it 'does nothing' do + expect(subject).not_to receive(:close_service) + expect(SystemNoteService).not_to receive(:mark_duplicate_issue) + expect(SystemNoteService).not_to receive(:mark_canonical_issue_of_duplicate) + + subject.execute(duplicate_issue, canonical_issue) + end + end + + context 'when the user cannot comment on the canonical issue' do + before do + duplicate_project.add_reporter(user) + end + + it 'does nothing' do + expect(subject).not_to receive(:close_service) + expect(SystemNoteService).not_to receive(:mark_duplicate_issue) + expect(SystemNoteService).not_to receive(:mark_canonical_issue_of_duplicate) + + subject.execute(duplicate_issue, canonical_issue) + end + end + + context 'when the user can mark the issue as a duplicate' do + before do + canonical_project.add_reporter(user) + duplicate_project.add_reporter(user) + end + + it 'closes the duplicate issue' do + subject.execute(duplicate_issue, canonical_issue) + + expect(duplicate_issue.reload).to be_closed + expect(canonical_issue.reload).to be_open + end + + it 'adds a system note to the duplicate issue' do + expect(SystemNoteService) + .to receive(:mark_duplicate_issue).with(duplicate_issue, duplicate_project, user, canonical_issue) + + subject.execute(duplicate_issue, canonical_issue) + end + + it 'adds a system note to the canonical issue' do + expect(SystemNoteService) + .to receive(:mark_canonical_issue_of_duplicate).with(canonical_issue, canonical_project, user, duplicate_issue) + + subject.execute(duplicate_issue, canonical_issue) + end + end + end +end diff --git a/spec/services/issues/update_service_spec.rb b/spec/services/issues/update_service_spec.rb index d0b991f19ab..064be940a1c 100644 --- a/spec/services/issues/update_service_spec.rb +++ b/spec/services/issues/update_service_spec.rb @@ -491,6 +491,27 @@ describe Issues::UpdateService, services: true do include_examples 'updating mentions', Issues::UpdateService end + context 'duplicate issue' do + let(:canonical_issue) { create(:issue, project: project) } + + context 'invalid canonical_issue_id' do + it 'does not call the duplicate service' do + expect(Issues::DuplicateService).not_to receive(:new) + + update_issue(canonical_issue_id: 123456789) + end + end + + context 'valid canonical_issue_id' do + it 'calls the duplicate service with both issues' do + expect_any_instance_of(Issues::DuplicateService) + .to receive(:execute).with(issue, canonical_issue) + + update_issue(canonical_issue_id: canonical_issue.id) + end + end + end + include_examples 'issuable update service' do let(:open_issuable) { issue } let(:closed_issuable) { create(:closed_issue, project: project) } diff --git a/spec/services/projects/destroy_service_spec.rb b/spec/services/projects/destroy_service_spec.rb index b399d3402fd..357e09bee95 100644 --- a/spec/services/projects/destroy_service_spec.rb +++ b/spec/services/projects/destroy_service_spec.rb @@ -36,6 +36,27 @@ describe Projects::DestroyService, services: true do end end + shared_examples 'handles errors thrown during async destroy' do |error_message| + it 'does not allow the error to bubble up' do + expect do + Sidekiq::Testing.inline! { destroy_project(project, user, {}) } + end.not_to raise_error + end + + it 'unmarks the project as "pending deletion"' do + Sidekiq::Testing.inline! { destroy_project(project, user, {}) } + + expect(project.reload.pending_delete).to be(false) + end + + it 'stores an error message in `projects.delete_error`' do + Sidekiq::Testing.inline! { destroy_project(project, user, {}) } + + expect(project.reload.delete_error).to be_present + expect(project.delete_error).to include(error_message) + end + end + context 'Sidekiq inline' do before do # Run sidekiq immediatly to check that renamed repository will be removed @@ -89,10 +110,51 @@ describe Projects::DestroyService, services: true do end it_behaves_like 'deleting the project with pipeline and build' - end - context 'with execute' do - it_behaves_like 'deleting the project with pipeline and build' + context 'errors' do + context 'when `remove_legacy_registry_tags` fails' do + before do + expect_any_instance_of(Projects::DestroyService) + .to receive(:remove_legacy_registry_tags).and_return(false) + end + + it_behaves_like 'handles errors thrown during async destroy', "Failed to remove some tags" + end + + context 'when `remove_repository` fails' do + before do + expect_any_instance_of(Projects::DestroyService) + .to receive(:remove_repository).and_return(false) + end + + it_behaves_like 'handles errors thrown during async destroy', "Failed to remove project repository" + end + + context 'when `execute` raises expected error' do + before do + expect_any_instance_of(Project) + .to receive(:destroy!).and_raise(StandardError.new("Other error message")) + end + + it_behaves_like 'handles errors thrown during async destroy', "Other error message" + end + + context 'when `execute` raises unexpected error' do + before do + expect_any_instance_of(Project) + .to receive(:destroy!).and_raise(Exception.new("Other error message")) + end + + it 'allows error to bubble up and rolls back project deletion' do + expect do + Sidekiq::Testing.inline! { destroy_project(project, user, {}) } + end.to raise_error + + expect(project.reload.pending_delete).to be(false) + expect(project.delete_error).to include("Other error message") + end + end + end end describe 'container registry' do @@ -119,8 +181,7 @@ describe Projects::DestroyService, services: true do expect_any_instance_of(ContainerRepository) .to receive(:delete_tags!).and_return(false) - expect{ destroy_project(project, user) } - .to raise_error(ActiveRecord::RecordNotDestroyed) + expect(destroy_project(project, user)).to be false end end end @@ -145,8 +206,7 @@ describe Projects::DestroyService, services: true do expect_any_instance_of(ContainerRepository) .to receive(:delete_tags!).and_return(false) - expect { destroy_project(project, user) } - .to raise_error(Projects::DestroyService::DestroyError) + expect(destroy_project(project, user)).to be false end end end diff --git a/spec/services/projects/update_pages_service_spec.rb b/spec/services/projects/update_pages_service_spec.rb index fc0a17296f3..aa6ad6340f5 100644 --- a/spec/services/projects/update_pages_service_spec.rb +++ b/spec/services/projects/update_pages_service_spec.rb @@ -96,6 +96,78 @@ describe Projects::UpdatePagesService do expect(execute).not_to eq(:success) end + describe 'maximum pages artifacts size' do + let(:metadata) { spy('metadata') } + + before do + file = fixture_file_upload(Rails.root + 'spec/fixtures/pages.zip') + metafile = fixture_file_upload(Rails.root + 'spec/fixtures/pages.zip.meta') + + build.update_attributes(artifacts_file: file) + build.update_attributes(artifacts_metadata: metafile) + + allow(build).to receive(:artifacts_metadata_entry) + .and_return(metadata) + end + + shared_examples 'pages size limit exceeded' do + it 'limits the maximum size of gitlab pages' do + subject.execute + + expect(deploy_status.description) + .to match(/artifacts for pages are too large/) + end + end + + context 'when maximum pages size is set to zero' do + before do + stub_application_setting(max_pages_size: 0) + end + + context 'when page size does not exceed internal maximum' do + before do + allow(metadata).to receive(:total_size).and_return(200.megabytes) + end + + it 'updates pages correctly' do + subject.execute + + expect(deploy_status.description).not_to be_present + end + end + + context 'when pages size does exceed internal maximum' do + before do + allow(metadata).to receive(:total_size).and_return(2.terabytes) + end + + it_behaves_like 'pages size limit exceeded' + end + end + + context 'when pages size is greater than max size setting' do + before do + stub_application_setting(max_pages_size: 200) + allow(metadata).to receive(:total_size).and_return(201.megabytes) + end + + it_behaves_like 'pages size limit exceeded' + end + + context 'when max size setting is greater than internal max size' do + before do + stub_application_setting(max_pages_size: 3.terabytes / 1.megabyte) + allow(metadata).to receive(:total_size).and_return(2.terabytes) + end + + it_behaves_like 'pages size limit exceeded' + end + end + + def deploy_status + GenericCommitStatus.find_by(name: 'pages:deploy') + end + def execute subject.execute[:status] end diff --git a/spec/services/quick_actions/interpret_service_spec.rb b/spec/services/quick_actions/interpret_service_spec.rb index a2db3f68ff7..2a2a5c38e4b 100644 --- a/spec/services/quick_actions/interpret_service_spec.rb +++ b/spec/services/quick_actions/interpret_service_spec.rb @@ -261,6 +261,15 @@ describe QuickActions::InterpretService, services: true do end end + shared_examples 'duplicate command' do + it 'fetches issue and populates canonical_issue_id if content contains /duplicate issue_reference' do + issue_duplicate # populate the issue + _, updates = service.execute(content, issuable) + + expect(updates).to eq(canonical_issue_id: issue_duplicate.id) + end + end + it_behaves_like 'reopen command' do let(:content) { '/reopen' } let(:issuable) { issue } @@ -644,6 +653,41 @@ describe QuickActions::InterpretService, services: true do let(:issuable) { issue } end + context '/duplicate command' do + it_behaves_like 'duplicate command' do + let(:issue_duplicate) { create(:issue, project: project) } + let(:content) { "/duplicate #{issue_duplicate.to_reference}" } + let(:issuable) { issue } + end + + it_behaves_like 'empty command' do + let(:content) { '/duplicate' } + let(:issuable) { issue } + end + + context 'cross project references' do + it_behaves_like 'duplicate command' do + let(:other_project) { create(:empty_project, :public) } + let(:issue_duplicate) { create(:issue, project: other_project) } + let(:content) { "/duplicate #{issue_duplicate.to_reference(project)}" } + let(:issuable) { issue } + end + + it_behaves_like 'empty command' do + let(:content) { "/duplicate imaginary#1234" } + let(:issuable) { issue } + end + + it_behaves_like 'empty command' do + let(:other_project) { create(:empty_project, :private) } + let(:issue_duplicate) { create(:issue, project: other_project) } + + let(:content) { "/duplicate #{issue_duplicate.to_reference(project)}" } + let(:issuable) { issue } + end + end + end + context 'when current_user cannot :admin_issue' do let(:visitor) { create(:user) } let(:issue) { create(:issue, project: project, author: visitor) } @@ -693,6 +737,11 @@ describe QuickActions::InterpretService, services: true do let(:content) { '/remove_due_date' } let(:issuable) { issue } end + + it_behaves_like 'empty command' do + let(:content) { '/duplicate #{issue.to_reference}' } + let(:issuable) { issue } + end end context '/award command' do diff --git a/spec/services/system_note_service_spec.rb b/spec/services/system_note_service_spec.rb index 60477b8e9ba..681b419aedf 100644 --- a/spec/services/system_note_service_spec.rb +++ b/spec/services/system_note_service_spec.rb @@ -1101,4 +1101,54 @@ describe SystemNoteService, services: true do expect(subject.note).to include(diffs_project_merge_request_url(project, merge_request, diff_id: diff_id, anchor: line_code)) end end + + describe '.mark_duplicate_issue' do + subject { described_class.mark_duplicate_issue(noteable, project, author, canonical_issue) } + + context 'within the same project' do + let(:canonical_issue) { create(:issue, project: project) } + + it_behaves_like 'a system note' do + let(:action) { 'duplicate' } + end + + it { expect(subject.note).to eq "marked this issue as a duplicate of #{canonical_issue.to_reference}" } + end + + context 'across different projects' do + let(:other_project) { create(:empty_project) } + let(:canonical_issue) { create(:issue, project: other_project) } + + it_behaves_like 'a system note' do + let(:action) { 'duplicate' } + end + + it { expect(subject.note).to eq "marked this issue as a duplicate of #{canonical_issue.to_reference(project)}" } + end + end + + describe '.mark_canonical_issue_of_duplicate' do + subject { described_class.mark_canonical_issue_of_duplicate(noteable, project, author, duplicate_issue) } + + context 'within the same project' do + let(:duplicate_issue) { create(:issue, project: project) } + + it_behaves_like 'a system note' do + let(:action) { 'duplicate' } + end + + it { expect(subject.note).to eq "marked #{duplicate_issue.to_reference} as a duplicate of this issue" } + end + + context 'across different projects' do + let(:other_project) { create(:empty_project) } + let(:duplicate_issue) { create(:issue, project: other_project) } + + it_behaves_like 'a system note' do + let(:action) { 'duplicate' } + end + + it { expect(subject.note).to eq "marked #{duplicate_issue.to_reference(project)} as a duplicate of this issue" } + end + end end diff --git a/spec/requests/api/milestones_spec.rb b/spec/support/api/milestones_shared_examples.rb index ab5ea3e8f2c..480e7d5151f 100644 --- a/spec/requests/api/milestones_spec.rb +++ b/spec/support/api/milestones_shared_examples.rb @@ -1,21 +1,14 @@ -require 'spec_helper' - -describe API::Milestones do - let(:user) { create(:user) } - let!(:project) { create(:empty_project, namespace: user.namespace ) } - let!(:closed_milestone) { create(:closed_milestone, project: project, title: 'version1', description: 'closed milestone') } - let!(:milestone) { create(:milestone, project: project, title: 'version2', description: 'open milestone') } +shared_examples_for 'group and project milestones' do |route_definition| + let(:resource_route) { "#{route}/#{milestone.id}" } let(:label_1) { create(:label, title: 'label_1', project: project, priority: 1) } let(:label_2) { create(:label, title: 'label_2', project: project, priority: 2) } let(:label_3) { create(:label, title: 'label_3', project: project) } + let(:merge_request) { create(:merge_request, source_project: project) } + let(:another_merge_request) { create(:merge_request, :simple, source_project: project) } - before do - project.team << [user, :developer] - end - - describe 'GET /projects/:id/milestones' do - it 'returns project milestones' do - get api("/projects/#{project.id}/milestones", user) + describe "GET #{route_definition}" do + it 'returns milestones list' do + get api(route, user) expect(response).to have_http_status(200) expect(response).to include_pagination_headers @@ -24,13 +17,13 @@ describe API::Milestones do end it 'returns a 401 error if user not authenticated' do - get api("/projects/#{project.id}/milestones") + get api(route) expect(response).to have_http_status(401) end it 'returns an array of active milestones' do - get api("/projects/#{project.id}/milestones?state=active", user) + get api("#{route}/?state=active", user) expect(response).to have_http_status(200) expect(response).to include_pagination_headers @@ -40,7 +33,7 @@ describe API::Milestones do end it 'returns an array of closed milestones' do - get api("/projects/#{project.id}/milestones?state=closed", user) + get api("#{route}/?state=closed", user) expect(response).to have_http_status(200) expect(response).to include_pagination_headers @@ -50,9 +43,9 @@ describe API::Milestones do end it 'returns an array of milestones specified by iids' do - other_milestone = create(:milestone, project: project) + other_milestone = create(:milestone, project: try(:project), group: try(:group)) - get api("/projects/#{project.id}/milestones", user), iids: [closed_milestone.iid, other_milestone.iid] + get api(route, user), iids: [closed_milestone.iid, other_milestone.iid] expect(response).to have_http_status(200) expect(json_response).to be_an Array @@ -61,25 +54,15 @@ describe API::Milestones do end it 'does not return any milestone if none found' do - get api("/projects/#{project.id}/milestones", user), iids: [Milestone.maximum(:iid).succ] + get api(route, user), iids: [Milestone.maximum(:iid).succ] expect(response).to have_http_status(200) expect(json_response).to be_an Array expect(json_response.length).to eq(0) end - end - - describe 'GET /projects/:id/milestones/:milestone_id' do - it 'returns a project milestone by id' do - get api("/projects/#{project.id}/milestones/#{milestone.id}", user) - - expect(response).to have_http_status(200) - expect(json_response['title']).to eq(milestone.title) - expect(json_response['iid']).to eq(milestone.iid) - end - it 'returns a project milestone by iids array' do - get api("/projects/#{project.id}/milestones?iids=#{closed_milestone.iid}", user) + it 'returns a milestone by iids array' do + get api("#{route}?iids=#{closed_milestone.iid}", user) expect(response.status).to eq 200 expect(response).to include_pagination_headers @@ -89,8 +72,8 @@ describe API::Milestones do expect(json_response.first['id']).to eq closed_milestone.id end - it 'returns a project milestone by searching for title' do - get api("/projects/#{project.id}/milestones", user), search: 'version2' + it 'returns a milestone by searching for title' do + get api(route, user), search: 'version2' expect(response).to have_http_status(200) expect(response).to include_pagination_headers @@ -99,8 +82,8 @@ describe API::Milestones do expect(json_response.first['id']).to eq milestone.id end - it 'returns a project milestones by searching for description' do - get api("/projects/#{project.id}/milestones", user), search: 'open' + it 'returns a milestones by searching for description' do + get api(route, user), search: 'open' expect(response).to have_http_status(200) expect(response).to include_pagination_headers @@ -110,9 +93,17 @@ describe API::Milestones do end end - describe 'GET /projects/:id/milestones/:milestone_id' do - it 'returns a project milestone by id' do - get api("/projects/#{project.id}/milestones/#{milestone.id}", user) + describe "GET #{route_definition}/:milestone_id" do + it 'returns a milestone by id' do + get api(resource_route, user) + + expect(response).to have_http_status(200) + expect(json_response['title']).to eq(milestone.title) + expect(json_response['iid']).to eq(milestone.iid) + end + + it 'returns a milestone by id' do + get api(resource_route, user) expect(response).to have_http_status(200) expect(json_response['title']).to eq(milestone.title) @@ -120,29 +111,29 @@ describe API::Milestones do end it 'returns 401 error if user not authenticated' do - get api("/projects/#{project.id}/milestones/#{milestone.id}") + get api(resource_route) expect(response).to have_http_status(401) end it 'returns a 404 error if milestone id not found' do - get api("/projects/#{project.id}/milestones/1234", user) + get api("#{route}/1234", user) expect(response).to have_http_status(404) end end - describe 'POST /projects/:id/milestones' do - it 'creates a new project milestone' do - post api("/projects/#{project.id}/milestones", user), title: 'new milestone' + describe "POST #{route_definition}" do + it 'creates a new milestone' do + post api(route, user), title: 'new milestone' expect(response).to have_http_status(201) expect(json_response['title']).to eq('new milestone') expect(json_response['description']).to be_nil end - it 'creates a new project milestone with description and dates' do - post api("/projects/#{project.id}/milestones", user), + it 'creates a new milestone with description and dates' do + post api(route, user), title: 'new milestone', description: 'release', due_date: '2013-03-02', start_date: '2013-02-02' expect(response).to have_http_status(201) @@ -152,20 +143,20 @@ describe API::Milestones do end it 'returns a 400 error if title is missing' do - post api("/projects/#{project.id}/milestones", user) + post api(route, user) expect(response).to have_http_status(400) end it 'returns a 400 error if params are invalid (duplicate title)' do - post api("/projects/#{project.id}/milestones", user), + post api(route, user), title: milestone.title, description: 'release', due_date: '2013-03-02' expect(response).to have_http_status(400) end - it 'creates a new project with reserved html characters' do - post api("/projects/#{project.id}/milestones", user), title: 'foo & bar 1.1 -> 2.2' + it 'creates a new milestone with reserved html characters' do + post api(route, user), title: 'foo & bar 1.1 -> 2.2' expect(response).to have_http_status(201) expect(json_response['title']).to eq('foo & bar 1.1 -> 2.2') @@ -173,9 +164,9 @@ describe API::Milestones do end end - describe 'PUT /projects/:id/milestones/:milestone_id' do - it 'updates a project milestone' do - put api("/projects/#{project.id}/milestones/#{milestone.id}", user), + describe "PUT #{route_definition}/:milestone_id" do + it 'updates a milestone' do + put api(resource_route, user), title: 'updated title' expect(response).to have_http_status(200) @@ -185,23 +176,21 @@ describe API::Milestones do it 'removes a due date if nil is passed' do milestone.update!(due_date: "2016-08-05") - put api("/projects/#{project.id}/milestones/#{milestone.id}", user), due_date: nil + put api(resource_route, user), due_date: nil expect(response).to have_http_status(200) expect(json_response['due_date']).to be_nil end it 'returns a 404 error if milestone id not found' do - put api("/projects/#{project.id}/milestones/1234", user), + put api("#{route}/1234", user), title: 'updated title' expect(response).to have_http_status(404) end - end - describe 'PUT /projects/:id/milestones/:milestone_id to close milestone' do - it 'updates a project milestone' do - put api("/projects/#{project.id}/milestones/#{milestone.id}", user), + it 'closes milestone' do + put api(resource_route, user), state_event: 'close' expect(response).to have_http_status(200) @@ -209,21 +198,14 @@ describe API::Milestones do end end - describe 'PUT /projects/:id/milestones/:milestone_id to test observer on close' do - it 'creates an activity event when an milestone is closed' do - expect(Event).to receive(:create) - - put api("/projects/#{project.id}/milestones/#{milestone.id}", user), - state_event: 'close' - end - end + describe "GET #{route_definition}/:milestone_id/issues" do + let(:issues_route) { "#{route}/#{milestone.id}/issues" } - describe 'GET /projects/:id/milestones/:milestone_id/issues' do before do milestone.issues << create(:issue, project: project) end - it 'returns project issues for a particular milestone' do - get api("/projects/#{project.id}/milestones/#{milestone.id}/issues", user) + it 'returns issues for a particular milestone' do + get api(issues_route, user) expect(response).to have_http_status(200) expect(response).to include_pagination_headers @@ -231,12 +213,12 @@ describe API::Milestones do expect(json_response.first['milestone']['title']).to eq(milestone.title) end - it 'returns project issues sorted by label priority' do + it 'returns issues sorted by label priority' do issue_1 = create(:labeled_issue, project: project, milestone: milestone, labels: [label_3]) issue_2 = create(:labeled_issue, project: project, milestone: milestone, labels: [label_1]) issue_3 = create(:labeled_issue, project: project, milestone: milestone, labels: [label_2]) - get api("/projects/#{project.id}/milestones/#{milestone.id}/issues", user) + get api(issues_route, user) expect(json_response.first['id']).to eq(issue_2.id) expect(json_response.second['id']).to eq(issue_3.id) @@ -244,44 +226,58 @@ describe API::Milestones do end it 'matches V4 response schema for a list of issues' do - get api("/projects/#{project.id}/milestones/#{milestone.id}/issues", user) + get api(issues_route, user) expect(response).to have_http_status(200) expect(response).to match_response_schema('public_api/v4/issues') end it 'returns a 401 error if user not authenticated' do - get api("/projects/#{project.id}/milestones/#{milestone.id}/issues") + get api(issues_route) expect(response).to have_http_status(401) end describe 'confidential issues' do - let(:public_project) { create(:empty_project, :public) } - let(:milestone) { create(:milestone, project: public_project) } - let(:issue) { create(:issue, project: public_project) } - let(:confidential_issue) { create(:issue, confidential: true, project: public_project) } + let!(:public_project) { create(:empty_project, :public) } + let!(:context_group) { try(:group) } + let!(:milestone) do + context_group ? create(:milestone, group: context_group) : create(:milestone, project: public_project) + end + let!(:issue) { create(:issue, project: public_project) } + let!(:confidential_issue) { create(:issue, confidential: true, project: public_project) } + let!(:issues_route) do + if context_group + "#{route}/#{milestone.id}/issues" + else + "/projects/#{public_project.id}/milestones/#{milestone.id}/issues" + end + end before do + # Add public project to the group in context + setup_for_group if context_group + public_project.team << [user, :developer] milestone.issues << issue << confidential_issue end it 'returns confidential issues to team members' do - get api("/projects/#{public_project.id}/milestones/#{milestone.id}/issues", user) + get api(issues_route, user) expect(response).to have_http_status(200) expect(response).to include_pagination_headers expect(json_response).to be_an Array - expect(json_response.size).to eq(2) + # 2 for projects, 3 for group(which has another project with an issue) + expect(json_response.size).to be_between(2, 3) expect(json_response.map { |issue| issue['id'] }).to include(issue.id, confidential_issue.id) end it 'does not return confidential issues to team members with guest role' do member = create(:user) - project.team << [member, :guest] + public_project.team << [member, :guest] - get api("/projects/#{public_project.id}/milestones/#{milestone.id}/issues", member) + get api(issues_route, member) expect(response).to have_http_status(200) expect(response).to include_pagination_headers @@ -291,7 +287,7 @@ describe API::Milestones do end it 'does not return confidential issues to regular users' do - get api("/projects/#{public_project.id}/milestones/#{milestone.id}/issues", create(:user)) + get api(issues_route, create(:user)) expect(response).to have_http_status(200) expect(response).to include_pagination_headers @@ -304,30 +300,30 @@ describe API::Milestones do issue.labels << label_2 confidential_issue.labels << label_1 - get api("/projects/#{public_project.id}/milestones/#{milestone.id}/issues", user) + get api(issues_route, user) expect(response).to have_http_status(200) expect(response).to include_pagination_headers expect(json_response).to be_an Array - expect(json_response.size).to eq(2) + # 2 for projects, 3 for group(which has another project with an issue) + expect(json_response.size).to be_between(2, 3) expect(json_response.first['id']).to eq(confidential_issue.id) expect(json_response.second['id']).to eq(issue.id) end end end - describe 'GET /projects/:id/milestones/:milestone_id/merge_requests' do - let(:merge_request) { create(:merge_request, source_project: project) } - let(:another_merge_request) { create(:merge_request, :simple, source_project: project) } + describe "GET #{route_definition}/:milestone_id/merge_requests" do + let(:merge_requests_route) { "#{route}/#{milestone.id}/merge_requests" } before do milestone.merge_requests << merge_request end - it 'returns project merge_requests for a particular milestone' do + it 'returns merge_requests for a particular milestone' do # eager-load another_merge_request another_merge_request - get api("/projects/#{project.id}/milestones/#{milestone.id}/merge_requests", user) + get api(merge_requests_route, user) expect(response).to have_http_status(200) expect(json_response).to be_an Array @@ -336,12 +332,12 @@ describe API::Milestones do expect(json_response.first['milestone']['title']).to eq(milestone.title) end - it 'returns project merge_requests sorted by label priority' do + it 'returns merge_requests sorted by label priority' do merge_request_1 = create(:labeled_merge_request, source_branch: 'branch_1', source_project: project, milestone: milestone, labels: [label_2]) merge_request_2 = create(:labeled_merge_request, source_branch: 'branch_2', source_project: project, milestone: milestone, labels: [label_1]) merge_request_3 = create(:labeled_merge_request, source_branch: 'branch_3', source_project: project, milestone: milestone, labels: [label_3]) - get api("/projects/#{project.id}/milestones/#{milestone.id}/merge_requests", user) + get api(merge_requests_route, user) expect(json_response.first['id']).to eq(merge_request_2.id) expect(json_response.second['id']).to eq(merge_request_1.id) @@ -349,20 +345,22 @@ describe API::Milestones do end it 'returns a 404 error if milestone id not found' do - get api("/projects/#{project.id}/milestones/1234/merge_requests", user) + not_found_route = "#{route}/1234/merge_requests" + + get api(not_found_route, user) expect(response).to have_http_status(404) end it 'returns a 404 if the user has no access to the milestone' do new_user = create :user - get api("/projects/#{project.id}/milestones/#{milestone.id}/merge_requests", new_user) + get api(merge_requests_route, new_user) expect(response).to have_http_status(404) end it 'returns a 401 error if user not authenticated' do - get api("/projects/#{project.id}/milestones/#{milestone.id}/merge_requests") + get api(merge_requests_route) expect(response).to have_http_status(401) end @@ -372,7 +370,7 @@ describe API::Milestones do another_merge_request.labels << label_1 merge_request.labels << label_2 - get api("/projects/#{project.id}/milestones/#{milestone.id}/merge_requests", user) + get api(merge_requests_route, user) expect(response).to have_http_status(200) expect(response).to include_pagination_headers diff --git a/spec/support/api/schema_matcher.rb b/spec/support/api/schema_matcher.rb index dff0dfba675..67599f77adb 100644 --- a/spec/support/api/schema_matcher.rb +++ b/spec/support/api/schema_matcher.rb @@ -5,7 +5,14 @@ end RSpec::Matchers.define :match_response_schema do |schema, **options| match do |response| - JSON::Validator.validate!(schema_path(schema), response.body, options) + @errors = JSON::Validator.fully_validate(schema_path(schema), response.body, options) + + @errors.empty? + end + + failure_message do |response| + "didn't match the schema defined by #{schema_path(schema)}" \ + " The validation errors were:\n#{@errors.join("\n")}" end end diff --git a/spec/support/forgery_protection.rb b/spec/support/forgery_protection.rb new file mode 100644 index 00000000000..a5e7b761651 --- /dev/null +++ b/spec/support/forgery_protection.rb @@ -0,0 +1,11 @@ +RSpec.configure do |config| + config.around(:each, :allow_forgery_protection) do |example| + begin + ActionController::Base.allow_forgery_protection = true + + example.call + ensure + ActionController::Base.allow_forgery_protection = false + end + end +end diff --git a/spec/support/jira_service_helper.rb b/spec/support/jira_service_helper.rb index 97ae0b6afc5..0b5f66597fd 100644 --- a/spec/support/jira_service_helper.rb +++ b/spec/support/jira_service_helper.rb @@ -51,7 +51,7 @@ module JiraServiceHelper end def jira_project_url - JIRA_API + "/project/#{jira_tracker.project_key}" + JIRA_API + "/project" end def jira_api_comment_url(issue_id) diff --git a/spec/support/api/status_shared_examples.rb b/spec/support/shared_examples/requests/api/status_shared_examples.rb index 3481749a7f0..226277411d6 100644 --- a/spec/support/api/status_shared_examples.rb +++ b/spec/support/shared_examples/requests/api/status_shared_examples.rb @@ -9,7 +9,7 @@ shared_examples_for '400 response' do end it 'returns 400' do - expect(response).to have_http_status(400) + expect(response).to have_gitlab_http_status(400) end end @@ -20,7 +20,7 @@ shared_examples_for '403 response' do end it 'returns 403' do - expect(response).to have_http_status(403) + expect(response).to have_gitlab_http_status(403) end end @@ -32,7 +32,7 @@ shared_examples_for '404 response' do end it 'returns 404' do - expect(response).to have_http_status(404) + expect(response).to have_gitlab_http_status(404) expect(json_response).to be_an Object if message.present? diff --git a/spec/support/stub_configuration.rb b/spec/support/stub_configuration.rb index 48f454c7187..516f8878679 100644 --- a/spec/support/stub_configuration.rb +++ b/spec/support/stub_configuration.rb @@ -4,29 +4,38 @@ module StubConfiguration # Stubbing both of these because we're not yet consistent with how we access # current application settings - allow_any_instance_of(ApplicationSetting).to receive_messages(messages) + allow_any_instance_of(ApplicationSetting).to receive_messages(to_settings(messages)) allow(Gitlab::CurrentSettings.current_application_settings) - .to receive_messages(messages) + .to receive_messages(to_settings(messages)) + end + + def stub_not_protect_default_branch + stub_application_setting( + default_branch_protection: Gitlab::Access::PROTECTION_NONE) end def stub_config_setting(messages) - allow(Gitlab.config.gitlab).to receive_messages(messages) + allow(Gitlab.config.gitlab).to receive_messages(to_settings(messages)) end def stub_gravatar_setting(messages) - allow(Gitlab.config.gravatar).to receive_messages(messages) + allow(Gitlab.config.gravatar).to receive_messages(to_settings(messages)) end def stub_incoming_email_setting(messages) - allow(Gitlab.config.incoming_email).to receive_messages(messages) + allow(Gitlab.config.incoming_email).to receive_messages(to_settings(messages)) end def stub_mattermost_setting(messages) - allow(Gitlab.config.mattermost).to receive_messages(messages) + allow(Gitlab.config.mattermost).to receive_messages(to_settings(messages)) end def stub_omniauth_setting(messages) - allow(Gitlab.config.omniauth).to receive_messages(messages) + allow(Gitlab.config.omniauth).to receive_messages(to_settings(messages)) + end + + def stub_backup_setting(messages) + allow(Gitlab.config.backup).to receive_messages(to_settings(messages)) end private @@ -49,4 +58,15 @@ module StubConfiguration messages[predicate.to_sym] = messages[key.to_sym] end end + + # Support nested hashes by converting all values into Settingslogic objects + def to_settings(hash) + hash.transform_values do |value| + if value.is_a? Hash + Settingslogic.new(value.deep_stringify_keys) + else + value + end + end + end end diff --git a/spec/support/test_env.rb b/spec/support/test_env.rb index 0a194ca4c90..c32c05b03e2 100644 --- a/spec/support/test_env.rb +++ b/spec/support/test_env.rb @@ -41,7 +41,8 @@ module TestEnv 'csv' => '3dd0896', 'v1.1.0' => 'b83d6e3', 'add-ipython-files' => '93ee732', - 'add-pdf-file' => 'e774ebd' + 'add-pdf-file' => 'e774ebd', + 'add-pdf-text-binary' => '79faa7b' }.freeze # gitlab-test-fork is a fork of gitlab-fork, but we don't necessarily diff --git a/spec/workers/post_receive_spec.rb b/spec/workers/post_receive_spec.rb index a8f4bb72acf..74a9f90195c 100644 --- a/spec/workers/post_receive_spec.rb +++ b/spec/workers/post_receive_spec.rb @@ -74,6 +74,7 @@ describe PostReceive do OpenStruct.new(id: '123456') end allow_any_instance_of(Ci::CreatePipelineService).to receive(:branch?).and_return(true) + allow_any_instance_of(Repository).to receive(:ref_exists?).and_return(true) stub_ci_pipeline_to_return_yaml_file end diff --git a/spec/workers/project_destroy_worker_spec.rb b/spec/workers/project_destroy_worker_spec.rb index 3d135f40c1f..f19c9dff941 100644 --- a/spec/workers/project_destroy_worker_spec.rb +++ b/spec/workers/project_destroy_worker_spec.rb @@ -1,24 +1,36 @@ require 'spec_helper' describe ProjectDestroyWorker do - let(:project) { create(:project, :repository) } + let(:project) { create(:project, :repository, pending_delete: true) } let(:path) { project.repository.path_to_repo } subject { described_class.new } - describe "#perform" do - it "deletes the project" do + describe '#perform' do + it 'deletes the project' do subject.perform(project.id, project.owner.id, {}) expect(Project.all).not_to include(project) expect(Dir.exist?(path)).to be_falsey end - it "deletes the project but skips repo deletion" do + it 'deletes the project but skips repo deletion' do subject.perform(project.id, project.owner.id, { "skip_repo" => true }) expect(Project.all).not_to include(project) expect(Dir.exist?(path)).to be_truthy end + + it 'does not raise error when project could not be found' do + expect do + subject.perform(-1, project.owner.id, {}) + end.not_to raise_error + end + + it 'does not raise error when user could not be found' do + expect do + subject.perform(project.id, -1, {}) + end.not_to raise_error + end end end |