diff options
Diffstat (limited to 'spec')
89 files changed, 2098 insertions, 882 deletions
diff --git a/spec/controllers/projects/issues_controller_spec.rb b/spec/controllers/projects/issues_controller_spec.rb index b4a22a46b51..053bd73fee3 100644 --- a/spec/controllers/projects/issues_controller_spec.rb +++ b/spec/controllers/projects/issues_controller_spec.rb @@ -207,162 +207,6 @@ describe Projects::IssuesController do end end - describe 'PUT #update' do - before do - sign_in(user) - project.team << [user, :developer] - end - - it_behaves_like 'update invalid issuable', Issue - - context 'changing the assignee' do - it 'limits the attributes exposed on the assignee' do - assignee = create(:user) - project.add_developer(assignee) - - put :update, - namespace_id: project.namespace.to_param, - project_id: project, - id: issue.iid, - issue: { assignee_ids: [assignee.id] }, - format: :json - body = JSON.parse(response.body) - - expect(body['assignees'].first.keys) - .to match_array(%w(id name username avatar_url state web_url)) - end - end - - context 'Akismet is enabled' do - let(:project) { create(:project_empty_repo, :public) } - - before do - stub_application_setting(recaptcha_enabled: true) - allow_any_instance_of(SpamService).to receive(:check_for_spam?).and_return(true) - end - - context 'when an issue is not identified as spam' do - before do - allow_any_instance_of(described_class).to receive(:verify_recaptcha).and_return(false) - allow_any_instance_of(AkismetService).to receive(:spam?).and_return(false) - end - - it 'normally updates the issue' do - expect { update_issue(title: 'Foo') }.to change { issue.reload.title }.to('Foo') - end - end - - context 'when an issue is identified as spam' do - before do - allow_any_instance_of(AkismetService).to receive(:spam?).and_return(true) - end - - context 'when captcha is not verified' do - def update_spam_issue - update_issue(title: 'Spam Title', description: 'Spam lives here') - end - - before do - allow_any_instance_of(described_class).to receive(:verify_recaptcha).and_return(false) - end - - it 'rejects an issue recognized as a spam' do - expect(Gitlab::Recaptcha).to receive(:load_configurations!).and_return(true) - expect { update_spam_issue }.not_to change { issue.reload.title } - end - - it 'rejects an issue recognized as a spam when recaptcha disabled' do - stub_application_setting(recaptcha_enabled: false) - - expect { update_spam_issue }.not_to change { issue.reload.title } - end - - it 'creates a spam log' do - update_spam_issue - - spam_logs = SpamLog.all - - expect(spam_logs.count).to eq(1) - expect(spam_logs.first.title).to eq('Spam Title') - expect(spam_logs.first.recaptcha_verified).to be_falsey - end - - context 'as HTML' do - it 'renders verify template' do - update_spam_issue - - expect(response).to render_template(:verify) - end - end - - context 'as JSON' do - before do - update_issue({ title: 'Spam Title', description: 'Spam lives here' }, format: :json) - end - - it 'renders json errors' do - expect(json_response) - .to eql("errors" => ["Your issue has been recognized as spam. Please, change the content or solve the reCAPTCHA to proceed."]) - end - - it 'returns 422 status' do - expect(response).to have_http_status(422) - end - end - end - - context 'when captcha is verified' do - let(:spammy_title) { 'Whatever' } - let!(:spam_logs) { create_list(:spam_log, 2, user: user, title: spammy_title) } - - def update_verified_issue - update_issue({ title: spammy_title }, - { spam_log_id: spam_logs.last.id, - recaptcha_verification: true }) - end - - before do - allow_any_instance_of(described_class).to receive(:verify_recaptcha) - .and_return(true) - end - - it 'redirect to issue page' do - update_verified_issue - - expect(response) - .to redirect_to(project_issue_path(project, issue)) - end - - it 'accepts an issue after recaptcha is verified' do - expect { update_verified_issue }.to change { issue.reload.title }.to(spammy_title) - end - - it 'marks spam log as recaptcha_verified' do - expect { update_verified_issue }.to change { SpamLog.last.recaptcha_verified }.from(false).to(true) - end - - it 'does not mark spam log as recaptcha_verified when it does not belong to current_user' do - spam_log = create(:spam_log) - - expect { update_issue(spam_log_id: spam_log.id, recaptcha_verification: true) } - .not_to change { SpamLog.last.recaptcha_verified } - end - end - end - - def update_issue(issue_params = {}, additional_params = {}) - params = { - namespace_id: project.namespace.to_param, - project_id: project, - id: issue.iid, - issue: issue_params - }.merge(additional_params) - - put :update, params - end - end - end - describe 'POST #move' do before do sign_in(user) @@ -533,6 +377,146 @@ describe Projects::IssuesController do end end + describe 'PUT #update' do + def update_issue(issue_params: {}, additional_params: {}, id: nil) + id ||= issue.iid + params = { + namespace_id: project.namespace.to_param, + project_id: project, + id: id, + issue: { title: 'New title' }.merge(issue_params), + format: :json + }.merge(additional_params) + + put :update, params + end + + def go(id:) + update_issue(id: id) + end + + before do + sign_in(user) + project.team << [user, :developer] + end + + it_behaves_like 'restricted action', success: 200 + it_behaves_like 'update invalid issuable', Issue + + context 'changing the assignee' do + it 'limits the attributes exposed on the assignee' do + assignee = create(:user) + project.add_developer(assignee) + + update_issue(issue_params: { assignee_ids: [assignee.id] }) + + body = JSON.parse(response.body) + + expect(body['assignees'].first.keys) + .to match_array(%w(id name username avatar_url state web_url)) + end + end + + context 'Akismet is enabled' do + before do + project.update!(visibility_level: Gitlab::VisibilityLevel::PUBLIC) + stub_application_setting(recaptcha_enabled: true) + allow_any_instance_of(SpamService).to receive(:check_for_spam?).and_return(true) + end + + context 'when an issue is not identified as spam' do + before do + allow_any_instance_of(described_class).to receive(:verify_recaptcha).and_return(false) + allow_any_instance_of(AkismetService).to receive(:spam?).and_return(false) + end + + it 'normally updates the issue' do + expect { update_issue(issue_params: { title: 'Foo' }) }.to change { issue.reload.title }.to('Foo') + end + end + + context 'when an issue is identified as spam' do + before do + allow_any_instance_of(AkismetService).to receive(:spam?).and_return(true) + end + + context 'when captcha is not verified' do + before do + allow_any_instance_of(described_class).to receive(:verify_recaptcha).and_return(false) + end + + it 'rejects an issue recognized as a spam' do + expect { update_issue }.not_to change { issue.reload.title } + end + + it 'rejects an issue recognized as a spam when recaptcha disabled' do + stub_application_setting(recaptcha_enabled: false) + + expect { update_issue }.not_to change { issue.reload.title } + end + + it 'creates a spam log' do + update_issue(issue_params: { title: 'Spam title' }) + + spam_logs = SpamLog.all + + expect(spam_logs.count).to eq(1) + expect(spam_logs.first.title).to eq('Spam title') + expect(spam_logs.first.recaptcha_verified).to be_falsey + end + + it 'renders json errors' do + update_issue + + expect(json_response) + .to eql("errors" => ["Your issue has been recognized as spam. Please, change the content or solve the reCAPTCHA to proceed."]) + end + + it 'returns 422 status' do + update_issue + + expect(response).to have_http_status(422) + end + end + + context 'when captcha is verified' do + let(:spammy_title) { 'Whatever' } + let!(:spam_logs) { create_list(:spam_log, 2, user: user, title: spammy_title) } + + def update_verified_issue + update_issue( + issue_params: { title: spammy_title }, + additional_params: { spam_log_id: spam_logs.last.id, recaptcha_verification: true }) + end + + before do + allow_any_instance_of(described_class).to receive(:verify_recaptcha) + .and_return(true) + end + + it 'returns 200 status' do + expect(response).to have_http_status(200) + end + + it 'accepts an issue after recaptcha is verified' do + expect { update_verified_issue }.to change { issue.reload.title }.to(spammy_title) + end + + it 'marks spam log as recaptcha_verified' do + expect { update_verified_issue }.to change { SpamLog.last.recaptcha_verified }.from(false).to(true) + end + + it 'does not mark spam log as recaptcha_verified when it does not belong to current_user' do + spam_log = create(:spam_log) + + expect { update_issue(issue_params: { spam_log_id: spam_log.id, recaptcha_verification: true }) } + .not_to change { SpamLog.last.recaptcha_verified } + end + end + end + end + end + describe 'GET #show' do it_behaves_like 'restricted action', success: 200 @@ -573,29 +557,6 @@ describe Projects::IssuesController do end end end - - describe 'GET #edit' do - it_behaves_like 'restricted action', success: 200 - - def go(id:) - get :edit, - namespace_id: project.namespace.to_param, - project_id: project, - id: id - end - end - - describe 'PUT #update' do - it_behaves_like 'restricted action', success: 302 - - def go(id:) - put :update, - namespace_id: project.namespace.to_param, - project_id: project, - id: id, - issue: { title: 'New title' } - end - end end describe 'POST #create' do diff --git a/spec/controllers/projects/jobs_controller_spec.rb b/spec/controllers/projects/jobs_controller_spec.rb index fdd7e6f173f..d01339a0b88 100644 --- a/spec/controllers/projects/jobs_controller_spec.rb +++ b/spec/controllers/projects/jobs_controller_spec.rb @@ -216,7 +216,7 @@ describe Projects::JobsController do expect(json_response['text']).to eq status.text expect(json_response['label']).to eq status.label expect(json_response['icon']).to eq status.icon - expect(json_response['favicon']).to eq "/assets/ci_favicons/#{status.favicon}.ico" + expect(json_response['favicon']).to match_asset_path "/assets/ci_favicons/#{status.favicon}.ico" end end diff --git a/spec/controllers/projects/merge_requests_controller_spec.rb b/spec/controllers/projects/merge_requests_controller_spec.rb index 55b72b7df97..e46d1995498 100644 --- a/spec/controllers/projects/merge_requests_controller_spec.rb +++ b/spec/controllers/projects/merge_requests_controller_spec.rb @@ -646,7 +646,7 @@ describe Projects::MergeRequestsController do expect(json_response['text']).to eq status.text expect(json_response['label']).to eq status.label expect(json_response['icon']).to eq status.icon - expect(json_response['favicon']).to eq "/assets/ci_favicons/#{status.favicon}.ico" + expect(json_response['favicon']).to match_asset_path "/assets/ci_favicons/#{status.favicon}.ico" end end diff --git a/spec/controllers/projects/notes_controller_spec.rb b/spec/controllers/projects/notes_controller_spec.rb index 6ffe41b8608..c0337f96fc6 100644 --- a/spec/controllers/projects/notes_controller_spec.rb +++ b/spec/controllers/projects/notes_controller_spec.rb @@ -120,6 +120,40 @@ describe Projects::NotesController do expect(note_json[:diff_discussion_html]).to be_nil end end + + context 'with cross-reference system note', :request_store do + let(:new_issue) { create(:issue) } + let(:cross_reference) { "mentioned in #{new_issue.to_reference(issue.project)}" } + + before do + note + create(:discussion_note_on_issue, :system, noteable: issue, project: issue.project, note: cross_reference) + end + + it 'filters notes that the user should not see' do + get :index, request_params + + expect(parsed_response[:notes].count).to eq(1) + expect(note_json[:id]).to eq(note.id) + end + + it 'does not result in N+1 queries' do + # Instantiate the controller variables to ensure QueryRecorder has an accurate base count + get :index, request_params + + RequestStore.clear! + + control_count = ActiveRecord::QueryRecorder.new do + get :index, request_params + end.count + + RequestStore.clear! + + create_list(:discussion_note_on_issue, 2, :system, noteable: issue, project: issue.project, note: cross_reference) + + expect { get :index, request_params }.not_to exceed_query_limit(control_count) + end + end end describe 'POST create' do diff --git a/spec/controllers/projects/pipelines_controller_spec.rb b/spec/controllers/projects/pipelines_controller_spec.rb index f9d77c7ad03..167e80ed9cd 100644 --- a/spec/controllers/projects/pipelines_controller_spec.rb +++ b/spec/controllers/projects/pipelines_controller_spec.rb @@ -142,7 +142,7 @@ describe Projects::PipelinesController do expect(json_response['text']).to eq status.text expect(json_response['label']).to eq status.label expect(json_response['icon']).to eq status.icon - expect(json_response['favicon']).to eq "/assets/ci_favicons/#{status.favicon}.ico" + expect(json_response['favicon']).to match_asset_path("/assets/ci_favicons/#{status.favicon}.ico") end end diff --git a/spec/controllers/projects_controller_spec.rb b/spec/controllers/projects_controller_spec.rb index 4459e227fb3..2a91a6613e6 100644 --- a/spec/controllers/projects_controller_spec.rb +++ b/spec/controllers/projects_controller_spec.rb @@ -289,6 +289,24 @@ describe ProjectsController do end end + it 'updates Fast Forward Merge attributes' do + controller.instance_variable_set(:@project, project) + + params = { + merge_method: :ff + } + + put :update, + namespace_id: project.namespace, + id: project.id, + project: params + + expect(response).to have_http_status(302) + params.each do |param, value| + expect(project.public_send(param)).to eq(value) + end + end + def update_project(**parameters) put :update, namespace_id: project.namespace.path, diff --git a/spec/factories/gitaly/commit.rb b/spec/factories/gitaly/commit.rb new file mode 100644 index 00000000000..e7966cee77b --- /dev/null +++ b/spec/factories/gitaly/commit.rb @@ -0,0 +1,17 @@ +FactoryGirl.define do + sequence(:gitaly_commit_id) { Digest::SHA1.hexdigest(Time.now.to_f.to_s) } + + factory :gitaly_commit, class: Gitaly::GitCommit do + skip_create + + id { generate(:gitaly_commit_id) } + parent_ids do + ids = [generate(:gitaly_commit_id), generate(:gitaly_commit_id)] + Google::Protobuf::RepeatedField.new(:string, ids) + end + subject { "My commit" } + body { subject + "\nMy body" } + author { build(:gitaly_commit_author) } + committer { build(:gitaly_commit_author) } + end +end diff --git a/spec/factories/gitaly/commit_author.rb b/spec/factories/gitaly/commit_author.rb new file mode 100644 index 00000000000..341873a2002 --- /dev/null +++ b/spec/factories/gitaly/commit_author.rb @@ -0,0 +1,9 @@ +FactoryGirl.define do + factory :gitaly_commit_author, class: Gitaly::CommitAuthor do + skip_create + + name { generate(:name) } + email { generate(:email) } + date { Google::Protobuf::Timestamp.new(seconds: Time.now.to_i) } + end +end diff --git a/spec/features/copy_as_gfm_spec.rb b/spec/features/copy_as_gfm_spec.rb index dfeba722ac6..ebcd0ba0dcd 100644 --- a/spec/features/copy_as_gfm_spec.rb +++ b/spec/features/copy_as_gfm_spec.rb @@ -446,7 +446,7 @@ describe 'Copy as GFM', js: true do def verify(label, *gfms) aggregate_failures(label) do gfms.each do |gfm| - html = gfm_to_html(gfm) + html = gfm_to_html(gfm).gsub(/\A
|
\z/, '') output_gfm = html_to_gfm(html) expect(output_gfm.strip).to eq(gfm.strip) end @@ -463,42 +463,98 @@ describe 'Copy as GFM', js: true do let(:project) { create(:project, :repository) } context 'from a diff' do - before do - visit project_commit_path(project, sample_commit.id) - end + shared_examples 'copying code from a diff' do + context 'selecting one word of text' do + it 'copies as inline code' do + verify( + '[id="2f6fcd96b88b36ce98c38da085c795a27d92a3dd_10_9"] .line .no', - context 'selecting one word of text' do - it 'copies as inline code' do - verify( - '[id="2f6fcd96b88b36ce98c38da085c795a27d92a3dd_10_9"] .line .no', + '`RuntimeError`', - '`RuntimeError`' - ) + target: '[id="2f6fcd96b88b36ce98c38da085c795a27d92a3dd_10_9"]' + ) + end end - end - context 'selecting one line of text' do - it 'copies as inline code' do - verify( - '[id="2f6fcd96b88b36ce98c38da085c795a27d92a3dd_10_9"] .line', + context 'selecting one line of text' do + it 'copies as inline code' do + verify( + '[id="2f6fcd96b88b36ce98c38da085c795a27d92a3dd_10_9"]', - '`raise RuntimeError, "System commands must be given as an array of strings"`' - ) + '`raise RuntimeError, "System commands must be given as an array of strings"`', + + target: '[id="2f6fcd96b88b36ce98c38da085c795a27d92a3dd_10_9"]' + ) + end + end + + context 'selecting multiple lines of text' do + it 'copies as a code block' do + verify( + '[id="2f6fcd96b88b36ce98c38da085c795a27d92a3dd_10_9"], [id="2f6fcd96b88b36ce98c38da085c795a27d92a3dd_10_10"]', + + <<-GFM.strip_heredoc, + ```ruby + raise RuntimeError, "System commands must be given as an array of strings" + end + ``` + GFM + + target: '[id="2f6fcd96b88b36ce98c38da085c795a27d92a3dd_10_9"]' + ) + end end end - context 'selecting multiple lines of text' do - it 'copies as a code block' do - verify( - '[id="2f6fcd96b88b36ce98c38da085c795a27d92a3dd_10_9"], [id="2f6fcd96b88b36ce98c38da085c795a27d92a3dd_10_10"]', + context 'inline diff' do + before do + visit project_commit_path(project, sample_commit.id, view: 'inline') + end - <<-GFM.strip_heredoc, - ```ruby - raise RuntimeError, "System commands must be given as an array of strings" - end - ``` - GFM - ) + it_behaves_like 'copying code from a diff' + end + + context 'parallel diff' do + before do + visit project_commit_path(project, sample_commit.id, view: 'parallel') + end + + it_behaves_like 'copying code from a diff' + + context 'selecting code on the left' do + it 'copies as a code block' do + verify( + '[id="2f6fcd96b88b36ce98c38da085c795a27d92a3dd_8_8"], [id="2f6fcd96b88b36ce98c38da085c795a27d92a3dd_9_9"], [id="2f6fcd96b88b36ce98c38da085c795a27d92a3dd_10_9"], [id="2f6fcd96b88b36ce98c38da085c795a27d92a3dd_10_10"]', + + <<-GFM.strip_heredoc, + ```ruby + unless cmd.is_a?(Array) + raise "System commands must be given as an array of strings" + end + ``` + GFM + + target: '[id="2f6fcd96b88b36ce98c38da085c795a27d92a3dd_8_8"].left-side' + ) + end + end + + context 'selecting code on the right' do + it 'copies as a code block' do + verify( + '[id="2f6fcd96b88b36ce98c38da085c795a27d92a3dd_8_8"], [id="2f6fcd96b88b36ce98c38da085c795a27d92a3dd_9_9"], [id="2f6fcd96b88b36ce98c38da085c795a27d92a3dd_10_9"], [id="2f6fcd96b88b36ce98c38da085c795a27d92a3dd_10_10"]', + + <<-GFM.strip_heredoc, + ```ruby + unless cmd.is_a?(Array) + raise RuntimeError, "System commands must be given as an array of strings" + end + ``` + GFM + + target: '[id="2f6fcd96b88b36ce98c38da085c795a27d92a3dd_8_8"].right-side' + ) + end end end end @@ -587,9 +643,9 @@ describe 'Copy as GFM', js: true do end end - def verify(selector, gfm) + def verify(selector, gfm, target: nil) html = html_for_selector(selector) - output_gfm = html_to_gfm(html, 'transformCodeSelection') + output_gfm = html_to_gfm(html, 'transformCodeSelection', target: target) expect(output_gfm.strip).to eq(gfm.strip) end end @@ -605,15 +661,21 @@ describe 'Copy as GFM', js: true do page.evaluate_script(js) end - def html_to_gfm(html, transformer = 'transformGFMSelection') + def html_to_gfm(html, transformer = 'transformGFMSelection', target: nil) js = <<-JS.strip_heredoc (function(html) { var transformer = window.gl.CopyAsGFM[#{transformer.inspect}]; var node = document.createElement('div'); - node.innerHTML = html; + $(html).each(function() { node.appendChild(this) }); + + var targetSelector = #{target.to_json}; + var target; + if (targetSelector) { + target = document.querySelector(targetSelector); + } - node = transformer(node); + node = transformer(node, target); if (!node) return null; return window.gl.CopyAsGFM.nodeToGFM(node); diff --git a/spec/features/issues/form_spec.rb b/spec/features/issues/form_spec.rb index 2db6f9a2982..8ce470fc288 100644 --- a/spec/features/issues/form_spec.rb +++ b/spec/features/issues/form_spec.rb @@ -218,54 +218,15 @@ describe 'New/edit issue', :js do context 'edit issue' do before do - visit edit_project_issue_path(project, issue) - end - - it 'allows user to update issue' do - expect(find('input[name="issue[assignee_ids][]"]', visible: false).value).to match(user.id.to_s) - expect(find('input[name="issue[milestone_id]"]', visible: false).value).to match(milestone.id.to_s) - expect(find('a', text: 'Assign to me', visible: false)).not_to be_visible - - page.within '.js-user-search' do - expect(page).to have_content user.name - end - - page.within '.js-milestone-select' do - expect(page).to have_content milestone.title - end - - click_button 'Labels' - page.within '.dropdown-menu-labels' do - click_link label.title - click_link label2.title - end - page.within '.js-label-select' do - expect(page).to have_content label.title - end - expect(page.all('input[name="issue[label_ids][]"]', visible: false)[1].value).to match(label.id.to_s) - expect(page.all('input[name="issue[label_ids][]"]', visible: false)[2].value).to match(label2.id.to_s) - - click_button 'Save changes' - - page.within '.issuable-sidebar' do - page.within '.assignee' do - expect(page).to have_content user.name - end - - page.within '.milestone' do - expect(page).to have_content milestone.title - end - - page.within '.labels' do - expect(page).to have_content label.title - expect(page).to have_content label2.title - end + visit project_issue_path(project, issue) + page.within('.content .issuable-actions') do + click_on 'Edit' end end it 'description has autocomplete' do - find('#issue_description').native.send_keys('') - fill_in 'issue_description', with: '@' + find_field('issue-description').native.send_keys('') + fill_in 'issue-description', with: '@' expect(page).to have_selector('.atwho-view') end diff --git a/spec/features/issues_spec.rb b/spec/features/issues_spec.rb index b4222edbcd0..aa8cf3b013c 100644 --- a/spec/features/issues_spec.rb +++ b/spec/features/issues_spec.rb @@ -1,6 +1,6 @@ require 'spec_helper' -describe 'Issues' do +describe 'Issues', :js do include DropzoneHelper include IssueHelpers include SortingHelper @@ -24,109 +24,15 @@ describe 'Issues' do end before do - visit edit_project_issue_path(project, issue) - find('.js-zen-enter').click - end - - it 'opens new issue popup' do - expect(page).to have_content("Issue ##{issue.iid}") - end - end - - describe 'Editing issue assignee' do - let!(:issue) do - create(:issue, - author: user, - assignees: [user], - project: project) - end - - it 'allows user to select unassigned', js: true do - visit edit_project_issue_path(project, issue) - - expect(page).to have_content "Assignee #{user.name}" - - first('.js-user-search').click - click_link 'Unassigned' - - click_button 'Save changes' - - page.within('.assignee') do - expect(page).to have_content 'No assignee - assign yourself' - end - - expect(issue.reload.assignees).to be_empty - end - end - - describe 'due date', js: true do - context 'on new form' do - before do - visit new_project_issue_path(project) - end - - it 'saves with due date' do - date = Date.today.at_beginning_of_month - - fill_in 'issue_title', with: 'bug 345' - fill_in 'issue_description', with: 'bug description' - find('#issuable-due-date').click - - page.within '.pika-single' do - click_button date.day - end - - expect(find('#issuable-due-date').value).to eq date.to_s - - click_button 'Submit issue' - - page.within '.issuable-sidebar' do - expect(page).to have_content date.to_s(:medium) - end + visit project_issue_path(project, issue) + page.within('.content .issuable-actions') do + find('.issuable-edit').click end + find('.issue-details .content-block .js-zen-enter').click end - context 'on edit form' do - let(:issue) { create(:issue, author: user, project: project, due_date: Date.today.at_beginning_of_month.to_s) } - - before do - visit edit_project_issue_path(project, issue) - end - - it 'saves with due date' do - date = Date.today.at_beginning_of_month - - expect(find('#issuable-due-date').value).to eq date.to_s - - date = date.tomorrow - - fill_in 'issue_title', with: 'bug 345' - fill_in 'issue_description', with: 'bug description' - find('#issuable-due-date').click - - page.within '.pika-single' do - click_button date.day - end - - expect(find('#issuable-due-date').value).to eq date.to_s - - click_button 'Save changes' - - page.within '.issuable-sidebar' do - expect(page).to have_content date.to_s(:medium) - end - end - - it 'warns about version conflict' do - issue.update(title: "New title") - - fill_in 'issue_title', with: 'bug 345' - fill_in 'issue_description', with: 'bug description' - - click_button 'Save changes' - - expect(page).to have_content 'Someone edited the issue the same time you did' - end + it 'opens new issue popup' do + expect(page).to have_content(issue.description) end end diff --git a/spec/features/merge_requests/diff_notes_avatars_spec.rb b/spec/features/merge_requests/diff_notes_avatars_spec.rb index 9bcb78d5206..4766cdf716f 100644 --- a/spec/features/merge_requests/diff_notes_avatars_spec.rb +++ b/spec/features/merge_requests/diff_notes_avatars_spec.rb @@ -84,7 +84,7 @@ feature 'Diff note avatars', js: true do end it 'shows note avatar' do - page.within find("[id='#{position.line_code(project.repository)}']") do + page.within find_line(position.line_code(project.repository)) do find('.diff-notes-collapse').click expect(page).to have_selector('img.js-diff-comment-avatar', count: 1) @@ -92,7 +92,7 @@ feature 'Diff note avatars', js: true do end it 'shows comment on note avatar' do - page.within find("[id='#{position.line_code(project.repository)}']") do + page.within find_line(position.line_code(project.repository)) do find('.diff-notes-collapse').click expect(first('img.js-diff-comment-avatar')["data-original-title"]).to eq("#{note.author.name}: #{note.note.truncate(17)}") @@ -100,13 +100,13 @@ feature 'Diff note avatars', js: true do end it 'toggles comments when clicking avatar' do - page.within find("[id='#{position.line_code(project.repository)}']") do + page.within find_line(position.line_code(project.repository)) do find('.diff-notes-collapse').click end expect(page).to have_selector('.notes_holder', visible: false) - page.within find("[id='#{position.line_code(project.repository)}']") do + page.within find_line(position.line_code(project.repository)) do first('img.js-diff-comment-avatar').click end @@ -122,7 +122,7 @@ feature 'Diff note avatars', js: true do wait_for_requests - page.within find("[id='#{position.line_code(project.repository)}']") do + page.within find_line(position.line_code(project.repository)) do expect(page).not_to have_selector('img.js-diff-comment-avatar') end end @@ -138,7 +138,7 @@ feature 'Diff note avatars', js: true do wait_for_requests end - page.within find("[id='#{position.line_code(project.repository)}']") do + page.within find_line(position.line_code(project.repository)) do find('.diff-notes-collapse').trigger('click') expect(page).to have_selector('img.js-diff-comment-avatar', count: 2) @@ -158,7 +158,7 @@ feature 'Diff note avatars', js: true do end end - page.within find("[id='#{position.line_code(project.repository)}']") do + page.within find_line(position.line_code(project.repository)) do find('.diff-notes-collapse').trigger('click') expect(page).to have_selector('img.js-diff-comment-avatar', count: 3) @@ -176,7 +176,7 @@ feature 'Diff note avatars', js: true do end it 'shows extra comment count' do - page.within find("[id='#{position.line_code(project.repository)}']") do + page.within find_line(position.line_code(project.repository)) do find('.diff-notes-collapse').click expect(find('.diff-comments-more-count')).to have_content '+1' @@ -185,4 +185,10 @@ feature 'Diff note avatars', js: true do end end end + + def find_line(line_code) + line = find("[id='#{line_code}']") + line = line.find(:xpath, 'preceding-sibling::*[1][self::td]') if line.tag_name == 'td' + line + end end diff --git a/spec/features/merge_requests/widget_spec.rb b/spec/features/merge_requests/widget_spec.rb index 443b596b3c6..791cfa308c3 100644 --- a/spec/features/merge_requests/widget_spec.rb +++ b/spec/features/merge_requests/widget_spec.rb @@ -202,6 +202,28 @@ describe 'Merge request', :js do end end + context 'view merge request where fast-forward merge is not possible' do + before do + project.update(merge_requests_ff_only_enabled: true) + + merge_request.update( + merge_user: merge_request.author, + merge_status: :cannot_be_merged + ) + + visit project_merge_request_path(project, merge_request) + end + + it 'shows information about the merge error' do + # Wait for the `ci_status` and `merge_check` requests + wait_for_requests + + page.within('.mr-widget-body') do + expect(page).to have_content('Fast-forward merge is not possible') + end + end + end + context 'merge error' do before do allow_any_instance_of(Repository).to receive(:merge).and_return(false) diff --git a/spec/features/projects/fork_spec.rb b/spec/features/projects/fork_spec.rb new file mode 100644 index 00000000000..e10d29e5eea --- /dev/null +++ b/spec/features/projects/fork_spec.rb @@ -0,0 +1,57 @@ +require 'spec_helper' + +describe 'Project fork' do + let(:user) { create(:user) } + let(:project) { create(:project, :public, :repository) } + + before do + sign_in user + end + + it 'allows user to fork project' do + visit project_path(project) + + expect(page).not_to have_css('a.disabled', text: 'Fork') + end + + it 'disables fork button when user has exceeded project limit' do + user.projects_limit = 0 + user.save! + + visit project_path(project) + + expect(page).to have_css('a.disabled', text: 'Fork') + end + + context 'master in group' do + before do + group = create(:group) + group.add_master(user) + end + + it 'allows user to fork project to group or to user namespace' do + visit project_path(project) + + expect(page).not_to have_css('a.disabled', text: 'Fork') + + click_link 'Fork' + + expect(page).to have_css('.fork-thumbnail', count: 2) + expect(page).not_to have_css('.fork-thumbnail.disabled') + end + + it 'allows user to fork project to group and not user when exceeded project limit' do + user.projects_limit = 0 + user.save! + + visit project_path(project) + + expect(page).not_to have_css('a.disabled', text: 'Fork') + + click_link 'Fork' + + expect(page).to have_css('.fork-thumbnail', count: 2) + expect(page).to have_css('.fork-thumbnail.disabled') + end + end +end diff --git a/spec/features/projects/issuable_templates_spec.rb b/spec/features/projects/issuable_templates_spec.rb index d2789d0aa52..1f9b52dd998 100644 --- a/spec/features/projects/issuable_templates_spec.rb +++ b/spec/features/projects/issuable_templates_spec.rb @@ -3,6 +3,7 @@ require 'spec_helper' feature 'issuable templates', js: true do let(:user) { create(:user) } let(:project) { create(:project, :public, :repository) } + let(:issue_form_location) { '#content-body .issuable-details .detail-page-description' } before do project.team << [user, :master] @@ -28,14 +29,17 @@ feature 'issuable templates', js: true do longtemplate_content, message: 'added issue template', branch_name: 'master') - visit edit_project_issue_path project, issue - fill_in :'issue[title]', with: 'test issue title' + visit project_issue_path project, issue + page.within('.content .issuable-actions') do + click_on 'Edit' + end + fill_in :'issue-title', with: 'test issue title' end scenario 'user selects "bug" template' do select_template 'bug' wait_for_requests - assert_template + assert_template(page_part: issue_form_location) save_changes end @@ -43,30 +47,19 @@ feature 'issuable templates', js: true do select_template 'bug' wait_for_requests select_option 'No template' - assert_template('') + assert_template(expected_content: '', page_part: issue_form_location) save_changes('') end scenario 'user selects "bug" template, edits description and then selects "reset template"' do select_template 'bug' wait_for_requests - find_field('issue_description').send_keys(description_addition) - assert_template(template_content + description_addition) + find_field('issue-description').send_keys(description_addition) + assert_template(expected_content: template_content + description_addition, page_part: issue_form_location) select_option 'Reset template' - assert_template + assert_template(page_part: issue_form_location) save_changes end - - it 'updates height of markdown textarea' do - start_height = page.evaluate_script('$(".markdown-area").outerHeight()') - - select_template 'test' - wait_for_requests - - end_height = page.evaluate_script('$(".markdown-area").outerHeight()') - - expect(end_height).not_to eq(start_height) - end end context 'user creates an issue using templates, with a prior description' do @@ -81,15 +74,18 @@ feature 'issuable templates', js: true do template_content, message: 'added issue template', branch_name: 'master') - visit edit_project_issue_path project, issue - fill_in :'issue[title]', with: 'test issue title' - fill_in :'issue[description]', with: prior_description + visit project_issue_path project, issue + page.within('.content .issuable-actions') do + click_on 'Edit' + end + fill_in :'issue-title', with: 'test issue title' + fill_in :'issue-description', with: prior_description end scenario 'user selects "bug" template' do select_template 'bug' wait_for_requests - assert_template("#{template_content}") + assert_template(page_part: issue_form_location) save_changes end end @@ -154,8 +150,10 @@ feature 'issuable templates', js: true do end end - def assert_template(expected_content = template_content) - expect(find('textarea')['value']).to eq(expected_content) + def assert_template(expected_content: template_content, page_part: '#content-body') + page.within(page_part) do + expect(find('textarea')['value']).to eq(expected_content) + end end def save_changes(expected_content = template_content) diff --git a/spec/features/projects/project_settings_spec.rb b/spec/features/projects/project_settings_spec.rb index 5d77cd1ccd5..06568817757 100644 --- a/spec/features/projects/project_settings_spec.rb +++ b/spec/features/projects/project_settings_spec.rb @@ -32,6 +32,32 @@ describe 'Edit Project Settings' do end end + describe 'Merge request settings section' do + it 'shows "Merge commit" strategy' do + visit edit_project_path(project) + + page.within '.merge-requests-feature' do + expect(page).to have_content 'Merge commit' + end + end + + it 'shows "Merge commit with semi-linear history " strategy' do + visit edit_project_path(project) + + page.within '.merge-requests-feature' do + expect(page).to have_content 'Merge commit with semi-linear history' + end + end + + it 'shows "Fast-forward merge" strategy' do + visit edit_project_path(project) + + page.within '.merge-requests-feature' do + expect(page).to have_content 'Fast-forward merge' + end + end + end + describe 'Rename repository section' do context 'with invalid characters' do it 'shows errors for invalid project path/name' do diff --git a/spec/features/projects/wiki/user_views_wiki_page_spec.rb b/spec/features/projects/wiki/user_views_wiki_page_spec.rb index 49ba2969ef0..470391dc66b 100644 --- a/spec/features/projects/wiki/user_views_wiki_page_spec.rb +++ b/spec/features/projects/wiki/user_views_wiki_page_spec.rb @@ -83,7 +83,7 @@ describe 'User views a wiki page' do it 'shows a file stored in a page' do file = Gollum::File.new(project.wiki) - allow_any_instance_of(Gollum::Wiki).to receive(:file).with('image.jpg', 'master', true).and_return(file) + allow_any_instance_of(Gollum::Wiki).to receive(:file).with('image.jpg', 'master').and_return(file) allow_any_instance_of(Gollum::File).to receive(:mime_type).and_return('image/jpeg') expect(page).to have_xpath('//img[@data-src="image.jpg"]') diff --git a/spec/features/security/project/internal_access_spec.rb b/spec/features/security/project/internal_access_spec.rb index a7928857b7d..d70cf1527e7 100644 --- a/spec/features/security/project/internal_access_spec.rb +++ b/spec/features/security/project/internal_access_spec.rb @@ -181,21 +181,6 @@ describe "Internal Project Access" do it { is_expected.to be_denied_for(:visitor) } end - describe "GET /:project_path/issues/:id/edit" do - let(:issue) { create(:issue, project: project) } - subject { edit_project_issue_path(project, issue) } - - it { is_expected.to be_allowed_for(:admin) } - it { is_expected.to be_allowed_for(:owner).of(project) } - it { is_expected.to be_allowed_for(:master).of(project) } - it { is_expected.to be_allowed_for(:developer).of(project) } - it { is_expected.to be_allowed_for(:reporter).of(project) } - it { is_expected.to be_denied_for(:guest).of(project) } - it { is_expected.to be_denied_for(:user) } - it { is_expected.to be_denied_for(:external) } - it { is_expected.to be_denied_for(:visitor) } - end - describe "GET /:project_path/snippets" do subject { project_snippets_path(project) } diff --git a/spec/features/security/project/private_access_spec.rb b/spec/features/security/project/private_access_spec.rb index a4396b20afd..ea130606545 100644 --- a/spec/features/security/project/private_access_spec.rb +++ b/spec/features/security/project/private_access_spec.rb @@ -181,21 +181,6 @@ describe "Private Project Access" do it { is_expected.to be_denied_for(:visitor) } end - describe "GET /:project_path/issues/:id/edit" do - let(:issue) { create(:issue, project: project) } - subject { edit_project_issue_path(project, issue) } - - it { is_expected.to be_allowed_for(:admin) } - it { is_expected.to be_allowed_for(:owner).of(project) } - it { is_expected.to be_allowed_for(:master).of(project) } - it { is_expected.to be_allowed_for(:developer).of(project) } - it { is_expected.to be_allowed_for(:reporter).of(project) } - it { is_expected.to be_denied_for(:guest).of(project) } - it { is_expected.to be_denied_for(:user) } - it { is_expected.to be_denied_for(:external) } - it { is_expected.to be_denied_for(:visitor) } - end - describe "GET /:project_path/snippets" do subject { project_snippets_path(project) } diff --git a/spec/features/security/project/public_access_spec.rb b/spec/features/security/project/public_access_spec.rb index fccdeb0e5b7..d15f5af66c9 100644 --- a/spec/features/security/project/public_access_spec.rb +++ b/spec/features/security/project/public_access_spec.rb @@ -394,21 +394,6 @@ describe "Public Project Access" do it { is_expected.to be_allowed_for(:visitor) } end - describe "GET /:project_path/issues/:id/edit" do - let(:issue) { create(:issue, project: project) } - subject { edit_project_issue_path(project, issue) } - - it { is_expected.to be_allowed_for(:admin) } - it { is_expected.to be_allowed_for(:owner).of(project) } - it { is_expected.to be_allowed_for(:master).of(project) } - it { is_expected.to be_allowed_for(:developer).of(project) } - it { is_expected.to be_allowed_for(:reporter).of(project) } - it { is_expected.to be_denied_for(:guest).of(project) } - it { is_expected.to be_denied_for(:user) } - it { is_expected.to be_denied_for(:external) } - it { is_expected.to be_denied_for(:visitor) } - end - describe "GET /:project_path/snippets" do subject { project_snippets_path(project) } diff --git a/spec/features/signup_spec.rb b/spec/features/signup_spec.rb index b6367b88e17..917fad74ef1 100644 --- a/spec/features/signup_spec.rb +++ b/spec/features/signup_spec.rb @@ -24,6 +24,24 @@ feature 'Signup' do end end + context "when sigining up with different cased emails" do + it "creates the user successfully" do + user = build(:user) + + visit root_path + + fill_in 'new_user_name', with: user.name + fill_in 'new_user_username', with: user.username + fill_in 'new_user_email', with: user.email + fill_in 'new_user_email_confirmation', with: user.email.capitalize + fill_in 'new_user_password', with: user.password + click_button "Register" + + expect(current_path).to eq dashboard_projects_path + expect(page).to have_content("Welcome! You have signed up successfully.") + end + end + context "when not sending confirmation email" do before do stub_application_setting(send_user_confirmation_email: false) diff --git a/spec/fixtures/api/schemas/entities/merge_request.json b/spec/fixtures/api/schemas/entities/merge_request.json index ce922ffad17..30b4e56bc98 100644 --- a/spec/fixtures/api/schemas/entities/merge_request.json +++ b/spec/fixtures/api/schemas/entities/merge_request.json @@ -96,7 +96,9 @@ "remove_wip_path": { "type": ["string", "null"] }, "commits_count": { "type": "integer" }, "remove_source_branch": { "type": ["boolean", "null"] }, - "merge_ongoing": { "type": "boolean" } + "merge_ongoing": { "type": "boolean" }, + "ff_only_enabled": { "type": ["boolean", false] }, + "should_be_rebased": { "type": "boolean" } }, "additionalProperties": false } diff --git a/spec/fixtures/config/kubeconfig.yml b/spec/fixtures/config/kubeconfig.yml index c4e8e573c32..5152dae0104 100644 --- a/spec/fixtures/config/kubeconfig.yml +++ b/spec/fixtures/config/kubeconfig.yml @@ -4,7 +4,7 @@ clusters: - name: gitlab-deploy cluster: server: https://kube.domain.com - certificate-authority-data: "UEVN\n" + certificate-authority-data: "UEVN" contexts: - name: gitlab-deploy context: diff --git a/spec/helpers/groups_helper_spec.rb b/spec/helpers/groups_helper_spec.rb index 36031ac1a28..76e5964ccf7 100644 --- a/spec/helpers/groups_helper_spec.rb +++ b/spec/helpers/groups_helper_spec.rb @@ -17,7 +17,7 @@ describe GroupsHelper do it 'gives default avatar_icon when no avatar is present' do group = create(:group) group.save! - expect(group_icon(group.path)).to match('group_avatar.png') + expect(group_icon(group.path)).to match_asset_path('group_avatar.png') end end diff --git a/spec/helpers/page_layout_helper_spec.rb b/spec/helpers/page_layout_helper_spec.rb index 9aca3987657..baf927a9acc 100644 --- a/spec/helpers/page_layout_helper_spec.rb +++ b/spec/helpers/page_layout_helper_spec.rb @@ -54,7 +54,7 @@ describe PageLayoutHelper do describe 'page_image' do it 'defaults to the GitLab logo' do - expect(helper.page_image).to end_with 'assets/gitlab_logo.png' + expect(helper.page_image).to match_asset_path 'assets/gitlab_logo.png' end %w(project user group).each do |type| @@ -70,13 +70,13 @@ describe PageLayoutHelper do object = double(avatar_url: nil) assign(type, object) - expect(helper.page_image).to end_with 'assets/gitlab_logo.png' + expect(helper.page_image).to match_asset_path 'assets/gitlab_logo.png' end end context "with no assignments" do it 'falls back to the default' do - expect(helper.page_image).to end_with 'assets/gitlab_logo.png' + expect(helper.page_image).to match_asset_path 'assets/gitlab_logo.png' end end end diff --git a/spec/helpers/projects_helper_spec.rb b/spec/helpers/projects_helper_spec.rb index 7ded95d01af..641971485de 100644 --- a/spec/helpers/projects_helper_spec.rb +++ b/spec/helpers/projects_helper_spec.rb @@ -200,13 +200,13 @@ describe ProjectsHelper do end it 'returns image tag for member avatar' do - expect(helper).to receive(:image_tag).with(expected, { width: 16, class: ["avatar", "avatar-inline", "s16"], alt: "" }) + expect(helper).to receive(:image_tag).with(expected, { width: 16, class: ["avatar", "avatar-inline", "s16"], alt: "", "data-src" => anything }) helper.link_to_member_avatar(user) end it 'returns image tag with avatar class' do - expect(helper).to receive(:image_tag).with(expected, { width: 16, class: ["avatar", "avatar-inline", "s16", "any-avatar-class"], alt: "" }) + expect(helper).to receive(:image_tag).with(expected, { width: 16, class: ["avatar", "avatar-inline", "s16", "any-avatar-class"], alt: "", "data-src" => anything }) helper.link_to_member_avatar(user, avatar_class: "any-avatar-class") end diff --git a/spec/javascripts/issuable_context_spec.js b/spec/javascripts/issuable_context_spec.js new file mode 100644 index 00000000000..bcb2b7b24a0 --- /dev/null +++ b/spec/javascripts/issuable_context_spec.js @@ -0,0 +1,34 @@ +/* global IssuableContext */ +import '~/issuable_context'; +import $ from 'jquery'; + +describe('IssuableContext', () => { + describe('toggleHiddenParticipants', () => { + const event = jasmine.createSpyObj('event', ['preventDefault']); + + beforeEach(() => { + spyOn($.fn, 'data').and.returnValue('data'); + spyOn($.fn, 'text').and.returnValue('data'); + }); + + afterEach(() => { + gl.lazyLoader = undefined; + }); + + it('calls loadCheck if lazyLoader is set', () => { + gl.lazyLoader = jasmine.createSpyObj('lazyLoader', ['loadCheck']); + + IssuableContext.prototype.toggleHiddenParticipants(event); + + expect(gl.lazyLoader.loadCheck).toHaveBeenCalled(); + }); + + it('does not throw if lazyLoader is not defined', () => { + gl.lazyLoader = undefined; + + const toggle = IssuableContext.prototype.toggleHiddenParticipants.bind(null, event); + + expect(toggle).not.toThrow(); + }); + }); +}); diff --git a/spec/javascripts/lib/utils/common_utils_spec.js b/spec/javascripts/lib/utils/common_utils_spec.js index 787b405de47..f86f2f260c3 100644 --- a/spec/javascripts/lib/utils/common_utils_spec.js +++ b/spec/javascripts/lib/utils/common_utils_spec.js @@ -84,6 +84,62 @@ describe('common_utils', () => { expectGetElementIdToHaveBeenCalledWith('definição'); expectGetElementIdToHaveBeenCalledWith('user-content-definição'); }); + + it('scrolls element into view', () => { + document.body.innerHTML += ` + <div id="parent"> + <div style="height: 2000px;"></div> + <div id="test" style="height: 2000px;"></div> + </div> + `; + + window.history.pushState({}, null, '#test'); + commonUtils.handleLocationHash(); + + expectGetElementIdToHaveBeenCalledWith('test'); + expect(window.scrollY).toBe(document.getElementById('test').offsetTop); + + document.getElementById('parent').remove(); + }); + + it('scrolls user content element into view', () => { + document.body.innerHTML += ` + <div id="parent"> + <div style="height: 2000px;"></div> + <div id="user-content-test" style="height: 2000px;"></div> + </div> + `; + + window.history.pushState({}, null, '#test'); + commonUtils.handleLocationHash(); + + expectGetElementIdToHaveBeenCalledWith('test'); + expectGetElementIdToHaveBeenCalledWith('user-content-test'); + expect(window.scrollY).toBe(document.getElementById('user-content-test').offsetTop); + + document.getElementById('parent').remove(); + }); + + it('scrolls to element with offset from navbar', () => { + spyOn(window, 'scrollBy').and.callThrough(); + document.body.innerHTML += ` + <div id="parent"> + <div class="navbar-gitlab" style="position: fixed; top: 0; height: 50px;"></div> + <div style="height: 2000px; margin-top: 50px;"></div> + <div id="user-content-test" style="height: 2000px;"></div> + </div> + `; + + window.history.pushState({}, null, '#test'); + commonUtils.handleLocationHash(); + + expectGetElementIdToHaveBeenCalledWith('test'); + expectGetElementIdToHaveBeenCalledWith('user-content-test'); + expect(window.scrollY).toBe(document.getElementById('user-content-test').offsetTop - 50); + expect(window.scrollBy).toHaveBeenCalledWith(0, -50); + + document.getElementById('parent').remove(); + }); }); describe('setParamInURL', () => { diff --git a/spec/javascripts/locale/sprintf_spec.js b/spec/javascripts/locale/sprintf_spec.js new file mode 100644 index 00000000000..52e903b819f --- /dev/null +++ b/spec/javascripts/locale/sprintf_spec.js @@ -0,0 +1,74 @@ +import sprintf from '~/locale/sprintf'; + +describe('locale', () => { + describe('sprintf', () => { + it('does not modify string without parameters', () => { + const input = 'No parameters'; + + const output = sprintf(input); + + expect(output).toBe(input); + }); + + it('ignores extraneous parameters', () => { + const input = 'No parameters'; + + const output = sprintf(input, { ignore: 'this' }); + + expect(output).toBe(input); + }); + + it('ignores extraneous placeholders', () => { + const input = 'No %{parameters}'; + + const output = sprintf(input); + + expect(output).toBe(input); + }); + + it('replaces parameters', () => { + const input = '%{name} has %{count} parameters'; + const parameters = { + name: 'this', + count: 2, + }; + + const output = sprintf(input, parameters); + + expect(output).toBe('this has 2 parameters'); + }); + + it('replaces multiple occurrences', () => { + const input = 'to %{verb} or not to %{verb}'; + const parameters = { + verb: 'be', + }; + + const output = sprintf(input, parameters); + + expect(output).toBe('to be or not to be'); + }); + + it('escapes parameters', () => { + const input = 'contains %{userContent}'; + const parameters = { + userContent: '<script>alert("malicious!")</script>', + }; + + const output = sprintf(input, parameters); + + expect(output).toBe('contains <script>alert("malicious!")</script>'); + }); + + it('does not escape parameters for escapeParameters = false', () => { + const input = 'contains %{safeContent}'; + const parameters = { + safeContent: '<strong>bold attempt</strong>', + }; + + const output = sprintf(input, parameters, false); + + expect(output).toBe('contains <strong>bold attempt</strong>'); + }); + }); +}); diff --git a/spec/javascripts/projects_dropdown/service/projects_service_spec.js b/spec/javascripts/projects_dropdown/service/projects_service_spec.js index d5dd8b3449a..cfd1bb7d24f 100644 --- a/spec/javascripts/projects_dropdown/service/projects_service_spec.js +++ b/spec/javascripts/projects_dropdown/service/projects_service_spec.js @@ -34,7 +34,7 @@ describe('ProjectsService', () => { const searchQuery = 'lab'; const queryParams = { - simple: false, + simple: true, per_page: 20, membership: true, order_by: 'last_activity_at', diff --git a/spec/javascripts/repo/components/repo_file_spec.js b/spec/javascripts/repo/components/repo_file_spec.js index 518a2d25ecf..f15633bd8b9 100644 --- a/spec/javascripts/repo/components/repo_file_spec.js +++ b/spec/javascripts/repo/components/repo_file_spec.js @@ -1,5 +1,6 @@ import Vue from 'vue'; import repoFile from '~/repo/components/repo_file.vue'; +import RepoStore from '~/repo/stores/repo_store'; describe('RepoFile', () => { const updated = 'updated'; @@ -12,8 +13,13 @@ describe('RepoFile', () => { level: 10, }; const activeFile = { + pageTitle: 'pageTitle', url: 'url', }; + const otherFile = { + html: '<p class="file-content">html</p>', + pageTitle: 'otherpageTitle', + }; function createComponent(propsData) { const RepoFile = Vue.extend(repoFile); @@ -60,6 +66,12 @@ describe('RepoFile', () => { expect(vm.$el.querySelector('.fa-spin.fa-spinner')).toBeFalsy(); }); + it('sets the document title correctly', () => { + RepoStore.setActiveFiles(otherFile); + + expect(document.title.trim()).toEqual(otherFile.pageTitle); + }); + it('renders a spinner if the file is loading', () => { file.loading = true; const vm = createComponent({ diff --git a/spec/javascripts/right_sidebar_spec.js b/spec/javascripts/right_sidebar_spec.js index f2072a6f350..5505f983d71 100644 --- a/spec/javascripts/right_sidebar_spec.js +++ b/spec/javascripts/right_sidebar_spec.js @@ -32,56 +32,86 @@ import '~/right_sidebar'; }; describe('RightSidebar', function() { - var fixtureName = 'issues/open-issue.html.raw'; - preloadFixtures(fixtureName); - loadJSONFixtures('todos/todos.json'); - - beforeEach(function() { - loadFixtures(fixtureName); - this.sidebar = new Sidebar; - $aside = $('.right-sidebar'); - $page = $('.page-with-sidebar'); - $icon = $aside.find('i'); - $toggle = $aside.find('.js-sidebar-toggle'); - return $labelsIcon = $aside.find('.sidebar-collapsed-icon'); - }); - it('should expand/collapse the sidebar when arrow is clicked', function() { - assertSidebarState('expanded'); - $toggle.click(); - assertSidebarState('collapsed'); - $toggle.click(); - assertSidebarState('expanded'); - }); - it('should float over the page and when sidebar icons clicked', function() { - $labelsIcon.click(); - return assertSidebarState('expanded'); - }); - it('should collapse when the icon arrow clicked while it is floating on page', function() { - $labelsIcon.click(); - assertSidebarState('expanded'); - $toggle.click(); - return assertSidebarState('collapsed'); + describe('fixture tests', () => { + var fixtureName = 'issues/open-issue.html.raw'; + preloadFixtures(fixtureName); + loadJSONFixtures('todos/todos.json'); + + beforeEach(function() { + loadFixtures(fixtureName); + this.sidebar = new Sidebar; + $aside = $('.right-sidebar'); + $page = $('.page-with-sidebar'); + $icon = $aside.find('i'); + $toggle = $aside.find('.js-sidebar-toggle'); + return $labelsIcon = $aside.find('.sidebar-collapsed-icon'); + }); + it('should expand/collapse the sidebar when arrow is clicked', function() { + assertSidebarState('expanded'); + $toggle.click(); + assertSidebarState('collapsed'); + $toggle.click(); + assertSidebarState('expanded'); + }); + it('should float over the page and when sidebar icons clicked', function() { + $labelsIcon.click(); + return assertSidebarState('expanded'); + }); + it('should collapse when the icon arrow clicked while it is floating on page', function() { + $labelsIcon.click(); + assertSidebarState('expanded'); + $toggle.click(); + return assertSidebarState('collapsed'); + }); + + it('should broadcast todo:toggle event when add todo clicked', function() { + var todos = getJSONFixture('todos/todos.json'); + spyOn(jQuery, 'ajax').and.callFake(function() { + var d = $.Deferred(); + var response = todos; + d.resolve(response); + return d.promise(); + }); + + var todoToggleSpy = spyOnEvent(document, 'todo:toggle'); + + $('.issuable-sidebar-header .js-issuable-todo').click(); + + expect(todoToggleSpy.calls.count()).toEqual(1); + }); + + it('should not hide collapsed icons', () => { + [].forEach.call(document.querySelectorAll('.sidebar-collapsed-icon'), (el) => { + expect(el.querySelector('.fa, svg').classList.contains('hidden')).toBeFalsy(); + }); + }); }); - it('should broadcast todo:toggle event when add todo clicked', function() { - var todos = getJSONFixture('todos/todos.json'); - spyOn(jQuery, 'ajax').and.callFake(function() { - var d = $.Deferred(); - var response = todos; - d.resolve(response); - return d.promise(); + describe('sidebarToggleClicked', () => { + const event = jasmine.createSpyObj('event', ['preventDefault']); + + beforeEach(() => { + spyOn($.fn, 'hasClass').and.returnValue(false); + }); + + afterEach(() => { + gl.lazyLoader = undefined; }); - var todoToggleSpy = spyOnEvent(document, 'todo:toggle'); + it('calls loadCheck if lazyLoader is set', () => { + gl.lazyLoader = jasmine.createSpyObj('lazyLoader', ['loadCheck']); - $('.issuable-sidebar-header .js-issuable-todo').click(); + Sidebar.prototype.sidebarToggleClicked(event); - expect(todoToggleSpy.calls.count()).toEqual(1); - }); + expect(gl.lazyLoader.loadCheck).toHaveBeenCalled(); + }); + + it('does not throw if lazyLoader is not defined', () => { + gl.lazyLoader = undefined; + + const toggle = Sidebar.prototype.sidebarToggleClicked.bind(null, event); - it('should not hide collapsed icons', () => { - [].forEach.call(document.querySelectorAll('.sidebar-collapsed-icon'), (el) => { - expect(el.querySelector('.fa, svg').classList.contains('hidden')).toBeFalsy(); + expect(toggle).not.toThrow(); }); }); }); diff --git a/spec/javascripts/vue_mr_widget/components/states/mr_widget_closed_spec.js b/spec/javascripts/vue_mr_widget/components/states/mr_widget_closed_spec.js index 47303d1e80f..d23b558f4ea 100644 --- a/spec/javascripts/vue_mr_widget/components/states/mr_widget_closed_spec.js +++ b/spec/javascripts/vue_mr_widget/components/states/mr_widget_closed_spec.js @@ -4,11 +4,15 @@ import closedComponent from '~/vue_merge_request_widget/components/states/mr_wid const mr = { targetBranch: 'good-branch', targetBranchPath: '/good-branch', - closedBy: { - name: 'Fatih Acet', - username: 'fatihacet', + closedEvent: { + author: { + name: 'Fatih Acet', + username: 'fatihacet', + }, + updatedAt: 'closedEventUpdatedAt', + formattedUpdatedAt: '', }, - updatedAt: '2017-03-23T20:08:08.845Z', + updatedAt: 'mrUpdatedAt', closedAt: '1 day ago', }; @@ -18,7 +22,7 @@ const createComponent = () => { return new Component({ el: document.createElement('div'), propsData: { mr }, - }).$el; + }); }; describe('MRWidgetClosed', () => { @@ -38,14 +42,30 @@ describe('MRWidgetClosed', () => { }); describe('template', () => { - it('should have correct elements', () => { - const el = createComponent(); + let vm; + let el; + beforeEach(() => { + vm = createComponent(); + el = vm.$el; + }); + + afterEach(() => { + vm.$destroy(); + }); + + it('should have correct elements', () => { expect(el.querySelector('h4').textContent).toContain('Closed by'); - expect(el.querySelector('h4').textContent).toContain(mr.closedBy.name); + expect(el.querySelector('h4').textContent).toContain(mr.closedEvent.author.name); expect(el.textContent).toContain('The changes were not merged into'); expect(el.querySelector('.label-branch').getAttribute('href')).toEqual(mr.targetBranchPath); expect(el.querySelector('.label-branch').textContent).toContain(mr.targetBranch); }); + + it('should use closedEvent updatedAt as tooltip title', () => { + expect( + el.querySelector('time').getAttribute('title'), + ).toBe('closedEventUpdatedAt'); + }); }); }); diff --git a/spec/javascripts/vue_mr_widget/components/states/mr_widget_conflicts_spec.js b/spec/javascripts/vue_mr_widget/components/states/mr_widget_conflicts_spec.js index 3b7b7d93662..5d4c7ec09dc 100644 --- a/spec/javascripts/vue_mr_widget/components/states/mr_widget_conflicts_spec.js +++ b/spec/javascripts/vue_mr_widget/components/states/mr_widget_conflicts_spec.js @@ -1,20 +1,9 @@ import Vue from 'vue'; import conflictsComponent from '~/vue_merge_request_widget/components/states/mr_widget_conflicts'; +import mountComponent from '../../../helpers/vue_mount_component_helper'; +const ConflictsComponent = Vue.extend(conflictsComponent); const path = '/conflicts'; -const createComponent = () => { - const Component = Vue.extend(conflictsComponent); - - return new Component({ - el: document.createElement('div'), - propsData: { - mr: { - canMerge: true, - conflictResolutionPath: path, - }, - }, - }); -}; describe('MRWidgetConflicts', () => { describe('props', () => { @@ -27,44 +16,90 @@ describe('MRWidgetConflicts', () => { }); describe('template', () => { - it('should have correct elements', () => { - const el = createComponent().$el; - const resolveButton = el.querySelector('.js-resolve-conflicts-button'); - const mergeButton = el.querySelector('.mr-widget-body .btn'); - const mergeLocallyButton = el.querySelector('.js-merge-locally-button'); - - expect(el.textContent).toContain('There are merge conflicts'); - expect(el.textContent).not.toContain('ask someone with write access'); - expect(el.querySelector('.btn-success').disabled).toBeTruthy(); - expect(resolveButton.textContent).toContain('Resolve conflicts'); - expect(resolveButton.getAttribute('href')).toEqual(path); - expect(mergeButton.textContent).toContain('Merge'); - expect(mergeLocallyButton.textContent).toContain('Merge locally'); + describe('when allowed to merge', () => { + let vm; + + beforeEach(() => { + vm = mountComponent(ConflictsComponent, { + mr: { + canMerge: true, + conflictResolutionPath: path, + }, + }); + }); + + afterEach(() => { + vm.$destroy(); + }); + + it('should tell you about conflicts without bothering other people', () => { + expect(vm.$el.textContent).toContain('There are merge conflicts'); + expect(vm.$el.textContent).not.toContain('ask someone with write access'); + }); + + it('should allow you to resolve the conflicts', () => { + const resolveButton = vm.$el.querySelector('.js-resolve-conflicts-button'); + + expect(resolveButton.textContent).toContain('Resolve conflicts'); + expect(resolveButton.getAttribute('href')).toEqual(path); + }); + + it('should have merge buttons', () => { + const mergeButton = vm.$el.querySelector('.js-disabled-merge-button'); + const mergeLocallyButton = vm.$el.querySelector('.js-merge-locally-button'); + + expect(mergeButton.textContent).toContain('Merge'); + expect(mergeButton.disabled).toBeTruthy(); + expect(mergeButton.classList.contains('btn-success')).toEqual(true); + expect(mergeLocallyButton.textContent).toContain('Merge locally'); + }); }); describe('when user does not have permission to merge', () => { let vm; beforeEach(() => { - vm = createComponent(); - vm.mr.canMerge = false; + vm = mountComponent(ConflictsComponent, { + mr: { + canMerge: false, + }, + }); }); - it('should show proper message', (done) => { - Vue.nextTick(() => { - expect(vm.$el.textContent).toContain('ask someone with write access'); - done(); - }); + afterEach(() => { + vm.$destroy(); + }); + + it('should show proper message', () => { + expect(vm.$el.textContent).toContain('ask someone with write access'); + }); + + it('should not have action buttons', () => { + expect(vm.$el.querySelector('.js-disabled-merge-button')).toBeDefined(); + expect(vm.$el.querySelector('.js-resolve-conflicts-button')).toBeNull(); + expect(vm.$el.querySelector('.js-merge-locally-button')).toBeNull(); }); + }); - it('should not have action buttons', (done) => { - Vue.nextTick(() => { - expect(vm.$el.querySelectorAll('.btn').length).toBe(1); - expect(vm.$el.querySelector('.js-resolve-conflicts-button')).toEqual(null); - expect(vm.$el.querySelector('.js-merge-locally-button')).toEqual(null); - done(); + describe('when fast-forward or semi-linear merge enabled', () => { + let vm; + + beforeEach(() => { + vm = mountComponent(ConflictsComponent, { + mr: { + shouldBeRebased: true, + }, }); }); + + afterEach(() => { + vm.$destroy(); + }); + + it('should tell you to rebase locally', () => { + expect(vm.$el.textContent).toContain('Fast-forward merge is not possible.'); + expect(vm.$el.textContent).toContain('To merge this request, first rebase locally'); + }); }); }); }); diff --git a/spec/javascripts/vue_mr_widget/components/states/mr_widget_merged_spec.js b/spec/javascripts/vue_mr_widget/components/states/mr_widget_merged_spec.js index afaa750199a..2714e8294fa 100644 --- a/spec/javascripts/vue_mr_widget/components/states/mr_widget_merged_spec.js +++ b/spec/javascripts/vue_mr_widget/components/states/mr_widget_merged_spec.js @@ -14,9 +14,12 @@ const createComponent = () => { canRevertInCurrentMR: true, canRemoveSourceBranch: true, sourceBranchRemoved: true, - mergedBy: {}, - mergedAt: '', - updatedAt: '', + mergedEvent: { + author: {}, + updatedAt: 'mergedUpdatedAt', + formattedUpdatedAt: '', + }, + updatedAt: 'mrUpdatedAt', targetBranch, }; @@ -170,5 +173,11 @@ describe('MRWidgetMerged', () => { done(); }); }); + + it('should use mergedEvent updatedAt as tooltip title', () => { + expect( + el.querySelector('time').getAttribute('title'), + ).toBe('mergedUpdatedAt'); + }); }); }); diff --git a/spec/javascripts/vue_mr_widget/components/states/mr_widget_ready_to_merge_spec.js b/spec/javascripts/vue_mr_widget/components/states/mr_widget_ready_to_merge_spec.js index 03a52f1f91c..2422e844e97 100644 --- a/spec/javascripts/vue_mr_widget/components/states/mr_widget_ready_to_merge_spec.js +++ b/spec/javascripts/vue_mr_widget/components/states/mr_widget_ready_to_merge_spec.js @@ -182,36 +182,6 @@ describe('MRWidgetReadyToMerge', () => { expect(vm.isMergeButtonDisabled).toBeTruthy(); }); }); - - describe('Remove source branch checkbox', () => { - describe('when user can merge but cannot delete branch', () => { - it('isRemoveSourceBranchButtonDisabled should be true', () => { - expect(vm.isRemoveSourceBranchButtonDisabled).toBe(true); - }); - - it('should be disabled in the rendered output', () => { - const checkboxElement = vm.$el.querySelector('#remove-source-branch-input'); - expect(checkboxElement.getAttribute('disabled')).toBe('disabled'); - }); - }); - - describe('when user can merge and can delete branch', () => { - beforeEach(() => { - this.customVm = createComponent({ - mr: { canRemoveSourceBranch: true }, - }); - }); - - it('isRemoveSourceBranchButtonDisabled should be false', () => { - expect(this.customVm.isRemoveSourceBranchButtonDisabled).toBe(false); - }); - - it('should be enabled in rendered output', () => { - const checkboxElement = this.customVm.$el.querySelector('#remove-source-branch-input'); - expect(checkboxElement.getAttribute('disabled')).toBeNull(); - }); - }); - }); }); describe('methods', () => { @@ -467,4 +437,54 @@ describe('MRWidgetReadyToMerge', () => { }); }); }); + + describe('Remove source branch checkbox', () => { + describe('when user can merge but cannot delete branch', () => { + it('isRemoveSourceBranchButtonDisabled should be true', () => { + expect(vm.isRemoveSourceBranchButtonDisabled).toBe(true); + }); + + it('should be disabled in the rendered output', () => { + const checkboxElement = vm.$el.querySelector('#remove-source-branch-input'); + expect(checkboxElement.getAttribute('disabled')).toBe('disabled'); + }); + }); + + describe('when user can merge and can delete branch', () => { + beforeEach(() => { + this.customVm = createComponent({ + mr: { canRemoveSourceBranch: true }, + }); + }); + + it('isRemoveSourceBranchButtonDisabled should be false', () => { + expect(this.customVm.isRemoveSourceBranchButtonDisabled).toBe(false); + }); + + it('should be enabled in rendered output', () => { + const checkboxElement = this.customVm.$el.querySelector('#remove-source-branch-input'); + expect(checkboxElement.getAttribute('disabled')).toBeNull(); + }); + }); + }); + + describe('Commit message area', () => { + it('when using merge commits, should show "Modify commit message" button', () => { + const customVm = createComponent({ + mr: { ffOnlyEnabled: false }, + }); + + expect(customVm.$el.querySelector('.js-fast-forward-message')).toBeNull(); + expect(customVm.$el.querySelector('.js-modify-commit-message-button')).toBeDefined(); + }); + + it('when fast-forward merge is enabled, only show fast-forward message', () => { + const customVm = createComponent({ + mr: { ffOnlyEnabled: true }, + }); + + expect(customVm.$el.querySelector('.js-fast-forward-message')).toBeDefined(); + expect(customVm.$el.querySelector('.js-modify-commit-message-button')).toBeNull(); + }); + }); }); diff --git a/spec/lib/github/import/legacy_diff_note_spec.rb b/spec/lib/github/import/legacy_diff_note_spec.rb new file mode 100644 index 00000000000..8c50b46cacb --- /dev/null +++ b/spec/lib/github/import/legacy_diff_note_spec.rb @@ -0,0 +1,9 @@ +require 'spec_helper' + +describe Github::Import::LegacyDiffNote do + describe '#type' do + it 'returns the original note type' do + expect(described_class.new.type).to eq('LegacyDiffNote') + end + end +end diff --git a/spec/lib/github/import/note_spec.rb b/spec/lib/github/import/note_spec.rb new file mode 100644 index 00000000000..fcdccd9e097 --- /dev/null +++ b/spec/lib/github/import/note_spec.rb @@ -0,0 +1,9 @@ +require 'spec_helper' + +describe Github::Import::Note do + describe '#type' do + it 'returns the original note type' do + expect(described_class.new.type).to eq('Note') + end + end +end diff --git a/spec/lib/gitlab/background_migration/deserialize_merge_request_diffs_and_commits_spec.rb b/spec/lib/gitlab/background_migration/deserialize_merge_request_diffs_and_commits_spec.rb index c0427639746..d2e7243ee05 100644 --- a/spec/lib/gitlab/background_migration/deserialize_merge_request_diffs_and_commits_spec.rb +++ b/spec/lib/gitlab/background_migration/deserialize_merge_request_diffs_and_commits_spec.rb @@ -1,9 +1,9 @@ require 'spec_helper' -describe Gitlab::BackgroundMigration::DeserializeMergeRequestDiffsAndCommits do +describe Gitlab::BackgroundMigration::DeserializeMergeRequestDiffsAndCommits, :truncate do describe '#perform' do - set(:merge_request) { create(:merge_request) } - set(:merge_request_diff) { merge_request.merge_request_diff } + let(:merge_request) { create(:merge_request) } + let(:merge_request_diff) { merge_request.merge_request_diff } let(:updated_merge_request_diff) { MergeRequestDiff.find(merge_request_diff.id) } def diffs_to_hashes(diffs) @@ -70,8 +70,8 @@ describe Gitlab::BackgroundMigration::DeserializeMergeRequestDiffsAndCommits do before do merge_request.reload_diff(true) - convert_to_yaml(start_id, merge_request_diff.commits, merge_request_diff.diffs) - convert_to_yaml(stop_id, updated_merge_request_diff.commits, updated_merge_request_diff.diffs) + convert_to_yaml(start_id, merge_request_diff.commits, diffs_to_hashes(merge_request_diff.merge_request_diff_files)) + convert_to_yaml(stop_id, updated_merge_request_diff.commits, diffs_to_hashes(updated_merge_request_diff.merge_request_diff_files)) MergeRequestDiffCommit.delete_all MergeRequestDiffFile.delete_all @@ -80,10 +80,32 @@ describe Gitlab::BackgroundMigration::DeserializeMergeRequestDiffsAndCommits do context 'when BUFFER_ROWS is exceeded' do before do stub_const("#{described_class}::BUFFER_ROWS", 1) + + allow(Gitlab::Database).to receive(:bulk_insert).and_call_original + end + + it 'inserts commit rows in chunks of BUFFER_ROWS' do + # There are 29 commits in each diff, so we should have slices of 20 + 9 + 20 + 9. + stub_const("#{described_class}::BUFFER_ROWS", 20) + + expect(Gitlab::Database).to receive(:bulk_insert) + .with('merge_request_diff_commits', anything) + .exactly(4) + .times + .and_call_original + + subject.perform(start_id, stop_id) end - it 'updates and continues' do - expect(described_class::MergeRequestDiff).to receive(:transaction).twice + it 'inserts diff rows in chunks of DIFF_FILE_BUFFER_ROWS' do + # There are 20 files in each diff, so we should have slices of 20 + 20. + stub_const("#{described_class}::DIFF_FILE_BUFFER_ROWS", 20) + + expect(Gitlab::Database).to receive(:bulk_insert) + .with('merge_request_diff_files', anything) + .exactly(2) + .times + .and_call_original subject.perform(start_id, stop_id) end @@ -91,27 +113,87 @@ describe Gitlab::BackgroundMigration::DeserializeMergeRequestDiffsAndCommits do context 'when BUFFER_ROWS is not exceeded' do it 'only updates once' do - expect(described_class::MergeRequestDiff).to receive(:transaction).once + expect(Gitlab::Database).to receive(:bulk_insert) + .with('merge_request_diff_commits', anything) + .once + .and_call_original + + expect(Gitlab::Database).to receive(:bulk_insert) + .with('merge_request_diff_files', anything) + .once + .and_call_original subject.perform(start_id, stop_id) end end - end - context 'when the merge request diff update fails' do - before do - allow(described_class::MergeRequestDiff) - .to receive(:update_all).and_raise(ActiveRecord::Rollback) - end + context 'when some rows were already inserted due to a previous failure' do + before do + subject.perform(start_id, stop_id) - it 'does not add any diff commits' do - expect { subject.perform(merge_request_diff.id, merge_request_diff.id) } - .not_to change { MergeRequestDiffCommit.count } + convert_to_yaml(start_id, merge_request_diff.commits, diffs_to_hashes(merge_request_diff.merge_request_diff_files)) + convert_to_yaml(stop_id, updated_merge_request_diff.commits, diffs_to_hashes(updated_merge_request_diff.merge_request_diff_files)) + end + + it 'does not raise' do + expect { subject.perform(start_id, stop_id) }.not_to raise_exception + end + + it 'logs a message' do + expect(Rails.logger).to receive(:info) + .with( + a_string_matching(described_class.name).and(matching([start_id, stop_id].inspect)) + ) + .twice + + subject.perform(start_id, stop_id) + end + + it 'ends up with the correct rows' do + expect(updated_merge_request_diff.commits.count).to eq(29) + expect(updated_merge_request_diff.raw_diffs.count).to eq(20) + end end - it 'does not add any diff files' do - expect { subject.perform(merge_request_diff.id, merge_request_diff.id) } - .not_to change { MergeRequestDiffFile.count } + context 'when the merge request diff update fails' do + let(:exception) { ActiveRecord::RecordNotFound } + + let(:perform_ignoring_exceptions) do + begin + subject.perform(start_id, stop_id) + rescue described_class::Error + end + end + + before do + allow_any_instance_of(described_class::MergeRequestDiff::ActiveRecord_Relation) + .to receive(:update_all).and_raise(exception) + end + + it 'raises an error' do + expect { subject.perform(start_id, stop_id) } + .to raise_exception(described_class::Error) + end + + it 'logs the error' do + expect(Rails.logger).to receive(:info).with( + a_string_matching(described_class.name) + .and(matching([start_id, stop_id].inspect)) + .and(matching(exception.name)) + ) + + perform_ignoring_exceptions + end + + it 'still adds diff commits' do + expect { perform_ignoring_exceptions } + .to change { MergeRequestDiffCommit.count } + end + + it 'still adds diff files' do + expect { perform_ignoring_exceptions } + .to change { MergeRequestDiffFile.count } + end end end diff --git a/spec/lib/gitlab/ci/ansi2html_spec.rb b/spec/lib/gitlab/ci/ansi2html_spec.rb index e6645985ba4..33540eab5d6 100644 --- a/spec/lib/gitlab/ci/ansi2html_spec.rb +++ b/spec/lib/gitlab/ci/ansi2html_spec.rb @@ -195,6 +195,32 @@ describe Gitlab::Ci::Ansi2html do end end + context "with section markers" do + let(:section_name) { 'test_section' } + let(:section_start_time) { Time.new(2017, 9, 20).utc } + let(:section_duration) { 3.seconds } + let(:section_end_time) { section_start_time + section_duration } + let(:section_start) { "section_start:#{section_start_time.to_i}:#{section_name}\r\033[0K"} + let(:section_end) { "section_end:#{section_end_time.to_i}:#{section_name}\r\033[0K"} + let(:section_start_html) do + '<div class="hidden" data-action="start"'\ + " data-timestamp=\"#{section_start_time.to_i}\" data-section=\"#{section_name}\">"\ + "#{section_start[0...-5]}</div>" + end + let(:section_end_html) do + '<div class="hidden" data-action="end"'\ + " data-timestamp=\"#{section_end_time.to_i}\" data-section=\"#{section_name}\">"\ + "#{section_end[0...-5]}</div>" + end + + it "prints light red" do + text = "#{section_start}\e[91mHello\e[0m\n#{section_end}" + html = %{#{section_start_html}<span class="term-fg-l-red">Hello</span><br>#{section_end_html}} + + expect(convert_html(text)).to eq(html) + end + end + describe "truncates" do let(:text) { "Hello World" } let(:stream) { StringIO.new(text) } diff --git a/spec/lib/gitlab/closing_issue_extractor_spec.rb b/spec/lib/gitlab/closing_issue_extractor_spec.rb index 9e528392756..ef7d766a13d 100644 --- a/spec/lib/gitlab/closing_issue_extractor_spec.rb +++ b/spec/lib/gitlab/closing_issue_extractor_spec.rb @@ -254,6 +254,46 @@ describe Gitlab::ClosingIssueExtractor do expect(subject.closed_by_message(message)).to eq([issue]) end + it do + message = "Implement: #{reference}" + expect(subject.closed_by_message(message)).to eq([issue]) + end + + it do + message = "Implements: #{reference}" + expect(subject.closed_by_message(message)).to eq([issue]) + end + + it do + message = "Implemented: #{reference}" + expect(subject.closed_by_message(message)).to eq([issue]) + end + + it do + message = "Implementing: #{reference}" + expect(subject.closed_by_message(message)).to eq([issue]) + end + + it do + message = "implement: #{reference}" + expect(subject.closed_by_message(message)).to eq([issue]) + end + + it do + message = "implements: #{reference}" + expect(subject.closed_by_message(message)).to eq([issue]) + end + + it do + message = "implemented: #{reference}" + expect(subject.closed_by_message(message)).to eq([issue]) + end + + it do + message = "implementing: #{reference}" + expect(subject.closed_by_message(message)).to eq([issue]) + end + context 'with an external issue tracker reference' do it 'extracts the referenced issue' do jira_project = create(:jira_project, name: 'JIRA_EXT1') diff --git a/spec/lib/gitlab/git/commit_spec.rb b/spec/lib/gitlab/git/commit_spec.rb index a3dff6d0d4b..3815055139a 100644 --- a/spec/lib/gitlab/git/commit_spec.rb +++ b/spec/lib/gitlab/git/commit_spec.rb @@ -65,34 +65,12 @@ describe Gitlab::Git::Commit, seed_helper: true do end describe "Commit info from gitaly commit" do - let(:id) { 'f00' } - let(:parent_ids) { %w(b45 b46) } let(:subject) { "My commit".force_encoding('ASCII-8BIT') } let(:body) { subject + "My body".force_encoding('ASCII-8BIT') } - let(:committer) do - Gitaly::CommitAuthor.new( - name: generate(:name), - email: generate(:email), - date: Google::Protobuf::Timestamp.new(seconds: 123) - ) - end - let(:author) do - Gitaly::CommitAuthor.new( - name: generate(:name), - email: generate(:email), - date: Google::Protobuf::Timestamp.new(seconds: 456) - ) - end - let(:gitaly_commit) do - Gitaly::GitCommit.new( - id: id, - subject: subject, - body: body, - author: author, - committer: committer, - parent_ids: parent_ids - ) - end + let(:gitaly_commit) { build(:gitaly_commit, subject: subject, body: body) } + let(:id) { gitaly_commit.id } + let(:committer) { gitaly_commit.committer } + let(:author) { gitaly_commit.author } let(:commit) { described_class.new(repository, gitaly_commit) } it { expect(commit.short_id).to eq(id[0..10]) } @@ -104,7 +82,7 @@ describe Gitlab::Git::Commit, seed_helper: true do it { expect(commit.author_name).to eq(author.name) } it { expect(commit.committer_name).to eq(committer.name) } it { expect(commit.committer_email).to eq(committer.email) } - it { expect(commit.parent_ids).to eq(parent_ids) } + it { expect(commit.parent_ids).to eq(gitaly_commit.parent_ids) } context 'no body' do let(:body) { "".force_encoding('ASCII-8BIT') } diff --git a/spec/lib/gitlab/git/diff_collection_spec.rb b/spec/lib/gitlab/git/diff_collection_spec.rb index 3494f0cc98d..ee657101f4c 100644 --- a/spec/lib/gitlab/git/diff_collection_spec.rb +++ b/spec/lib/gitlab/git/diff_collection_spec.rb @@ -341,8 +341,7 @@ describe Gitlab::Git::DiffCollection, seed_helper: true do end context 'when diff is quite large will collapse by default' do - let(:iterator) { [{ diff: 'a' * (Gitlab::Git::Diff.collapse_limit + 1) }] } - let(:max_files) { 100 } + let(:iterator) { [{ diff: 'a' * 20480 }] } context 'when no collapse is set' do let(:expanded) { true } diff --git a/spec/lib/gitlab/git/diff_spec.rb b/spec/lib/gitlab/git/diff_spec.rb index d39b33a0c05..4a7b06003fc 100644 --- a/spec/lib/gitlab/git/diff_spec.rb +++ b/spec/lib/gitlab/git/diff_spec.rb @@ -31,36 +31,6 @@ EOT [".gitmodules"]).patches.first end - describe 'size limit feature toggles' do - context 'when the feature gitlab_git_diff_size_limit_increase is enabled' do - before do - stub_feature_flags(gitlab_git_diff_size_limit_increase: true) - end - - it 'returns 200 KB for size_limit' do - expect(described_class.size_limit).to eq(200.kilobytes) - end - - it 'returns 100 KB for collapse_limit' do - expect(described_class.collapse_limit).to eq(100.kilobytes) - end - end - - context 'when the feature gitlab_git_diff_size_limit_increase is disabled' do - before do - stub_feature_flags(gitlab_git_diff_size_limit_increase: false) - end - - it 'returns 100 KB for size_limit' do - expect(described_class.size_limit).to eq(100.kilobytes) - end - - it 'returns 10 KB for collapse_limit' do - expect(described_class.collapse_limit).to eq(10.kilobytes) - end - end - end - describe '.new' do context 'using a Hash' do context 'with a small diff' do @@ -77,7 +47,7 @@ EOT context 'using a diff that is too large' do it 'prunes the diff' do - diff = described_class.new(diff: 'a' * (described_class.size_limit + 1)) + diff = described_class.new(diff: 'a' * 204800) expect(diff.diff).to be_empty expect(diff).to be_too_large @@ -115,8 +85,8 @@ EOT # The patch total size is 200, with lines between 21 and 54. # This is a quick-and-dirty way to test this. Ideally, a new patch is # added to the test repo with a size that falls between the real limits. - allow(Gitlab::Git::Diff).to receive(:size_limit).and_return(150) - allow(Gitlab::Git::Diff).to receive(:collapse_limit).and_return(100) + stub_const("#{described_class}::SIZE_LIMIT", 150) + stub_const("#{described_class}::COLLAPSE_LIMIT", 100) end it 'prunes the diff as a large diff instead of as a collapsed diff' do @@ -356,7 +326,7 @@ EOT describe '#collapsed?' do it 'returns true for a diff that is quite large' do - diff = described_class.new({ diff: 'a' * (described_class.collapse_limit + 1) }, expanded: false) + diff = described_class.new({ diff: 'a' * 20480 }, expanded: false) expect(diff).to be_collapsed end diff --git a/spec/lib/gitlab/git/hook_spec.rb b/spec/lib/gitlab/git/hook_spec.rb index 0ff4f3bd105..2fe1f5603ce 100644 --- a/spec/lib/gitlab/git/hook_spec.rb +++ b/spec/lib/gitlab/git/hook_spec.rb @@ -14,6 +14,7 @@ describe Gitlab::Git::Hook do let(:repo_path) { repository.path } let(:user) { create(:user) } let(:gl_id) { Gitlab::GlId.gl_id(user) } + let(:gl_username) { user.username } def create_hook(name) FileUtils.mkdir_p(File.join(repo_path, 'hooks')) @@ -42,6 +43,7 @@ describe Gitlab::Git::Hook do let(:env) do { 'GL_ID' => gl_id, + 'GL_USERNAME' => gl_username, 'PWD' => repo_path, 'GL_PROTOCOL' => 'web', 'GL_REPOSITORY' => gl_repository @@ -59,7 +61,7 @@ describe Gitlab::Git::Hook do .with(env, hook_path, chdir: repo_path).and_call_original end - status, errors = hook.trigger(gl_id, blank, blank, ref) + status, errors = hook.trigger(gl_id, gl_username, blank, blank, ref) expect(status).to be true expect(errors).to be_blank end @@ -72,7 +74,7 @@ describe Gitlab::Git::Hook do blank = Gitlab::Git::BLANK_SHA ref = Gitlab::Git::BRANCH_REF_PREFIX + 'new_branch' - status, errors = hook.trigger(gl_id, blank, blank, ref) + status, errors = hook.trigger(gl_id, gl_username, blank, blank, ref) expect(status).to be false expect(errors).to eq("error message from the hook<br>error message from the hook line 2<br>") end @@ -86,7 +88,7 @@ describe Gitlab::Git::Hook do blank = Gitlab::Git::BLANK_SHA ref = Gitlab::Git::BRANCH_REF_PREFIX + 'new_branch' - status, errors = hook.trigger(gl_id, blank, blank, ref) + status, errors = hook.trigger(gl_id, gl_username, blank, blank, ref) expect(status).to be true expect(errors).to be_nil end diff --git a/spec/lib/gitlab/git/hooks_service_spec.rb b/spec/lib/gitlab/git/hooks_service_spec.rb index d4d75b66659..51e4e3fdad1 100644 --- a/spec/lib/gitlab/git/hooks_service_spec.rb +++ b/spec/lib/gitlab/git/hooks_service_spec.rb @@ -1,7 +1,7 @@ require 'spec_helper' describe Gitlab::Git::HooksService, seed_helper: true do - let(:user) { Gitlab::Git::User.new('Jane Doe', 'janedoe@example.com', 'user-456') } + let(:user) { Gitlab::Git::User.new('janedoe', 'Jane Doe', 'janedoe@example.com', 'user-456') } let(:repository) { Gitlab::Git::Repository.new('default', TEST_REPO_PATH, 'project-123') } let(:service) { described_class.new } diff --git a/spec/lib/gitlab/git/repository_spec.rb b/spec/lib/gitlab/git/repository_spec.rb index 5effaf2b043..a0482e30a33 100644 --- a/spec/lib/gitlab/git/repository_spec.rb +++ b/spec/lib/gitlab/git/repository_spec.rb @@ -389,6 +389,40 @@ describe Gitlab::Git::Repository, seed_helper: true do end end + describe '#has_local_branches?' do + shared_examples 'check for local branches' do + it { expect(repository.has_local_branches?).to eq(true) } + + context 'mutable' do + let(:repository) { Gitlab::Git::Repository.new('default', TEST_MUTABLE_REPO_PATH, '') } + + after do + ensure_seeds + end + + it 'returns false when there are no branches' do + # Sanity check + expect(repository.has_local_branches?).to eq(true) + + FileUtils.rm_rf(File.join(repository.path, 'packed-refs')) + heads_dir = File.join(repository.path, 'refs/heads') + FileUtils.rm_rf(heads_dir) + FileUtils.mkdir_p(heads_dir) + + expect(repository.has_local_branches?).to eq(false) + end + end + end + + context 'with gitaly' do + it_behaves_like 'check for local branches' + end + + context 'without gitaly', skip_gitaly_mock: true do + it_behaves_like 'check for local branches' + end + end + describe "#delete_branch" do shared_examples "deleting a branch" do let(:repository) { Gitlab::Git::Repository.new('default', TEST_MUTABLE_REPO_PATH, '') } diff --git a/spec/lib/gitlab/git/user_spec.rb b/spec/lib/gitlab/git/user_spec.rb index 0ebcecb26c0..31d5f59a562 100644 --- a/spec/lib/gitlab/git/user_spec.rb +++ b/spec/lib/gitlab/git/user_spec.rb @@ -1,22 +1,38 @@ require 'spec_helper' describe Gitlab::Git::User do + let(:username) { 'janedo' } let(:name) { 'Jane Doe' } let(:email) { 'janedoe@example.com' } let(:gl_id) { 'user-123' } - subject { described_class.new(name, email, gl_id) } + subject { described_class.new(username, name, email, gl_id) } + + describe '.from_gitaly' do + let(:gitaly_user) { Gitaly::User.new(name: name, email: email, gl_id: gl_id) } + subject { described_class.from_gitaly(gitaly_user) } + + it { expect(subject).to eq(described_class.new('', name, email, gl_id)) } + end + + describe '.from_gitlab' do + let(:user) { build(:user) } + subject { described_class.from_gitlab(user) } + + it { expect(subject).to eq(described_class.new(user.username, user.name, user.email, 'user-')) } + end describe '#==' do - def eq_other(name, email, gl_id) - eq(described_class.new(name, email, gl_id)) + def eq_other(username, name, email, gl_id) + eq(described_class.new(username, name, email, gl_id)) end - it { expect(subject).to eq_other(name, email, gl_id) } + it { expect(subject).to eq_other(username, name, email, gl_id) } - it { expect(subject).not_to eq_other(nil, nil, nil) } - it { expect(subject).not_to eq_other(name + 'x', email, gl_id) } - it { expect(subject).not_to eq_other(name, email + 'x', gl_id) } - it { expect(subject).not_to eq_other(name, email, gl_id + 'x') } + it { expect(subject).not_to eq_other(nil, nil, nil, nil) } + it { expect(subject).not_to eq_other(username + 'x', name, email, gl_id) } + it { expect(subject).not_to eq_other(username, name + 'x', email, gl_id) } + it { expect(subject).not_to eq_other(username, name, email + 'x', gl_id) } + it { expect(subject).not_to eq_other(username, name, email, gl_id + 'x') } end end diff --git a/spec/lib/gitlab/gitaly_client/commit_service_spec.rb b/spec/lib/gitlab/gitaly_client/commit_service_spec.rb index 1ef3e2e3a5d..b2275119a04 100644 --- a/spec/lib/gitlab/gitaly_client/commit_service_spec.rb +++ b/spec/lib/gitlab/gitaly_client/commit_service_spec.rb @@ -53,7 +53,7 @@ describe Gitlab::GitalyClient::CommitService do end it 'encodes paths correctly' do - expect { client.diff_from_parent(commit, paths: ['encoding/test.txt', 'encoding/テスト.txt']) }.not_to raise_error + expect { client.diff_from_parent(commit, paths: ['encoding/test.txt', 'encoding/テスト.txt', nil]) }.not_to raise_error end end diff --git a/spec/lib/gitlab/gitaly_client/operation_service_spec.rb b/spec/lib/gitlab/gitaly_client/operation_service_spec.rb new file mode 100644 index 00000000000..769b14687ac --- /dev/null +++ b/spec/lib/gitlab/gitaly_client/operation_service_spec.rb @@ -0,0 +1,55 @@ +require 'spec_helper' + +describe Gitlab::GitalyClient::OperationService do + let(:project) { create(:project) } + let(:repository) { project.repository.raw } + let(:client) { described_class.new(repository) } + + describe '#user_create_branch' do + let(:user) { create(:user) } + let(:gitaly_user) { Gitlab::GitalyClient::Util.gitaly_user(user) } + let(:branch_name) { 'new' } + let(:start_point) { 'master' } + let(:request) do + Gitaly::UserCreateBranchRequest.new( + repository: repository.gitaly_repository, + branch_name: branch_name, + start_point: start_point, + user: gitaly_user + ) + end + let(:gitaly_commit) { build(:gitaly_commit) } + let(:commit_id) { gitaly_commit.id } + let(:gitaly_branch) do + Gitaly::Branch.new(name: branch_name, target_commit: gitaly_commit) + end + let(:response) { Gitaly::UserCreateBranchResponse.new(branch: gitaly_branch) } + let(:commit) { Gitlab::Git::Commit.new(repository, gitaly_commit) } + + subject { client.user_create_branch(branch_name, user, start_point) } + + it 'sends a user_create_branch message and returns a Gitlab::git::Branch' do + expect_any_instance_of(Gitaly::OperationService::Stub) + .to receive(:user_create_branch).with(request, kind_of(Hash)) + .and_return(response) + + expect(subject.name).to eq(branch_name) + expect(subject.dereferenced_target).to eq(commit) + end + + context "when pre_receive_error is present" do + let(:response) do + Gitaly::UserCreateBranchResponse.new(pre_receive_error: "something failed") + end + + it "throws a PreReceive exception" do + expect_any_instance_of(Gitaly::OperationService::Stub) + .to receive(:user_create_branch).with(request, kind_of(Hash)) + .and_return(response) + + expect { subject }.to raise_error( + Gitlab::Git::HooksService::PreReceiveError, "something failed") + end + end + end +end diff --git a/spec/lib/gitlab/gitaly_client/util_spec.rb b/spec/lib/gitlab/gitaly_client/util_spec.rb new file mode 100644 index 00000000000..498f6886bee --- /dev/null +++ b/spec/lib/gitlab/gitaly_client/util_spec.rb @@ -0,0 +1,43 @@ +require 'spec_helper' + +describe Gitlab::GitalyClient::Util do + describe '.repository' do + let(:repository_storage) { 'default' } + let(:relative_path) { 'my/repo.git' } + let(:gl_repository) { 'project-1' } + let(:git_object_directory) { '.git/objects' } + let(:git_alternate_object_directory) { '/dir/one:/dir/two' } + + subject do + described_class.repository(repository_storage, relative_path, gl_repository) + end + + it 'creates a Gitaly::Repository with the given data' do + expect(Gitlab::Git::Env).to receive(:[]).with('GIT_OBJECT_DIRECTORY') + .and_return(git_object_directory) + expect(Gitlab::Git::Env).to receive(:[]).with('GIT_ALTERNATE_OBJECT_DIRECTORIES') + .and_return(git_alternate_object_directory) + + expect(subject).to be_a(Gitaly::Repository) + expect(subject.storage_name).to eq(repository_storage) + expect(subject.relative_path).to eq(relative_path) + expect(subject.gl_repository).to eq(gl_repository) + expect(subject.git_object_directory).to eq(git_object_directory) + expect(subject.git_alternate_object_directories).to eq([git_alternate_object_directory]) + end + end + + describe '.gitaly_user' do + let(:user) { create(:user) } + let(:gl_id) { Gitlab::GlId.gl_id(user) } + + subject { described_class.gitaly_user(user) } + + it 'creates a Gitaly::User from a GitLab user' do + expect(subject).to be_a(Gitaly::User) + expect(subject.name).to eq(user.name) + expect(subject.email).to eq(user.email) + expect(subject.gl_id).to eq(gl_id) + end + end +end diff --git a/spec/lib/gitlab/gitaly_client_spec.rb b/spec/lib/gitlab/gitaly_client_spec.rb index 9a84d6e6a67..a1f4e65b8d4 100644 --- a/spec/lib/gitlab/gitaly_client_spec.rb +++ b/spec/lib/gitlab/gitaly_client_spec.rb @@ -38,6 +38,20 @@ describe Gitlab::GitalyClient, skip_gitaly_mock: true do end end + describe 'encode' do + [ + [nil, ""], + ["", ""], + [" ", " "], + %w(a1 a1), + ["编码", "\xE7\xBC\x96\xE7\xA0\x81".b] + ].each do |input, result| + it "encodes #{input.inspect} to #{result.inspect}" do + expect(described_class.encode(input)).to eq result + end + end + end + describe 'allow_n_plus_1_calls' do context 'when RequestStore is enabled', :request_store do it 'returns the result of the allow_n_plus_1_calls block' do diff --git a/spec/lib/gitlab/import_export/safe_model_attributes.yml b/spec/lib/gitlab/import_export/safe_model_attributes.yml index 899d17d97c2..7268226112c 100644 --- a/spec/lib/gitlab/import_export/safe_model_attributes.yml +++ b/spec/lib/gitlab/import_export/safe_model_attributes.yml @@ -412,6 +412,8 @@ Project: - last_repository_updated_at - ci_config_path - delete_error +- merge_requests_ff_only_enabled +- merge_requests_rebase_enabled Author: - name ProjectFeature: diff --git a/spec/lib/gitlab/o_auth/user_spec.rb b/spec/lib/gitlab/o_auth/user_spec.rb index 8aaf320cbf5..db26e16e3b2 100644 --- a/spec/lib/gitlab/o_auth/user_spec.rb +++ b/spec/lib/gitlab/o_auth/user_spec.rb @@ -4,6 +4,7 @@ describe Gitlab::OAuth::User do let(:oauth_user) { described_class.new(auth_hash) } let(:gl_user) { oauth_user.gl_user } let(:uid) { 'my-uid' } + let(:dn) { 'uid=user1,ou=People,dc=example' } let(:provider) { 'my-provider' } let(:auth_hash) { OmniAuth::AuthHash.new(uid: uid, provider: provider, info: info_hash) } let(:info_hash) do @@ -197,7 +198,7 @@ describe Gitlab::OAuth::User do allow(ldap_user).to receive(:uid) { uid } allow(ldap_user).to receive(:username) { uid } allow(ldap_user).to receive(:email) { ['johndoe@example.com', 'john2@example.com'] } - allow(ldap_user).to receive(:dn) { 'uid=user1,ou=People,dc=example' } + allow(ldap_user).to receive(:dn) { dn } end context "and no account for the LDAP user" do @@ -213,7 +214,7 @@ describe Gitlab::OAuth::User do identities_as_hash = gl_user.identities.map { |id| { provider: id.provider, extern_uid: id.extern_uid } } expect(identities_as_hash).to match_array( [ - { provider: 'ldapmain', extern_uid: 'uid=user1,ou=People,dc=example' }, + { provider: 'ldapmain', extern_uid: dn }, { provider: 'twitter', extern_uid: uid } ] ) @@ -221,7 +222,7 @@ describe Gitlab::OAuth::User do end context "and LDAP user has an account already" do - let!(:existing_user) { create(:omniauth_user, email: 'john@example.com', extern_uid: 'uid=user1,ou=People,dc=example', provider: 'ldapmain', username: 'john') } + let!(:existing_user) { create(:omniauth_user, email: 'john@example.com', extern_uid: dn, provider: 'ldapmain', username: 'john') } it "adds the omniauth identity to the LDAP account" do allow(Gitlab::LDAP::Person).to receive(:find_by_uid).and_return(ldap_user) @@ -234,7 +235,7 @@ describe Gitlab::OAuth::User do identities_as_hash = gl_user.identities.map { |id| { provider: id.provider, extern_uid: id.extern_uid } } expect(identities_as_hash).to match_array( [ - { provider: 'ldapmain', extern_uid: 'uid=user1,ou=People,dc=example' }, + { provider: 'ldapmain', extern_uid: dn }, { provider: 'twitter', extern_uid: uid } ] ) @@ -252,7 +253,7 @@ describe Gitlab::OAuth::User do expect(identities_as_hash) .to match_array( [ - { provider: 'ldapmain', extern_uid: 'uid=user1,ou=People,dc=example' }, + { provider: 'ldapmain', extern_uid: dn }, { provider: 'twitter', extern_uid: uid } ] ) @@ -310,8 +311,8 @@ describe Gitlab::OAuth::User do allow(ldap_user).to receive(:uid) { uid } allow(ldap_user).to receive(:username) { uid } allow(ldap_user).to receive(:email) { ['johndoe@example.com', 'john2@example.com'] } - allow(ldap_user).to receive(:dn) { 'uid=user1,ou=People,dc=example' } - allow(oauth_user).to receive(:ldap_person).and_return(ldap_user) + allow(ldap_user).to receive(:dn) { dn } + allow(Gitlab::LDAP::Person).to receive(:find_by_uid).and_return(ldap_user) end context "and no account for the LDAP user" do @@ -341,7 +342,7 @@ describe Gitlab::OAuth::User do end context 'and LDAP user has an account already' do - let!(:existing_user) { create(:omniauth_user, email: 'john@example.com', extern_uid: 'uid=user1,ou=People,dc=example', provider: 'ldapmain', username: 'john') } + let!(:existing_user) { create(:omniauth_user, email: 'john@example.com', extern_uid: dn, provider: 'ldapmain', username: 'john') } context 'dont block on create (LDAP)' do before do diff --git a/spec/lib/gitlab/path_regex_spec.rb b/spec/lib/gitlab/path_regex_spec.rb index 2f989397f7e..1f1c48ee9b5 100644 --- a/spec/lib/gitlab/path_regex_spec.rb +++ b/spec/lib/gitlab/path_regex_spec.rb @@ -84,9 +84,9 @@ describe Gitlab::PathRegex do let(:top_level_words) do words = routes_not_starting_in_wildcard.map do |route| route.split('/')[1] - end.compact.uniq + end.compact - words + ee_top_level_words + files_in_public + Array(API::API.prefix.to_s) + (words + ee_top_level_words + files_in_public + Array(API::API.prefix.to_s)).uniq end let(:ee_top_level_words) do @@ -95,10 +95,11 @@ describe Gitlab::PathRegex do let(:files_in_public) do git = Gitlab.config.git.bin_path - `cd #{Rails.root} && #{git} ls-files public` + tracked = `cd #{Rails.root} && #{git} ls-files public` .split("\n") .map { |entry| entry.gsub('public/', '') } .uniq + tracked + %w(assets uploads) end # All routes that start with a namespaced path, that have 1 or more diff --git a/spec/lib/gitlab/popen_spec.rb b/spec/lib/gitlab/popen_spec.rb index 4567f220c11..b145ca36f26 100644 --- a/spec/lib/gitlab/popen_spec.rb +++ b/spec/lib/gitlab/popen_spec.rb @@ -14,7 +14,7 @@ describe 'Gitlab::Popen' do end it { expect(@status).to be_zero } - it { expect(@output).to include('cache') } + it { expect(@output).to include('tests') } end context 'non-zero status' do diff --git a/spec/lib/gitlab/saml/user_spec.rb b/spec/lib/gitlab/saml/user_spec.rb index 19710029224..59923bfb14d 100644 --- a/spec/lib/gitlab/saml/user_spec.rb +++ b/spec/lib/gitlab/saml/user_spec.rb @@ -1,9 +1,12 @@ require 'spec_helper' describe Gitlab::Saml::User do + include LdapHelpers + let(:saml_user) { described_class.new(auth_hash) } let(:gl_user) { saml_user.gl_user } let(:uid) { 'my-uid' } + let(:dn) { 'uid=user1,ou=People,dc=example' } let(:provider) { 'saml' } let(:auth_hash) { OmniAuth::AuthHash.new(uid: uid, provider: provider, info: info_hash, extra: { raw_info: OneLogin::RubySaml::Attributes.new({ 'groups' => %w(Developers Freelancers Designers) }) }) } let(:info_hash) do @@ -163,13 +166,17 @@ describe Gitlab::Saml::User do end context 'and a corresponding LDAP person' do + let(:adapter) { ldap_adapter('ldapmain') } + before do allow(ldap_user).to receive(:uid) { uid } allow(ldap_user).to receive(:username) { uid } allow(ldap_user).to receive(:email) { %w(john@mail.com john2@example.com) } - allow(ldap_user).to receive(:dn) { 'uid=user1,ou=People,dc=example' } - allow(Gitlab::LDAP::Person).to receive(:find_by_uid).and_return(ldap_user) - allow(Gitlab::LDAP::Person).to receive(:find_by_dn).and_return(ldap_user) + allow(ldap_user).to receive(:dn) { dn } + allow(Gitlab::LDAP::Adapter).to receive(:new).and_return(adapter) + allow(Gitlab::LDAP::Person).to receive(:find_by_uid).with(uid, adapter).and_return(ldap_user) + allow(Gitlab::LDAP::Person).to receive(:find_by_dn).with(dn, adapter).and_return(ldap_user) + allow(Gitlab::LDAP::Person).to receive(:find_by_email).with('john@mail.com', adapter).and_return(ldap_user) end context 'and no account for the LDAP user' do @@ -181,20 +188,86 @@ describe Gitlab::Saml::User do expect(gl_user.email).to eql 'john@mail.com' expect(gl_user.identities.length).to be 2 identities_as_hash = gl_user.identities.map { |id| { provider: id.provider, extern_uid: id.extern_uid } } - expect(identities_as_hash).to match_array([{ provider: 'ldapmain', extern_uid: 'uid=user1,ou=People,dc=example' }, + expect(identities_as_hash).to match_array([{ provider: 'ldapmain', extern_uid: dn }, { provider: 'saml', extern_uid: uid }]) end end context 'and LDAP user has an account already' do + let(:auth_hash_base_attributes) do + { + uid: uid, + provider: provider, + info: info_hash, + extra: { + raw_info: OneLogin::RubySaml::Attributes.new( + { 'groups' => %w(Developers Freelancers Designers) } + ) + } + } + end + let(:auth_hash) { OmniAuth::AuthHash.new(auth_hash_base_attributes) } + let(:uid_types) { %w(uid dn email) } + before do create(:omniauth_user, email: 'john@mail.com', - extern_uid: 'uid=user1,ou=People,dc=example', + extern_uid: dn, provider: 'ldapmain', username: 'john') end + shared_examples 'find LDAP person' do |uid_type, uid| + let(:auth_hash) { OmniAuth::AuthHash.new(auth_hash_base_attributes.merge(uid: extern_uid)) } + + before do + nil_types = uid_types - [uid_type] + + nil_types.each do |type| + allow(Gitlab::LDAP::Person).to receive(:"find_by_#{type}").and_return(nil) + end + + allow(Gitlab::LDAP::Person).to receive(:"find_by_#{uid_type}").and_return(ldap_user) + end + + it 'adds the omniauth identity to the LDAP account' do + identities = [ + { provider: 'ldapmain', extern_uid: dn }, + { provider: 'saml', extern_uid: extern_uid } + ] + + identities_as_hash = gl_user.identities.map do |id| + { provider: id.provider, extern_uid: id.extern_uid } + end + + saml_user.save + + expect(gl_user).to be_valid + expect(gl_user.username).to eql 'john' + expect(gl_user.email).to eql 'john@mail.com' + expect(gl_user.identities.length).to be 2 + expect(identities_as_hash).to match_array(identities) + end + end + + context 'when uid is an uid' do + it_behaves_like 'find LDAP person', 'uid' do + let(:extern_uid) { uid } + end + end + + context 'when uid is a dn' do + it_behaves_like 'find LDAP person', 'dn' do + let(:extern_uid) { dn } + end + end + + context 'when uid is an email' do + it_behaves_like 'find LDAP person', 'email' do + let(:extern_uid) { 'john@mail.com' } + end + end + it 'adds the omniauth identity to the LDAP account' do saml_user.save @@ -203,7 +276,7 @@ describe Gitlab::Saml::User do expect(gl_user.email).to eql 'john@mail.com' expect(gl_user.identities.length).to be 2 identities_as_hash = gl_user.identities.map { |id| { provider: id.provider, extern_uid: id.extern_uid } } - expect(identities_as_hash).to match_array([{ provider: 'ldapmain', extern_uid: 'uid=user1,ou=People,dc=example' }, + expect(identities_as_hash).to match_array([{ provider: 'ldapmain', extern_uid: dn }, { provider: 'saml', extern_uid: uid }]) end @@ -219,17 +292,21 @@ describe Gitlab::Saml::User do context 'user has SAML user, and wants to add their LDAP identity' do it 'adds the LDAP identity to the existing SAML user' do - create(:omniauth_user, email: 'john@mail.com', extern_uid: 'uid=user1,ou=People,dc=example', provider: 'saml', username: 'john') - local_hash = OmniAuth::AuthHash.new(uid: 'uid=user1,ou=People,dc=example', provider: provider, info: info_hash) + create(:omniauth_user, email: 'john@mail.com', extern_uid: dn, provider: 'saml', username: 'john') + + allow(Gitlab::LDAP::Person).to receive(:find_by_uid).with(dn, adapter).and_return(ldap_user) + + local_hash = OmniAuth::AuthHash.new(uid: dn, provider: provider, info: info_hash) local_saml_user = described_class.new(local_hash) + local_saml_user.save local_gl_user = local_saml_user.gl_user expect(local_gl_user).to be_valid expect(local_gl_user.identities.length).to be 2 identities_as_hash = local_gl_user.identities.map { |id| { provider: id.provider, extern_uid: id.extern_uid } } - expect(identities_as_hash).to match_array([{ provider: 'ldapmain', extern_uid: 'uid=user1,ou=People,dc=example' }, - { provider: 'saml', extern_uid: 'uid=user1,ou=People,dc=example' }]) + expect(identities_as_hash).to match_array([{ provider: 'ldapmain', extern_uid: dn }, + { provider: 'saml', extern_uid: dn }]) end end end diff --git a/spec/lib/gitlab/shell_spec.rb b/spec/lib/gitlab/shell_spec.rb index 8edf83864da..9efdd7940ca 100644 --- a/spec/lib/gitlab/shell_spec.rb +++ b/spec/lib/gitlab/shell_spec.rb @@ -126,30 +126,42 @@ describe Gitlab::Shell do end describe '#add_repository' do - it 'creates a repository' do - created_path = File.join(TestEnv.repos_path, 'project', 'path.git') - hooks_path = File.join(created_path, 'hooks') + shared_examples '#add_repository' do + let(:repository_storage) { 'default' } + let(:repository_storage_path) { Gitlab.config.repositories.storages[repository_storage]['path'] } + let(:repo_name) { 'project/path' } + let(:created_path) { File.join(repository_storage_path, repo_name + '.git') } - begin - result = gitlab_shell.add_repository(TestEnv.repos_path, 'project/path') - - repo_stat = File.stat(created_path) rescue nil - hooks_stat = File.lstat(hooks_path) rescue nil - hooks_dir = File.realpath(hooks_path) - ensure + after do FileUtils.rm_rf(created_path) end - expect(result).to be_truthy - expect(repo_stat.mode & 0o777).to eq(0o770) - expect(hooks_stat.symlink?).to be_truthy - expect(hooks_dir).to eq(gitlab_shell_hooks_path) + it 'creates a repository' do + expect(gitlab_shell.add_repository(repository_storage, repo_name)).to be_truthy + + expect(File.stat(created_path).mode & 0o777).to eq(0o770) + + hooks_path = File.join(created_path, 'hooks') + expect(File.lstat(hooks_path)).to be_symlink + expect(File.realpath(hooks_path)).to eq(gitlab_shell_hooks_path) + end + + it 'returns false when the command fails' do + FileUtils.mkdir_p(File.dirname(created_path)) + # This file will block the creation of the repo's .git directory. That + # should cause #add_repository to fail. + FileUtils.touch(created_path) + + expect(gitlab_shell.add_repository(repository_storage, repo_name)).to be_falsy + end end - it 'returns false when the command fails' do - expect(FileUtils).to receive(:mkdir_p).and_raise(Errno::EEXIST) + context 'with gitaly' do + it_behaves_like '#add_repository' + end - expect(gitlab_shell.add_repository('current/storage', 'project/path')).to be_falsy + context 'without gitaly', skip_gitaly_mock: true do + it_behaves_like '#add_repository' end end diff --git a/spec/lib/gitlab/url_sanitizer_spec.rb b/spec/lib/gitlab/url_sanitizer_spec.rb index 59c28431e1e..fc8991fd31f 100644 --- a/spec/lib/gitlab/url_sanitizer_spec.rb +++ b/spec/lib/gitlab/url_sanitizer_spec.rb @@ -39,7 +39,8 @@ describe Gitlab::UrlSanitizer do false | nil false | '' false | '123://invalid:url' - true | 'valid@project:url.git' + false | 'valid@project:url.git' + false | 'valid:pass@project:url.git' true | 'ssh://example.com' true | 'ssh://:@example.com' true | 'ssh://foo@example.com' @@ -81,24 +82,6 @@ describe Gitlab::UrlSanitizer do describe '#credentials' do context 'credentials in hash' do - where(:input, :output) do - { user: 'foo', password: 'bar' } | { user: 'foo', password: 'bar' } - { user: 'foo', password: '' } | { user: 'foo', password: nil } - { user: 'foo', password: nil } | { user: 'foo', password: nil } - { user: '', password: 'bar' } | { user: nil, password: 'bar' } - { user: '', password: '' } | { user: nil, password: nil } - { user: '', password: nil } | { user: nil, password: nil } - { user: nil, password: 'bar' } | { user: nil, password: 'bar' } - { user: nil, password: '' } | { user: nil, password: nil } - { user: nil, password: nil } | { user: nil, password: nil } - end - - with_them do - subject { described_class.new('user@example.com:path.git', credentials: input).credentials } - - it { is_expected.to eq(output) } - end - it 'overrides URL-provided credentials' do sanitizer = described_class.new('http://a:b@example.com', credentials: { user: 'c', password: 'd' }) @@ -116,10 +99,6 @@ describe Gitlab::UrlSanitizer do 'http://@example.com' | { user: nil, password: nil } 'http://example.com' | { user: nil, password: nil } - # Credentials from SCP-style URLs are not supported at present - 'foo@example.com:path' | { user: nil, password: nil } - 'foo:bar@example.com:path' | { user: nil, password: nil } - # Other invalid URLs nil | { user: nil, password: nil } '' | { user: nil, password: nil } diff --git a/spec/lib/gitlab/workhorse_spec.rb b/spec/lib/gitlab/workhorse_spec.rb index a333ae33972..4dffe2bd82f 100644 --- a/spec/lib/gitlab/workhorse_spec.rb +++ b/spec/lib/gitlab/workhorse_spec.rb @@ -13,13 +13,51 @@ describe Gitlab::Workhorse do end describe ".send_git_archive" do + let(:ref) { 'master' } + let(:format) { 'zip' } + let(:storage_path) { Gitlab.config.gitlab.repository_downloads_path } + let(:base_params) { repository.archive_metadata(ref, storage_path, format) } + let(:gitaly_params) do + base_params.merge( + 'GitalyServer' => { + 'address' => Gitlab::GitalyClient.address(project.repository_storage), + 'token' => Gitlab::GitalyClient.token(project.repository_storage) + }, + 'GitalyRepository' => repository.gitaly_repository.to_h.deep_stringify_keys + ) + end + + subject do + described_class.send_git_archive(repository, ref: ref, format: format) + end + + context 'when Gitaly workhorse_archive feature is enabled' do + it 'sets the header correctly' do + key, command, params = decode_workhorse_header(subject) + + expect(key).to eq('Gitlab-Workhorse-Send-Data') + expect(command).to eq('git-archive') + expect(params).to include(gitaly_params) + end + end + + context 'when Gitaly workhorse_archive feature is disabled', skip_gitaly_mock: true do + it 'sets the header correctly' do + key, command, params = decode_workhorse_header(subject) + + expect(key).to eq('Gitlab-Workhorse-Send-Data') + expect(command).to eq('git-archive') + expect(params).to eq(base_params) + end + end + context "when the repository doesn't have an archive file path" do before do allow(project.repository).to receive(:archive_metadata).and_return(Hash.new) end it "raises an error" do - expect { described_class.send_git_archive(project.repository, ref: "master", format: "zip") }.to raise_error(RuntimeError) + expect { subject }.to raise_error(RuntimeError) end end end @@ -182,7 +220,12 @@ describe Gitlab::Workhorse do let(:repo_path) { repository.path_to_repo } let(:action) { 'info_refs' } let(:params) do - { GL_ID: "user-#{user.id}", GL_REPOSITORY: "project-#{project.id}", RepoPath: repo_path } + { + GL_ID: "user-#{user.id}", + GL_USERNAME: user.username, + GL_REPOSITORY: "project-#{project.id}", + RepoPath: repo_path + } end subject { described_class.git_http_ok(repository, false, user, action) } @@ -191,7 +234,12 @@ describe Gitlab::Workhorse do context 'when is_wiki' do let(:params) do - { GL_ID: "user-#{user.id}", GL_REPOSITORY: "wiki-#{project.id}", RepoPath: repo_path } + { + GL_ID: "user-#{user.id}", + GL_USERNAME: user.username, + GL_REPOSITORY: "wiki-#{project.id}", + RepoPath: repo_path + } end subject { described_class.git_http_ok(repository, true, user, action) } @@ -214,15 +262,13 @@ describe Gitlab::Workhorse do end it 'includes a Repository param' do - repo_param = { Repository: { + repo_param = { storage_name: 'default', relative_path: project.full_path + '.git', - git_object_directory: '', - git_alternate_object_directories: [], - gl_repository: '' - } } + gl_repository: "project-#{project.id}" + } - expect(subject).to include(repo_param) + expect(subject[:Repository]).to include(repo_param) end context "when git_upload_pack action is passed" do diff --git a/spec/lib/system_check/app/git_user_default_ssh_config_check_spec.rb b/spec/lib/system_check/app/git_user_default_ssh_config_check_spec.rb index 7125bfcab59..a0fb86345f3 100644 --- a/spec/lib/system_check/app/git_user_default_ssh_config_check_spec.rb +++ b/spec/lib/system_check/app/git_user_default_ssh_config_check_spec.rb @@ -16,7 +16,12 @@ describe SystemCheck::App::GitUserDefaultSSHConfigCheck do end it 'only whitelists safe files' do - expect(described_class::WHITELIST).to contain_exactly('authorized_keys', 'authorized_keys2', 'known_hosts') + expect(described_class::WHITELIST).to contain_exactly( + 'authorized_keys', + 'authorized_keys2', + 'authorized_keys.lock', + 'known_hosts' + ) end describe '#skip?' do diff --git a/spec/migrations/schedule_merge_request_diff_migrations_take_two_spec.rb b/spec/migrations/schedule_merge_request_diff_migrations_take_two_spec.rb new file mode 100644 index 00000000000..4ab1bb67058 --- /dev/null +++ b/spec/migrations/schedule_merge_request_diff_migrations_take_two_spec.rb @@ -0,0 +1,59 @@ +require 'spec_helper' +require Rails.root.join('db', 'post_migrate', '20170926150348_schedule_merge_request_diff_migrations_take_two') + +describe ScheduleMergeRequestDiffMigrationsTakeTwo, :migration, :sidekiq do + matcher :be_scheduled_migration do |time, *expected| + match do |migration| + BackgroundMigrationWorker.jobs.any? do |job| + job['args'] == [migration, expected] && + job['at'].to_i == time.to_i + end + end + + failure_message do |migration| + "Migration `#{migration}` with args `#{expected.inspect}` not scheduled!" + end + end + + let(:merge_request_diffs) { table(:merge_request_diffs) } + let(:merge_requests) { table(:merge_requests) } + let(:projects) { table(:projects) } + + before do + stub_const("#{described_class.name}::BATCH_SIZE", 1) + + projects.create!(id: 1, name: 'gitlab', path: 'gitlab') + + merge_requests.create!(id: 1, target_project_id: 1, source_project_id: 1, target_branch: 'feature', source_branch: 'master') + + merge_request_diffs.create!(id: 1, merge_request_id: 1, st_commits: YAML.dump([]), st_diffs: nil) + merge_request_diffs.create!(id: 2, merge_request_id: 1, st_commits: nil, st_diffs: YAML.dump([])) + merge_request_diffs.create!(id: 3, merge_request_id: 1, st_commits: nil, st_diffs: nil) + merge_request_diffs.create!(id: 4, merge_request_id: 1, st_commits: YAML.dump([]), st_diffs: YAML.dump([])) + end + + it 'correctly schedules background migrations' do + Sidekiq::Testing.fake! do + Timecop.freeze do + migrate! + + expect(described_class::MIGRATION).to be_scheduled_migration(10.minutes.from_now, 1, 1) + expect(described_class::MIGRATION).to be_scheduled_migration(20.minutes.from_now, 2, 2) + expect(described_class::MIGRATION).to be_scheduled_migration(30.minutes.from_now, 4, 4) + expect(BackgroundMigrationWorker.jobs.size).to eq 3 + end + end + end + + it 'migrates the data' do + Sidekiq::Testing.inline! do + non_empty = 'st_commits IS NOT NULL OR st_diffs IS NOT NULL' + + expect(merge_request_diffs.where(non_empty).count).to eq 3 + + migrate! + + expect(merge_request_diffs.where(non_empty).count).to eq 0 + end + end +end diff --git a/spec/migrations/update_legacy_diff_notes_type_for_import_spec.rb b/spec/migrations/update_legacy_diff_notes_type_for_import_spec.rb new file mode 100644 index 00000000000..d625b60ff50 --- /dev/null +++ b/spec/migrations/update_legacy_diff_notes_type_for_import_spec.rb @@ -0,0 +1,22 @@ +require 'spec_helper' +require Rails.root.join('db', 'post_migrate', '20170927112318_update_legacy_diff_notes_type_for_import.rb') + +describe UpdateLegacyDiffNotesTypeForImport, :migration do + let(:notes) { table(:notes) } + + before do + notes.inheritance_column = nil + + notes.create(type: 'Note') + notes.create(type: 'LegacyDiffNote') + notes.create(type: 'Github::Import::Note') + notes.create(type: 'Github::Import::LegacyDiffNote') + end + + it 'updates the notes type' do + migrate! + + expect(notes.pluck(:type)) + .to contain_exactly('Note', 'Github::Import::Note', 'LegacyDiffNote', 'LegacyDiffNote') + end +end diff --git a/spec/migrations/update_notes_type_for_import_spec.rb b/spec/migrations/update_notes_type_for_import_spec.rb new file mode 100644 index 00000000000..06195d970d8 --- /dev/null +++ b/spec/migrations/update_notes_type_for_import_spec.rb @@ -0,0 +1,22 @@ +require 'spec_helper' +require Rails.root.join('db', 'post_migrate', '20170927112319_update_notes_type_for_import.rb') + +describe UpdateNotesTypeForImport, :migration do + let(:notes) { table(:notes) } + + before do + notes.inheritance_column = nil + + notes.create(type: 'Note') + notes.create(type: 'LegacyDiffNote') + notes.create(type: 'Github::Import::Note') + notes.create(type: 'Github::Import::LegacyDiffNote') + end + + it 'updates the notes type' do + migrate! + + expect(notes.pluck(:type)) + .to contain_exactly('Note', 'Note', 'LegacyDiffNote', 'Github::Import::LegacyDiffNote') + end +end diff --git a/spec/models/gpg_key_spec.rb b/spec/models/gpg_key_spec.rb index fadc8bfeb61..4a4d079b721 100644 --- a/spec/models/gpg_key_spec.rb +++ b/spec/models/gpg_key_spec.rb @@ -138,6 +138,14 @@ describe GpgKey do expect(gpg_key.verified?).to be_truthy expect(gpg_key.verified_and_belongs_to_email?('bette.cartwright@example.com')).to be_truthy end + + it 'returns true if one of the email addresses in the key belongs to the user and case-insensitively matches the provided email' do + user = create :user, email: 'bette.cartwright@example.com' + gpg_key = create :gpg_key, key: GpgHelpers::User2.public_key, user: user + + expect(gpg_key.verified?).to be_truthy + expect(gpg_key.verified_and_belongs_to_email?('Bette.Cartwright@example.com')).to be_truthy + end end describe '#revoke' do diff --git a/spec/models/project_services/kubernetes_service_spec.rb b/spec/models/project_services/kubernetes_service_spec.rb index 537cdadd528..2298dcab55f 100644 --- a/spec/models/project_services/kubernetes_service_spec.rb +++ b/spec/models/project_services/kubernetes_service_spec.rb @@ -208,7 +208,7 @@ describe KubernetesService, :use_clean_rails_memory_store_caching do config.dig('users', 0, 'user')['token'] = 'token' config.dig('contexts', 0, 'context')['namespace'] = namespace config.dig('clusters', 0, 'cluster')['certificate-authority-data'] = - Base64.encode64('CA PEM DATA') + Base64.strict_encode64('CA PEM DATA') YAML.dump(config) end diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index 3ca88071ced..176bb568cbe 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -408,6 +408,18 @@ describe Project do end end + describe '#merge_method' do + it 'returns "ff" merge_method when ff is enabled' do + project = build(:project, merge_requests_ff_only_enabled: true) + expect(project.merge_method).to be :ff + end + + it 'returns "merge" merge_method when ff is disabled' do + project = build(:project, merge_requests_ff_only_enabled: false) + expect(project.merge_method).to be :merge + end + end + describe '#repository_storage_path' do let(:project) { create(:project, repository_storage: 'custom') } @@ -1303,7 +1315,7 @@ describe Project do context 'using a regular repository' do it 'creates the repository' do expect(shell).to receive(:add_repository) - .with(project.repository_storage_path, project.disk_path) + .with(project.repository_storage, project.disk_path) .and_return(true) expect(project.repository).to receive(:after_create) @@ -1313,7 +1325,7 @@ describe Project do it 'adds an error if the repository could not be created' do expect(shell).to receive(:add_repository) - .with(project.repository_storage_path, project.disk_path) + .with(project.repository_storage, project.disk_path) .and_return(false) expect(project.repository).not_to receive(:after_create) @@ -1370,7 +1382,7 @@ describe Project do .and_return(false) expect(shell).to receive(:add_repository) - .with(project.repository_storage_path, project.disk_path) + .with(project.repository_storage, project.disk_path) .and_return(true) project.ensure_repository @@ -2793,6 +2805,17 @@ describe Project do end end + describe '#check_repository_path_availability' do + let(:project) { build(:project) } + + it 'skips gitlab-shell exists?' do + project.skip_disk_validation = true + + expect(project.gitlab_shell).not_to receive(:exists?) + expect(project.check_repository_path_availability).to be_truthy + end + end + describe '#latest_successful_pipeline_for_default_branch' do let(:project) { build(:project) } diff --git a/spec/models/project_wiki_spec.rb b/spec/models/project_wiki_spec.rb index 953df7746eb..78fb2df884a 100644 --- a/spec/models/project_wiki_spec.rb +++ b/spec/models/project_wiki_spec.rb @@ -6,13 +6,10 @@ describe ProjectWiki do let(:user) { project.owner } let(:gitlab_shell) { Gitlab::Shell.new } let(:project_wiki) { described_class.new(project, user) } + let(:raw_repository) { Gitlab::Git::Repository.new(project.repository_storage, subject.disk_path + '.git', 'foo') } subject { project_wiki } - before do - project_wiki.wiki - end - describe "#path_with_namespace" do it "returns the project path with namespace with the .wiki extension" do expect(subject.path_with_namespace).to eq(project.full_path + '.wiki') @@ -61,8 +58,8 @@ describe ProjectWiki do end describe "#wiki" do - it "contains a Gollum::Wiki instance" do - expect(subject.wiki).to be_a Gollum::Wiki + it "contains a Gitlab::Git::Wiki instance" do + expect(subject.wiki).to be_a Gitlab::Git::Wiki end it "creates a new wiki repo if one does not yet exist" do @@ -70,20 +67,18 @@ describe ProjectWiki do end it "raises CouldNotCreateWikiError if it can't create the wiki repository" do - allow(project_wiki).to receive(:init_repo).and_return(false) - expect { project_wiki.send(:create_repo!) }.to raise_exception(ProjectWiki::CouldNotCreateWikiError) + # Create a fresh project which will not have a wiki + project_wiki = described_class.new(create(:project), user) + gitlab_shell = double(:gitlab_shell) + allow(gitlab_shell).to receive(:add_repository) + allow(project_wiki).to receive(:gitlab_shell).and_return(gitlab_shell) + + expect { project_wiki.send(:wiki) }.to raise_exception(ProjectWiki::CouldNotCreateWikiError) end end describe "#empty?" do context "when the wiki repository is empty" do - before do - allow_any_instance_of(Gitlab::Shell).to receive(:add_repository) do - create_temp_repo("#{Rails.root}/tmp/test-git-base-path/non-existant.wiki.git") - end - allow(project).to receive(:full_path).and_return("non-existant") - end - describe '#empty?' do subject { super().empty? } it { is_expected.to be_truthy } @@ -154,13 +149,13 @@ describe ProjectWiki do before do file = Gollum::File.new(subject.wiki) allow_any_instance_of(Gollum::Wiki) - .to receive(:file).with('image.jpg', 'master', true) + .to receive(:file).with('image.jpg', 'master') .and_return(file) allow_any_instance_of(Gollum::File) .to receive(:mime_type) .and_return('image/jpeg') allow_any_instance_of(Gollum::Wiki) - .to receive(:file).with('non-existant', 'master', true) + .to receive(:file).with('non-existant', 'master') .and_return(nil) end @@ -178,9 +173,9 @@ describe ProjectWiki do expect(subject.find_file('non-existant')).to eq(nil) end - it 'returns a Gollum::File instance' do + it 'returns a Gitlab::Git::WikiFile instance' do file = subject.find_file('image.jpg') - expect(file).to be_a Gollum::File + expect(file).to be_a Gitlab::Git::WikiFile end end @@ -222,9 +217,9 @@ describe ProjectWiki do describe "#update_page" do before do create_page("update-page", "some content") - @gollum_page = subject.wiki.paged("update-page") + @gitlab_git_wiki_page = subject.wiki.page(title: "update-page") subject.update_page( - @gollum_page, + @gitlab_git_wiki_page, content: "some other content", format: :markdown, message: "updated page" @@ -246,7 +241,7 @@ describe ProjectWiki do it 'updates project activity' do subject.update_page( - @gollum_page, + @gitlab_git_wiki_page, content: 'Yet more content', format: :markdown, message: 'Updated page again' @@ -262,7 +257,7 @@ describe ProjectWiki do describe "#delete_page" do before do create_page("index", "some content") - @page = subject.wiki.paged("index") + @page = subject.wiki.page(title: "index") end it "deletes the page" do @@ -282,27 +277,28 @@ describe ProjectWiki do describe '#create_repo!' do it 'creates a repository' do - expect(subject).to receive(:init_repo) - .with(subject.full_path) - .and_return(true) - + expect(raw_repository.exists?).to eq(false) expect(subject.repository).to receive(:after_create) - expect(subject.create_repo!).to be_an_instance_of(Gollum::Wiki) + subject.send(:create_repo!, raw_repository) + + expect(raw_repository.exists?).to eq(true) end end describe '#ensure_repository' do it 'creates the repository if it not exist' do - allow(subject).to receive(:repository_exists?).and_return(false) - - expect(subject).to receive(:create_repo!) + expect(raw_repository.exists?).to eq(false) + expect(subject).to receive(:create_repo!).and_call_original subject.ensure_repository + + expect(raw_repository.exists?).to eq(true) end it 'does not create the repository if it exists' do - allow(subject).to receive(:repository_exists?).and_return(true) + subject.wiki + expect(raw_repository.exists?).to eq(true) expect(subject).not_to receive(:create_repo!) @@ -329,7 +325,7 @@ describe ProjectWiki do end def commit_details - { name: user.name, email: user.email, message: "test commit" } + Gitlab::Git::Wiki::CommitDetails.new(user.name, user.email, "test commit") end def create_page(name, content) @@ -337,6 +333,6 @@ describe ProjectWiki do end def destroy_page(page) - subject.wiki.delete_page(page, commit_details) + subject.delete_page(page, commit_details) end end diff --git a/spec/models/repository_spec.rb b/spec/models/repository_spec.rb index 525c4027e5f..8a4dcdc311e 100644 --- a/spec/models/repository_spec.rb +++ b/spec/models/repository_spec.rb @@ -815,45 +815,70 @@ describe Repository do end describe '#add_branch' do - context 'when pre hooks were successful' do - it 'runs without errors' do - hook = double(trigger: [true, nil]) - expect(Gitlab::Git::Hook).to receive(:new).exactly(3).times.and_return(hook) + let(:branch_name) { 'new_feature' } + let(:target) { 'master' } - expect { repository.add_branch(user, 'new_feature', 'master') }.not_to raise_error - end + subject { repository.add_branch(user, branch_name, target) } - it 'creates the branch' do - allow_any_instance_of(Gitlab::Git::Hook).to receive(:trigger).and_return([true, nil]) + context 'with Gitaly enabled' do + it "calls Gitaly's OperationService" do + expect_any_instance_of(Gitlab::GitalyClient::OperationService) + .to receive(:user_create_branch).with(branch_name, user, target) + .and_return(nil) - branch = repository.add_branch(user, 'new_feature', 'master') + subject + end - expect(branch.name).to eq('new_feature') + it 'creates_the_branch' do + expect(subject.name).to eq(branch_name) + expect(repository.find_branch(branch_name)).not_to be_nil end - it 'calls the after_create_branch hook' do - expect(repository).to receive(:after_create_branch) + context 'with a non-existing target' do + let(:target) { 'fake-target' } - repository.add_branch(user, 'new_feature', 'master') + it "returns false and doesn't create the branch" do + expect(subject).to be(false) + expect(repository.find_branch(branch_name)).to be_nil + end end end - context 'when pre hooks failed' do - it 'gets an error' do - allow_any_instance_of(Gitlab::Git::Hook).to receive(:trigger).and_return([false, '']) + context 'with Gitaly disabled', skip_gitaly_mock: true do + context 'when pre hooks were successful' do + it 'runs without errors' do + hook = double(trigger: [true, nil]) + expect(Gitlab::Git::Hook).to receive(:new).exactly(3).times.and_return(hook) - expect do - repository.add_branch(user, 'new_feature', 'master') - end.to raise_error(Gitlab::Git::HooksService::PreReceiveError) + expect { subject }.not_to raise_error + end + + it 'creates the branch' do + allow_any_instance_of(Gitlab::Git::Hook).to receive(:trigger).and_return([true, nil]) + + expect(subject.name).to eq(branch_name) + end + + it 'calls the after_create_branch hook' do + expect(repository).to receive(:after_create_branch) + + subject + end end - it 'does not create the branch' do - allow_any_instance_of(Gitlab::Git::Hook).to receive(:trigger).and_return([false, '']) + context 'when pre hooks failed' do + it 'gets an error' do + allow_any_instance_of(Gitlab::Git::Hook).to receive(:trigger).and_return([false, '']) - expect do - repository.add_branch(user, 'new_feature', 'master') - end.to raise_error(Gitlab::Git::HooksService::PreReceiveError) - expect(repository.find_branch('new_feature')).to be_nil + expect { subject }.to raise_error(Gitlab::Git::HooksService::PreReceiveError) + end + + it 'does not create the branch' do + allow_any_instance_of(Gitlab::Git::Hook).to receive(:trigger).and_return([false, '']) + + expect { subject }.to raise_error(Gitlab::Git::HooksService::PreReceiveError) + expect(repository.find_branch(branch_name)).to be_nil + end end end end @@ -1131,21 +1156,31 @@ describe Repository do end describe '#has_visible_content?' do - subject { repository.has_visible_content? } + before do + # If raw_repository.has_visible_content? gets called more than once then + # caching is broken. We don't want that. + expect(repository.raw_repository).to receive(:has_visible_content?) + .once + .and_return(result) + end - describe 'when there are no branches' do - before do - allow(repository.raw_repository).to receive(:branch_count).and_return(0) - end + context 'when true' do + let(:result) { true } - it { is_expected.to eq(false) } + it 'returns true and caches it' do + expect(repository.has_visible_content?).to eq(true) + # Second call hits the cache + expect(repository.has_visible_content?).to eq(true) + end end - describe 'when there are branches' do - it 'returns true' do - expect(repository.raw_repository).to receive(:branch_count).and_return(3) + context 'when false' do + let(:result) { false } - expect(subject).to eq(true) + it 'returns false and caches it' do + expect(repository.has_visible_content?).to eq(false) + # Second call hits the cache + expect(repository.has_visible_content?).to eq(false) end end end @@ -1262,6 +1297,7 @@ describe Repository do allow(repository).to receive(:empty?).and_return(true) expect(cache).to receive(:expire).with(:empty?) + expect(cache).to receive(:expire).with(:has_visible_content?) repository.expire_emptiness_caches end @@ -1270,6 +1306,7 @@ describe Repository do allow(repository).to receive(:empty?).and_return(false) expect(cache).not_to receive(:expire).with(:empty?) + expect(cache).not_to receive(:expire).with(:has_visible_content?) repository.expire_emptiness_caches end @@ -1308,6 +1345,34 @@ describe Repository do end end + describe '#ff_merge' do + before do + repository.add_branch(user, 'ff-target', 'feature~5') + end + + it 'merges the code and return the commit id' do + merge_request = create(:merge_request, source_branch: 'feature', target_branch: 'ff-target', source_project: project) + merge_commit_id = repository.ff_merge(user, + merge_request.diff_head_sha, + merge_request.target_branch, + merge_request: merge_request) + merge_commit = repository.commit(merge_commit_id) + + expect(merge_commit).to be_present + expect(repository.blob_at(merge_commit.id, 'files/ruby/feature.rb')).to be_present + end + + it 'sets the `in_progress_merge_commit_sha` flag for the given merge request' do + merge_request = create(:merge_request, source_branch: 'feature', target_branch: 'ff-target', source_project: project) + merge_commit_id = repository.ff_merge(user, + merge_request.diff_head_sha, + merge_request.target_branch, + merge_request: merge_request) + + expect(merge_request.in_progress_merge_commit_sha).to eq(merge_commit_id) + end + end + describe '#revert' do let(:new_image_commit) { repository.commit('33f3729a45c02fc67d00adb1b8bca394b0e761d9') } let(:update_image_commit) { repository.commit('2f63565e7aac07bcdadb654e253078b727143ec4') } @@ -1599,7 +1664,7 @@ describe Repository do describe '#expire_branches_cache' do it 'expires the cache' do expect(repository).to receive(:expire_method_caches) - .with(%i(branch_names branch_count)) + .with(%i(branch_names branch_count has_visible_content?)) .and_call_original repository.expire_branches_cache @@ -1669,11 +1734,11 @@ describe Repository do tag_sha = tag.target expect(pre_receive_hook).to have_received(:trigger) - .with(anything, anything, commit_sha, anything) + .with(anything, anything, anything, commit_sha, anything) expect(update_hook).to have_received(:trigger) - .with(anything, anything, commit_sha, anything) + .with(anything, anything, anything, commit_sha, anything) expect(post_receive_hook).to have_received(:trigger) - .with(anything, anything, tag_sha, anything) + .with(anything, anything, anything, tag_sha, anything) end end end @@ -1878,6 +1943,15 @@ describe Repository do repository.expire_all_method_caches end + + it 'all cache_method definitions are in the lists of method caches' do + methods = repository.methods.map do |method| + match = /^_uncached_(.*)/.match(method) + match[1].to_sym if match + end.compact + + expect(methods).to match_array(Repository::CACHED_METHODS + Repository::MEMOIZED_CACHED_METHODS) + end end describe '#file_on_head' do diff --git a/spec/models/wiki_page_spec.rb b/spec/models/wiki_page_spec.rb index 9ef8d117123..1f14d06997e 100644 --- a/spec/models/wiki_page_spec.rb +++ b/spec/models/wiki_page_spec.rb @@ -80,7 +80,7 @@ describe WikiPage do context "when initialized with an existing gollum page" do before do create_page("test page", "test content") - @page = wiki.wiki.paged("test page") + @page = wiki.wiki.page(title: "test page") @wiki_page = described_class.new(wiki, @page, true) end @@ -105,7 +105,7 @@ describe WikiPage do end it "sets the version attribute" do - expect(@wiki_page.version).to be_a Gollum::Git::Commit + expect(@wiki_page.version).to be_a Gitlab::Git::WikiPageVersion end end end @@ -321,14 +321,14 @@ describe WikiPage do end it 'returns true when requesting an old version' do - old_version = @page.versions.last.to_s + old_version = @page.versions.last.id old_page = wiki.find_page('Update', old_version) expect(old_page.historical?).to eq true end it 'returns false when requesting latest version' do - latest_version = @page.versions.first.to_s + latest_version = @page.versions.first.id latest_page = wiki.find_page('Update', latest_version) expect(latest_page.historical?).to eq false @@ -393,7 +393,7 @@ describe WikiPage do end def commit_details - { name: user.name, email: user.email, message: "test commit" } + Gitlab::Git::Wiki::CommitDetails.new(user.name, user.email, "test commit") end def create_page(name, content) @@ -401,8 +401,8 @@ describe WikiPage do end def destroy_page(title) - page = wiki.wiki.paged(title) - wiki.wiki.delete_page(page, commit_details) + page = wiki.wiki.page(title: title) + wiki.delete_page(page, commit_details) end def get_slugs(page_or_dir) diff --git a/spec/policies/global_policy_spec.rb b/spec/policies/global_policy_spec.rb index 983f0e52d31..5b8cf2e6ab5 100644 --- a/spec/policies/global_policy_spec.rb +++ b/spec/policies/global_policy_spec.rb @@ -52,6 +52,29 @@ describe GlobalPolicy do end end + describe "create fork" do + context "when user has not exceeded project limit" do + it { is_expected.to be_allowed(:create_fork) } + end + + context "when user has exceeded project limit" do + let(:current_user) { create(:user, projects_limit: 0) } + + it { is_expected.not_to be_allowed(:create_fork) } + end + + context "when user is a master in a group" do + let(:group) { create(:group) } + let(:current_user) { create(:user, projects_limit: 0) } + + before do + group.add_master(current_user) + end + + it { is_expected.to be_allowed(:create_fork) } + end + end + describe 'custom attributes' do context 'regular user' do it { is_expected.not_to be_allowed(:read_custom_attribute) } diff --git a/spec/policies/namespace_policy_spec.rb b/spec/policies/namespace_policy_spec.rb new file mode 100644 index 00000000000..e52ff02e5f0 --- /dev/null +++ b/spec/policies/namespace_policy_spec.rb @@ -0,0 +1,20 @@ +require 'spec_helper' + +describe NamespacePolicy do + let(:current_user) { create(:user) } + let(:namespace) { current_user.namespace } + + subject { described_class.new(current_user, namespace) } + + context "create projects" do + context "user namespace" do + it { is_expected.to be_allowed(:create_projects) } + end + + context "user who has exceeded project limit" do + let(:current_user) { create(:user, projects_limit: 0) } + + it { is_expected.not_to be_allowed(:create_projects) } + end + end +end diff --git a/spec/requests/api/helpers_spec.rb b/spec/requests/api/helpers_spec.rb index 98c49d3364c..060c8902471 100644 --- a/spec/requests/api/helpers_spec.rb +++ b/spec/requests/api/helpers_spec.rb @@ -480,6 +480,27 @@ describe API::Helpers do handle_api_exception(exception) end + + context 'with a personal access token given' do + let(:token) { create(:personal_access_token, scopes: ['api'], user: user) } + + # Regression test for https://gitlab.com/gitlab-org/gitlab-ce/issues/38571 + it 'does not raise an additional exception because of missing `request`' do + # We need to stub at a lower level than #sentry_enabled? otherwise + # Sentry is not enabled when the request below is made, and the test + # would pass even without the fix + expect(Gitlab::Sentry).to receive(:enabled?).twice.and_return(true) + expect(ProjectsFinder).to receive(:new).and_raise('Runtime Error!') + + get api('/projects', personal_access_token: token) + + # The 500 status is expected as we're testing a case where an exception + # is raised, but Grape shouldn't raise an additional exception + expect(response).to have_gitlab_http_status(500) + expect(json_response['message']).not_to include("undefined local variable or method `request'") + expect(json_response['message']).to start_with("\nRuntimeError (Runtime Error!):") + end + end end describe '.authenticate_non_get!' do diff --git a/spec/requests/api/runner_spec.rb b/spec/requests/api/runner_spec.rb index 12720355a6d..5068df5b43a 100644 --- a/spec/requests/api/runner_spec.rb +++ b/spec/requests/api/runner_spec.rb @@ -360,6 +360,8 @@ describe API::Runner do 'policy' => 'pull-push' }] end + let(:expected_features) { { 'trace_sections' => true } } + it 'picks a job' do request_job info: { platform: :darwin } @@ -379,6 +381,7 @@ describe API::Runner do expect(json_response['artifacts']).to eq(expected_artifacts) expect(json_response['cache']).to eq(expected_cache) expect(json_response['variables']).to include(*expected_variables) + expect(json_response['features']).to eq(expected_features) end context 'when job is made for tag' do diff --git a/spec/serializers/build_serializer_spec.rb b/spec/serializers/build_serializer_spec.rb index 01e2cfed6f8..9673b11c2a2 100644 --- a/spec/serializers/build_serializer_spec.rb +++ b/spec/serializers/build_serializer_spec.rb @@ -38,7 +38,7 @@ describe BuildSerializer do expect(subject[:text]).to eq(status.text) expect(subject[:label]).to eq(status.label) expect(subject[:icon]).to eq(status.icon) - expect(subject[:favicon]).to eq("/assets/ci_favicons/#{status.favicon}.ico") + expect(subject[:favicon]).to match_asset_path("/assets/ci_favicons/#{status.favicon}.ico") end end end diff --git a/spec/serializers/merge_request_entity_spec.rb b/spec/serializers/merge_request_entity_spec.rb index d14cbe5ff92..4aeb593da44 100644 --- a/spec/serializers/merge_request_entity_spec.rb +++ b/spec/serializers/merge_request_entity_spec.rb @@ -37,7 +37,8 @@ describe MergeRequestEntity do :cancel_merge_when_pipeline_succeeds_path, :create_issue_to_resolve_discussions_path, :source_branch_path, :target_branch_commits_path, - :target_branch_tree_path, :commits_count, :merge_ongoing) + :target_branch_tree_path, :commits_count, :merge_ongoing, + :ff_only_enabled) end it 'has email_patches_path' do diff --git a/spec/serializers/pipeline_serializer_spec.rb b/spec/serializers/pipeline_serializer_spec.rb index 3baf9b1edab..8fc1ceedc34 100644 --- a/spec/serializers/pipeline_serializer_spec.rb +++ b/spec/serializers/pipeline_serializer_spec.rb @@ -168,7 +168,7 @@ describe PipelineSerializer do expect(subject[:text]).to eq(status.text) expect(subject[:label]).to eq(status.label) expect(subject[:icon]).to eq(status.icon) - expect(subject[:favicon]).to eq("/assets/ci_favicons/#{status.favicon}.ico") + expect(subject[:favicon]).to match_asset_path("/assets/ci_favicons/#{status.favicon}.ico") end end end diff --git a/spec/serializers/status_entity_spec.rb b/spec/serializers/status_entity_spec.rb index 3964b998084..16431ed4188 100644 --- a/spec/serializers/status_entity_spec.rb +++ b/spec/serializers/status_entity_spec.rb @@ -18,12 +18,12 @@ describe StatusEntity do it 'contains status details' do expect(subject).to include :text, :icon, :favicon, :label, :group expect(subject).to include :has_details, :details_path - expect(subject[:favicon]).to eq('/assets/ci_favicons/favicon_status_success.ico') + expect(subject[:favicon]).to match_asset_path('/assets/ci_favicons/favicon_status_success.ico') end it 'contains a dev namespaced favicon if dev env' do allow(Rails.env).to receive(:development?) { true } - expect(entity.as_json[:favicon]).to eq('/assets/ci_favicons/dev/favicon_status_success.ico') + expect(entity.as_json[:favicon]).to match_asset_path('/assets/ci_favicons/dev/favicon_status_success.ico') end end end diff --git a/spec/services/merge_requests/ff_merge_service_spec.rb b/spec/services/merge_requests/ff_merge_service_spec.rb new file mode 100644 index 00000000000..aaabf3ed2b0 --- /dev/null +++ b/spec/services/merge_requests/ff_merge_service_spec.rb @@ -0,0 +1,84 @@ +require 'spec_helper' + +describe MergeRequests::FfMergeService do + let(:user) { create(:user) } + let(:user2) { create(:user) } + let(:merge_request) do + create(:merge_request, + source_branch: 'flatten-dir', + target_branch: 'improve/awesome', + assignee: user2) + end + let(:project) { merge_request.project } + + before do + project.team << [user, :master] + project.team << [user2, :developer] + end + + describe '#execute' do + context 'valid params' do + let(:service) { described_class.new(project, user, {}) } + + before do + allow(service).to receive(:execute_hooks) + + perform_enqueued_jobs do + service.execute(merge_request) + end + end + + it "does not create merge commit" do + source_branch_sha = merge_request.source_project.repository.commit(merge_request.source_branch).sha + target_branch_sha = merge_request.target_project.repository.commit(merge_request.target_branch).sha + expect(source_branch_sha).to eq(target_branch_sha) + end + + it { expect(merge_request).to be_valid } + it { expect(merge_request).to be_merged } + + it 'sends email to user2 about merge of new merge_request' do + email = ActionMailer::Base.deliveries.last + expect(email.to.first).to eq(user2.email) + expect(email.subject).to include(merge_request.title) + end + + it 'creates system note about merge_request merge' do + note = merge_request.notes.last + expect(note.note).to include 'merged' + end + end + + context "error handling" do + let(:service) { described_class.new(project, user, commit_message: 'Awesome message') } + + before do + allow(Rails.logger).to receive(:error) + end + + it 'logs and saves error if there is an exception' do + error_message = 'error message' + + allow(service).to receive(:repository).and_raise("error message") + allow(service).to receive(:execute_hooks) + + service.execute(merge_request) + + 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 an PreReceiveError exception' do + error_message = 'error message' + + allow(service).to receive(:repository).and_raise(Gitlab::Git::HooksService::PreReceiveError, error_message) + allow(service).to receive(:execute_hooks) + + service.execute(merge_request) + + 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/projects/create_service_spec.rb b/spec/services/projects/create_service_spec.rb index 5da634e2fb1..dc89fdebce7 100644 --- a/spec/services/projects/create_service_spec.rb +++ b/spec/services/projects/create_service_spec.rb @@ -76,9 +76,8 @@ describe Projects::CreateService, '#execute' do context 'wiki_enabled true creates wiki repository directory' do it do project = create_project(user, opts) - path = ProjectWiki.new(project, user).send(:path_to_repo) - expect(File.exist?(path)).to be_truthy + expect(wiki_repo(project).exists?).to be_truthy end end @@ -86,11 +85,15 @@ describe Projects::CreateService, '#execute' do it do opts[:wiki_enabled] = false project = create_project(user, opts) - path = ProjectWiki.new(project, user).send(:path_to_repo) - expect(File.exist?(path)).to be_falsey + expect(wiki_repo(project).exists?).to be_falsey end end + + def wiki_repo(project) + relative_path = ProjectWiki.new(project).disk_path + '.git' + Gitlab::Git::Repository.new(project.repository_storage, relative_path, 'foobar') + end end context 'builds_enabled global setting' do @@ -149,6 +152,9 @@ describe Projects::CreateService, '#execute' do end context 'when another repository already exists on disk' do + let(:repository_storage) { 'default' } + let(:repository_storage_path) { Gitlab.config.repositories.storages[repository_storage]['path'] } + let(:opts) do { name: 'Existing', @@ -156,30 +162,59 @@ describe Projects::CreateService, '#execute' do } end - let(:repository_storage_path) { Gitlab.config.repositories.storages['default']['path'] } + context 'with legacy storage' do + before do + gitlab_shell.add_repository(repository_storage, "#{user.namespace.full_path}/existing") + end - before do - gitlab_shell.add_repository(repository_storage_path, "#{user.namespace.full_path}/existing") - end + after do + gitlab_shell.remove_repository(repository_storage_path, "#{user.namespace.full_path}/existing") + end - after do - gitlab_shell.remove_repository(repository_storage_path, "#{user.namespace.full_path}/existing") - end + it 'does not allow to create a project when path matches existing repository on disk' do + project = create_project(user, opts) - it 'does not allow to create project with same path' do - project = create_project(user, opts) + expect(project).not_to be_persisted + expect(project).to respond_to(:errors) + expect(project.errors.messages).to have_key(:base) + expect(project.errors.messages[:base].first).to match('There is already a repository with that name on disk') + end - expect(project).to respond_to(:errors) - expect(project.errors.messages).to have_key(:base) - expect(project.errors.messages[:base].first).to match('There is already a repository with that name on disk') + it 'does not allow to import project when path matches existing repository on disk' do + project = create_project(user, opts.merge({ import_url: 'https://gitlab.com/gitlab-org/gitlab-test.git' })) + + expect(project).not_to be_persisted + expect(project).to respond_to(:errors) + expect(project.errors.messages).to have_key(:base) + expect(project.errors.messages[:base].first).to match('There is already a repository with that name on disk') + end end - it 'does not allow to import a project with the same path' do - project = create_project(user, opts.merge({ import_url: 'https://gitlab.com/gitlab-org/gitlab-test.git' })) + context 'with hashed storage' do + let(:hash) { '6b86b273ff34fce19d6b804eff5a3f5747ada4eaa22f1d49c01e52ddb7875b4b' } + let(:hashed_path) { '@hashed/6b/86/6b86b273ff34fce19d6b804eff5a3f5747ada4eaa22f1d49c01e52ddb7875b4b' } + + before do + stub_application_setting(hashed_storage_enabled: true) + allow(Digest::SHA2).to receive(:hexdigest) { hash } + end - expect(project).to respond_to(:errors) - expect(project.errors.messages).to have_key(:base) - expect(project.errors.messages[:base].first).to match('There is already a repository with that name on disk') + before do + gitlab_shell.add_repository(repository_storage, hashed_path) + end + + after do + gitlab_shell.remove_repository(repository_storage_path, hashed_path) + end + + it 'does not allow to create a project when path matches existing repository on disk' do + project = create_project(user, opts) + + expect(project).not_to be_persisted + expect(project).to respond_to(:errors) + expect(project.errors.messages).to have_key(:base) + expect(project.errors.messages[:base].first).to match('There is already a repository with that name on disk') + end end end end @@ -208,6 +243,15 @@ describe Projects::CreateService, '#execute' do end end + context 'when skip_disk_validation is used' do + it 'sets the project attribute' do + opts[:skip_disk_validation] = true + project = create_project(user, opts) + + expect(project.skip_disk_validation).to be_truthy + end + end + def create_project(user, opts) Projects::CreateService.new(user, opts).execute end diff --git a/spec/services/projects/fork_service_spec.rb b/spec/services/projects/fork_service_spec.rb index a6e0364d44c..fa9d6969830 100644 --- a/spec/services/projects/fork_service_spec.rb +++ b/spec/services/projects/fork_service_spec.rb @@ -76,10 +76,11 @@ describe Projects::ForkService do end context 'repository already exists' do - let(:repository_storage_path) { Gitlab.config.repositories.storages['default']['path'] } + let(:repository_storage) { 'default' } + let(:repository_storage_path) { Gitlab.config.repositories.storages[repository_storage]['path'] } before do - gitlab_shell.add_repository(repository_storage_path, "#{@to_user.namespace.full_path}/#{@from_project.path}") + gitlab_shell.add_repository(repository_storage, "#{@to_user.namespace.full_path}/#{@from_project.path}") end after do diff --git a/spec/services/projects/transfer_service_spec.rb b/spec/services/projects/transfer_service_spec.rb index a14ed526f68..2459f371a91 100644 --- a/spec/services/projects/transfer_service_spec.rb +++ b/spec/services/projects/transfer_service_spec.rb @@ -121,11 +121,14 @@ describe Projects::TransferService do end context 'namespace which contains orphan repository with same projects path name' do - let(:repository_storage_path) { Gitlab.config.repositories.storages['default']['path'] } + let(:repository_storage) { 'default' } + let(:repository_storage_path) { Gitlab.config.repositories.storages[repository_storage]['path'] } before do group.add_owner(user) - gitlab_shell.add_repository(repository_storage_path, "#{group.full_path}/#{project.path}") + unless gitlab_shell.add_repository(repository_storage, "#{group.full_path}/#{project.path}") + raise 'failed to add repository' + end @result = transfer_project(project, user, group) end diff --git a/spec/services/projects/update_service_spec.rb b/spec/services/projects/update_service_spec.rb index c551083ac90..d400304622e 100644 --- a/spec/services/projects/update_service_spec.rb +++ b/spec/services/projects/update_service_spec.rb @@ -149,24 +149,43 @@ describe Projects::UpdateService, '#execute' do end context 'when renaming a project' do - let(:repository_storage_path) { Gitlab.config.repositories.storages['default']['path'] } + let(:repository_storage) { 'default' } + let(:repository_storage_path) { Gitlab.config.repositories.storages[repository_storage]['path'] } - before do - gitlab_shell.add_repository(repository_storage_path, "#{user.namespace.full_path}/existing") - end + context 'with legacy storage' do + before do + gitlab_shell.add_repository(repository_storage, "#{user.namespace.full_path}/existing") + end - after do - gitlab_shell.remove_repository(repository_storage_path, "#{user.namespace.full_path}/existing") + after do + gitlab_shell.remove_repository(repository_storage_path, "#{user.namespace.full_path}/existing") + end + + it 'does not allow renaming when new path matches existing repository on disk' do + result = update_project(project, admin, path: 'existing') + + expect(result).to include(status: :error) + expect(result[:message]).to match('There is already a repository with that name on disk') + expect(project).not_to be_valid + expect(project.errors.messages).to have_key(:base) + expect(project.errors.messages[:base]).to include('There is already a repository with that name on disk') + end end - it 'does not allow renaming when new path matches existing repository on disk' do - result = update_project(project, admin, path: 'existing') + context 'with hashed storage' do + let(:project) { create(:project, :repository, creator: user, namespace: user.namespace) } - expect(result).to include(status: :error) - expect(result[:message]).to match('There is already a repository with that name on disk') - expect(project).not_to be_valid - expect(project.errors.messages).to have_key(:base) - expect(project.errors.messages[:base]).to include('There is already a repository with that name on disk') + before do + stub_application_setting(hashed_storage_enabled: true) + end + + it 'does not check if new path matches existing repository on disk' do + expect(project).not_to receive(:repository_with_same_path_already_exists?) + + result = update_project(project, admin, path: 'existing') + + expect(result).to include(status: :success) + end end end diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index dbf05b7f004..576e4ae1d38 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -169,6 +169,24 @@ RSpec.configure do |config| end end +# add simpler way to match asset paths containing digest strings +RSpec::Matchers.define :match_asset_path do |expected| + match do |actual| + path = Regexp.escape(expected) + extname = Regexp.escape(File.extname(expected)) + digest_regex = Regexp.new(path.sub(extname, "(?:-\\h+)?#{extname}") << '$') + digest_regex =~ actual + end + + failure_message do |actual| + "expected that #{actual} would include an asset path for #{expected}" + end + + failure_message_when_negated do |actual| + "expected that #{actual} would not include an asset path for #{expected}" + end +end + FactoryGirl::SyntaxRunner.class_eval do include RSpec::Mocks::ExampleMethods end diff --git a/spec/support/test_env.rb b/spec/support/test_env.rb index b4e8b5ea67b..79395f4c564 100644 --- a/spec/support/test_env.rb +++ b/spec/support/test_env.rb @@ -17,6 +17,7 @@ module TestEnv 'feature_conflict' => 'bb5206f', 'fix' => '48f0be4', 'improve/awesome' => '5937ac0', + 'merged-target' => '21751bf', 'markdown' => '0ed8c6c', 'lfs' => 'be93687', 'master' => 'b83d6e3', diff --git a/spec/support/update_invalid_issuable.rb b/spec/support/update_invalid_issuable.rb index 1490287681b..50a1d4a56e2 100644 --- a/spec/support/update_invalid_issuable.rb +++ b/spec/support/update_invalid_issuable.rb @@ -25,11 +25,13 @@ shared_examples 'update invalid issuable' do |klass| .and_raise(ActiveRecord::StaleObjectError.new(issuable, :save)) end - it 'renders edit when format is html' do - put :update, params + if klass == MergeRequest + it 'renders edit when format is html' do + put :update, params - expect(response).to render_template(:edit) - expect(assigns[:conflict]).to be_truthy + expect(response).to render_template(:edit) + expect(assigns[:conflict]).to be_truthy + end end it 'renders json error message when format is json' do @@ -42,16 +44,17 @@ shared_examples 'update invalid issuable' do |klass| end end - context 'when updating an invalid issuable' do - before do - key = klass == Issue ? :issue : :merge_request - params[key][:title] = "" - end + if klass == MergeRequest + context 'when updating an invalid issuable' do + before do + params[:merge_request][:title] = "" + end - it 'renders edit when merge request is invalid' do - put :update, params + it 'renders edit when merge request is invalid' do + put :update, params - expect(response).to render_template(:edit) + expect(response).to render_template(:edit) + end end end end diff --git a/spec/views/shared/issuable/_participants.html.haml.rb b/spec/views/shared/issuable/_participants.html.haml.rb new file mode 100644 index 00000000000..51059d4c0d7 --- /dev/null +++ b/spec/views/shared/issuable/_participants.html.haml.rb @@ -0,0 +1,26 @@ +require 'spec_helper' +require 'nokogiri' + +describe 'shared/issuable/_participants.html.haml' do + let(:project) { create(:project) } + let(:participants) { create_list(:user, 100) } + + before do + allow(view).to receive_messages(project: project, + participants: participants) + end + + it 'renders lazy loaded avatars' do + render 'shared/issuable/participants' + + html = Nokogiri::HTML(rendered) + + avatars = html.css('.participants-author img') + + avatars.each do |avatar| + expect(avatar[:class]).to include('lazy') + expect(avatar[:src]).to eql(LazyImageTagHelper.placeholder_image) + expect(avatar[:"data-src"]).to match('http://www.gravatar.com/avatar/') + end + end +end diff --git a/spec/workers/git_garbage_collect_worker_spec.rb b/spec/workers/git_garbage_collect_worker_spec.rb index f7b67b8efc6..ab656d619f4 100644 --- a/spec/workers/git_garbage_collect_worker_spec.rb +++ b/spec/workers/git_garbage_collect_worker_spec.rb @@ -32,7 +32,7 @@ describe GitGarbageCollectWorker do expect_any_instance_of(Repository).to receive(:after_create_branch).and_call_original expect_any_instance_of(Repository).to receive(:branch_names).and_call_original expect_any_instance_of(Repository).to receive(:has_visible_content?).and_call_original - expect_any_instance_of(Gitlab::Git::Repository).to receive(:branch_count).and_call_original + expect_any_instance_of(Gitlab::Git::Repository).to receive(:has_visible_content?).and_call_original subject.perform(project.id, :gc, lease_key, lease_uuid) end @@ -47,7 +47,6 @@ describe GitGarbageCollectWorker do expect(subject).not_to receive(:command) expect_any_instance_of(Repository).not_to receive(:after_create_branch).and_call_original expect_any_instance_of(Repository).not_to receive(:branch_names).and_call_original - expect_any_instance_of(Repository).not_to receive(:branch_count).and_call_original expect_any_instance_of(Repository).not_to receive(:has_visible_content?).and_call_original subject.perform(project.id, :gc, lease_key, lease_uuid) @@ -78,7 +77,7 @@ describe GitGarbageCollectWorker do expect_any_instance_of(Repository).to receive(:after_create_branch).and_call_original expect_any_instance_of(Repository).to receive(:branch_names).and_call_original expect_any_instance_of(Repository).to receive(:has_visible_content?).and_call_original - expect_any_instance_of(Gitlab::Git::Repository).to receive(:branch_count).and_call_original + expect_any_instance_of(Gitlab::Git::Repository).to receive(:has_visible_content?).and_call_original subject.perform(project.id) end @@ -93,7 +92,6 @@ describe GitGarbageCollectWorker do expect(subject).not_to receive(:command) expect_any_instance_of(Repository).not_to receive(:after_create_branch).and_call_original expect_any_instance_of(Repository).not_to receive(:branch_names).and_call_original - expect_any_instance_of(Repository).not_to receive(:branch_count).and_call_original expect_any_instance_of(Repository).not_to receive(:has_visible_content?).and_call_original subject.perform(project.id) diff --git a/spec/workers/repository_check/single_repository_worker_spec.rb b/spec/workers/repository_check/single_repository_worker_spec.rb index d2609d21546..1d9bbf2ca62 100644 --- a/spec/workers/repository_check/single_repository_worker_spec.rb +++ b/spec/workers/repository_check/single_repository_worker_spec.rb @@ -69,7 +69,12 @@ describe RepositoryCheck::SingleRepositoryWorker do end def break_wiki(project) - FileUtils.rm_rf(wiki_path(project) + '/objects') + objects_dir = wiki_path(project) + '/objects' + + # Replace the /objects directory with a file so that the repo is + # invalid, _and_ 'git init' cannot fix it. + FileUtils.rm_rf(objects_dir) + FileUtils.touch(objects_dir) if File.directory?(wiki_path(project)) end def wiki_path(project) |