diff options
Diffstat (limited to 'spec/services')
| -rw-r--r-- | spec/services/application_settings/update_service_spec.rb | 88 | ||||
| -rw-r--r-- | spec/services/check_gcp_project_billing_service_spec.rb | 32 | ||||
| -rw-r--r-- | spec/services/merge_requests/merge_service_spec.rb | 60 | ||||
| -rw-r--r-- | spec/services/merge_requests/squash_service_spec.rb | 199 | ||||
| -rw-r--r-- | spec/services/notes/create_service_spec.rb | 82 |
5 files changed, 415 insertions, 46 deletions
diff --git a/spec/services/application_settings/update_service_spec.rb b/spec/services/application_settings/update_service_spec.rb index fb07ecc6ae8..6337ee7d724 100644 --- a/spec/services/application_settings/update_service_spec.rb +++ b/spec/services/application_settings/update_service_spec.rb @@ -1,7 +1,7 @@ require 'spec_helper' describe ApplicationSettings::UpdateService do - let(:application_settings) { Gitlab::CurrentSettings.current_application_settings } + let(:application_settings) { create(:application_setting) } let(:admin) { create(:user, :admin) } let(:params) { {} } @@ -54,4 +54,90 @@ describe ApplicationSettings::UpdateService do end end end + + describe 'performance bar settings' do + using RSpec::Parameterized::TableSyntax + + where(:params_performance_bar_enabled, + :params_performance_bar_allowed_group_path, + :previous_performance_bar_allowed_group_id, + :expected_performance_bar_allowed_group_id) do + true | '' | nil | nil + true | '' | 42_000_000 | nil + true | nil | nil | nil + true | nil | 42_000_000 | nil + true | 'foo' | nil | nil + true | 'foo' | 42_000_000 | nil + true | 'group_a' | nil | 42_000_000 + true | 'group_b' | 42_000_000 | 43_000_000 + true | 'group_a' | 42_000_000 | 42_000_000 + false | '' | nil | nil + false | '' | 42_000_000 | nil + false | nil | nil | nil + false | nil | 42_000_000 | nil + false | 'foo' | nil | nil + false | 'foo' | 42_000_000 | nil + false | 'group_a' | nil | nil + false | 'group_b' | 42_000_000 | nil + false | 'group_a' | 42_000_000 | nil + end + + with_them do + let(:params) do + { + performance_bar_enabled: params_performance_bar_enabled, + performance_bar_allowed_group_path: params_performance_bar_allowed_group_path + } + end + + before do + if previous_performance_bar_allowed_group_id == 42_000_000 || params_performance_bar_allowed_group_path == 'group_a' + create(:group, id: 42_000_000, path: 'group_a') + end + + if expected_performance_bar_allowed_group_id == 43_000_000 || params_performance_bar_allowed_group_path == 'group_b' + create(:group, id: 43_000_000, path: 'group_b') + end + + application_settings.update!(performance_bar_allowed_group_id: previous_performance_bar_allowed_group_id) + end + + it 'sets performance_bar_allowed_group_id when present and performance_bar_enabled == true' do + expect(application_settings.performance_bar_allowed_group_id).to eq(previous_performance_bar_allowed_group_id) + + if previous_performance_bar_allowed_group_id != expected_performance_bar_allowed_group_id + expect { subject.execute } + .to change(application_settings, :performance_bar_allowed_group_id) + .from(previous_performance_bar_allowed_group_id).to(expected_performance_bar_allowed_group_id) + else + expect { subject.execute } + .not_to change(application_settings, :performance_bar_allowed_group_id) + end + end + end + + context 'when :performance_bar_allowed_group_path is not present' do + let(:group) { create(:group) } + + before do + application_settings.update!(performance_bar_allowed_group_id: group.id) + end + + it 'does not change the performance bar settings' do + expect { subject.execute } + .not_to change(application_settings, :performance_bar_allowed_group_id) + end + end + + context 'when :performance_bar_enabled is not present' do + let(:group) { create(:group) } + let(:params) { { performance_bar_allowed_group_path: group.full_path } } + + it 'implicitely defaults to true' do + expect { subject.execute } + .to change(application_settings, :performance_bar_allowed_group_id) + .from(nil).to(group.id) + end + end + end end diff --git a/spec/services/check_gcp_project_billing_service_spec.rb b/spec/services/check_gcp_project_billing_service_spec.rb deleted file mode 100644 index 3e68d906e71..00000000000 --- a/spec/services/check_gcp_project_billing_service_spec.rb +++ /dev/null @@ -1,32 +0,0 @@ -require 'spec_helper' - -describe CheckGcpProjectBillingService do - include GoogleApi::CloudPlatformHelpers - - let(:service) { described_class.new } - let(:project_id) { 'test-project-1234' } - - describe '#execute' do - before do - stub_cloud_platform_projects_list(project_id: project_id) - end - - subject { service.execute('bogustoken') } - - context 'google account has a billing enabled gcp project' do - before do - stub_cloud_platform_projects_get_billing_info(project_id, true) - end - - it { is_expected.to all(satisfy { |project| project.project_id == project_id }) } - end - - context 'google account does not have a billing enabled gcp project' do - before do - stub_cloud_platform_projects_get_billing_info(project_id, false) - end - - it { is_expected.to eq([]) } - end - end -end diff --git a/spec/services/merge_requests/merge_service_spec.rb b/spec/services/merge_requests/merge_service_spec.rb index e8568bf8bb3..dc30a9bccc1 100644 --- a/spec/services/merge_requests/merge_service_spec.rb +++ b/spec/services/merge_requests/merge_service_spec.rb @@ -249,24 +249,58 @@ describe MergeRequests::MergeService do expect(Rails.logger).to have_received(:error).with(a_string_matching(error_message)) end - context "when fast-forward merge is not allowed" do + context 'when squashing' do before do - allow_any_instance_of(Repository).to receive(:ancestor?).and_return(nil) + merge_request.update!(source_branch: 'master', target_branch: 'feature') end - %w(semi-linear ff).each do |merge_method| - it "logs and saves error if merge is #{merge_method} only" do - merge_method = 'rebase_merge' if merge_method == 'semi-linear' - merge_request.project.update(merge_method: merge_method) - error_message = 'Only fast-forward merge is allowed for your project. Please update your source branch' - allow(service).to receive(:execute_hooks) + it 'logs and saves error if there is an error when squashing' do + error_message = 'Failed to squash. Should be done manually' - service.execute(merge_request) + allow_any_instance_of(MergeRequests::SquashService).to receive(:squash).and_return(nil) + merge_request.update(squash: true) + + service.execute(merge_request) + + expect(merge_request).to be_open + expect(merge_request.merge_commit_sha).to be_nil + expect(merge_request.merge_error).to include(error_message) + expect(Rails.logger).to have_received(:error).with(a_string_matching(error_message)) + end + + it 'logs and saves error if there is a squash in progress' do + error_message = 'another squash is already in progress' + + allow_any_instance_of(MergeRequest).to receive(:squash_in_progress?).and_return(true) + merge_request.update(squash: true) + + service.execute(merge_request) - expect(merge_request).to be_open - expect(merge_request.merge_commit_sha).to be_nil - expect(merge_request.merge_error).to include(error_message) - expect(Rails.logger).to have_received(:error).with(a_string_matching(error_message)) + expect(merge_request).to be_open + expect(merge_request.merge_commit_sha).to be_nil + expect(merge_request.merge_error).to include(error_message) + expect(Rails.logger).to have_received(:error).with(a_string_matching(error_message)) + end + + context "when fast-forward merge is not allowed" do + before do + allow_any_instance_of(Repository).to receive(:ancestor?).and_return(nil) + end + + %w(semi-linear ff).each do |merge_method| + it "logs and saves error if merge is #{merge_method} only" do + merge_method = 'rebase_merge' if merge_method == 'semi-linear' + merge_request.project.update(merge_method: merge_method) + error_message = 'Only fast-forward merge is allowed for your project. Please update your source branch' + allow(service).to receive(:execute_hooks) + + service.execute(merge_request) + + expect(merge_request).to be_open + expect(merge_request.merge_commit_sha).to be_nil + expect(merge_request.merge_error).to include(error_message) + expect(Rails.logger).to have_received(:error).with(a_string_matching(error_message)) + end end end end diff --git a/spec/services/merge_requests/squash_service_spec.rb b/spec/services/merge_requests/squash_service_spec.rb new file mode 100644 index 00000000000..bd884787425 --- /dev/null +++ b/spec/services/merge_requests/squash_service_spec.rb @@ -0,0 +1,199 @@ +require 'spec_helper' + +describe MergeRequests::SquashService do + let(:service) { described_class.new(project, user, {}) } + let(:user) { project.owner } + let(:project) { create(:project, :repository) } + let(:repository) { project.repository.raw } + let(:log_error) { "Failed to squash merge request #{merge_request.to_reference(full: true)}:" } + let(:squash_dir_path) do + File.join(Gitlab.config.shared.path, 'tmp/squash', repository.gl_repository, merge_request.id.to_s) + end + let(:merge_request_with_one_commit) do + create(:merge_request, + source_branch: 'feature', source_project: project, + target_branch: 'master', target_project: project) + end + + let(:merge_request_with_only_new_files) do + create(:merge_request, + source_branch: 'video', source_project: project, + target_branch: 'master', target_project: project) + end + + let(:merge_request_with_large_files) do + create(:merge_request, + source_branch: 'squash-large-files', source_project: project, + target_branch: 'master', target_project: project) + end + + shared_examples 'the squash succeeds' do + it 'returns the squashed commit SHA' do + result = service.execute(merge_request) + + expect(result).to match(status: :success, squash_sha: a_string_matching(/\h{40}/)) + expect(result[:squash_sha]).not_to eq(merge_request.diff_head_sha) + end + + it 'cleans up the temporary directory' do + service.execute(merge_request) + + expect(File.exist?(squash_dir_path)).to be(false) + end + + it 'does not keep the branch push event' do + expect { service.execute(merge_request) }.not_to change { Event.count } + end + + context 'the squashed commit' do + let(:squash_sha) { service.execute(merge_request)[:squash_sha] } + let(:squash_commit) { project.repository.commit(squash_sha) } + + it 'copies the author info and message from the merge request' do + expect(squash_commit.author_name).to eq(merge_request.author.name) + expect(squash_commit.author_email).to eq(merge_request.author.email) + + # Commit messages have a trailing newline, but titles don't. + expect(squash_commit.message.chomp).to eq(merge_request.title) + end + + it 'sets the current user as the committer' do + expect(squash_commit.committer_name).to eq(user.name.chomp('.')) + expect(squash_commit.committer_email).to eq(user.email) + end + + it 'has the same diff as the merge request, but a different SHA' do + rugged = project.repository.rugged + mr_diff = rugged.diff(merge_request.diff_base_sha, merge_request.diff_head_sha) + squash_diff = rugged.diff(merge_request.diff_start_sha, squash_sha) + + expect(squash_diff.patch.length).to eq(mr_diff.patch.length) + expect(squash_commit.sha).not_to eq(merge_request.diff_head_sha) + end + end + end + + describe '#execute' do + context 'when there is only one commit in the merge request' do + it 'returns that commit SHA' do + result = service.execute(merge_request_with_one_commit) + + expect(result).to match(status: :success, squash_sha: merge_request_with_one_commit.diff_head_sha) + end + + it 'does not perform any git actions' do + expect(repository).not_to receive(:popen) + + service.execute(merge_request_with_one_commit) + end + end + + context 'when squashing only new files' do + let(:merge_request) { merge_request_with_only_new_files } + + include_examples 'the squash succeeds' + end + + context 'when squashing with files too large to display' do + let(:merge_request) { merge_request_with_large_files } + + include_examples 'the squash succeeds' + end + + context 'git errors' do + let(:merge_request) { merge_request_with_only_new_files } + let(:error) { 'A test error' } + + context 'with gitaly enabled' do + before do + allow(repository.gitaly_operation_client).to receive(:user_squash) + .and_raise(Gitlab::Git::Repository::GitError, error) + end + + it 'logs the stage and output' do + expect(service).to receive(:log_error).with(log_error) + expect(service).to receive(:log_error).with(error) + + service.execute(merge_request) + end + + it 'returns an error' do + expect(service.execute(merge_request)).to match(status: :error, + message: a_string_including('squash')) + end + end + + context 'with Gitaly disabled', :skip_gitaly_mock do + stages = { + 'add worktree for squash' => 'worktree', + 'configure sparse checkout' => 'config', + 'get files in diff' => 'diff --name-only', + 'check out target branch' => 'checkout', + 'apply patch' => 'diff --binary', + 'commit squashed changes' => 'commit', + 'get SHA of squashed commit' => 'rev-parse' + } + + stages.each do |stage, command| + context "when the #{stage} stage fails" do + before do + git_command = a_collection_containing_exactly( + a_string_starting_with("#{Gitlab.config.git.bin_path} #{command}") + ).or( + a_collection_starting_with([Gitlab.config.git.bin_path] + command.split) + ) + + allow(repository).to receive(:popen).and_return(['', 0]) + allow(repository).to receive(:popen).with(git_command, anything, anything, anything).and_return([error, 1]) + end + + it 'logs the stage and output' do + expect(service).to receive(:log_error).with(log_error) + expect(service).to receive(:log_error).with(error) + + service.execute(merge_request) + end + + it 'returns an error' do + expect(service.execute(merge_request)).to match(status: :error, + message: a_string_including('squash')) + end + + it 'cleans up the temporary directory' do + expect(File.exist?(squash_dir_path)).to be(false) + + service.execute(merge_request) + end + end + end + end + end + + context 'when any other exception is thrown' do + let(:merge_request) { merge_request_with_only_new_files } + let(:error) { 'A test error' } + + before do + allow(merge_request).to receive(:commits_count).and_raise(error) + end + + it 'logs the MR reference and exception' do + expect(service).to receive(:log_error).with(a_string_including("#{project.full_path}#{merge_request.to_reference}")) + expect(service).to receive(:log_error).with(error) + + service.execute(merge_request) + end + + it 'returns an error' do + expect(service.execute(merge_request)).to match(status: :error, + message: a_string_including('squash')) + end + + it 'cleans up the temporary directory' do + service.execute(merge_request) + + expect(File.exist?(squash_dir_path)).to be(false) + end + end + end +end diff --git a/spec/services/notes/create_service_spec.rb b/spec/services/notes/create_service_spec.rb index f5cff66de6d..2b2b983494f 100644 --- a/spec/services/notes/create_service_spec.rb +++ b/spec/services/notes/create_service_spec.rb @@ -57,6 +57,88 @@ describe Notes::CreateService do end end + context 'note diff file' do + let(:project_with_repo) { create(:project, :repository) } + let(:merge_request) do + create(:merge_request, + source_project: project_with_repo, + target_project: project_with_repo) + end + let(:line_number) { 14 } + let(:position) do + Gitlab::Diff::Position.new(old_path: "files/ruby/popen.rb", + new_path: "files/ruby/popen.rb", + old_line: nil, + new_line: line_number, + diff_refs: merge_request.diff_refs) + end + let(:previous_note) do + create(:diff_note_on_merge_request, noteable: merge_request, project: project_with_repo) + end + + context 'when eligible to have a note diff file' do + let(:new_opts) do + opts.merge(in_reply_to_discussion_id: nil, + type: 'DiffNote', + noteable_type: 'MergeRequest', + noteable_id: merge_request.id, + position: position.to_h) + end + + it 'note is associated with a note diff file' do + note = described_class.new(project_with_repo, user, new_opts).execute + + expect(note).to be_persisted + expect(note.note_diff_file).to be_present + end + end + + context 'when DiffNote is a reply' do + let(:new_opts) do + opts.merge(in_reply_to_discussion_id: previous_note.discussion_id, + type: 'DiffNote', + noteable_type: 'MergeRequest', + noteable_id: merge_request.id, + position: position.to_h) + end + + it 'note is not associated with a note diff file' do + note = described_class.new(project_with_repo, user, new_opts).execute + + expect(note).to be_persisted + expect(note.note_diff_file).to be_nil + end + + context 'when DiffNote from an image' do + let(:image_position) do + Gitlab::Diff::Position.new(old_path: "files/images/6049019_460s.jpg", + new_path: "files/images/6049019_460s.jpg", + width: 100, + height: 100, + x: 1, + y: 100, + diff_refs: merge_request.diff_refs, + position_type: 'image') + end + + let(:new_opts) do + opts.merge(in_reply_to_discussion_id: nil, + type: 'DiffNote', + noteable_type: 'MergeRequest', + noteable_id: merge_request.id, + position: image_position.to_h) + end + + it 'note is not associated with a note diff file' do + note = described_class.new(project_with_repo, user, new_opts).execute + + expect(note).to be_persisted + expect(note.note_diff_file).to be_nil + end + end + end + end + context 'note with commands' do context 'as a user who can update the target' do context '/close, /label, /assign & /milestone' do |