diff options
| author | Douwe Maan <douwe@selenight.nl> | 2016-07-07 18:05:34 -0400 |
|---|---|---|
| committer | Douwe Maan <douwe@selenight.nl> | 2016-07-07 18:05:34 -0400 |
| commit | 5a8f727fd5400634a99eae82e7bac5e26cf36d4e (patch) | |
| tree | dde30050a8992e4ba0b5d3f0e98794d1f6089df8 /spec | |
| parent | d925aea9f388f8ca5c858e825e5303232fa16c1d (diff) | |
| parent | 86d238e4bda6424a79eb9d8ea7cfe41af714f49f (diff) | |
| download | gitlab-ce-5a8f727fd5400634a99eae82e7bac5e26cf36d4e.tar.gz | |
Merge branch 'master' into faster-diffs
# Conflicts:
# app/helpers/notes_helper.rb
# app/views/projects/diffs/_line.html.haml
# app/views/projects/diffs/_parallel_view.html.haml
# app/views/projects/diffs/_text_file.html.haml
# features/steps/shared/diff_note.rb
Diffstat (limited to 'spec')
39 files changed, 3441 insertions, 122 deletions
diff --git a/spec/controllers/admin/projects_controller_spec.rb b/spec/controllers/admin/projects_controller_spec.rb index 4cb8b8da150..8eaacef2024 100644 --- a/spec/controllers/admin/projects_controller_spec.rb +++ b/spec/controllers/admin/projects_controller_spec.rb @@ -11,12 +11,12 @@ describe Admin::ProjectsController do render_views it 'retrieves the project for the given visibility level' do - get :index, visibility_levels: [Gitlab::VisibilityLevel::PUBLIC] + get :index, visibility_level: [Gitlab::VisibilityLevel::PUBLIC] expect(response.body).to match(project.name) end it 'does not retrieve the project' do - get :index, visibility_levels: [Gitlab::VisibilityLevel::INTERNAL] + get :index, visibility_level: [Gitlab::VisibilityLevel::INTERNAL] expect(response.body).not_to match(project.name) end end diff --git a/spec/controllers/projects/merge_requests_controller_spec.rb b/spec/controllers/projects/merge_requests_controller_spec.rb index eff74e12869..c4b57e77804 100644 --- a/spec/controllers/projects/merge_requests_controller_spec.rb +++ b/spec/controllers/projects/merge_requests_controller_spec.rb @@ -103,7 +103,7 @@ describe Projects::MergeRequestsController do id: merge_request.iid, format: :patch) - expect(response.headers['Gitlab-Workhorse-Send-Data']).to start_with("git-format-patch:") + expect(response.headers[Gitlab::Workhorse::SEND_DATA_HEADER]).to start_with("git-format-patch:") end end end @@ -212,7 +212,7 @@ describe Projects::MergeRequestsController do context 'when the sha parameter matches the source SHA' do def merge_with_sha - post :merge, base_params.merge(sha: merge_request.source_sha) + post :merge, base_params.merge(sha: merge_request.diff_head_sha) end it 'returns :success' do @@ -229,11 +229,11 @@ describe Projects::MergeRequestsController do context 'when merge_when_build_succeeds is passed' do def merge_when_build_succeeds - post :merge, base_params.merge(sha: merge_request.source_sha, merge_when_build_succeeds: '1') + post :merge, base_params.merge(sha: merge_request.diff_head_sha, merge_when_build_succeeds: '1') end before do - create(:ci_empty_pipeline, project: project, sha: merge_request.source_sha, ref: merge_request.source_branch) + create(:ci_empty_pipeline, project: project, sha: merge_request.diff_head_sha, ref: merge_request.source_branch) end it 'returns :merge_when_build_succeeds' do diff --git a/spec/factories/notes.rb b/spec/factories/notes.rb index 696cf276e57..83e38095feb 100644 --- a/spec/factories/notes.rb +++ b/spec/factories/notes.rb @@ -10,21 +10,46 @@ FactoryGirl.define do on_issue factory :note_on_commit, traits: [:on_commit] - factory :note_on_commit_diff, traits: [:on_commit, :on_diff], class: LegacyDiffNote factory :note_on_issue, traits: [:on_issue], aliases: [:votable_note] factory :note_on_merge_request, traits: [:on_merge_request] - factory :note_on_merge_request_diff, traits: [:on_merge_request, :on_diff], class: LegacyDiffNote factory :note_on_project_snippet, traits: [:on_project_snippet] factory :system_note, traits: [:system] + factory :legacy_diff_note_on_commit, traits: [:on_commit, :legacy_diff_note], class: LegacyDiffNote + factory :legacy_diff_note_on_merge_request, traits: [:on_merge_request, :legacy_diff_note], class: LegacyDiffNote + + factory :diff_note_on_merge_request, traits: [:on_merge_request], class: DiffNote do + position do + Gitlab::Diff::Position.new( + old_path: "files/ruby/popen.rb", + new_path: "files/ruby/popen.rb", + old_line: nil, + new_line: 14, + diff_refs: noteable.diff_refs + ) + end + end + + factory :diff_note_on_commit, traits: [:on_commit], class: DiffNote do + position do + Gitlab::Diff::Position.new( + old_path: "files/ruby/popen.rb", + new_path: "files/ruby/popen.rb", + old_line: nil, + new_line: 14, + diff_refs: project.commit(commit_id).try(:diff_refs) + ) + end + end + trait :on_commit do noteable nil - noteable_id nil noteable_type 'Commit' + noteable_id nil commit_id RepoHelpers.sample_commit.id end - trait :on_diff do + trait :legacy_diff_note do line_code "0_184_184" end diff --git a/spec/factories/notification_settings.rb b/spec/factories/notification_settings.rb new file mode 100644 index 00000000000..b5e96d18b8f --- /dev/null +++ b/spec/factories/notification_settings.rb @@ -0,0 +1,8 @@ +FactoryGirl.define do + factory :notification_setting do + source factory: :empty_project + user + level 3 + events [] + end +end diff --git a/spec/features/merge_requests/created_from_fork_spec.rb b/spec/features/merge_requests/created_from_fork_spec.rb index b4d2201c729..f676200ecf3 100644 --- a/spec/features/merge_requests/created_from_fork_spec.rb +++ b/spec/features/merge_requests/created_from_fork_spec.rb @@ -30,7 +30,7 @@ feature 'Merge request created from fork' do given(:pipeline) do create(:ci_pipeline_with_two_job, project: fork_project, - sha: merge_request.last_commit.id, + sha: merge_request.diff_head_sha, ref: merge_request.source_branch) end diff --git a/spec/features/merge_requests/merge_when_build_succeeds_spec.rb b/spec/features/merge_requests/merge_when_build_succeeds_spec.rb index c5e6412d7bf..96f7b8c9932 100644 --- a/spec/features/merge_requests/merge_when_build_succeeds_spec.rb +++ b/spec/features/merge_requests/merge_when_build_succeeds_spec.rb @@ -12,7 +12,7 @@ feature 'Merge When Build Succeeds', feature: true, js: true do end context "Active build for Merge Request" do - let!(:pipeline) { create(:ci_pipeline, project: project, sha: merge_request.last_commit.id, ref: merge_request.source_branch) } + let!(:pipeline) { create(:ci_pipeline, project: project, sha: merge_request.diff_head_sha, ref: merge_request.source_branch) } let!(:ci_build) { create(:ci_build, pipeline: pipeline) } before do @@ -47,7 +47,7 @@ feature 'Merge When Build Succeeds', feature: true, js: true do merge_user: user, title: "MepMep", merge_when_build_succeeds: true) end - let!(:pipeline) { create(:ci_pipeline, project: project, sha: merge_request.last_commit.id, ref: merge_request.source_branch) } + let!(:pipeline) { create(:ci_pipeline, project: project, sha: merge_request.diff_head_sha, ref: merge_request.source_branch) } let!(:ci_build) { create(:ci_build, pipeline: pipeline) } before do diff --git a/spec/features/merge_requests/only_allow_merge_if_build_succeeds.rb b/spec/features/merge_requests/only_allow_merge_if_build_succeeds.rb index 65e9185ec24..80e8b8fc642 100644 --- a/spec/features/merge_requests/only_allow_merge_if_build_succeeds.rb +++ b/spec/features/merge_requests/only_allow_merge_if_build_succeeds.rb @@ -19,7 +19,7 @@ feature 'Only allow merge requests to be merged if the build succeeds', feature: end context 'when project has CI enabled' do - let(:pipeline) { create(:ci_empty_pipeline, project: project, sha: merge_request.last_commit.id, ref: merge_request.source_branch) } + let(:pipeline) { create(:ci_empty_pipeline, project: project, sha: merge_request.diff_head_sha, ref: merge_request.source_branch) } context 'when merge requests can only be merged if the build succeeds' do before do diff --git a/spec/features/notes_on_merge_requests_spec.rb b/spec/features/notes_on_merge_requests_spec.rb index bb53475bf6c..61eec0afbee 100644 --- a/spec/features/notes_on_merge_requests_spec.rb +++ b/spec/features/notes_on_merge_requests_spec.rb @@ -166,12 +166,14 @@ describe 'Comments', feature: true do click_diff_line is_expected. - to have_css("tr[id='#{line_code}'] + .js-temp-notes-holder form", + to have_css("form[data-line-code='#{line_code}']", count: 1) end it 'should be removed when canceled' do - page.within(".diff-file form[id$='#{line_code}-true']") do + is_expected.to have_css('.js-temp-notes-holder') + + page.within("form[data-line-code='#{line_code}']") do find('.js-close-discussion-note-form').trigger('click') end diff --git a/spec/features/pipelines_spec.rb b/spec/features/pipelines_spec.rb index 98703ef3ac4..e7ee0aaea3c 100644 --- a/spec/features/pipelines_spec.rb +++ b/spec/features/pipelines_spec.rb @@ -123,7 +123,7 @@ describe "Pipelines" do before { visit namespace_project_pipeline_path(project.namespace, project, pipeline) } it 'showing a list of builds' do - expect(page).to have_content('Tests') + expect(page).to have_content('Test') expect(page).to have_content(@success.id) expect(page).to have_content('Deploy') expect(page).to have_content(@failed.id) diff --git a/spec/features/projects/files/gitlab_ci_yml_dropdown_spec.rb b/spec/features/projects/files/gitlab_ci_yml_dropdown_spec.rb index b8c06c383fb..fca40f68b01 100644 --- a/spec/features/projects/files/gitlab_ci_yml_dropdown_spec.rb +++ b/spec/features/projects/files/gitlab_ci_yml_dropdown_spec.rb @@ -19,12 +19,12 @@ feature 'User wants to add a .gitlab-ci.yml file', feature: true do find('.js-gitlab-ci-yml-selector').click wait_for_ajax within '.gitlab-ci-yml-selector' do - find('.dropdown-input-field').set('jekyll') - find('.dropdown-content li', text: 'jekyll').click + find('.dropdown-input-field').set('Jekyll') + find('.dropdown-content li', text: 'Jekyll').click end wait_for_ajax - expect(page).to have_css('.gitlab-ci-yml-selector .dropdown-toggle-text', text: 'jekyll') + expect(page).to have_css('.gitlab-ci-yml-selector .dropdown-toggle-text', text: 'Jekyll') expect(page).to have_content('This file is a template, and might need editing before it works on your project') expect(page).to have_content('jekyll build -d test') end diff --git a/spec/features/projects/import_export/import_file_spec.rb b/spec/features/projects/import_export/import_file_spec.rb index 9d66f76ef58..bc3bf53fe9d 100644 --- a/spec/features/projects/import_export/import_file_spec.rb +++ b/spec/features/projects/import_export/import_file_spec.rb @@ -42,6 +42,23 @@ feature 'project import', feature: true, js: true do expect(project.import_status).to eq('finished') end + scenario 'invalid project' do + project = create(:project, namespace_id: 2) + + visit new_project_path + + select2('2', from: '#project_namespace_id') + fill_in :project_path, with: project.name, visible: true + click_link 'GitLab export' + + attach_file('file', file) + click_on 'Import project' + + page.within('.flash-container') do + expect(page).to have_content('Project could not be imported') + end + end + def wiki_exists? wiki = ProjectWiki.new(project) File.exist?(wiki.repository.path_to_repo) && !wiki.repository.empty? diff --git a/spec/fixtures/parallel_diff_result.yml b/spec/fixtures/parallel_diff_result.yml index a8b7907d4ba..7d01183e3ef 100644 --- a/spec/fixtures/parallel_diff_result.yml +++ b/spec/fixtures/parallel_diff_result.yml @@ -3,322 +3,818 @@ :type: match :number: 6 :text: "@@ -6,12 +6,18 @@ module Popen" - :line_code: 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_6_6 + :line_code: + :position: !ruby/object:Gitlab::Diff::Position + attributes: + :old_path: files/ruby/popen.rb + :new_path: files/ruby/popen.rb + :old_line: + :new_line: + :base_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9 + :start_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9 + :head_sha: 570e7b2abdd848b95f2f578043fc23bd6f6fd24d :right: :type: match :number: 6 :text: "@@ -6,12 +6,18 @@ module Popen" - :line_code: 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_6_6 + :line_code: + :position: !ruby/object:Gitlab::Diff::Position + attributes: + :old_path: files/ruby/popen.rb + :new_path: files/ruby/popen.rb + :old_line: + :new_line: + :base_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9 + :start_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9 + :head_sha: 570e7b2abdd848b95f2f578043fc23bd6f6fd24d - :left: :type: :number: 6 :text: |2 <span id="LC6" class="line"></span> :line_code: 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_6_6 + :position: !ruby/object:Gitlab::Diff::Position + attributes: + :old_path: files/ruby/popen.rb + :new_path: files/ruby/popen.rb + :old_line: 6 + :new_line: 6 + :base_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9 + :start_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9 + :head_sha: 570e7b2abdd848b95f2f578043fc23bd6f6fd24d :right: :type: :number: 6 :text: |2 <span id="LC6" class="line"></span> :line_code: 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_6_6 + :position: !ruby/object:Gitlab::Diff::Position + attributes: + :old_path: files/ruby/popen.rb + :new_path: files/ruby/popen.rb + :old_line: 6 + :new_line: 6 + :base_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9 + :start_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9 + :head_sha: 570e7b2abdd848b95f2f578043fc23bd6f6fd24d - :left: :type: :number: 7 :text: |2 <span id="LC7" class="line"> <span class="k">def</span> <span class="nf">popen</span><span class="p">(</span><span class="n">cmd</span><span class="p">,</span> <span class="n">path</span><span class="o">=</span><span class="kp">nil</span><span class="p">)</span></span> :line_code: 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_7_7 + :position: !ruby/object:Gitlab::Diff::Position + attributes: + :old_path: files/ruby/popen.rb + :new_path: files/ruby/popen.rb + :old_line: 7 + :new_line: 7 + :base_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9 + :start_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9 + :head_sha: 570e7b2abdd848b95f2f578043fc23bd6f6fd24d :right: :type: :number: 7 :text: |2 <span id="LC7" class="line"> <span class="k">def</span> <span class="nf">popen</span><span class="p">(</span><span class="n">cmd</span><span class="p">,</span> <span class="n">path</span><span class="o">=</span><span class="kp">nil</span><span class="p">)</span></span> :line_code: 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_7_7 + :position: !ruby/object:Gitlab::Diff::Position + attributes: + :old_path: files/ruby/popen.rb + :new_path: files/ruby/popen.rb + :old_line: 7 + :new_line: 7 + :base_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9 + :start_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9 + :head_sha: 570e7b2abdd848b95f2f578043fc23bd6f6fd24d - :left: :type: :number: 8 :text: |2 <span id="LC8" class="line"> <span class="k">unless</span> <span class="n">cmd</span><span class="p">.</span><span class="nf">is_a?</span><span class="p">(</span><span class="no">Array</span><span class="p">)</span></span> :line_code: 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_8_8 + :position: !ruby/object:Gitlab::Diff::Position + attributes: + :old_path: files/ruby/popen.rb + :new_path: files/ruby/popen.rb + :old_line: 8 + :new_line: 8 + :base_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9 + :start_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9 + :head_sha: 570e7b2abdd848b95f2f578043fc23bd6f6fd24d :right: :type: :number: 8 :text: |2 <span id="LC8" class="line"> <span class="k">unless</span> <span class="n">cmd</span><span class="p">.</span><span class="nf">is_a?</span><span class="p">(</span><span class="no">Array</span><span class="p">)</span></span> :line_code: 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_8_8 + :position: !ruby/object:Gitlab::Diff::Position + attributes: + :old_path: files/ruby/popen.rb + :new_path: files/ruby/popen.rb + :old_line: 8 + :new_line: 8 + :base_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9 + :start_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9 + :head_sha: 570e7b2abdd848b95f2f578043fc23bd6f6fd24d - :left: :type: old :number: 9 :text: | -<span id="LC9" class="line"> <span class="k">raise</span> <span class="s2">"System commands must be given as an array of strings"</span></span> :line_code: 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_9_9 + :position: !ruby/object:Gitlab::Diff::Position + attributes: + :old_path: files/ruby/popen.rb + :new_path: files/ruby/popen.rb + :old_line: 9 + :new_line: + :base_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9 + :start_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9 + :head_sha: 570e7b2abdd848b95f2f578043fc23bd6f6fd24d :right: :type: new :number: 9 :text: | +<span id="LC9" class="line"> <span class="k">raise</span> <span class="no"><span class='idiff left'>RuntimeError</span></span><span class="p"><span class='idiff'>,</span></span><span class='idiff right'> </span><span class="s2">"System commands must be given as an array of strings"</span></span> :line_code: 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_10_9 + :position: !ruby/object:Gitlab::Diff::Position + attributes: + :old_path: files/ruby/popen.rb + :new_path: files/ruby/popen.rb + :old_line: + :new_line: 9 + :base_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9 + :start_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9 + :head_sha: 570e7b2abdd848b95f2f578043fc23bd6f6fd24d - :left: :type: :number: 10 :text: |2 <span id="LC10" class="line"> <span class="k">end</span></span> :line_code: 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_10_10 + :position: !ruby/object:Gitlab::Diff::Position + attributes: + :old_path: files/ruby/popen.rb + :new_path: files/ruby/popen.rb + :old_line: 10 + :new_line: 10 + :base_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9 + :start_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9 + :head_sha: 570e7b2abdd848b95f2f578043fc23bd6f6fd24d :right: :type: :number: 10 :text: |2 <span id="LC10" class="line"> <span class="k">end</span></span> :line_code: 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_10_10 + :position: !ruby/object:Gitlab::Diff::Position + attributes: + :old_path: files/ruby/popen.rb + :new_path: files/ruby/popen.rb + :old_line: 10 + :new_line: 10 + :base_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9 + :start_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9 + :head_sha: 570e7b2abdd848b95f2f578043fc23bd6f6fd24d - :left: :type: :number: 11 :text: |2 <span id="LC11" class="line"></span> :line_code: 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_11_11 + :position: !ruby/object:Gitlab::Diff::Position + attributes: + :old_path: files/ruby/popen.rb + :new_path: files/ruby/popen.rb + :old_line: 11 + :new_line: 11 + :base_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9 + :start_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9 + :head_sha: 570e7b2abdd848b95f2f578043fc23bd6f6fd24d :right: :type: :number: 11 :text: |2 <span id="LC11" class="line"></span> :line_code: 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_11_11 + :position: !ruby/object:Gitlab::Diff::Position + attributes: + :old_path: files/ruby/popen.rb + :new_path: files/ruby/popen.rb + :old_line: 11 + :new_line: 11 + :base_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9 + :start_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9 + :head_sha: 570e7b2abdd848b95f2f578043fc23bd6f6fd24d - :left: :type: :number: 12 :text: |2 <span id="LC12" class="line"> <span class="n">path</span> <span class="o">||=</span> <span class="no">Dir</span><span class="p">.</span><span class="nf">pwd</span></span> :line_code: 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_12_12 + :position: !ruby/object:Gitlab::Diff::Position + attributes: + :old_path: files/ruby/popen.rb + :new_path: files/ruby/popen.rb + :old_line: 12 + :new_line: 12 + :base_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9 + :start_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9 + :head_sha: 570e7b2abdd848b95f2f578043fc23bd6f6fd24d :right: :type: :number: 12 :text: |2 <span id="LC12" class="line"> <span class="n">path</span> <span class="o">||=</span> <span class="no">Dir</span><span class="p">.</span><span class="nf">pwd</span></span> :line_code: 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_12_12 + :position: !ruby/object:Gitlab::Diff::Position + attributes: + :old_path: files/ruby/popen.rb + :new_path: files/ruby/popen.rb + :old_line: 12 + :new_line: 12 + :base_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9 + :start_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9 + :head_sha: 570e7b2abdd848b95f2f578043fc23bd6f6fd24d - :left: :type: old :number: 13 :text: | -<span id="LC13" class="line"> <span class="n">vars</span> <span class="o">=</span> <span class="p">{</span> <span class="s2">"PWD"</span> <span class="o">=></span> <span class="n">path</span> <span class="p">}</span></span> :line_code: 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_13_13 + :position: !ruby/object:Gitlab::Diff::Position + attributes: + :old_path: files/ruby/popen.rb + :new_path: files/ruby/popen.rb + :old_line: 13 + :new_line: + :base_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9 + :start_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9 + :head_sha: 570e7b2abdd848b95f2f578043fc23bd6f6fd24d :right: :type: old :number: :text: '' :line_code: + :position: - :left: :type: old :number: 14 :text: | -<span id="LC14" class="line"> <span class="n">options</span> <span class="o">=</span> <span class="p">{</span> <span class="ss">chdir: </span><span class="n">path</span> <span class="p">}</span></span> :line_code: 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_14_13 + :position: !ruby/object:Gitlab::Diff::Position + attributes: + :old_path: files/ruby/popen.rb + :new_path: files/ruby/popen.rb + :old_line: 14 + :new_line: + :base_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9 + :start_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9 + :head_sha: 570e7b2abdd848b95f2f578043fc23bd6f6fd24d :right: :type: new :number: 13 :text: | +<span id="LC13" class="line"></span> :line_code: 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_15_13 + :position: !ruby/object:Gitlab::Diff::Position + attributes: + :old_path: files/ruby/popen.rb + :new_path: files/ruby/popen.rb + :old_line: + :new_line: 13 + :base_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9 + :start_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9 + :head_sha: 570e7b2abdd848b95f2f578043fc23bd6f6fd24d - :left: :type: :number: :text: '' :line_code: 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_15_14 + :position: !ruby/object:Gitlab::Diff::Position + attributes: + :old_path: files/ruby/popen.rb + :new_path: files/ruby/popen.rb + :old_line: + :new_line: 14 + :base_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9 + :start_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9 + :head_sha: 570e7b2abdd848b95f2f578043fc23bd6f6fd24d :right: :type: new :number: 14 :text: | +<span id="LC14" class="line"> <span class="n">vars</span> <span class="o">=</span> <span class="p">{</span></span> :line_code: 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_15_14 + :position: !ruby/object:Gitlab::Diff::Position + attributes: + :old_path: files/ruby/popen.rb + :new_path: files/ruby/popen.rb + :old_line: + :new_line: 14 + :base_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9 + :start_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9 + :head_sha: 570e7b2abdd848b95f2f578043fc23bd6f6fd24d - :left: :type: :number: :text: '' :line_code: 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_15_15 + :position: !ruby/object:Gitlab::Diff::Position + attributes: + :old_path: files/ruby/popen.rb + :new_path: files/ruby/popen.rb + :old_line: + :new_line: 15 + :base_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9 + :start_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9 + :head_sha: 570e7b2abdd848b95f2f578043fc23bd6f6fd24d :right: :type: new :number: 15 :text: | +<span id="LC15" class="line"> <span class="s2">"PWD"</span> <span class="o">=></span> <span class="n">path</span></span> :line_code: 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_15_15 + :position: !ruby/object:Gitlab::Diff::Position + attributes: + :old_path: files/ruby/popen.rb + :new_path: files/ruby/popen.rb + :old_line: + :new_line: 15 + :base_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9 + :start_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9 + :head_sha: 570e7b2abdd848b95f2f578043fc23bd6f6fd24d - :left: :type: :number: :text: '' :line_code: 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_15_16 + :position: !ruby/object:Gitlab::Diff::Position + attributes: + :old_path: files/ruby/popen.rb + :new_path: files/ruby/popen.rb + :old_line: + :new_line: 16 + :base_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9 + :start_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9 + :head_sha: 570e7b2abdd848b95f2f578043fc23bd6f6fd24d :right: :type: new :number: 16 :text: | +<span id="LC16" class="line"> <span class="p">}</span></span> :line_code: 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_15_16 + :position: !ruby/object:Gitlab::Diff::Position + attributes: + :old_path: files/ruby/popen.rb + :new_path: files/ruby/popen.rb + :old_line: + :new_line: 16 + :base_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9 + :start_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9 + :head_sha: 570e7b2abdd848b95f2f578043fc23bd6f6fd24d - :left: :type: :number: :text: '' :line_code: 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_15_17 + :position: !ruby/object:Gitlab::Diff::Position + attributes: + :old_path: files/ruby/popen.rb + :new_path: files/ruby/popen.rb + :old_line: + :new_line: 17 + :base_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9 + :start_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9 + :head_sha: 570e7b2abdd848b95f2f578043fc23bd6f6fd24d :right: :type: new :number: 17 :text: | +<span id="LC17" class="line"></span> :line_code: 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_15_17 + :position: !ruby/object:Gitlab::Diff::Position + attributes: + :old_path: files/ruby/popen.rb + :new_path: files/ruby/popen.rb + :old_line: + :new_line: 17 + :base_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9 + :start_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9 + :head_sha: 570e7b2abdd848b95f2f578043fc23bd6f6fd24d - :left: :type: :number: :text: '' :line_code: 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_15_18 + :position: !ruby/object:Gitlab::Diff::Position + attributes: + :old_path: files/ruby/popen.rb + :new_path: files/ruby/popen.rb + :old_line: + :new_line: 18 + :base_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9 + :start_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9 + :head_sha: 570e7b2abdd848b95f2f578043fc23bd6f6fd24d :right: :type: new :number: 18 :text: | +<span id="LC18" class="line"> <span class="n">options</span> <span class="o">=</span> <span class="p">{</span></span> :line_code: 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_15_18 + :position: !ruby/object:Gitlab::Diff::Position + attributes: + :old_path: files/ruby/popen.rb + :new_path: files/ruby/popen.rb + :old_line: + :new_line: 18 + :base_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9 + :start_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9 + :head_sha: 570e7b2abdd848b95f2f578043fc23bd6f6fd24d - :left: :type: :number: :text: '' :line_code: 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_15_19 + :position: !ruby/object:Gitlab::Diff::Position + attributes: + :old_path: files/ruby/popen.rb + :new_path: files/ruby/popen.rb + :old_line: + :new_line: 19 + :base_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9 + :start_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9 + :head_sha: 570e7b2abdd848b95f2f578043fc23bd6f6fd24d :right: :type: new :number: 19 :text: | +<span id="LC19" class="line"> <span class="ss">chdir: </span><span class="n">path</span></span> :line_code: 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_15_19 + :position: !ruby/object:Gitlab::Diff::Position + attributes: + :old_path: files/ruby/popen.rb + :new_path: files/ruby/popen.rb + :old_line: + :new_line: 19 + :base_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9 + :start_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9 + :head_sha: 570e7b2abdd848b95f2f578043fc23bd6f6fd24d - :left: :type: :number: :text: '' :line_code: 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_15_20 + :position: !ruby/object:Gitlab::Diff::Position + attributes: + :old_path: files/ruby/popen.rb + :new_path: files/ruby/popen.rb + :old_line: + :new_line: 20 + :base_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9 + :start_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9 + :head_sha: 570e7b2abdd848b95f2f578043fc23bd6f6fd24d :right: :type: new :number: 20 :text: | +<span id="LC20" class="line"> <span class="p">}</span></span> :line_code: 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_15_20 + :position: !ruby/object:Gitlab::Diff::Position + attributes: + :old_path: files/ruby/popen.rb + :new_path: files/ruby/popen.rb + :old_line: + :new_line: 20 + :base_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9 + :start_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9 + :head_sha: 570e7b2abdd848b95f2f578043fc23bd6f6fd24d - :left: :type: :number: 15 :text: |2 <span id="LC21" class="line"></span> :line_code: 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_15_21 + :position: !ruby/object:Gitlab::Diff::Position + attributes: + :old_path: files/ruby/popen.rb + :new_path: files/ruby/popen.rb + :old_line: 15 + :new_line: 21 + :base_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9 + :start_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9 + :head_sha: 570e7b2abdd848b95f2f578043fc23bd6f6fd24d :right: :type: :number: 21 :text: |2 <span id="LC21" class="line"></span> :line_code: 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_15_21 + :position: !ruby/object:Gitlab::Diff::Position + attributes: + :old_path: files/ruby/popen.rb + :new_path: files/ruby/popen.rb + :old_line: 15 + :new_line: 21 + :base_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9 + :start_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9 + :head_sha: 570e7b2abdd848b95f2f578043fc23bd6f6fd24d - :left: :type: :number: 16 :text: |2 <span id="LC22" class="line"> <span class="k">unless</span> <span class="no">File</span><span class="p">.</span><span class="nf">directory?</span><span class="p">(</span><span class="n">path</span><span class="p">)</span></span> :line_code: 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_16_22 + :position: !ruby/object:Gitlab::Diff::Position + attributes: + :old_path: files/ruby/popen.rb + :new_path: files/ruby/popen.rb + :old_line: 16 + :new_line: 22 + :base_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9 + :start_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9 + :head_sha: 570e7b2abdd848b95f2f578043fc23bd6f6fd24d :right: :type: :number: 22 :text: |2 <span id="LC22" class="line"> <span class="k">unless</span> <span class="no">File</span><span class="p">.</span><span class="nf">directory?</span><span class="p">(</span><span class="n">path</span><span class="p">)</span></span> :line_code: 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_16_22 + :position: !ruby/object:Gitlab::Diff::Position + attributes: + :old_path: files/ruby/popen.rb + :new_path: files/ruby/popen.rb + :old_line: 16 + :new_line: 22 + :base_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9 + :start_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9 + :head_sha: 570e7b2abdd848b95f2f578043fc23bd6f6fd24d - :left: :type: :number: 17 :text: |2 <span id="LC23" class="line"> <span class="no">FileUtils</span><span class="p">.</span><span class="nf">mkdir_p</span><span class="p">(</span><span class="n">path</span><span class="p">)</span></span> :line_code: 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_17_23 + :position: !ruby/object:Gitlab::Diff::Position + attributes: + :old_path: files/ruby/popen.rb + :new_path: files/ruby/popen.rb + :old_line: 17 + :new_line: 23 + :base_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9 + :start_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9 + :head_sha: 570e7b2abdd848b95f2f578043fc23bd6f6fd24d :right: :type: :number: 23 :text: |2 <span id="LC23" class="line"> <span class="no">FileUtils</span><span class="p">.</span><span class="nf">mkdir_p</span><span class="p">(</span><span class="n">path</span><span class="p">)</span></span> :line_code: 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_17_23 + :position: !ruby/object:Gitlab::Diff::Position + attributes: + :old_path: files/ruby/popen.rb + :new_path: files/ruby/popen.rb + :old_line: 17 + :new_line: 23 + :base_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9 + :start_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9 + :head_sha: 570e7b2abdd848b95f2f578043fc23bd6f6fd24d - :left: :type: match :number: 19 :text: "@@ -19,6 +25,7 @@ module Popen" - :line_code: 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_19_25 + :line_code: + :position: !ruby/object:Gitlab::Diff::Position + attributes: + :old_path: files/ruby/popen.rb + :new_path: files/ruby/popen.rb + :old_line: + :new_line: + :base_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9 + :start_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9 + :head_sha: 570e7b2abdd848b95f2f578043fc23bd6f6fd24d :right: :type: match :number: 25 :text: "@@ -19,6 +25,7 @@ module Popen" - :line_code: 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_19_25 + :line_code: + :position: !ruby/object:Gitlab::Diff::Position + attributes: + :old_path: files/ruby/popen.rb + :new_path: files/ruby/popen.rb + :old_line: + :new_line: + :base_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9 + :start_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9 + :head_sha: 570e7b2abdd848b95f2f578043fc23bd6f6fd24d - :left: :type: :number: 19 :text: |2 <span id="LC25" class="line"></span> :line_code: 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_19_25 + :position: !ruby/object:Gitlab::Diff::Position + attributes: + :old_path: files/ruby/popen.rb + :new_path: files/ruby/popen.rb + :old_line: 19 + :new_line: 25 + :base_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9 + :start_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9 + :head_sha: 570e7b2abdd848b95f2f578043fc23bd6f6fd24d :right: :type: :number: 25 :text: |2 <span id="LC25" class="line"></span> :line_code: 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_19_25 + :position: !ruby/object:Gitlab::Diff::Position + attributes: + :old_path: files/ruby/popen.rb + :new_path: files/ruby/popen.rb + :old_line: 19 + :new_line: 25 + :base_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9 + :start_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9 + :head_sha: 570e7b2abdd848b95f2f578043fc23bd6f6fd24d - :left: :type: :number: 20 :text: |2 <span id="LC26" class="line"> <span class="vi">@cmd_output</span> <span class="o">=</span> <span class="s2">""</span></span> :line_code: 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_20_26 + :position: !ruby/object:Gitlab::Diff::Position + attributes: + :old_path: files/ruby/popen.rb + :new_path: files/ruby/popen.rb + :old_line: 20 + :new_line: 26 + :base_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9 + :start_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9 + :head_sha: 570e7b2abdd848b95f2f578043fc23bd6f6fd24d :right: :type: :number: 26 :text: |2 <span id="LC26" class="line"> <span class="vi">@cmd_output</span> <span class="o">=</span> <span class="s2">""</span></span> :line_code: 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_20_26 + :position: !ruby/object:Gitlab::Diff::Position + attributes: + :old_path: files/ruby/popen.rb + :new_path: files/ruby/popen.rb + :old_line: 20 + :new_line: 26 + :base_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9 + :start_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9 + :head_sha: 570e7b2abdd848b95f2f578043fc23bd6f6fd24d - :left: :type: :number: 21 :text: |2 <span id="LC27" class="line"> <span class="vi">@cmd_status</span> <span class="o">=</span> <span class="mi">0</span></span> :line_code: 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_21_27 + :position: !ruby/object:Gitlab::Diff::Position + attributes: + :old_path: files/ruby/popen.rb + :new_path: files/ruby/popen.rb + :old_line: 21 + :new_line: 27 + :base_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9 + :start_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9 + :head_sha: 570e7b2abdd848b95f2f578043fc23bd6f6fd24d :right: :type: :number: 27 :text: |2 <span id="LC27" class="line"> <span class="vi">@cmd_status</span> <span class="o">=</span> <span class="mi">0</span></span> :line_code: 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_21_27 + :position: !ruby/object:Gitlab::Diff::Position + attributes: + :old_path: files/ruby/popen.rb + :new_path: files/ruby/popen.rb + :old_line: 21 + :new_line: 27 + :base_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9 + :start_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9 + :head_sha: 570e7b2abdd848b95f2f578043fc23bd6f6fd24d - :left: :type: :number: :text: '' :line_code: 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_22_28 + :position: !ruby/object:Gitlab::Diff::Position + attributes: + :old_path: files/ruby/popen.rb + :new_path: files/ruby/popen.rb + :old_line: + :new_line: 28 + :base_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9 + :start_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9 + :head_sha: 570e7b2abdd848b95f2f578043fc23bd6f6fd24d :right: :type: new :number: 28 :text: | +<span id="LC28" class="line"></span> :line_code: 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_22_28 + :position: !ruby/object:Gitlab::Diff::Position + attributes: + :old_path: files/ruby/popen.rb + :new_path: files/ruby/popen.rb + :old_line: + :new_line: 28 + :base_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9 + :start_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9 + :head_sha: 570e7b2abdd848b95f2f578043fc23bd6f6fd24d - :left: :type: :number: 22 :text: |2 <span id="LC29" class="line"> <span class="no">Open3</span><span class="p">.</span><span class="nf">popen3</span><span class="p">(</span><span class="n">vars</span><span class="p">,</span> <span class="o">*</span><span class="n">cmd</span><span class="p">,</span> <span class="n">options</span><span class="p">)</span> <span class="k">do</span> <span class="o">|</span><span class="n">stdin</span><span class="p">,</span> <span class="n">stdout</span><span class="p">,</span> <span class="n">stderr</span><span class="p">,</span> <span class="n">wait_thr</span><span class="o">|</span></span> :line_code: 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_22_29 + :position: !ruby/object:Gitlab::Diff::Position + attributes: + :old_path: files/ruby/popen.rb + :new_path: files/ruby/popen.rb + :old_line: 22 + :new_line: 29 + :base_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9 + :start_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9 + :head_sha: 570e7b2abdd848b95f2f578043fc23bd6f6fd24d :right: :type: :number: 29 :text: |2 <span id="LC29" class="line"> <span class="no">Open3</span><span class="p">.</span><span class="nf">popen3</span><span class="p">(</span><span class="n">vars</span><span class="p">,</span> <span class="o">*</span><span class="n">cmd</span><span class="p">,</span> <span class="n">options</span><span class="p">)</span> <span class="k">do</span> <span class="o">|</span><span class="n">stdin</span><span class="p">,</span> <span class="n">stdout</span><span class="p">,</span> <span class="n">stderr</span><span class="p">,</span> <span class="n">wait_thr</span><span class="o">|</span></span> :line_code: 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_22_29 + :position: !ruby/object:Gitlab::Diff::Position + attributes: + :old_path: files/ruby/popen.rb + :new_path: files/ruby/popen.rb + :old_line: 22 + :new_line: 29 + :base_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9 + :start_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9 + :head_sha: 570e7b2abdd848b95f2f578043fc23bd6f6fd24d - :left: :type: :number: 23 :text: |2 <span id="LC30" class="line"> <span class="vi">@cmd_output</span> <span class="o"><<</span> <span class="n">stdout</span><span class="p">.</span><span class="nf">read</span></span> :line_code: 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_23_30 + :position: !ruby/object:Gitlab::Diff::Position + attributes: + :old_path: files/ruby/popen.rb + :new_path: files/ruby/popen.rb + :old_line: 23 + :new_line: 30 + :base_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9 + :start_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9 + :head_sha: 570e7b2abdd848b95f2f578043fc23bd6f6fd24d :right: :type: :number: 30 :text: |2 <span id="LC30" class="line"> <span class="vi">@cmd_output</span> <span class="o"><<</span> <span class="n">stdout</span><span class="p">.</span><span class="nf">read</span></span> :line_code: 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_23_30 + :position: !ruby/object:Gitlab::Diff::Position + attributes: + :old_path: files/ruby/popen.rb + :new_path: files/ruby/popen.rb + :old_line: 23 + :new_line: 30 + :base_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9 + :start_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9 + :head_sha: 570e7b2abdd848b95f2f578043fc23bd6f6fd24d - :left: :type: :number: 24 :text: |2 <span id="LC31" class="line"> <span class="vi">@cmd_output</span> <span class="o"><<</span> <span class="n">stderr</span><span class="p">.</span><span class="nf">read</span></span> :line_code: 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_24_31 + :position: !ruby/object:Gitlab::Diff::Position + attributes: + :old_path: files/ruby/popen.rb + :new_path: files/ruby/popen.rb + :old_line: 24 + :new_line: 31 + :base_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9 + :start_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9 + :head_sha: 570e7b2abdd848b95f2f578043fc23bd6f6fd24d :right: :type: :number: 31 :text: |2 <span id="LC31" class="line"> <span class="vi">@cmd_output</span> <span class="o"><<</span> <span class="n">stderr</span><span class="p">.</span><span class="nf">read</span></span> :line_code: 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_24_31 + :position: !ruby/object:Gitlab::Diff::Position + attributes: + :old_path: files/ruby/popen.rb + :new_path: files/ruby/popen.rb + :old_line: 24 + :new_line: 31 + :base_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9 + :start_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9 + :head_sha: 570e7b2abdd848b95f2f578043fc23bd6f6fd24d diff --git a/spec/helpers/diff_helper_spec.rb b/spec/helpers/diff_helper_spec.rb index a71dd53e6ed..7ea9fa933f0 100644 --- a/spec/helpers/diff_helper_spec.rb +++ b/spec/helpers/diff_helper_spec.rb @@ -9,7 +9,7 @@ describe DiffHelper do let(:diffs) { commit.diffs } let(:diff) { diffs.first } let(:diff_refs) { [commit.parent, commit] } - let(:diff_file) { Gitlab::Diff::File.new(diff, diff_refs) } + let(:diff_file) { Gitlab::Diff::File.new(diff, diff_refs: diff_refs, repository: repository) } describe 'diff_view' do it 'returns a valid value when cookie is set' do diff --git a/spec/lib/banzai/filter/label_reference_filter_spec.rb b/spec/lib/banzai/filter/label_reference_filter_spec.rb index f1064a701d8..9e3d2f5825d 100644 --- a/spec/lib/banzai/filter/label_reference_filter_spec.rb +++ b/spec/lib/banzai/filter/label_reference_filter_spec.rb @@ -104,6 +104,31 @@ describe Banzai::Filter::LabelReferenceFilter, lib: true do end end + context 'String-based single-word references with special characters' do + let(:label) { create(:label, name: '?gfm&', project: project) } + let(:reference) { "#{Label.reference_prefix}#{label.name}" } + + it 'links to a valid reference' do + doc = reference_filter("See #{reference}") + + expect(doc.css('a').first.attr('href')).to eq urls. + namespace_project_issues_url(project.namespace, project, label_name: label.name) + expect(doc.text).to eq 'See ?gfm&' + end + + it 'links with adjacent text' do + doc = reference_filter("Label (#{reference}.)") + expect(doc.to_html).to match(%r(\(<a.+><span.+>\?gfm&</span></a>\.\))) + end + + it 'ignores invalid label names' do + act = "Label #{Label.reference_prefix}#{label.name.reverse}" + exp = "Label #{Label.reference_prefix}&mfg?" + + expect(reference_filter(act).to_html).to eq exp + end + end + context 'String-based multi-word references in quotes' do let(:label) { create(:label, name: 'gfm references', project: project) } let(:reference) { label.to_reference(format: :name) } @@ -128,6 +153,31 @@ describe Banzai::Filter::LabelReferenceFilter, lib: true do end end + context 'String-based multi-word references with special characters in quotes' do + let(:label) { create(:label, name: 'gfm & references?', project: project) } + let(:reference) { label.to_reference(format: :name) } + + it 'links to a valid reference' do + doc = reference_filter("See #{reference}") + + expect(doc.css('a').first.attr('href')).to eq urls. + namespace_project_issues_url(project.namespace, project, label_name: label.name) + expect(doc.text).to eq 'See gfm & references?' + end + + it 'links with adjacent text' do + doc = reference_filter("Label (#{reference}.)") + expect(doc.to_html).to match(%r(\(<a.+><span.+>gfm & references\?</span></a>\.\))) + end + + it 'ignores invalid label names' do + act = %(Label #{Label.reference_prefix}"#{label.name.reverse}") + exp = %(Label #{Label.reference_prefix}"?secnerefer & mfg\") + + expect(reference_filter(act).to_html).to eq exp + end + end + describe 'edge cases' do it 'gracefully handles non-references matching the pattern' do exp = act = '(format nil "~0f" 3.0) ; 3.0' diff --git a/spec/lib/gitlab/diff/file_spec.rb b/spec/lib/gitlab/diff/file_spec.rb index a0cbef6e6a4..1cb513d5229 100644 --- a/spec/lib/gitlab/diff/file_spec.rb +++ b/spec/lib/gitlab/diff/file_spec.rb @@ -6,7 +6,7 @@ describe Gitlab::Diff::File, lib: true do let(:project) { create(:project) } let(:commit) { project.commit(sample_commit.id) } let(:diff) { commit.diffs.first } - let(:diff_file) { Gitlab::Diff::File.new(diff, [commit.parent, commit]) } + let(:diff_file) { Gitlab::Diff::File.new(diff, diff_refs: commit.diff_refs, repository: project.repository) } describe :diff_lines do let(:diff_lines) { diff_file.diff_lines } diff --git a/spec/lib/gitlab/diff/highlight_spec.rb b/spec/lib/gitlab/diff/highlight_spec.rb index d19bf4ac84b..fb5d50a5c68 100644 --- a/spec/lib/gitlab/diff/highlight_spec.rb +++ b/spec/lib/gitlab/diff/highlight_spec.rb @@ -6,11 +6,11 @@ describe Gitlab::Diff::Highlight, lib: true do let(:project) { create(:project) } let(:commit) { project.commit(sample_commit.id) } let(:diff) { commit.diffs.first } - let(:diff_file) { Gitlab::Diff::File.new(diff, [commit.parent, commit]) } + let(:diff_file) { Gitlab::Diff::File.new(diff, diff_refs: commit.diff_refs, repository: project.repository) } describe '#highlight' do context "with a diff file" do - let(:subject) { Gitlab::Diff::Highlight.new(diff_file).highlight } + let(:subject) { Gitlab::Diff::Highlight.new(diff_file, repository: project.repository).highlight } it 'should return Gitlab::Diff::Line elements' do expect(subject.first).to be_an_instance_of(Gitlab::Diff::Line) @@ -41,7 +41,7 @@ describe Gitlab::Diff::Highlight, lib: true do end context "with diff lines" do - let(:subject) { Gitlab::Diff::Highlight.new(diff_file.diff_lines).highlight } + let(:subject) { Gitlab::Diff::Highlight.new(diff_file.diff_lines, repository: project.repository).highlight } it 'should return Gitlab::Diff::Line elements' do expect(subject.first).to be_an_instance_of(Gitlab::Diff::Line) diff --git a/spec/lib/gitlab/diff/line_mapper_spec.rb b/spec/lib/gitlab/diff/line_mapper_spec.rb new file mode 100644 index 00000000000..4e50e03bb7e --- /dev/null +++ b/spec/lib/gitlab/diff/line_mapper_spec.rb @@ -0,0 +1,137 @@ +require 'spec_helper' + +describe Gitlab::Diff::LineMapper, lib: true do + include RepoHelpers + + let(:project) { create(:project) } + let(:repository) { project.repository } + let(:commit) { project.commit(sample_commit.id) } + let(:diffs) { commit.diffs } + let(:diff) { diffs.first } + let(:diff_file) { Gitlab::Diff::File.new(diff, diff_refs: commit.diff_refs, repository: repository) } + subject { described_class.new(diff_file) } + + describe '#old_to_new' do + context "with a diff file" do + let(:mapping) do + { + 1 => 1, + 2 => 2, + 3 => 3, + 4 => 4, + 5 => 5, + 6 => 6, + 7 => 7, + 8 => 8, + 9 => nil, + # nil => 9, + 10 => 10, + 11 => 11, + 12 => 12, + 13 => nil, + 14 => nil, + # nil => 15, + # nil => 16, + # nil => 17, + # nil => 18, + # nil => 19, + # nil => 20, + 15 => 21, + 16 => 22, + 17 => 23, + 18 => 24, + 19 => 25, + 20 => 26, + 21 => 27, + # nil => 28, + 22 => 29, + 23 => 30, + 24 => 31, + 25 => 32, + 26 => 33, + 27 => 34, + 28 => 35, + 29 => 36, + 30 => 37 + } + end + + it 'returns the new line number for the old line number' do + mapping.each do |old_line, new_line| + expect(subject.old_to_new(old_line)).to eq(new_line) + end + end + end + + context "without a diff file" do + let(:diff_file) { nil } + + it "returns the same line number" do + expect(subject.old_to_new(100)).to eq(100) + end + end + end + + describe '#new_to_old' do + context "with a diff file" do + let(:mapping) do + { + 1 => 1, + 2 => 2, + 3 => 3, + 4 => 4, + 5 => 5, + 6 => 6, + 7 => 7, + 8 => 8, + # nil => 9, + 9 => nil, + 10 => 10, + 11 => 11, + 12 => 12, + # nil => 13, + # nil => 14, + 13 => nil, + 14 => nil, + 15 => nil, + 16 => nil, + 17 => nil, + 18 => nil, + 19 => nil, + 20 => nil, + 21 => 15, + 22 => 16, + 23 => 17, + 24 => 18, + 25 => 19, + 26 => 20, + 27 => 21, + 28 => nil, + 29 => 22, + 30 => 23, + 31 => 24, + 32 => 25, + 33 => 26, + 34 => 27, + 35 => 28, + 36 => 29, + 37 => 30 + } + end + + it 'returns the old line number for the new line number' do + mapping.each do |new_line, old_line| + expect(subject.new_to_old(new_line)).to eq(old_line) + end + end + end + + context "without a diff file" do + let(:diff_file) { nil } + + it "returns the same line number" do + expect(subject.new_to_old(100)).to eq(100) + end + end + end +end diff --git a/spec/lib/gitlab/diff/parallel_diff_spec.rb b/spec/lib/gitlab/diff/parallel_diff_spec.rb index 1c5bbc47120..5f76b70c6f5 100644 --- a/spec/lib/gitlab/diff/parallel_diff_spec.rb +++ b/spec/lib/gitlab/diff/parallel_diff_spec.rb @@ -8,8 +8,7 @@ describe Gitlab::Diff::ParallelDiff, lib: true do let(:commit) { project.commit(sample_commit.id) } let(:diffs) { commit.diffs } let(:diff) { diffs.first } - let(:diff_refs) { [commit.parent, commit] } - let(:diff_file) { Gitlab::Diff::File.new(diff, diff_refs) } + let(:diff_file) { Gitlab::Diff::File.new(diff, diff_refs: commit.diff_refs, repository: repository) } subject { described_class.new(diff_file) } let(:parallel_diff_result_array) { YAML.load_file("#{Rails.root}/spec/fixtures/parallel_diff_result.yml") } diff --git a/spec/lib/gitlab/diff/position_spec.rb b/spec/lib/gitlab/diff/position_spec.rb new file mode 100644 index 00000000000..cf28628cb96 --- /dev/null +++ b/spec/lib/gitlab/diff/position_spec.rb @@ -0,0 +1,341 @@ +require 'spec_helper' + +describe Gitlab::Diff::Position, lib: true do + include RepoHelpers + + let(:project) { create(:project) } + + describe "position for an added file" do + let(:commit) { project.commit("2ea1f3dec713d940208fb5ce4a38765ecb5d3f73") } + + subject do + described_class.new( + old_path: "files/images/wm.svg", + new_path: "files/images/wm.svg", + old_line: nil, + new_line: 5, + diff_refs: commit.diff_refs + ) + end + + describe "#diff_file" do + it "returns the correct diff file" do + diff_file = subject.diff_file(project.repository) + + expect(diff_file.new_file).to be true + expect(diff_file.new_path).to eq(subject.new_path) + expect(diff_file.diff_refs).to eq(subject.diff_refs) + end + end + + describe "#diff_line" do + it "returns the correct diff line" do + diff_line = subject.diff_line(project.repository) + + expect(diff_line.added?).to be true + expect(diff_line.new_line).to eq(subject.new_line) + expect(diff_line.text).to eq("+ <desc>Created with Sketch.</desc>") + end + end + + describe "#line_code" do + it "returns the correct line code" do + line_code = Gitlab::Diff::LineCode.generate(subject.file_path, subject.new_line, 0) + + expect(subject.line_code(project.repository)).to eq(line_code) + end + end + end + + describe "position for a changed file" do + let(:commit) { project.commit("570e7b2abdd848b95f2f578043fc23bd6f6fd24d") } + + describe "position for an added line" do + subject do + described_class.new( + old_path: "files/ruby/popen.rb", + new_path: "files/ruby/popen.rb", + old_line: nil, + new_line: 14, + diff_refs: commit.diff_refs + ) + end + + describe "#diff_file" do + it "returns the correct diff file" do + diff_file = subject.diff_file(project.repository) + + expect(diff_file.old_path).to eq(subject.old_path) + expect(diff_file.new_path).to eq(subject.new_path) + expect(diff_file.diff_refs).to eq(subject.diff_refs) + end + end + + describe "#diff_line" do + it "returns the correct diff line" do + diff_line = subject.diff_line(project.repository) + + expect(diff_line.added?).to be true + expect(diff_line.new_line).to eq(subject.new_line) + expect(diff_line.text).to eq("+ vars = {") + end + end + + describe "#line_code" do + it "returns the correct line code" do + line_code = Gitlab::Diff::LineCode.generate(subject.file_path, subject.new_line, 15) + + expect(subject.line_code(project.repository)).to eq(line_code) + end + end + end + + describe "position for an unchanged line" do + subject do + described_class.new( + old_path: "files/ruby/popen.rb", + new_path: "files/ruby/popen.rb", + old_line: 16, + new_line: 22, + diff_refs: commit.diff_refs + ) + end + + describe "#diff_file" do + it "returns the correct diff file" do + diff_file = subject.diff_file(project.repository) + + expect(diff_file.old_path).to eq(subject.old_path) + expect(diff_file.new_path).to eq(subject.new_path) + expect(diff_file.diff_refs).to eq(subject.diff_refs) + end + end + + describe "#diff_line" do + it "returns the correct diff line" do + diff_line = subject.diff_line(project.repository) + + expect(diff_line.unchanged?).to be true + expect(diff_line.old_line).to eq(subject.old_line) + expect(diff_line.new_line).to eq(subject.new_line) + expect(diff_line.text).to eq(" unless File.directory?(path)") + end + end + + describe "#line_code" do + it "returns the correct line code" do + line_code = Gitlab::Diff::LineCode.generate(subject.file_path, subject.new_line, subject.old_line) + + expect(subject.line_code(project.repository)).to eq(line_code) + end + end + end + + describe "position for a removed line" do + subject do + described_class.new( + old_path: "files/ruby/popen.rb", + new_path: "files/ruby/popen.rb", + old_line: 14, + new_line: nil, + diff_refs: commit.diff_refs + ) + end + + describe "#diff_file" do + it "returns the correct diff file" do + diff_file = subject.diff_file(project.repository) + + expect(diff_file.old_path).to eq(subject.old_path) + expect(diff_file.new_path).to eq(subject.new_path) + expect(diff_file.diff_refs).to eq(subject.diff_refs) + end + end + + describe "#diff_line" do + it "returns the correct diff line" do + diff_line = subject.diff_line(project.repository) + + expect(diff_line.removed?).to be true + expect(diff_line.old_line).to eq(subject.old_line) + expect(diff_line.text).to eq("- options = { chdir: path }") + end + end + + describe "#line_code" do + it "returns the correct line code" do + line_code = Gitlab::Diff::LineCode.generate(subject.file_path, 13, subject.old_line) + + expect(subject.line_code(project.repository)).to eq(line_code) + end + end + end + end + + describe "position for a renamed file" do + let(:commit) { project.commit("6907208d755b60ebeacb2e9dfea74c92c3449a1f") } + + describe "position for an added line" do + subject do + described_class.new( + old_path: "files/js/commit.js.coffee", + new_path: "files/js/commit.coffee", + old_line: nil, + new_line: 4, + diff_refs: commit.diff_refs + ) + end + + describe "#diff_file" do + it "returns the correct diff file" do + diff_file = subject.diff_file(project.repository) + + expect(diff_file.old_path).to eq(subject.old_path) + expect(diff_file.new_path).to eq(subject.new_path) + expect(diff_file.diff_refs).to eq(subject.diff_refs) + end + end + + describe "#diff_line" do + it "returns the correct diff line" do + diff_line = subject.diff_line(project.repository) + + expect(diff_line.added?).to be true + expect(diff_line.new_line).to eq(subject.new_line) + expect(diff_line.text).to eq("+ new CommitFile(@)") + end + end + + describe "#line_code" do + it "returns the correct line code" do + line_code = Gitlab::Diff::LineCode.generate(subject.file_path, subject.new_line, 5) + + expect(subject.line_code(project.repository)).to eq(line_code) + end + end + end + + describe "position for an unchanged line" do + subject do + described_class.new( + old_path: "files/js/commit.js.coffee", + new_path: "files/js/commit.coffee", + old_line: 3, + new_line: 3, + diff_refs: commit.diff_refs + ) + end + + describe "#diff_file" do + it "returns the correct diff file" do + diff_file = subject.diff_file(project.repository) + + expect(diff_file.old_path).to eq(subject.old_path) + expect(diff_file.new_path).to eq(subject.new_path) + expect(diff_file.diff_refs).to eq(subject.diff_refs) + end + end + + describe "#diff_line" do + it "returns the correct diff line" do + diff_line = subject.diff_line(project.repository) + + expect(diff_line.unchanged?).to be true + expect(diff_line.old_line).to eq(subject.old_line) + expect(diff_line.new_line).to eq(subject.new_line) + expect(diff_line.text).to eq(" $('.files .diff-file').each ->") + end + end + + describe "#line_code" do + it "returns the correct line code" do + line_code = Gitlab::Diff::LineCode.generate(subject.file_path, subject.new_line, subject.old_line) + + expect(subject.line_code(project.repository)).to eq(line_code) + end + end + end + + describe "position for a removed line" do + subject do + described_class.new( + old_path: "files/js/commit.js.coffee", + new_path: "files/js/commit.coffee", + old_line: 4, + new_line: nil, + diff_refs: commit.diff_refs + ) + end + + describe "#diff_file" do + it "returns the correct diff file" do + diff_file = subject.diff_file(project.repository) + + expect(diff_file.old_path).to eq(subject.old_path) + expect(diff_file.new_path).to eq(subject.new_path) + expect(diff_file.diff_refs).to eq(subject.diff_refs) + end + end + + describe "#diff_line" do + it "returns the correct diff line" do + diff_line = subject.diff_line(project.repository) + + expect(diff_line.removed?).to be true + expect(diff_line.old_line).to eq(subject.old_line) + expect(diff_line.text).to eq("- new CommitFile(this)") + end + end + + describe "#line_code" do + it "returns the correct line code" do + line_code = Gitlab::Diff::LineCode.generate(subject.file_path, 4, subject.old_line) + + expect(subject.line_code(project.repository)).to eq(line_code) + end + end + end + end + + describe "position for a deleted file" do + let(:commit) { project.commit("8634272bfad4cf321067c3e94d64d5a253f8321d") } + + subject do + described_class.new( + old_path: "LICENSE", + new_path: "LICENSE", + old_line: 3, + new_line: nil, + diff_refs: commit.diff_refs + ) + end + + describe "#diff_file" do + it "returns the correct diff file" do + diff_file = subject.diff_file(project.repository) + + expect(diff_file.deleted_file).to be true + expect(diff_file.old_path).to eq(subject.old_path) + expect(diff_file.diff_refs).to eq(subject.diff_refs) + end + end + + describe "#diff_line" do + it "returns the correct diff line" do + diff_line = subject.diff_line(project.repository) + + expect(diff_line.removed?).to be true + expect(diff_line.old_line).to eq(subject.old_line) + expect(diff_line.text).to eq("-Copyright (c) 2014 gitlabhq") + end + end + + describe "#line_code" do + it "returns the correct line code" do + line_code = Gitlab::Diff::LineCode.generate(subject.file_path, 0, subject.old_line) + + expect(subject.line_code(project.repository)).to eq(line_code) + end + end + end +end diff --git a/spec/lib/gitlab/diff/position_tracer_spec.rb b/spec/lib/gitlab/diff/position_tracer_spec.rb new file mode 100644 index 00000000000..08312e60f4a --- /dev/null +++ b/spec/lib/gitlab/diff/position_tracer_spec.rb @@ -0,0 +1,1758 @@ +require 'spec_helper' + +describe Gitlab::Diff::PositionTracer, lib: true do + # Douwe's diary New York City, 2016-06-28 + # -------------------------------------------------------------------------- + # + # Dear diary, + # + # Ideally, we would have a test for every single diff scenario that can + # occur and that the PositionTracer should correctly trace a position + # through, across the following variables: + # + # - Old diff file type: created, changed, renamed, deleted, unchanged (5) + # - Old diff line type: added, removed, unchanged (3) + # - New diff file type: created, changed, renamed, deleted, unchanged (5) + # - New diff line type: added, removed, unchanged (3) + # - Old-to-new diff line change: kept, moved, undone (3) + # + # This adds up to 5 * 3 * 5 * 3 * 3 = 675 different potential scenarios, + # and 675 different tests to cover them all. In reality, it would be fewer, + # since one cannot have a removed line in a created file diff, for example, + # but for the sake of this diary entry, let's be pessimistic. + # + # Writing these tests is a manual and time consuming process, as every test + # requires the manual construction or finding of a combination of diffs that + # create the exact diff scenario we are looking for, and can take between + # 1 and 10 minutes, depending on the farfetchedness of the scenario and + # complexity of creating it. + # + # This means that writing tests to cover all of these scenarios would end up + # taking between 11 and 112 hours in total, which I do not believe is the + # best use of my time. + # + # A better course of action would be to think of scenarios that are likely + # to occur, but also potentially tricky to trace correctly, and only cover + # those, with a few more obvious scenarios thrown in to cover our bases. + # + # Unfortunately, I only came to the above realization once I was about + # 1/5th of the way through the process of writing ALL THE SPECS, having + # already wasted about 3 hours trying to be thorough. + # + # I did find 2 bugs while writing those though, so that's good. + # + # In any case, all of this means that the tests below will be extremely + # (excessively, unjustifiably) thorough for scenarios where "the file was + # created in the old diff" and then drop off to comparitively lackluster + # testing of other scenarios. + # + # I did still try to cover most of the obvious and potentially tricky + # scenarios, though. + + include RepoHelpers + + let(:project) { create(:project) } + let(:current_user) { project.owner } + let(:repository) { project.repository } + let(:file_name) { "test-file" } + let(:new_file_name) { "#{file_name}-new" } + let(:second_file_name) { "#{file_name}-2" } + let(:branch_name) { "position-tracer-test" } + + let(:old_diff_refs) { raise NotImplementedError } + let(:new_diff_refs) { raise NotImplementedError } + let(:old_position) { raise NotImplementedError } + + let(:position_tracer) { described_class.new(repository: project.repository, old_diff_refs: old_diff_refs, new_diff_refs: new_diff_refs) } + subject { position_tracer.trace(old_position) } + + def diff_refs(base_commit, head_commit) + Gitlab::Diff::DiffRefs.new(base_sha: base_commit.id, head_sha: head_commit.id) + end + + def position(attrs = {}) + attrs.reverse_merge!( + diff_refs: old_diff_refs + ) + Gitlab::Diff::Position.new(attrs) + end + + def expect_new_position(attrs, new_position = subject) + if attrs.nil? + expect(new_position).to be_nil + else + expect(new_position).not_to be_nil + + expect(new_position.diff_refs).to eq(new_diff_refs) + + attrs.each do |attr, value| + expect(new_position.send(attr)).to eq(value) + end + end + end + + def create_branch(new_name, branch_name) + CreateBranchService.new(project, current_user).execute(new_name, branch_name) + end + + def create_file(branch_name, file_name, content) + Files::CreateService.new( + project, + current_user, + source_branch: branch_name, + target_branch: branch_name, + commit_message: "Create file", + file_path: file_name, + file_content: content + ).execute + project.commit(branch_name) + end + + def update_file(branch_name, file_name, content) + Files::UpdateService.new( + project, + current_user, + source_branch: branch_name, + target_branch: branch_name, + commit_message: "Update file", + file_path: file_name, + file_content: content + ).execute + project.commit(branch_name) + end + + def delete_file(branch_name, file_name) + Files::DeleteService.new( + project, + current_user, + source_branch: branch_name, + target_branch: branch_name, + commit_message: "Delete file", + file_path: file_name + ).execute + project.commit(branch_name) + end + + let(:initial_commit) do + create_branch(branch_name, "master")[:branch].name + project.commit(branch_name) + end + + describe "#trace" do + describe "diff scenarios" do + let(:create_file_commit) do + initial_commit + + create_file( + branch_name, + file_name, + <<-CONTENT.strip_heredoc + A + B + C + CONTENT + ) + end + + let(:create_second_file_commit) do + create_file_commit + + create_file( + branch_name, + second_file_name, + <<-CONTENT.strip_heredoc + D + E + CONTENT + ) + end + + let(:update_line_commit) do + create_second_file_commit + + update_file( + branch_name, + file_name, + <<-CONTENT.strip_heredoc + A + BB + C + CONTENT + ) + end + + let(:update_second_file_line_commit) do + update_line_commit + + update_file( + branch_name, + second_file_name, + <<-CONTENT.strip_heredoc + D + EE + CONTENT + ) + end + + let(:move_line_commit) do + update_second_file_line_commit + + update_file( + branch_name, + file_name, + <<-CONTENT.strip_heredoc + BB + A + C + CONTENT + ) + end + + let(:add_second_file_line_commit) do + move_line_commit + + update_file( + branch_name, + second_file_name, + <<-CONTENT.strip_heredoc + D + EE + F + CONTENT + ) + end + + let(:move_second_file_line_commit) do + add_second_file_line_commit + + update_file( + branch_name, + second_file_name, + <<-CONTENT.strip_heredoc + D + F + EE + CONTENT + ) + end + + let(:delete_line_commit) do + move_second_file_line_commit + + update_file( + branch_name, + file_name, + <<-CONTENT.strip_heredoc + BB + A + CONTENT + ) + end + + let(:delete_second_file_line_commit) do + delete_line_commit + + update_file( + branch_name, + second_file_name, + <<-CONTENT.strip_heredoc + D + F + CONTENT + ) + end + + let(:delete_file_commit) do + delete_second_file_line_commit + + delete_file(branch_name, file_name) + end + + let(:rename_file_commit) do + delete_file_commit + + create_file( + branch_name, + new_file_name, + <<-CONTENT.strip_heredoc + BB + A + CONTENT + ) + end + + let(:update_line_again_commit) do + rename_file_commit + + update_file( + branch_name, + new_file_name, + <<-CONTENT.strip_heredoc + BB + AA + CONTENT + ) + end + + let(:move_line_again_commit) do + update_line_again_commit + + update_file( + branch_name, + new_file_name, + <<-CONTENT.strip_heredoc + AA + BB + CONTENT + ) + end + + let(:delete_line_again_commit) do + move_line_again_commit + + update_file( + branch_name, + new_file_name, + <<-CONTENT.strip_heredoc + AA + CONTENT + ) + end + + context "when the file was created in the old diff" do + context "when the file is created in the new diff" do + context "when the position pointed at an added line in the old diff" do + context "when the file's content was unchanged between the old and the new diff" do + let(:old_diff_refs) { diff_refs(initial_commit, create_file_commit) } + let(:new_diff_refs) { diff_refs(initial_commit, create_second_file_commit) } + let(:old_position) { position(new_path: file_name, new_line: 2) } + + # old diff: + # 1 + A + # 2 + B + # 3 + C + # + # new diff: + # 1 + A + # 2 + B + # 3 + C + + it "returns the new position" do + expect_new_position( + new_path: old_position.new_path, + new_line: old_position.new_line + ) + end + end + + context "when the file's content was changed between the old and the new diff" do + context "when that line was unchanged between the old and the new diff" do + let(:old_diff_refs) { diff_refs(initial_commit, create_file_commit) } + let(:new_diff_refs) { diff_refs(initial_commit, update_line_commit) } + let(:old_position) { position(new_path: file_name, new_line: 1) } + + # old diff: + # 1 + A + # 2 + B + # 3 + C + # + # new diff: + # 1 + A + # 2 + BB + # 3 + C + + it "returns the new position" do + expect_new_position( + new_path: old_position.new_path, + new_line: old_position.new_line + ) + end + end + + context "when that line was moved between the old and the new diff" do + let(:old_diff_refs) { diff_refs(initial_commit, update_line_commit) } + let(:new_diff_refs) { diff_refs(initial_commit, move_line_commit) } + let(:old_position) { position(new_path: file_name, new_line: 2) } + + # old diff: + # 1 + A + # 2 + BB + # 3 + C + # + # new diff: + # 1 + BB + # 2 + A + # 3 + C + + it "returns the new position" do + expect_new_position( + new_path: old_position.new_path, + new_line: 1 + ) + end + end + + context "when that line was changed between the old and the new diff" do + let(:old_diff_refs) { diff_refs(initial_commit, create_file_commit) } + let(:new_diff_refs) { diff_refs(initial_commit, update_line_commit) } + let(:old_position) { position(new_path: file_name, new_line: 2) } + + # old diff: + # 1 + A + # 2 + B + # 3 + C + # + # new diff: + # 1 + A + # 2 + BB + # 3 + C + + it "returns nil" do + expect(subject).to be_nil + end + end + + context "when that line was deleted between the old and the new diff" do + let(:old_diff_refs) { diff_refs(initial_commit, update_line_commit) } + let(:new_diff_refs) { diff_refs(initial_commit, delete_line_commit) } + let(:old_position) { position(new_path: file_name, new_line: 3) } + + # old diff: + # 1 + A + # 2 + BB + # 3 + C + # + # new diff: + # 1 + A + # 2 + BB + + it "returns nil" do + expect(subject).to be_nil + end + end + end + end + end + + context "when the file is changed in the new diff" do + context "when the position pointed at an added line in the old diff" do + context "when the file's content was unchanged between the old and the new diff" do + let(:old_diff_refs) { diff_refs(initial_commit, update_line_commit) } + let(:new_diff_refs) { diff_refs(create_file_commit, update_line_commit) } + let(:old_position) { position(new_path: file_name, new_line: 1) } + + # old diff: + # 1 + A + # 2 + BB + # 3 + C + # + # new diff: + # 1 1 A + # 2 - B + # 2 + BB + # 3 3 C + + it "returns the new position" do + expect_new_position( + new_path: old_position.new_path, + new_line: old_position.new_line + ) + end + end + + context "when the file's content was changed between the old and the new diff" do + context "when that line was unchanged between the old and the new diff" do + let(:old_diff_refs) { diff_refs(initial_commit, update_line_commit) } + let(:new_diff_refs) { diff_refs(update_line_commit, move_line_commit) } + let(:old_position) { position(new_path: file_name, new_line: 3) } + + # old diff: + # 1 + A + # 2 + BB + # 3 + C + # + # new diff: + # 1 - A + # 2 1 BB + # 2 + A + # 3 3 C + + it "returns the new position" do + expect_new_position( + new_path: old_position.new_path, + new_line: old_position.new_line + ) + end + end + + context "when that line was moved between the old and the new diff" do + let(:old_diff_refs) { diff_refs(initial_commit, update_line_commit) } + let(:new_diff_refs) { diff_refs(update_line_commit, move_line_commit) } + let(:old_position) { position(new_path: file_name, new_line: 2) } + + # old diff: + # 1 + A + # 2 + BB + # 3 + C + # + # new diff: + # 1 - A + # 2 1 BB + # 2 + A + # 3 3 C + + it "returns the new position" do + expect_new_position( + new_path: old_position.new_path, + new_line: 1 + ) + end + end + + context "when that line was changed between the old and the new diff" do + let(:old_diff_refs) { diff_refs(initial_commit, create_file_commit) } + let(:new_diff_refs) { diff_refs(create_file_commit, update_line_commit) } + let(:old_position) { position(new_path: file_name, new_line: 2) } + + # old diff: + # 1 + A + # 2 + B + # 3 + C + # + # new diff: + # 1 1 A + # 2 - B + # 2 + BB + # 3 3 C + + it "returns nil" do + expect(subject).to be_nil + end + end + + context "when that line was deleted between the old and the new diff" do + let(:old_diff_refs) { diff_refs(initial_commit, move_line_commit) } + let(:new_diff_refs) { diff_refs(move_line_commit, delete_line_commit) } + let(:old_position) { position(new_path: file_name, new_line: 3) } + + # old diff: + # 1 + BB + # 2 + A + # 3 + C + # + # new diff: + # 1 1 BB + # 2 2 A + # 3 - C + + it "returns nil" do + expect(subject).to be_nil + end + end + end + end + end + + context "when the file is renamed in the new diff" do + context "when the position pointed at an added line in the old diff" do + context "when the file's content was unchanged between the old and the new diff" do + let(:old_diff_refs) { diff_refs(initial_commit, delete_line_commit) } + let(:new_diff_refs) { diff_refs(delete_line_commit, rename_file_commit) } + let(:old_position) { position(new_path: file_name, new_line: 2) } + + # old diff: + # 1 + BB + # 2 + A + # + # new diff: + # file_name -> new_file_name + # 1 1 BB + # 2 2 A + + it "returns the new position" do + expect_new_position( + old_path: file_name, + new_path: new_file_name, + old_line: old_position.new_line, + new_line: old_position.new_line + ) + end + end + + context "when the file's content was changed between the old and the new diff" do + context "when that line was unchanged between the old and the new diff" do + let(:old_diff_refs) { diff_refs(initial_commit, delete_line_commit) } + let(:new_diff_refs) { diff_refs(delete_line_commit, update_line_again_commit) } + let(:old_position) { position(new_path: file_name, new_line: 1) } + + # old diff: + # 1 + BB + # 2 + A + # + # new diff: + # file_name -> new_file_name + # 1 1 BB + # 2 - A + # 2 + AA + + it "returns the new position" do + expect_new_position( + old_path: file_name, + new_path: new_file_name, + old_line: old_position.new_line, + new_line: old_position.new_line + ) + end + end + + context "when that line was moved between the old and the new diff" do + let(:old_diff_refs) { diff_refs(initial_commit, delete_line_commit) } + let(:new_diff_refs) { diff_refs(delete_line_commit, move_line_again_commit) } + let(:old_position) { position(new_path: file_name, new_line: 1) } + + # old diff: + # 1 + BB + # 2 + A + # + # new diff: + # file_name -> new_file_name + # 1 + AA + # 1 2 BB + # 2 - A + + it "returns the new position" do + expect_new_position( + old_path: file_name, + new_path: new_file_name, + old_line: 1, + new_line: 2 + ) + end + end + + context "when that line was changed between the old and the new diff" do + let(:old_diff_refs) { diff_refs(initial_commit, delete_line_commit) } + let(:new_diff_refs) { diff_refs(delete_line_commit, update_line_again_commit) } + let(:old_position) { position(new_path: file_name, new_line: 2) } + + # old diff: + # 1 + BB + # 2 + A + # + # new diff: + # file_name -> new_file_name + # 1 1 BB + # 2 - A + # 2 + AA + + it "returns nil" do + expect(subject).to be_nil + end + end + + context "when that line was deleted between the old and the new diff" do + let(:old_diff_refs) { diff_refs(initial_commit, delete_line_commit) } + let(:new_diff_refs) { diff_refs(delete_line_commit, delete_line_again_commit) } + let(:old_position) { position(new_path: file_name, new_line: 1) } + + # old diff: + # 1 + BB + # 2 + A + # + # new diff: + # file_name -> new_file_name + # 1 - BB + # 2 - A + # 1 + AA + + it "returns nil" do + expect(subject).to be_nil + end + end + end + end + end + + context "when the file is deleted in the new diff" do + context "when the position pointed at an added line in the old diff" do + context "when the file's content was unchanged between the old and the new diff" do + let(:old_diff_refs) { diff_refs(initial_commit, delete_line_commit) } + let(:new_diff_refs) { diff_refs(delete_line_commit, delete_file_commit) } + let(:old_position) { position(new_path: file_name, new_line: 2) } + + # old diff: + # 1 + BB + # 2 + A + # + # new diff: + # 1 - BB + # 2 - A + + it "returns nil" do + expect(subject).to be_nil + end + end + + context "when the file's content was changed between the old and the new diff" do + context "when that line was unchanged between the old and the new diff" do + let(:old_diff_refs) { diff_refs(initial_commit, move_line_commit) } + let(:new_diff_refs) { diff_refs(delete_line_commit, delete_file_commit) } + let(:old_position) { position(new_path: file_name, new_line: 2) } + + # old diff: + # 1 + BB + # 2 + A + # 3 + C + # + # new diff: + # 1 - BB + # 2 - A + + it "returns nil" do + expect(subject).to be_nil + end + end + + context "when that line was moved between the old and the new diff" do + let(:old_diff_refs) { diff_refs(initial_commit, update_line_commit) } + let(:new_diff_refs) { diff_refs(move_line_commit, delete_file_commit) } + let(:old_position) { position(new_path: file_name, new_line: 2) } + + # old diff: + # 1 + A + # 2 + BB + # 3 + C + # + # new diff: + # 1 - BB + # 2 - A + # 3 - C + + it "returns nil" do + expect(subject).to be_nil + end + end + + context "when that line was changed between the old and the new diff" do + let(:old_diff_refs) { diff_refs(initial_commit, create_file_commit) } + let(:new_diff_refs) { diff_refs(update_line_commit, delete_file_commit) } + let(:old_position) { position(new_path: file_name, new_line: 2) } + + # old diff: + # 1 + A + # 2 + B + # 3 + C + # + # new diff: + # 1 - A + # 2 - BB + # 3 - C + + it "returns nil" do + expect(subject).to be_nil + end + end + + context "when that line was deleted between the old and the new diff" do + let(:old_diff_refs) { diff_refs(initial_commit, move_line_commit) } + let(:new_diff_refs) { diff_refs(delete_line_commit, delete_file_commit) } + let(:old_position) { position(new_path: file_name, new_line: 3) } + + # old diff: + # 1 + BB + # 2 + A + # 3 + C + # + # new diff: + # 1 - BB + # 2 - A + + it "returns nil" do + expect(subject).to be_nil + end + end + end + end + end + + context "when the file is unchanged in the new diff" do + context "when the position pointed at an added line in the old diff" do + context "when the file's content was unchanged between the old and the new diff" do + let(:old_diff_refs) { diff_refs(initial_commit, create_file_commit) } + let(:new_diff_refs) { diff_refs(create_file_commit, create_second_file_commit) } + let(:old_position) { position(new_path: file_name, new_line: 2) } + + # old diff: + # 1 + A + # 2 + B + # 3 + C + # + # new diff: + # 1 1 A + # 2 2 B + # 3 3 C + + it "returns nil" do + expect(subject).to be_nil + end + end + + context "when the file's content was changed between the old and the new diff" do + context "when that line was unchanged between the old and the new diff" do + let(:old_diff_refs) { diff_refs(initial_commit, create_file_commit) } + let(:new_diff_refs) { diff_refs(update_line_commit, update_second_file_line_commit) } + let(:old_position) { position(new_path: file_name, new_line: 1) } + + # old diff: + # 1 + A + # 2 + B + # 3 + C + # + # new diff: + # 1 1 A + # 2 2 BB + # 3 3 C + + it "returns nil" do + expect(subject).to be_nil + end + end + + context "when that line was moved between the old and the new diff" do + let(:old_diff_refs) { diff_refs(initial_commit, update_line_commit) } + let(:new_diff_refs) { diff_refs(move_line_commit, move_second_file_line_commit) } + let(:old_position) { position(new_path: file_name, new_line: 2) } + + # old diff: + # 1 + A + # 2 + BB + # 3 + C + # + # new diff: + # 1 1 BB + # 2 2 A + # 3 3 C + + it "returns nil" do + expect(subject).to be_nil + end + end + + context "when that line was changed between the old and the new diff" do + let(:old_diff_refs) { diff_refs(initial_commit, create_file_commit) } + let(:new_diff_refs) { diff_refs(update_line_commit, update_second_file_line_commit) } + let(:old_position) { position(new_path: file_name, new_line: 2) } + + # old diff: + # 1 + A + # 2 + B + # 3 + C + # + # new diff: + # 1 1 A + # 2 2 BB + # 3 3 C + + it "returns nil" do + expect(subject).to be_nil + end + end + + context "when that line was deleted between the old and the new diff" do + let(:old_diff_refs) { diff_refs(initial_commit, move_line_commit) } + let(:new_diff_refs) { diff_refs(delete_line_commit, delete_second_file_line_commit) } + let(:old_position) { position(new_path: file_name, new_line: 3) } + + # old diff: + # 1 + BB + # 2 + A + # 3 + C + # + # new diff: + # 1 1 BB + # 2 2 A + + it "returns nil" do + expect(subject).to be_nil + end + end + end + end + end + end + + context "when the file was changed in the old diff" do + context "when the file is created in the new diff" do + context "when the position pointed at an added line in the old diff" do + context "when the file's content was unchanged between the old and the new diff" do + let(:old_diff_refs) { diff_refs(create_file_commit, update_line_commit) } + let(:new_diff_refs) { diff_refs(initial_commit, update_line_commit) } + let(:old_position) { position(old_path: file_name, new_path: file_name, new_line: 2) } + + # old diff: + # 1 1 A + # 2 - B + # 2 + BB + # 3 3 C + # + # new diff: + # 1 + A + # 2 + BB + # 3 + C + + it "returns the new position" do + expect_new_position( + new_path: old_position.new_path, + old_line: nil, + new_line: old_position.new_line + ) + end + end + + context "when the file's content was changed between the old and the new diff" do + context "when that line was unchanged between the old and the new diff" do + let(:old_diff_refs) { diff_refs(create_file_commit, move_line_commit) } + let(:new_diff_refs) { diff_refs(initial_commit, move_line_commit) } + let(:old_position) { position(old_path: file_name, new_path: file_name, new_line: 1) } + + # old diff: + # 1 + BB + # 1 2 A + # 2 - B + # 3 3 C + # + # new diff: + # 1 + BB + # 2 + A + + it "returns the new position" do + expect_new_position( + new_path: old_position.new_path, + old_line: nil, + new_line: old_position.new_line + ) + end + end + + context "when that line was moved between the old and the new diff" do + let(:old_diff_refs) { diff_refs(create_file_commit, update_line_commit) } + let(:new_diff_refs) { diff_refs(initial_commit, move_line_commit) } + let(:old_position) { position(old_path: file_name, new_path: file_name, new_line: 2) } + + # old diff: + # 1 1 A + # 2 - B + # 2 + BB + # 3 3 C + # + # new diff: + # 1 + BB + # 2 + A + # 3 + C + + it "returns the new position" do + expect_new_position( + new_path: old_position.new_path, + old_line: nil, + new_line: 1 + ) + end + end + + context "when that line was changed or deleted between the old and the new diff" do + let(:old_diff_refs) { diff_refs(create_file_commit, move_line_commit) } + let(:new_diff_refs) { diff_refs(initial_commit, create_file_commit) } + let(:old_position) { position(old_path: file_name, new_path: file_name, new_line: 1) } + + # old diff: + # 1 + BB + # 1 2 A + # 2 - B + # 3 3 C + # + # new diff: + # 1 + A + # 2 + B + # 3 + C + + it "returns nil" do + expect(subject).to be_nil + end + end + end + end + + context "when the position pointed at a deleted line in the old diff" do + let(:old_diff_refs) { diff_refs(create_file_commit, update_line_commit) } + let(:new_diff_refs) { diff_refs(initial_commit, update_line_commit) } + let(:old_position) { position(old_path: file_name, new_path: file_name, old_line: 2) } + + # old diff: + # 1 1 A + # 2 - B + # 2 + BB + # 3 3 C + # + # new diff: + # 1 + A + # 2 + BB + # 3 + C + + it "returns nil" do + expect(subject).to be_nil + end + end + + context "when the position pointed at an unchanged line in the old diff" do + context "when the file's content was unchanged between the old and the new diff" do + let(:old_diff_refs) { diff_refs(create_file_commit, update_line_commit) } + let(:new_diff_refs) { diff_refs(initial_commit, update_line_commit) } + let(:old_position) { position(old_path: file_name, new_path: file_name, old_line: 1, new_line: 1) } + + # old diff: + # 1 1 A + # 2 - B + # 2 + BB + # 3 3 C + # + # new diff: + # 1 + A + # 2 + BB + # 3 + C + + it "returns the new position" do + expect_new_position( + new_path: old_position.new_path, + old_line: nil, + new_line: old_position.new_line + ) + end + end + + context "when the file's content was changed between the old and the new diff" do + context "when that line was unchanged between the old and the new diff" do + let(:old_diff_refs) { diff_refs(create_file_commit, move_line_commit) } + let(:new_diff_refs) { diff_refs(initial_commit, move_line_commit) } + let(:old_position) { position(old_path: file_name, new_path: file_name, old_line: 1, new_line: 2) } + + # old diff: + # 1 + BB + # 1 2 A + # 2 - B + # 3 3 C + # + # new diff: + # 1 + BB + # 2 + A + + it "returns the new position" do + expect_new_position( + new_path: old_position.new_path, + old_line: nil, + new_line: old_position.new_line + ) + end + end + + context "when that line was moved between the old and the new diff" do + let(:old_diff_refs) { diff_refs(move_line_commit, delete_line_commit) } + let(:new_diff_refs) { diff_refs(initial_commit, update_line_commit) } + let(:old_position) { position(old_path: file_name, new_path: file_name, old_line: 2, new_line: 2) } + + # old diff: + # 1 1 BB + # 2 2 A + # 3 - C + # + # new diff: + # 1 + A + # 2 + BB + # 3 + C + + it "returns the new position" do + expect_new_position( + new_path: old_position.new_path, + old_line: nil, + new_line: 1 + ) + end + end + + context "when that line was changed or deleted between the old and the new diff" do + let(:old_diff_refs) { diff_refs(create_file_commit, move_line_commit) } + let(:new_diff_refs) { diff_refs(initial_commit, delete_line_commit) } + let(:old_position) { position(old_path: file_name, new_path: file_name, old_line: 3, new_line: 3) } + + # old diff: + # 1 + BB + # 1 2 A + # 2 - B + # 3 3 C + # + # new diff: + # 1 + A + # 2 + B + + it "returns nil" do + expect(subject).to be_nil + end + end + end + end + end + + context "when the file is changed in the new diff" do + context "when the position pointed at an added line in the old diff" do + context "when the file's content was unchanged between the old and the new diff" do + let(:old_diff_refs) { diff_refs(create_file_commit, update_line_commit) } + let(:new_diff_refs) { diff_refs(create_file_commit, update_second_file_line_commit) } + let(:old_position) { position(old_path: file_name, new_path: file_name, new_line: 2) } + + # old diff: + # 1 1 A + # 2 - B + # 2 + BB + # 3 3 C + # + # new diff: + # 1 1 A + # 2 - B + # 2 + BB + # 3 3 C + + it "returns the new position" do + expect_new_position( + old_path: old_position.old_path, + new_path: old_position.new_path, + old_line: nil, + new_line: old_position.new_line + ) + end + end + + context "when the file's content was changed between the old and the new diff" do + context "when that line was unchanged between the old and the new diff" do + let(:old_diff_refs) { diff_refs(create_file_commit, move_line_commit) } + let(:new_diff_refs) { diff_refs(move_line_commit, delete_line_commit) } + let(:old_position) { position(old_path: file_name, new_path: file_name, new_line: 1) } + + # old diff: + # 1 + BB + # 1 2 A + # 2 - B + # 3 3 C + # + # new diff: + # 1 1 BB + # 2 2 A + # 3 - C + + it "returns the new position" do + expect_new_position( + old_path: old_position.old_path, + new_path: old_position.new_path, + old_line: 1, + new_line: 1 + ) + end + end + + context "when that line was moved between the old and the new diff" do + let(:old_diff_refs) { diff_refs(create_file_commit, update_line_commit) } + let(:new_diff_refs) { diff_refs(update_line_commit, move_line_commit) } + let(:old_position) { position(old_path: file_name, new_path: file_name, new_line: 2) } + + # old diff: + # 1 1 A + # 2 - B + # 2 + BB + # 3 3 C + # + # new diff: + # 1 - A + # 2 1 BB + # 2 + A + # 3 3 C + + it "returns the new position" do + expect_new_position( + old_path: old_position.old_path, + new_path: old_position.new_path, + old_line: 2, + new_line: 1 + ) + end + end + + context "when that line was changed or deleted between the old and the new diff" do + let(:old_diff_refs) { diff_refs(create_file_commit, move_line_commit) } + let(:new_diff_refs) { diff_refs(create_file_commit, update_line_commit) } + let(:old_position) { position(old_path: file_name, new_path: file_name, new_line: 1) } + + # old diff: + # 1 + BB + # 1 2 A + # 2 - B + # 3 3 C + # + # new diff: + # 1 1 A + # 2 - B + # 2 + BB + # 3 3 C + + it "returns nil" do + expect(subject).to be_nil + end + end + end + end + + context "when the position pointed at a deleted line in the old diff" do + context "when the file's content was unchanged between the old and the new diff" do + let(:old_diff_refs) { diff_refs(create_file_commit, update_line_commit) } + let(:new_diff_refs) { diff_refs(create_file_commit, update_second_file_line_commit) } + let(:old_position) { position(old_path: file_name, new_path: file_name, old_line: 2) } + + # old diff: + # 1 1 A + # 2 - B + # 2 + BB + # 3 3 C + # + # new diff: + # 1 1 A + # 2 - B + # 2 + BB + # 3 3 C + + it "returns the new position" do + expect_new_position( + old_path: old_position.old_path, + new_path: old_position.new_path, + old_line: old_position.old_line, + new_line: nil + ) + end + end + end + end + end + end + end + + describe "typical use scenarios" do + let(:second_branch_name) { "#{branch_name}-2" } + + def expect_positions(old_attrs, new_attrs) + old_positions = old_attrs.map do |old_attrs| + position(old_attrs) + end + + new_positions = old_positions.map do |old_position| + position_tracer.trace(old_position) + end + + new_positions.zip(new_attrs).each do |new_position, new_attrs| + expect_new_position(new_attrs, new_position) + end + end + + let(:create_file_commit) do + initial_commit + + create_file( + branch_name, + file_name, + <<-CONTENT.strip_heredoc + A + B + C + D + E + F + CONTENT + ) + end + + let(:second_create_file_commit) do + create_file_commit + + create_branch(second_branch_name, branch_name) + + update_file( + second_branch_name, + file_name, + <<-CONTENT.strip_heredoc + Z + Z + Z + A + B + C + D + E + F + CONTENT + ) + end + + let(:update_file_commit) do + second_create_file_commit + + update_file( + branch_name, + file_name, + <<-CONTENT.strip_heredoc + A + C + DD + E + F + G + CONTENT + ) + end + + let(:update_file_again_commit) do + update_file_commit + + update_file( + branch_name, + file_name, + <<-CONTENT.strip_heredoc + A + BB + C + D + E + FF + G + CONTENT + ) + end + + describe "simple push of new commit" do + let(:old_diff_refs) { diff_refs(create_file_commit, update_file_commit) } + let(:new_diff_refs) { diff_refs(create_file_commit, update_file_again_commit) } + + # old diff: + # 1 1 A + # 2 - B + # 3 2 C + # 4 - D + # 3 + DD + # 5 4 E + # 6 5 F + # 6 + G + # + # new diff: + # 1 1 A + # 2 - B + # 2 + BB + # 3 3 C + # 4 4 D + # 5 5 E + # 6 - F + # 6 + FF + # 7 + G + + it "returns the new positions" do + old_position_attrs = [ + { old_path: file_name, new_path: file_name, old_line: 1, new_line: 1 }, # A + { old_path: file_name, old_line: 2 }, # - B + { old_path: file_name, new_path: file_name, old_line: 3, new_line: 2 }, # C + { old_path: file_name, old_line: 4 }, # - D + { new_path: file_name, new_line: 3 }, # + DD + { old_path: file_name, new_path: file_name, old_line: 5, new_line: 4 }, # E + { old_path: file_name, new_path: file_name, old_line: 6, new_line: 5 }, # F + { new_path: file_name, new_line: 6 }, # + G + ] + + new_position_attrs = [ + { old_path: file_name, new_path: file_name, old_line: 1, new_line: 1 }, + { old_path: file_name, old_line: 2 }, + { old_path: file_name, new_path: file_name, old_line: 3, new_line: 3 }, + { old_path: file_name, old_line: 4, new_line: 4 }, + nil, + { old_path: file_name, new_path: file_name, old_line: 5, new_line: 5 }, + { old_path: file_name, old_line: 6 }, + { new_path: file_name, new_line: 7 }, + ] + + expect_positions(old_position_attrs, new_position_attrs) + end + end + + describe "force push to overwrite last commit" do + let(:second_create_file_commit) do + create_file_commit + + create_branch(second_branch_name, branch_name) + + update_file( + second_branch_name, + file_name, + <<-CONTENT.strip_heredoc + A + BB + C + D + E + FF + G + CONTENT + ) + end + + let(:old_diff_refs) { diff_refs(create_file_commit, update_file_commit) } + let(:new_diff_refs) { diff_refs(create_file_commit, second_create_file_commit) } + + # old diff: + # 1 1 A + # 2 - B + # 3 2 C + # 4 - D + # 3 + DD + # 5 4 E + # 6 5 F + # 6 + G + # + # new diff: + # 1 1 A + # 2 - B + # 2 + BB + # 3 3 C + # 4 4 D + # 5 5 E + # 6 - F + # 6 + FF + # 7 + G + + it "returns the new positions" do + old_position_attrs = [ + { old_path: file_name, new_path: file_name, old_line: 1, new_line: 1 }, # A + { old_path: file_name, old_line: 2 }, # - B + { old_path: file_name, new_path: file_name, old_line: 3, new_line: 2 }, # C + { old_path: file_name, old_line: 4 }, # - D + { new_path: file_name, new_line: 3 }, # + DD + { old_path: file_name, new_path: file_name, old_line: 5, new_line: 4 }, # E + { old_path: file_name, new_path: file_name, old_line: 6, new_line: 5 }, # F + { new_path: file_name, new_line: 6 }, # + G + ] + + new_position_attrs = [ + { old_path: file_name, new_path: file_name, old_line: 1, new_line: 1 }, + { old_path: file_name, old_line: 2 }, + { old_path: file_name, new_path: file_name, old_line: 3, new_line: 3 }, + { old_path: file_name, old_line: 4, new_line: 4 }, + nil, + { old_path: file_name, new_path: file_name, old_line: 5, new_line: 5 }, + { old_path: file_name, old_line: 6 }, + { new_path: file_name, new_line: 7 }, + ] + + expect_positions(old_position_attrs, new_position_attrs) + end + end + + describe "force push to delete last commit" do + let(:old_diff_refs) { diff_refs(create_file_commit, update_file_again_commit) } + let(:new_diff_refs) { diff_refs(create_file_commit, update_file_commit) } + + # old diff: + # 1 1 A + # 2 - B + # 2 + BB + # 3 3 C + # 4 4 D + # 5 5 E + # 6 - F + # 6 + FF + # 7 + G + # + # new diff: + # 1 1 A + # 2 - B + # 3 2 C + # 4 - D + # 3 + DD + # 5 4 E + # 6 5 F + # 6 + G + + it "returns the new positions" do + old_position_attrs = [ + { old_path: file_name, new_path: file_name, old_line: 1, new_line: 1 }, # A + { old_path: file_name, old_line: 2 }, # - B + { new_path: file_name, new_line: 2 }, # + BB + { old_path: file_name, new_path: file_name, old_line: 3, new_line: 3 }, # C + { old_path: file_name, new_path: file_name, old_line: 4, new_line: 4 }, # D + { old_path: file_name, new_path: file_name, old_line: 5, new_line: 5 }, # E + { old_path: file_name, old_line: 6 }, # - F + { new_path: file_name, new_line: 6 }, # + FF + { new_path: file_name, new_line: 7 }, # + G + ] + + new_position_attrs = [ + { old_path: file_name, new_path: file_name, old_line: 1, new_line: 1 }, + { old_path: file_name, old_line: 2 }, + nil, + { old_path: file_name, new_path: file_name, old_line: 3, new_line: 2 }, + { old_path: file_name, old_line: 4 }, + { old_path: file_name, new_path: file_name, old_line: 5, new_line: 4 }, + { old_path: file_name, new_path: file_name, old_line: 6, new_line: 5 }, + nil, + { new_path: file_name, new_line: 6 }, + ] + + expect_positions(old_position_attrs, new_position_attrs) + end + end + + describe "rebase on top of target branch" do + let(:second_update_file_commit) do + update_file_commit + + update_file( + second_branch_name, + file_name, + <<-CONTENT.strip_heredoc + Z + Z + Z + A + C + DD + E + F + G + CONTENT + ) + end + + let(:update_file_again_commit) do + second_update_file_commit + + update_file( + branch_name, + file_name, + <<-CONTENT.strip_heredoc + A + BB + C + D + E + FF + G + CONTENT + ) + end + + let(:overwrite_update_file_again_commit) do + update_file_again_commit + + update_file( + second_branch_name, + file_name, + <<-CONTENT.strip_heredoc + Z + Z + Z + A + BB + C + D + E + FF + G + CONTENT + ) + end + + let(:old_diff_refs) { diff_refs(create_file_commit, update_file_again_commit) } + let(:new_diff_refs) { diff_refs(create_file_commit, overwrite_update_file_again_commit) } + + # old diff: + # 1 1 A + # 2 - B + # 2 + BB + # 3 3 C + # 4 4 D + # 5 5 E + # 6 - F + # 6 + FF + # 7 + G + # + # new diff: + # 1 + Z + # 2 + Z + # 3 + Z + # 1 4 A + # 2 - B + # 5 + BB + # 3 6 C + # 4 7 D + # 5 8 E + # 6 - F + # 9 + FF + # 0 + G + + it "returns the new positions" do + old_position_attrs = [ + { old_path: file_name, new_path: file_name, old_line: 1, new_line: 1 }, # A + { old_path: file_name, old_line: 2 }, # - B + { new_path: file_name, new_line: 2 }, # + BB + { old_path: file_name, new_path: file_name, old_line: 3, new_line: 3 }, # C + { old_path: file_name, new_path: file_name, old_line: 4, new_line: 4 }, # D + { old_path: file_name, new_path: file_name, old_line: 5, new_line: 5 }, # E + { old_path: file_name, old_line: 6 }, # - F + { new_path: file_name, new_line: 6 }, # + FF + { new_path: file_name, new_line: 7 }, # + G + ] + + new_position_attrs = [ + { old_path: file_name, new_path: file_name, old_line: 1, new_line: 4 }, # A + { old_path: file_name, old_line: 2 }, # - B + { new_path: file_name, new_line: 5 }, # + BB + { old_path: file_name, new_path: file_name, old_line: 3, new_line: 6 }, # C + { old_path: file_name, new_path: file_name, old_line: 4, new_line: 7 }, # D + { old_path: file_name, new_path: file_name, old_line: 5, new_line: 8 }, # E + { old_path: file_name, old_line: 6 }, # - F + { new_path: file_name, new_line: 9 }, # + FF + { new_path: file_name, new_line: 10 }, # + G + ] + + expect_positions(old_position_attrs, new_position_attrs) + end + end + + describe "merge of target branch" do + let(:merge_commit) do + update_file_again_commit + + committer = repository.user_to_committer(current_user) + + options = { + message: "Merge branches", + author: committer, + committer: committer + } + + repository.merge(current_user, second_create_file_commit.sha, branch_name, options) + project.commit(branch_name) + end + + let(:old_diff_refs) { diff_refs(create_file_commit, update_file_again_commit) } + let(:new_diff_refs) { diff_refs(create_file_commit, merge_commit) } + + # old diff: + # 1 1 A + # 2 - B + # 2 + BB + # 3 3 C + # 4 4 D + # 5 5 E + # 6 - F + # 6 + FF + # 7 + G + # + # new diff: + # 1 + Z + # 2 + Z + # 3 + Z + # 1 4 A + # 2 - B + # 5 + BB + # 3 6 C + # 4 7 D + # 5 8 E + # 6 - F + # 9 + FF + # 0 + G + + it "returns the new positions" do + old_position_attrs = [ + { old_path: file_name, new_path: file_name, old_line: 1, new_line: 1 }, # A + { old_path: file_name, old_line: 2 }, # - B + { new_path: file_name, new_line: 2 }, # + BB + { old_path: file_name, new_path: file_name, old_line: 3, new_line: 3 }, # C + { old_path: file_name, new_path: file_name, old_line: 4, new_line: 4 }, # D + { old_path: file_name, new_path: file_name, old_line: 5, new_line: 5 }, # E + { old_path: file_name, old_line: 6 }, # - F + { new_path: file_name, new_line: 6 }, # + FF + { new_path: file_name, new_line: 7 }, # + G + ] + + new_position_attrs = [ + { old_path: file_name, new_path: file_name, old_line: 1, new_line: 4 }, # A + { old_path: file_name, old_line: 2 }, # - B + { new_path: file_name, new_line: 5 }, # + BB + { old_path: file_name, new_path: file_name, old_line: 3, new_line: 6 }, # C + { old_path: file_name, new_path: file_name, old_line: 4, new_line: 7 }, # D + { old_path: file_name, new_path: file_name, old_line: 5, new_line: 8 }, # E + { old_path: file_name, old_line: 6 }, # - F + { new_path: file_name, new_line: 9 }, # + FF + { new_path: file_name, new_line: 10 }, # + G + ] + + expect_positions(old_position_attrs, new_position_attrs) + end + end + + describe "changing target branch" do + let(:old_diff_refs) { diff_refs(create_file_commit, update_file_again_commit) } + let(:new_diff_refs) { diff_refs(update_file_commit, update_file_again_commit) } + + # old diff: + # 1 1 A + # 2 - B + # 2 + BB + # 3 3 C + # 4 4 D + # 5 5 E + # 6 - F + # 6 + FF + # 7 + G + # + # new diff: + # 1 1 A + # 2 + BB + # 2 3 C + # 3 - DD + # 4 + D + # 4 5 E + # 5 - F + # 6 + FF + # 7 G + + it "returns the new positions" do + old_position_attrs = [ + { old_path: file_name, new_path: file_name, old_line: 1, new_line: 1 }, # A + { old_path: file_name, old_line: 2 }, # - B + { new_path: file_name, new_line: 2 }, # + BB + { old_path: file_name, new_path: file_name, old_line: 3, new_line: 3 }, # C + { old_path: file_name, new_path: file_name, old_line: 4, new_line: 4 }, # D + { old_path: file_name, new_path: file_name, old_line: 5, new_line: 5 }, # E + { old_path: file_name, old_line: 6 }, # - F + { new_path: file_name, new_line: 6 }, # + FF + { new_path: file_name, new_line: 7 }, # + G + ] + + new_position_attrs = [ + { old_path: file_name, new_path: file_name, old_line: 1, new_line: 1 }, + nil, + { new_path: file_name, new_line: 2 }, + { old_path: file_name, new_path: file_name, old_line: 2, new_line: 3 }, + { new_path: file_name, new_line: 4 }, + { old_path: file_name, new_path: file_name, old_line: 4, new_line: 5 }, + { old_path: file_name, old_line: 5 }, + { new_path: file_name, new_line: 6 }, + { new_path: file_name, new_line: 7 }, + ] + + expect_positions(old_position_attrs, new_position_attrs) + end + end + end +end diff --git a/spec/lib/gitlab/github_import/branch_formatter_spec.rb b/spec/lib/gitlab/github_import/branch_formatter_spec.rb index 3cb634ba010..fc9d5204148 100644 --- a/spec/lib/gitlab/github_import/branch_formatter_spec.rb +++ b/spec/lib/gitlab/github_import/branch_formatter_spec.rb @@ -2,17 +2,18 @@ require 'spec_helper' describe Gitlab::GithubImport::BranchFormatter, lib: true do let(:project) { create(:project) } + let(:commit) { create(:commit, project: project) } let(:repo) { double } let(:raw) do { ref: 'feature', repo: repo, - sha: '2e5d3239642f9161dcbbc4b70a211a68e5e45e2b' + sha: commit.id } end describe '#exists?' do - it 'returns true when branch exists' do + it 'returns true when both branch, and commit exists' do branch = described_class.new(project, double(raw)) expect(branch.exists?).to eq true @@ -23,6 +24,12 @@ describe Gitlab::GithubImport::BranchFormatter, lib: true do expect(branch.exists?).to eq false end + + it 'returns false when commit does not exist' do + branch = described_class.new(project, double(raw.merge(sha: '2e5d3239642f9161dcbbc4b70a211a68e5e45e2b'))) + + expect(branch.exists?).to eq false + end end describe '#name' do @@ -33,7 +40,7 @@ describe Gitlab::GithubImport::BranchFormatter, lib: true do end it 'returns formatted ref when branch does not exist' do - branch = described_class.new(project, double(raw.merge(ref: 'removed-branch'))) + branch = described_class.new(project, double(raw.merge(ref: 'removed-branch', sha: '2e5d3239642f9161dcbbc4b70a211a68e5e45e2b'))) expect(branch.name).to eq 'removed-branch-2e5d3239' end @@ -51,18 +58,18 @@ describe Gitlab::GithubImport::BranchFormatter, lib: true do it 'returns raw sha' do branch = described_class.new(project, double(raw)) - expect(branch.sha).to eq '2e5d3239642f9161dcbbc4b70a211a68e5e45e2b' + expect(branch.sha).to eq commit.id end end describe '#valid?' do - it 'returns true when repository exists' do + it 'returns true when raw repo is present' do branch = described_class.new(project, double(raw)) expect(branch.valid?).to eq true end - it 'returns false when repository does not exist' do + it 'returns false when raw repo is blank' do branch = described_class.new(project, double(raw.merge(repo: nil))) expect(branch.valid?).to eq false diff --git a/spec/lib/gitlab/github_import/pull_request_formatter_spec.rb b/spec/lib/gitlab/github_import/pull_request_formatter_spec.rb index 120f59e6e71..79931ecd134 100644 --- a/spec/lib/gitlab/github_import/pull_request_formatter_spec.rb +++ b/spec/lib/gitlab/github_import/pull_request_formatter_spec.rb @@ -2,11 +2,13 @@ require 'spec_helper' describe Gitlab::GithubImport::PullRequestFormatter, lib: true do let(:project) { create(:project) } + let(:source_sha) { create(:commit, project: project).id } + let(:target_sha) { create(:commit, project: project, git_commit: RepoHelpers.another_sample_commit).id } let(:repository) { double(id: 1, fork: false) } let(:source_repo) { repository } - let(:source_branch) { double(ref: 'feature', repo: source_repo, sha: '2e5d3239642f9161dcbbc4b70a211a68e5e45e2b') } + let(:source_branch) { double(ref: 'feature', repo: source_repo, sha: source_sha) } let(:target_repo) { repository } - let(:target_branch) { double(ref: 'master', repo: target_repo, sha: '8ffb3c15a5475e59ae909384297fede4badcb4c7') } + let(:target_branch) { double(ref: 'master', repo: target_repo, sha: target_sha) } let(:octocat) { double(id: 123456, login: 'octocat') } let(:created_at) { DateTime.strptime('2011-01-26T19:01:12Z') } let(:updated_at) { DateTime.strptime('2011-01-27T19:01:12Z') } @@ -41,10 +43,10 @@ describe Gitlab::GithubImport::PullRequestFormatter, lib: true do description: "*Created by: octocat*\n\nPlease pull these awesome changes", source_project: project, source_branch: 'feature', - head_source_sha: '2e5d3239642f9161dcbbc4b70a211a68e5e45e2b', + source_branch_sha: source_sha, target_project: project, target_branch: 'master', - base_target_sha: '8ffb3c15a5475e59ae909384297fede4badcb4c7', + target_branch_sha: target_sha, state: 'opened', milestone: nil, author_id: project.creator_id, @@ -68,10 +70,10 @@ describe Gitlab::GithubImport::PullRequestFormatter, lib: true do description: "*Created by: octocat*\n\nPlease pull these awesome changes", source_project: project, source_branch: 'feature', - head_source_sha: '2e5d3239642f9161dcbbc4b70a211a68e5e45e2b', + source_branch_sha: source_sha, target_project: project, target_branch: 'master', - base_target_sha: '8ffb3c15a5475e59ae909384297fede4badcb4c7', + target_branch_sha: target_sha, state: 'closed', milestone: nil, author_id: project.creator_id, @@ -95,10 +97,10 @@ describe Gitlab::GithubImport::PullRequestFormatter, lib: true do description: "*Created by: octocat*\n\nPlease pull these awesome changes", source_project: project, source_branch: 'feature', - head_source_sha: '2e5d3239642f9161dcbbc4b70a211a68e5e45e2b', + source_branch_sha: source_sha, target_project: project, target_branch: 'master', - base_target_sha: '8ffb3c15a5475e59ae909384297fede4badcb4c7', + target_branch_sha: target_sha, state: 'merged', milestone: nil, author_id: project.creator_id, diff --git a/spec/lib/gitlab/import_export/project_tree_saver_spec.rb b/spec/lib/gitlab/import_export/project_tree_saver_spec.rb index a75eaa4d51f..1424de9e60b 100644 --- a/spec/lib/gitlab/import_export/project_tree_saver_spec.rb +++ b/spec/lib/gitlab/import_export/project_tree_saver_spec.rb @@ -125,7 +125,7 @@ describe Gitlab::ImportExport::ProjectTreeSaver, services: true do ci_pipeline = create(:ci_pipeline, project: project, - sha: merge_request.last_commit.id, + sha: merge_request.diff_head_sha, ref: merge_request.source_branch, statuses: [commit_status]) diff --git a/spec/lib/gitlab/note_data_builder_spec.rb b/spec/lib/gitlab/note_data_builder_spec.rb index e848d88182f..3d6bcdfd873 100644 --- a/spec/lib/gitlab/note_data_builder_spec.rb +++ b/spec/lib/gitlab/note_data_builder_spec.rb @@ -27,7 +27,7 @@ describe 'Gitlab::NoteDataBuilder', lib: true do end describe 'When asking for a note on commit diff' do - let(:note) { create(:note_on_commit_diff, project: project) } + let(:note) { create(:diff_note_on_commit, project: project) } it 'returns the note and commit-specific data' do expect(data).to have_key(:commit) @@ -90,7 +90,7 @@ describe 'Gitlab::NoteDataBuilder', lib: true do end let(:note) do - create(:note_on_merge_request_diff, noteable: merge_request, + create(:diff_note_on_merge_request, noteable: merge_request, project: project) end diff --git a/spec/lib/gitlab/url_builder_spec.rb b/spec/lib/gitlab/url_builder_spec.rb index bf11472407a..a826b24419a 100644 --- a/spec/lib/gitlab/url_builder_spec.rb +++ b/spec/lib/gitlab/url_builder_spec.rb @@ -43,9 +43,9 @@ describe Gitlab::UrlBuilder, lib: true do end end - context 'on a CommitDiff' do + context 'on a Commit Diff' do it 'returns a proper URL' do - note = build_stubbed(:note_on_commit_diff) + note = build_stubbed(:diff_note_on_commit) url = described_class.build(note) @@ -75,10 +75,10 @@ describe Gitlab::UrlBuilder, lib: true do end end - context 'on a MergeRequestDiff' do + context 'on a MergeRequest Diff' do it 'returns a proper URL' do merge_request = create(:merge_request, iid: 42) - note = build_stubbed(:note_on_merge_request_diff, noteable: merge_request) + note = build_stubbed(:diff_note_on_merge_request, noteable: merge_request) url = described_class.build(note) diff --git a/spec/mailers/notify_spec.rb b/spec/mailers/notify_spec.rb index aa382f930d7..0a9b10bebea 100644 --- a/spec/mailers/notify_spec.rb +++ b/spec/mailers/notify_spec.rb @@ -948,7 +948,7 @@ describe Notify do let(:commits) { Commit.decorate(compare.commits, nil) } let(:diff_path) { namespace_project_compare_path(project.namespace, project, from: Commit.new(compare.base, project), to: Commit.new(compare.head, project)) } let(:send_from_committer_email) { false } - let(:diff_refs) { [project.merge_base_commit(sample_image_commit.id, sample_commit.id), project.commit(sample_commit.id)] } + let(:diff_refs) { Gitlab::Diff::DiffRefs.new(base_sha: project.merge_base_commit(sample_image_commit.id, sample_commit.id).id, head_sha: sample_commit.id) } subject { Notify.repository_push_email(project.id, author_id: user.id, ref: 'refs/heads/master', action: :push, compare: compare, reverse_compare: false, diff_refs: diff_refs, send_from_committer_email: send_from_committer_email) } @@ -1049,7 +1049,7 @@ describe Notify do let(:compare) { Gitlab::Git::Compare.new(project.repository.raw_repository, sample_commit.parent_id, sample_commit.id) } let(:commits) { Commit.decorate(compare.commits, nil) } let(:diff_path) { namespace_project_commit_path(project.namespace, project, commits.first) } - let(:diff_refs) { [project.merge_base_commit(sample_commit.parent_id, sample_commit.id), project.commit(sample_commit.id)] } + let(:diff_refs) { Gitlab::Diff::DiffRefs.new(base_sha: project.merge_base_commit(sample_image_commit.id, sample_commit.id).id, head_sha: sample_commit.id) } subject { Notify.repository_push_email(project.id, author_id: user.id, ref: 'refs/heads/master', action: :push, compare: compare, diff_refs: diff_refs) } diff --git a/spec/models/diff_note_spec.rb b/spec/models/diff_note_spec.rb new file mode 100644 index 00000000000..af8e890ca95 --- /dev/null +++ b/spec/models/diff_note_spec.rb @@ -0,0 +1,191 @@ +require 'spec_helper' + +describe DiffNote, models: true do + include RepoHelpers + + let(:project) { create(:project) } + let(:merge_request) { create(:merge_request, source_project: project) } + let(:commit) { project.commit(sample_commit.id) } + + let(:path) { "files/ruby/popen.rb" } + + let!(:position) do + Gitlab::Diff::Position.new( + old_path: path, + new_path: path, + old_line: nil, + new_line: 14, + diff_refs: merge_request.diff_refs + ) + end + + let!(:new_position) do + Gitlab::Diff::Position.new( + old_path: path, + new_path: path, + old_line: 16, + new_line: 22, + diff_refs: merge_request.diff_refs + ) + end + + subject { create(:diff_note_on_merge_request, project: project, position: position, noteable: merge_request) } + + describe "#position=" do + context "when provided a string" do + it "sets the position" do + subject.position = new_position.to_json + + expect(subject.position).to eq(new_position) + end + end + + context "when provided a hash" do + it "sets the position" do + subject.position = new_position.to_h + + expect(subject.position).to eq(new_position) + end + end + + context "when provided a position object" do + it "sets the position" do + subject.position = new_position + + expect(subject.position).to eq(new_position) + end + end + end + + describe "#diff_file" do + it "returns the correct diff file" do + diff_file = subject.diff_file + + expect(diff_file.old_path).to eq(position.old_path) + expect(diff_file.new_path).to eq(position.new_path) + expect(diff_file.diff_refs).to eq(position.diff_refs) + end + end + + describe "#diff_line" do + it "returns the correct diff line" do + diff_line = subject.diff_line + + expect(diff_line.added?).to be true + expect(diff_line.new_line).to eq(position.new_line) + expect(diff_line.text).to eq("+ vars = {") + end + end + + describe "#line_code" do + it "returns the correct line code" do + line_code = Gitlab::Diff::LineCode.generate(position.file_path, position.new_line, 15) + + expect(subject.line_code).to eq(line_code) + end + end + + describe "#for_line?" do + context "when provided the correct diff line" do + it "returns true" do + expect(subject.for_line?(subject.diff_line)).to be true + end + end + + context "when provided a different diff line" do + it "returns false" do + some_line = subject.diff_file.diff_lines.first + + expect(subject.for_line?(some_line)).to be false + end + end + end + + describe "#active?" do + context "when noteable is a commit" do + subject { create(:diff_note_on_commit, project: project, position: position) } + + it "returns true" do + expect(subject.active?).to be true + end + end + + context "when noteable is a merge request" do + context "when the merge request's diff refs match that of the diff note" do + it "returns true" do + expect(subject.active?).to be true + end + end + + context "when the merge request's diff refs don't match that of the diff note" do + before do + allow(subject.noteable).to receive(:diff_refs).and_return(commit.diff_refs) + end + + it "returns false" do + expect(subject.active?).to be false + end + end + end + end + + describe "creation" do + describe "updating of position" do + context "when noteable is a commit" do + let(:diff_note) { create(:diff_note_on_commit, project: project, position: position) } + + it "doesn't use the DiffPositionUpdateService" do + expect(Notes::DiffPositionUpdateService).not_to receive(:new) + + diff_note + end + + it "doesn't update the position" do + diff_note + + expect(diff_note.original_position).to eq(position) + expect(diff_note.position).to eq(position) + end + end + + context "when noteable is a merge request" do + let(:diff_note) { create(:diff_note_on_merge_request, project: project, position: position, noteable: merge_request) } + + context "when the note is active" do + it "doesn't use the DiffPositionUpdateService" do + expect(Notes::DiffPositionUpdateService).not_to receive(:new) + + diff_note + end + + it "doesn't update the position" do + diff_note + + expect(diff_note.original_position).to eq(position) + expect(diff_note.position).to eq(position) + end + end + + context "when the note is outdated" do + before do + allow(merge_request).to receive(:diff_refs).and_return(commit.diff_refs) + end + + it "uses the DiffPositionUpdateService" do + service = instance_double("Notes::DiffPositionUpdateService") + expect(Notes::DiffPositionUpdateService).to receive(:new).with( + project, + nil, + old_diff_refs: position.diff_refs, + new_diff_refs: commit.diff_refs, + paths: [path] + ).and_return(service) + expect(service).to receive(:execute) + + diff_note + end + end + end + end + end +end diff --git a/spec/models/event_spec.rb b/spec/models/event_spec.rb index 166a1dc4ddb..b5d0d79e14e 100644 --- a/spec/models/event_spec.rb +++ b/spec/models/event_spec.rb @@ -46,6 +46,22 @@ describe Event, models: true do it { expect(@event.author).to eq(@user) } end + describe '#note?' do + subject { Event.new(project: target.project, target: target) } + + context 'issue note event' do + let(:target) { create(:note_on_issue) } + + it { is_expected.to be_note } + end + + context 'merge request diff note event' do + let(:target) { create(:legacy_diff_note_on_merge_request) } + + it { is_expected.to be_note } + end + end + describe '#visible_to_user?' do let(:project) { create(:empty_project, :public) } let(:non_member) { create(:user) } @@ -89,7 +105,7 @@ describe Event, models: true do end end - context 'note event' do + context 'issue note event' do context 'on non confidential issues' do let(:target) { note_on_issue } @@ -112,6 +128,20 @@ describe Event, models: true do it { expect(event.visible_to_user?(admin)).to eq true } end end + + context 'merge request diff note event' do + let(:project) { create(:project, :public) } + let(:merge_request) { create(:merge_request, source_project: project, author: author, assignee: assignee) } + let(:note_on_merge_request) { create(:legacy_diff_note_on_merge_request, noteable: merge_request, project: project) } + let(:target) { note_on_merge_request } + + it { expect(event.visible_to_user?(non_member)).to eq true } + it { expect(event.visible_to_user?(author)).to eq true } + it { expect(event.visible_to_user?(assignee)).to eq true } + it { expect(event.visible_to_user?(member)).to eq true } + it { expect(event.visible_to_user?(guest)).to eq true } + it { expect(event.visible_to_user?(admin)).to eq true } + end end describe '.limit_recent' do diff --git a/spec/models/label_spec.rb b/spec/models/label_spec.rb index dad2628651b..f37f44a608e 100644 --- a/spec/models/label_spec.rb +++ b/spec/models/label_spec.rb @@ -32,21 +32,20 @@ describe Label, models: true do it 'should validate title' do expect(label).not_to allow_value('G,ITLAB').for(:title) - expect(label).not_to allow_value('G?ITLAB').for(:title) - expect(label).not_to allow_value('G&ITLAB').for(:title) expect(label).not_to allow_value('').for(:title) expect(label).to allow_value('GITLAB').for(:title) expect(label).to allow_value('gitlab').for(:title) + expect(label).to allow_value('G?ITLAB').for(:title) + expect(label).to allow_value('G&ITLAB').for(:title) expect(label).to allow_value("customer's request").for(:title) end end - describe "#title" do - let(:label) { create(:label, title: "<b>test</b>") } - - it "sanitizes title" do - expect(label.title).to eq("test") + describe '#title' do + it 'sanitizes title' do + label = described_class.new(title: '<b>foo & bar?</b>') + expect(label.title).to eq('foo & bar?') end end diff --git a/spec/models/legacy_diff_note_spec.rb b/spec/models/legacy_diff_note_spec.rb index b2d06853886..d64d89edbd3 100644 --- a/spec/models/legacy_diff_note_spec.rb +++ b/spec/models/legacy_diff_note_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' describe LegacyDiffNote, models: true do describe "Commit diff line notes" do - let!(:note) { create(:note_on_commit_diff, note: "+1 from me") } + let!(:note) { create(:legacy_diff_note_on_commit, note: "+1 from me") } let!(:commit) { note.noteable } it "should save a valid note" do @@ -17,7 +17,7 @@ describe LegacyDiffNote, models: true do describe '#active?' do it 'is always true when the note has no associated diff' do - note = build(:note_on_merge_request_diff) + note = build(:legacy_diff_note_on_merge_request) expect(note).to receive(:diff).and_return(nil) @@ -25,7 +25,7 @@ describe LegacyDiffNote, models: true do end it 'is never true when the note has no noteable associated' do - note = build(:note_on_merge_request_diff) + note = build(:legacy_diff_note_on_merge_request) expect(note).to receive(:diff).and_return(double) expect(note).to receive(:noteable).and_return(nil) @@ -34,7 +34,7 @@ describe LegacyDiffNote, models: true do end it 'returns the memoized value if defined' do - note = build(:note_on_merge_request_diff) + note = build(:legacy_diff_note_on_merge_request) note.instance_variable_set(:@active, 'foo') expect(note).not_to receive(:find_noteable_diff) @@ -45,7 +45,7 @@ describe LegacyDiffNote, models: true do context 'for a merge request noteable' do it 'is false when noteable has no matching diff' do merge = build_stubbed(:merge_request, :simple) - note = build(:note_on_merge_request_diff, noteable: merge) + note = build(:legacy_diff_note_on_merge_request, noteable: merge) allow(note).to receive(:diff).and_return(double) expect(note).to receive(:find_noteable_diff).and_return(nil) @@ -63,9 +63,9 @@ describe LegacyDiffNote, models: true do code = Gitlab::Diff::LineCode.generate(diff.new_path, line.new_pos, line.old_pos) # We're persisting in order to trigger the set_diff callback - note = create(:note_on_merge_request_diff, noteable: merge, - line_code: code, - project: merge.source_project) + note = create(:legacy_diff_note_on_merge_request, noteable: merge, + line_code: code, + project: merge.source_project) # Make sure we don't get a false positive from a guard clause expect(note).to receive(:find_noteable_diff).and_call_original diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb index ceb4d64698f..a4b6ff8f8ad 100644 --- a/spec/models/merge_request_spec.rb +++ b/spec/models/merge_request_spec.rb @@ -1,6 +1,8 @@ require 'spec_helper' describe MergeRequest, models: true do + include RepoHelpers + subject { create(:merge_request) } describe 'associations' do @@ -62,7 +64,7 @@ describe MergeRequest, models: true do end end - describe '#target_sha' do + describe '#target_branch_sha' do context 'when the target branch does not exist anymore' do let(:project) { create(:project) } @@ -73,32 +75,32 @@ describe MergeRequest, models: true do end it 'returns nil' do - expect(subject.target_sha).to be_nil + expect(subject.target_branch_sha).to be_nil end end end - describe '#source_sha' do + describe '#source_branch_sha' do let(:last_branch_commit) { subject.source_project.repository.commit(subject.source_branch) } context 'with diffs' do subject { create(:merge_request, :with_diffs) } it 'returns the sha of the source branch last commit' do - expect(subject.source_sha).to eq(last_branch_commit.sha) + expect(subject.source_branch_sha).to eq(last_branch_commit.sha) end end context 'without diffs' do subject { create(:merge_request, :without_diffs) } it 'returns the sha of the source branch last commit' do - expect(subject.source_sha).to eq(last_branch_commit.sha) + expect(subject.source_branch_sha).to eq(last_branch_commit.sha) end end context 'when the merge request is being created' do subject { build(:merge_request, source_branch: nil, compare_commits: []) } it 'returns nil' do - expect(subject.source_sha).to be_nil + expect(subject.source_branch_sha).to be_nil end end end @@ -252,12 +254,14 @@ describe MergeRequest, models: true do end it "can be removed if the last commit is the head of the source branch" do - allow(subject.source_project).to receive(:commit).and_return(subject.last_commit) + allow(subject.source_project).to receive(:commit).and_return(subject.diff_head_commit) expect(subject.can_remove_source_branch?(user)).to be_truthy end it "cannot be removed if the last commit is not also the head of the source branch" do + subject.source_branch = "lfs" + expect(subject.can_remove_source_branch?(user)).to be_falsey end end @@ -363,7 +367,7 @@ describe MergeRequest, models: true do and_return(2) subject.diverged_commits_count - allow(subject).to receive(:source_sha).and_return('123abc') + allow(subject).to receive(:source_branch_sha).and_return('123abc') subject.diverged_commits_count end @@ -373,7 +377,7 @@ describe MergeRequest, models: true do and_return(2) subject.diverged_commits_count - allow(subject).to receive(:target_sha).and_return('123abc') + allow(subject).to receive(:target_branch_sha).and_return('123abc') subject.diverged_commits_count end end @@ -392,11 +396,10 @@ describe MergeRequest, models: true do describe '#pipeline' do describe 'when the source project exists' do - it 'returns the latest commit' do - commit = double(:commit, id: '123abc') + it 'returns the latest pipeline' do pipeline = double(:ci_pipeline, ref: 'master') - allow(subject).to receive(:last_commit).and_return(commit) + allow(subject).to receive(:diff_head_sha).and_return('123abc') expect(subject.source_project).to receive(:pipeline). with('123abc', 'master'). @@ -464,7 +467,7 @@ describe MergeRequest, models: true do context 'when it is not broken and has no conflicts' do it 'is marked as mergeable' do allow(subject).to receive(:broken?) { false } - allow(project.repository).to receive(:can_be_merged?) { true } + allow(project.repository).to receive(:can_be_merged?).and_return(true) expect { subject.check_if_can_be_merged }.to change { subject.merge_status }.to('can_be_merged') end @@ -481,7 +484,7 @@ describe MergeRequest, models: true do context 'when it has conflicts' do before do allow(subject).to receive(:broken?) { false } - allow(project.repository).to receive(:can_be_merged?) { false } + allow(project.repository).to receive(:can_be_merged?).and_return(false) end it 'becomes unmergeable' do @@ -608,4 +611,42 @@ describe MergeRequest, models: true do end end end + + describe "#reload_diff" do + let(:note) { create(:diff_note_on_merge_request, project: subject.project, noteable: subject) } + + let(:commit) { subject.project.commit(sample_commit.id) } + + it "reloads the diff content" do + expect(subject.merge_request_diff).to receive(:reload_content) + + subject.reload_diff + end + + it "updates diff note positions" do + old_diff_refs = subject.diff_refs + + merge_request_diff = subject.merge_request_diff + + # Update merge_request_diff so that #diff_refs will return commit.diff_refs + allow(merge_request_diff).to receive(:reload_content) do + merge_request_diff.base_commit_sha = commit.parent_id + merge_request_diff.start_commit_sha = commit.parent_id + merge_request_diff.head_commit_sha = commit.sha + end + + expect(Notes::DiffPositionUpdateService).to receive(:new).with( + subject.project, + nil, + old_diff_refs: old_diff_refs, + new_diff_refs: commit.diff_refs, + paths: note.position.paths + ).and_call_original + expect_any_instance_of(Notes::DiffPositionUpdateService).to receive(:execute).with(note) + + expect_any_instance_of(DiffNote).to receive(:save).once + + subject.reload_diff + end + end end diff --git a/spec/models/notification_setting_spec.rb b/spec/models/notification_setting_spec.rb index df336a6effe..d58673413c8 100644 --- a/spec/models/notification_setting_spec.rb +++ b/spec/models/notification_setting_spec.rb @@ -38,4 +38,21 @@ RSpec.describe NotificationSetting, type: :model do end end end + + describe '#for_projects' do + let(:user) { create(:user) } + + before do + 1.upto(4) do |i| + setting = create(:notification_setting, user: user) + + setting.project.update_attributes(pending_delete: true) if i.even? + end + end + + it 'excludes projects pending delete' do + expect(user.notification_settings.for_projects).to all(have_attributes(project: an_instance_of(Project))) + expect(user.notification_settings.for_projects.map(&:project)).to all(have_attributes(pending_delete: false)) + end + end end diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index a8c777d1e3e..1b434a726dc 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -129,6 +129,18 @@ describe Project, models: true do expect(project2.errors[:repository_storage].first).to match(/is not included in the list/) end end + + it 'should not allow an invalid URI as import_url' do + project2 = build(:project, import_url: 'invalid://') + + expect(project2).not_to be_valid + end + + it 'should allow a valid URI as import_url' do + project2 = build(:project, import_url: 'ssh://test@gitlab.com/project.git') + + expect(project2).to be_valid + end end describe 'default_scope' do @@ -300,7 +312,7 @@ describe Project, models: true do it 'should update merge request commits with new one if pushed to source branch' do project.update_merge_requests(prev_commit_id, commit_id, "refs/heads/#{merge_request.source_branch}", key.user) merge_request.reload - expect(merge_request.last_commit.id).to eq(commit_id) + expect(merge_request.diff_head_sha).to eq(commit_id) end end diff --git a/spec/models/repository_spec.rb b/spec/models/repository_spec.rb index 7975fc64e59..24e49c8def3 100644 --- a/spec/models/repository_spec.rb +++ b/spec/models/repository_spec.rb @@ -12,8 +12,8 @@ describe Repository, models: true do end let(:merge_commit) do source_sha = repository.find_branch('feature').target - merge_commit_id = repository.merge(user, source_sha, 'master', commit_options) - repository.commit(merge_commit_id) + merge_commit_sha = repository.merge(user, source_sha, 'master', commit_options) + repository.commit(merge_commit_sha) end describe :branch_names_contains do diff --git a/spec/requests/api/issues_spec.rb b/spec/requests/api/issues_spec.rb index 2cf130df328..6adccb4ebae 100644 --- a/spec/requests/api/issues_spec.rb +++ b/spec/requests/api/issues_spec.rb @@ -482,12 +482,16 @@ describe API::API, api: true do expect(response).to have_http_status(400) end - it 'should return 400 on invalid label names' do + it 'should allow special label names' do post api("/projects/#{project.id}/issues", user), title: 'new issue', - labels: 'label, ?' - expect(response).to have_http_status(400) - expect(json_response['message']['labels']['?']['title']).to eq(['is invalid']) + labels: 'label, label?, label&foo, ?, &' + expect(response.status).to eq(201) + expect(json_response['labels']).to include 'label' + expect(json_response['labels']).to include 'label?' + expect(json_response['labels']).to include 'label&foo' + expect(json_response['labels']).to include '?' + expect(json_response['labels']).to include '&' end it 'should return 400 if title is too long' do @@ -557,12 +561,17 @@ describe API::API, api: true do expect(response).to have_http_status(404) end - it 'should return 400 on invalid label names' do + it 'should allow special label names' do put api("/projects/#{project.id}/issues/#{issue.id}", user), title: 'updated title', - labels: 'label, ?' - expect(response).to have_http_status(400) - expect(json_response['message']['labels']['?']['title']).to eq(['is invalid']) + labels: 'label, label?, label&foo, ?, &' + + expect(response.status).to eq(200) + expect(json_response['labels']).to include 'label' + expect(json_response['labels']).to include 'label?' + expect(json_response['labels']).to include 'label&foo' + expect(json_response['labels']).to include '?' + expect(json_response['labels']).to include '&' end context 'confidential issues' do @@ -627,21 +636,18 @@ describe API::API, api: true do expect(json_response['labels']).to include 'bar' end - it 'should return 400 on invalid label names' do - put api("/projects/#{project.id}/issues/#{issue.id}", user), - labels: 'label, ?' - expect(response).to have_http_status(400) - expect(json_response['message']['labels']['?']['title']).to eq(['is invalid']) - end - it 'should allow special label names' do put api("/projects/#{project.id}/issues/#{issue.id}", user), - labels: 'label:foo, label-bar,label_bar,label/bar' - expect(response).to have_http_status(200) + labels: 'label:foo, label-bar,label_bar,label/bar,label?bar,label&bar,?,&' + expect(response.status).to eq(200) expect(json_response['labels']).to include 'label:foo' expect(json_response['labels']).to include 'label-bar' expect(json_response['labels']).to include 'label_bar' expect(json_response['labels']).to include 'label/bar' + expect(json_response['labels']).to include 'label?bar' + expect(json_response['labels']).to include 'label&bar' + expect(json_response['labels']).to include '?' + expect(json_response['labels']).to include '&' end it 'should return 400 if title is too long' do diff --git a/spec/requests/api/labels_spec.rb b/spec/requests/api/labels_spec.rb index 0404cf31ff7..63636b4a1b6 100644 --- a/spec/requests/api/labels_spec.rb +++ b/spec/requests/api/labels_spec.rb @@ -35,10 +35,10 @@ describe API::API, api: true do it 'should return created label when only required params' do post api("/projects/#{project.id}/labels", user), - name: 'Foo', + name: 'Foo & Bar', color: '#FFAABB' - expect(response).to have_http_status(201) - expect(json_response['name']).to eq('Foo') + expect(response.status).to eq(201) + expect(json_response['name']).to eq('Foo & Bar') expect(json_response['color']).to eq('#FFAABB') expect(json_response['description']).to be_nil end @@ -71,7 +71,7 @@ describe API::API, api: true do it 'should return 400 for invalid name' do post api("/projects/#{project.id}/labels", user), - name: '?', + name: ',', color: '#FFAABB' expect(response).to have_http_status(400) expect(json_response['message']['title']).to eq(['is invalid']) @@ -167,7 +167,7 @@ describe API::API, api: true do it 'should return 400 for invalid name' do put api("/projects/#{project.id}/labels", user), name: 'label1', - new_name: '?', + new_name: ',', color: '#FFFFFF' expect(response).to have_http_status(400) expect(json_response['message']['title']).to eq(['is invalid']) diff --git a/spec/requests/api/merge_requests_spec.rb b/spec/requests/api/merge_requests_spec.rb index 61e897edf87..4a1b5600bdf 100644 --- a/spec/requests/api/merge_requests_spec.rb +++ b/spec/requests/api/merge_requests_spec.rb @@ -243,17 +243,19 @@ describe API::API, api: true do expect(response).to have_http_status(400) end - it 'should return 400 on invalid label names' do + it 'should allow special label names' do post api("/projects/#{project.id}/merge_requests", user), title: 'Test merge_request', source_branch: 'markdown', target_branch: 'master', author: user, - labels: 'label, ?' - expect(response).to have_http_status(400) - expect(json_response['message']['labels']['?']['title']).to eq( - ['is invalid'] - ) + labels: 'label, label?, label&foo, ?, &' + expect(response.status).to eq(201) + expect(json_response['labels']).to include 'label' + expect(json_response['labels']).to include 'label?' + expect(json_response['labels']).to include 'label&foo' + expect(json_response['labels']).to include '?' + expect(json_response['labels']).to include '&' end context 'with existing MR' do @@ -437,14 +439,14 @@ describe API::API, api: true do end it "returns 409 if the SHA parameter doesn't match" do - put api("/projects/#{project.id}/merge_requests/#{merge_request.id}/merge", user), sha: merge_request.source_sha.succ + put api("/projects/#{project.id}/merge_requests/#{merge_request.id}/merge", user), sha: merge_request.diff_head_sha.reverse expect(response).to have_http_status(409) expect(json_response['message']).to start_with('SHA does not match HEAD of source branch') end it "succeeds if the SHA parameter matches" do - put api("/projects/#{project.id}/merge_requests/#{merge_request.id}/merge", user), sha: merge_request.source_sha + put api("/projects/#{project.id}/merge_requests/#{merge_request.id}/merge", user), sha: merge_request.diff_head_sha expect(response).to have_http_status(200) end @@ -492,13 +494,17 @@ describe API::API, api: true do expect(json_response['target_branch']).to eq('wiki') end - it 'should return 400 on invalid label names' do + it 'should allow special label names' do put api("/projects/#{project.id}/merge_requests/#{merge_request.id}", user), title: 'new issue', - labels: 'label, ?' - expect(response).to have_http_status(400) - expect(json_response['message']['labels']['?']['title']).to eq(['is invalid']) + labels: 'label, label?, label&foo, ?, &' + expect(response.status).to eq(200) + expect(json_response['labels']).to include 'label' + expect(json_response['labels']).to include 'label?' + expect(json_response['labels']).to include 'label&foo' + expect(json_response['labels']).to include '?' + expect(json_response['labels']).to include '&' end end diff --git a/spec/services/notes/diff_position_update_service_spec.rb b/spec/services/notes/diff_position_update_service_spec.rb new file mode 100644 index 00000000000..110efb54fa0 --- /dev/null +++ b/spec/services/notes/diff_position_update_service_spec.rb @@ -0,0 +1,175 @@ +require 'spec_helper' + +describe Notes::DiffPositionUpdateService, services: true do + let(:project) { create(:project) } + let(:create_commit) { project.commit("913c66a37b4a45b9769037c55c2d238bd0942d2e") } + let(:modify_commit) { project.commit("874797c3a73b60d2187ed6e2fcabd289ff75171e") } + let(:edit_commit) { project.commit("570e7b2abdd848b95f2f578043fc23bd6f6fd24d") } + + let(:path) { "files/ruby/popen.rb" } + + let(:old_diff_refs) do + Gitlab::Diff::DiffRefs.new( + base_sha: create_commit.parent_id, + head_sha: modify_commit.sha + ) + end + + let(:new_diff_refs) do + Gitlab::Diff::DiffRefs.new( + base_sha: create_commit.parent_id, + head_sha: edit_commit.sha + ) + end + + subject do + described_class.new( + project, + nil, + old_diff_refs: old_diff_refs, + new_diff_refs: new_diff_refs, + paths: [path] + ) + end + + # old diff: + # 1 + require 'fileutils' + # 2 + require 'open3' + # 3 + + # 4 + module Popen + # 5 + extend self + # 6 + + # 7 + def popen(cmd, path=nil) + # 8 + unless cmd.is_a?(Array) + # 9 + raise "System commands must be given as an array of strings" + # 10 + end + # 11 + + # 12 + path ||= Dir.pwd + # 13 + vars = { "PWD" => path } + # 14 + options = { chdir: path } + # 15 + + # 16 + unless File.directory?(path) + # 17 + FileUtils.mkdir_p(path) + # 18 + end + # 19 + + # 20 + @cmd_output = "" + # 21 + @cmd_status = 0 + # 22 + Open3.popen3(vars, *cmd, options) do |stdin, stdout, stderr, wait_thr| + # 23 + @cmd_output << stdout.read + # 24 + @cmd_output << stderr.read + # 25 + @cmd_status = wait_thr.value.exitstatus + # 26 + end + # 27 + + # 28 + return @cmd_output, @cmd_status + # 29 + end + # 30 + end + # + # new diff: + # 1 + require 'fileutils' + # 2 + require 'open3' + # 3 + + # 4 + module Popen + # 5 + extend self + # 6 + + # 7 + def popen(cmd, path=nil) + # 8 + unless cmd.is_a?(Array) + # 9 + raise RuntimeError, "System commands must be given as an array of strings" + # 10 + end + # 11 + + # 12 + path ||= Dir.pwd + # 13 + + # 14 + vars = { + # 15 + "PWD" => path + # 16 + } + # 17 + + # 18 + options = { + # 19 + chdir: path + # 20 + } + # 21 + + # 22 + unless File.directory?(path) + # 23 + FileUtils.mkdir_p(path) + # 24 + end + # 25 + + # 26 + @cmd_output = "" + # 27 + @cmd_status = 0 + # 28 + + # 29 + Open3.popen3(vars, *cmd, options) do |stdin, stdout, stderr, wait_thr| + # 30 + @cmd_output << stdout.read + # 31 + @cmd_output << stderr.read + # 32 + @cmd_status = wait_thr.value.exitstatus + # 33 + end + # 34 + + # 35 + return @cmd_output, @cmd_status + # 36 + end + # 37 + end + # + # old->new diff: + # .. .. @@ -6,12 +6,18 @@ module Popen + # 6 6 + # 7 7 def popen(cmd, path=nil) + # 8 8 unless cmd.is_a?(Array) + # 9 - raise "System commands must be given as an array of strings" + # 9 + raise RuntimeError, "System commands must be given as an array of strings" + # 10 10 end + # 11 11 + # 12 12 path ||= Dir.pwd + # 13 - vars = { "PWD" => path } + # 14 - options = { chdir: path } + # 13 + + # 14 + vars = { + # 15 + "PWD" => path + # 16 + } + # 17 + + # 18 + options = { + # 19 + chdir: path + # 20 + } + # 15 21 + # 16 22 unless File.directory?(path) + # 17 23 FileUtils.mkdir_p(path) + # 18 24 end + # 19 25 + # 20 26 @cmd_output = "" + # 21 27 @cmd_status = 0 + # 28 + + # 22 29 Open3.popen3(vars, *cmd, options) do |stdin, stdout, stderr, wait_thr| + # 23 30 @cmd_output << stdout.read + # 24 31 @cmd_output << stderr.read + # .. .. + + describe "#execute" do + let(:note) { create(:diff_note_on_merge_request, project: project, position: old_position) } + + let(:old_position) do + Gitlab::Diff::Position.new( + old_path: path, + new_path: path, + old_line: nil, + new_line: line, + diff_refs: old_diff_refs + ) + end + + context "when the diff line is the same" do + let(:line) { 16 } + + it "updates the position" do + subject.execute(note) + + expect(note.original_position).to eq(old_position) + expect(note.position).not_to eq(old_position) + expect(note.position.new_line).to eq(22) + end + end + + context "when the diff line has changed" do + let(:line) { 9 } + + it "doesn't update the position" do + subject.execute(note) + + expect(note.original_position).to eq(old_position) + expect(note.position).to eq(old_position) + end + end + end +end diff --git a/spec/services/system_note_service_spec.rb b/spec/services/system_note_service_spec.rb index 85dd30bf48c..43693441450 100644 --- a/spec/services/system_note_service_spec.rb +++ b/spec/services/system_note_service_spec.rb @@ -213,7 +213,7 @@ describe SystemNoteService, services: true do create(:merge_request, source_project: project, target_project: project) end - subject { described_class.merge_when_build_succeeds(noteable, project, author, noteable.last_commit) } + subject { described_class.merge_when_build_succeeds(noteable, project, author, noteable.diff_head_commit) } it_behaves_like 'a system note' |