diff options
| author | Douwe Maan <douwe@gitlab.com> | 2019-07-05 14:03:48 +0300 |
|---|---|---|
| committer | Douwe Maan <douwe@gitlab.com> | 2019-07-05 14:03:48 +0300 |
| commit | 74cf3a11b4510bb10d356fb75df06708cf44488a (patch) | |
| tree | d2f55dee037e7258ce8eec829bca4bb77f34a745 | |
| parent | a1edc5a5cb3e740b0c215c54ce282535b143a9db (diff) | |
| parent | 6ef6693e3767d480362ce528dd0beff159c895ff (diff) | |
Merge branch '57793-fix-line-age' into 'master'
Support note position tracing on an image
See merge request gitlab-org/gitlab-ce!30158
| -rw-r--r-- | app/services/discussions/update_diff_position_service.rb | 3 | ||||
| -rw-r--r-- | app/services/system_note_service.rb | 8 | ||||
| -rw-r--r-- | changelogs/unreleased/57793-fix-line-age.yml | 5 | ||||
| -rw-r--r-- | lib/gitlab/diff/position.rb | 4 | ||||
| -rw-r--r-- | lib/gitlab/diff/position_tracer.rb | 192 | ||||
| -rw-r--r-- | lib/gitlab/diff/position_tracer/base_strategy.rb | 26 | ||||
| -rw-r--r-- | lib/gitlab/diff/position_tracer/image_strategy.rb | 50 | ||||
| -rw-r--r-- | lib/gitlab/diff/position_tracer/line_strategy.rb | 201 | ||||
| -rw-r--r-- | spec/lib/gitlab/diff/position_spec.rb | 13 | ||||
| -rw-r--r-- | spec/lib/gitlab/diff/position_tracer/image_strategy_spec.rb | 238 | ||||
| -rw-r--r-- | spec/lib/gitlab/diff/position_tracer/line_strategy_spec.rb | 1805 | ||||
| -rw-r--r-- | spec/lib/gitlab/diff/position_tracer_spec.rb | 1918 | ||||
| -rw-r--r-- | spec/services/system_note_service_spec.rb | 24 | ||||
| -rw-r--r-- | spec/support/helpers/position_tracer_helpers.rb | 93 |
14 files changed, 2527 insertions, 2053 deletions
diff --git a/app/services/discussions/update_diff_position_service.rb b/app/services/discussions/update_diff_position_service.rb index c61437fb2e3..7bdf7711155 100644 --- a/app/services/discussions/update_diff_position_service.rb +++ b/app/services/discussions/update_diff_position_service.rb @@ -3,7 +3,8 @@ module Discussions class UpdateDiffPositionService < BaseService def execute(discussion) - result = tracer.trace(discussion.position) + old_position = discussion.position + result = tracer.trace(old_position) return unless result position = result[:position] diff --git a/app/services/system_note_service.rb b/app/services/system_note_service.rb index 4783417ad6d..1d02d7ed787 100644 --- a/app/services/system_note_service.rb +++ b/app/services/system_note_service.rb @@ -268,11 +268,13 @@ module SystemNoteService merge_request = discussion.noteable diff_refs = change_position.diff_refs version_index = merge_request.merge_request_diffs.viewable.count + position_on_text = change_position.on_text? + text_parts = ["changed this #{position_on_text ? 'line' : 'file'} in"] - text_parts = ["changed this line in"] if version_params = merge_request.version_params_for(diff_refs) - line_code = change_position.line_code(project.repository) - url = url_helpers.diffs_project_merge_request_path(project, merge_request, version_params.merge(anchor: line_code)) + repository = project.repository + anchor = position_on_text ? change_position.line_code(repository) : change_position.file_hash + url = url_helpers.diffs_project_merge_request_path(project, merge_request, version_params.merge(anchor: anchor)) text_parts << "[version #{version_index} of the diff](#{url})" else diff --git a/changelogs/unreleased/57793-fix-line-age.yml b/changelogs/unreleased/57793-fix-line-age.yml new file mode 100644 index 00000000000..cf4e328e54a --- /dev/null +++ b/changelogs/unreleased/57793-fix-line-age.yml @@ -0,0 +1,5 @@ +--- +title: Support note position tracing on an image +merge_request: 30158 +author: +type: fixed diff --git a/lib/gitlab/diff/position.rb b/lib/gitlab/diff/position.rb index d349c378e53..dfa80eb4a64 100644 --- a/lib/gitlab/diff/position.rb +++ b/lib/gitlab/diff/position.rb @@ -134,6 +134,10 @@ module Gitlab @line_code ||= diff_file(repository)&.line_code_for_position(self) end + def file_hash + @file_hash ||= Digest::SHA1.hexdigest(file_path) + end + def on_image? position_type == 'image' end diff --git a/lib/gitlab/diff/position_tracer.rb b/lib/gitlab/diff/position_tracer.rb index af3df820422..a1c82ce9afc 100644 --- a/lib/gitlab/diff/position_tracer.rb +++ b/lib/gitlab/diff/position_tracer.rb @@ -17,187 +17,13 @@ module Gitlab @paths = paths end - def trace(ab_position) + def trace(old_position) return unless old_diff_refs&.complete? && new_diff_refs&.complete? - return unless ab_position.diff_refs == old_diff_refs + return unless old_position.diff_refs == old_diff_refs - # Suppose we have an MR with source branch `feature` and target branch `master`. - # When the MR was created, the head of `master` was commit A, and the - # head of `feature` was commit B, resulting in the original diff A->B. - # Since creation, `master` was updated to C. - # Now `feature` is being updated to D, and the newly generated MR diff is C->D. - # It is possible that C and D are direct descendants of A and B respectively, - # but this isn't necessarily the case as rebases and merges come into play. - # - # Suppose we have a diff note on the original diff A->B. Now that the MR - # is updated, we need to find out what line in C->D corresponds to the - # line the note was originally created on, so that we can update the diff note's - # records and continue to display it in the right place in the diffs. - # If we cannot find this line in the new diff, this means the diff note is now - # outdated, and we will display that fact to the user. - # - # In the new diff, the file the diff note was originally created on may - # have been renamed, deleted or even created, if the file existed in A and B, - # but was removed in C, and restored in D. - # - # Every diff note stores a Position object that defines a specific location, - # identified by paths and line numbers, within a specific diff, identified - # by start, head and base commit ids. - # - # For diff notes for diff A->B, the position looks like this: - # Position - # start_sha - ID of commit A - # head_sha - ID of commit B - # base_sha - ID of base commit of A and B - # old_path - path as of A (nil if file was newly created) - # new_path - path as of B (nil if file was deleted) - # old_line - line number as of A (nil if file was newly created) - # new_line - line number as of B (nil if file was deleted) - # - # We can easily update `start_sha` and `head_sha` to hold the IDs of - # commits C and D, and can trivially determine `base_sha` based on those, - # but need to find the paths and line numbers as of C and D. - # - # If the file was unchanged or newly created in A->B, the path as of D can be found - # by generating diff B->D ("head to head"), finding the diff file with - # `diff_file.old_path == position.new_path`, and taking `diff_file.new_path`. - # The path as of C can be found by taking diff C->D, finding the diff file - # with that same `new_path` and taking `diff_file.old_path`. - # The line number as of D can be found by using the LineMapper on diff B->D - # and providing the line number as of B. - # The line number as of C can be found by using the LineMapper on diff C->D - # and providing the line number as of D. - # - # If the file was deleted in A->B, the path as of C can be found - # by generating diff A->C ("base to base"), finding the diff file with - # `diff_file.old_path == position.old_path`, and taking `diff_file.new_path`. - # The path as of D can be found by taking diff C->D, finding the diff file - # with `old_path` set to that `diff_file.new_path` and taking `diff_file.new_path`. - # The line number as of C can be found by using the LineMapper on diff A->C - # and providing the line number as of A. - # The line number as of D can be found by using the LineMapper on diff C->D - # and providing the line number as of C. + strategy = old_position.on_text? ? LineStrategy : ImageStrategy - if ab_position.added? - trace_added_line(ab_position) - elsif ab_position.removed? - trace_removed_line(ab_position) - else # unchanged - trace_unchanged_line(ab_position) - end - end - - private - - def trace_added_line(ab_position) - b_path = ab_position.new_path - b_line = ab_position.new_line - - bd_diff = bd_diffs.diff_file_with_old_path(b_path) - - d_path = bd_diff&.new_path || b_path - d_line = LineMapper.new(bd_diff).old_to_new(b_line) - - if d_line - cd_diff = cd_diffs.diff_file_with_new_path(d_path) - - c_path = cd_diff&.old_path || d_path - c_line = LineMapper.new(cd_diff).new_to_old(d_line) - - if c_line - # If the line is still in D but also in C, it has turned from an - # added line into an unchanged one. - new_position = position(cd_diff, c_line, d_line) - if valid_position?(new_position) - # If the line is still in the MR, we don't treat this as outdated. - { position: new_position, outdated: false } - else - # If the line is no longer in the MR, we unfortunately cannot show - # the current state on the CD diff, so we treat it as outdated. - ac_diff = ac_diffs.diff_file_with_new_path(c_path) - - { position: position(ac_diff, nil, c_line), outdated: true } - end - else - # If the line is still in D and not in C, it is still added. - { position: position(cd_diff, nil, d_line), outdated: false } - end - else - # If the line is no longer in D, it has been removed from the MR. - { position: position(bd_diff, b_line, nil), outdated: true } - end - end - - def trace_removed_line(ab_position) - a_path = ab_position.old_path - a_line = ab_position.old_line - - ac_diff = ac_diffs.diff_file_with_old_path(a_path) - - c_path = ac_diff&.new_path || a_path - c_line = LineMapper.new(ac_diff).old_to_new(a_line) - - if c_line - cd_diff = cd_diffs.diff_file_with_old_path(c_path) - - d_path = cd_diff&.new_path || c_path - d_line = LineMapper.new(cd_diff).old_to_new(c_line) - - if d_line - # If the line is still in C but also in D, it has turned from a - # removed line into an unchanged one. - bd_diff = bd_diffs.diff_file_with_new_path(d_path) - - { position: position(bd_diff, nil, d_line), outdated: true } - else - # If the line is still in C and not in D, it is still removed. - { position: position(cd_diff, c_line, nil), outdated: false } - end - else - # If the line is no longer in C, it has been removed outside of the MR. - { position: position(ac_diff, a_line, nil), outdated: true } - end - end - - def trace_unchanged_line(ab_position) - a_path = ab_position.old_path - a_line = ab_position.old_line - b_path = ab_position.new_path - b_line = ab_position.new_line - - ac_diff = ac_diffs.diff_file_with_old_path(a_path) - - c_path = ac_diff&.new_path || a_path - c_line = LineMapper.new(ac_diff).old_to_new(a_line) - - bd_diff = bd_diffs.diff_file_with_old_path(b_path) - - d_line = LineMapper.new(bd_diff).old_to_new(b_line) - - cd_diff = cd_diffs.diff_file_with_old_path(c_path) - - if c_line && d_line - # If the line is still in C and D, it is still unchanged. - new_position = position(cd_diff, c_line, d_line) - if valid_position?(new_position) - # If the line is still in the MR, we don't treat this as outdated. - { position: new_position, outdated: false } - else - # If the line is no longer in the MR, we unfortunately cannot show - # the current state on the CD diff or any change on the BD diff, - # so we treat it as outdated. - { position: nil, outdated: true } - end - elsif d_line # && !c_line - # If the line is still in D but no longer in C, it has turned from - # an unchanged line into an added one. - # We don't treat this as outdated since the line is still in the MR. - { position: position(cd_diff, nil, d_line), outdated: false } - else # !d_line && (c_line || !c_line) - # If the line is no longer in D, it has turned from an unchanged line - # into a removed one. - { position: position(bd_diff, b_line, nil), outdated: true } - end + strategy.new(self).trace(old_position) end def ac_diffs @@ -216,18 +42,12 @@ module Gitlab @cd_diffs ||= compare(new_diff_refs.start_sha, new_diff_refs.head_sha) end + private + def compare(start_sha, head_sha, straight: false) compare = CompareService.new(project, head_sha).execute(project, start_sha, straight: straight) compare.diffs(paths: paths, expanded: true) end - - def position(diff_file, old_line, new_line) - Position.new(diff_file: diff_file, old_line: old_line, new_line: new_line) - end - - def valid_position?(position) - !!position.diff_line(project.repository) - end end end end diff --git a/lib/gitlab/diff/position_tracer/base_strategy.rb b/lib/gitlab/diff/position_tracer/base_strategy.rb new file mode 100644 index 00000000000..65049daabf4 --- /dev/null +++ b/lib/gitlab/diff/position_tracer/base_strategy.rb @@ -0,0 +1,26 @@ +# frozen_string_literal: true + +module Gitlab + module Diff + class PositionTracer + class BaseStrategy + attr_reader :tracer + + delegate \ + :project, + :ac_diffs, + :bd_diffs, + :cd_diffs, + to: :tracer + + def initialize(tracer) + @tracer = tracer + end + + def trace(position) + raise NotImplementedError + end + end + end + end +end diff --git a/lib/gitlab/diff/position_tracer/image_strategy.rb b/lib/gitlab/diff/position_tracer/image_strategy.rb new file mode 100644 index 00000000000..79244a17951 --- /dev/null +++ b/lib/gitlab/diff/position_tracer/image_strategy.rb @@ -0,0 +1,50 @@ +# frozen_string_literal: true + +module Gitlab + module Diff + class PositionTracer + class ImageStrategy < BaseStrategy + def trace(position) + b_path = position.new_path + + # If file exists in B->D (e.g. updated, renamed, removed), let the + # note become outdated. + bd_diff = bd_diffs.diff_file_with_old_path(b_path) + + return { position: new_position(position, bd_diff), outdated: true } if bd_diff + + # If file still exists in the new diff, update the position. + cd_diff = cd_diffs.diff_file_with_new_path(bd_diff&.new_path || b_path) + + return { position: new_position(position, cd_diff), outdated: false } if cd_diff + + # If file exists in A->C (e.g. rebased and same changes were present + # in target branch), let the note become outdated. + ac_diff = ac_diffs.diff_file_with_old_path(position.old_path) + + return { position: new_position(position, ac_diff), outdated: true } if ac_diff + + # If ever there's a case that the file no longer exists in any diff, + # don't set a change position and let the note become outdated. + # + # This should never happen given the file should exist in one of the + # diffs above. + { outdated: true } + end + + private + + def new_position(position, diff_file) + Position.new( + diff_file: diff_file, + x: position.x, + y: position.y, + width: position.width, + height: position.height, + position_type: position.position_type + ) + end + end + end + end +end diff --git a/lib/gitlab/diff/position_tracer/line_strategy.rb b/lib/gitlab/diff/position_tracer/line_strategy.rb new file mode 100644 index 00000000000..8db0fc6f963 --- /dev/null +++ b/lib/gitlab/diff/position_tracer/line_strategy.rb @@ -0,0 +1,201 @@ +# frozen_string_literal: true + +module Gitlab + module Diff + class PositionTracer + class LineStrategy < BaseStrategy + def trace(position) + # Suppose we have an MR with source branch `feature` and target branch `master`. + # When the MR was created, the head of `master` was commit A, and the + # head of `feature` was commit B, resulting in the original diff A->B. + # Since creation, `master` was updated to C. + # Now `feature` is being updated to D, and the newly generated MR diff is C->D. + # It is possible that C and D are direct descendants of A and B respectively, + # but this isn't necessarily the case as rebases and merges come into play. + # + # Suppose we have a diff note on the original diff A->B. Now that the MR + # is updated, we need to find out what line in C->D corresponds to the + # line the note was originally created on, so that we can update the diff note's + # records and continue to display it in the right place in the diffs. + # If we cannot find this line in the new diff, this means the diff note is now + # outdated, and we will display that fact to the user. + # + # In the new diff, the file the diff note was originally created on may + # have been renamed, deleted or even created, if the file existed in A and B, + # but was removed in C, and restored in D. + # + # Every diff note stores a Position object that defines a specific location, + # identified by paths and line numbers, within a specific diff, identified + # by start, head and base commit ids. + # + # For diff notes for diff A->B, the position looks like this: + # Position + # start_sha - ID of commit A + # head_sha - ID of commit B + # base_sha - ID of base commit of A and B + # old_path - path as of A (nil if file was newly created) + # new_path - path as of B (nil if file was deleted) + # old_line - line number as of A (nil if file was newly created) + # new_line - line number as of B (nil if file was deleted) + # + # We can easily update `start_sha` and `head_sha` to hold the IDs of + # commits C and D, and can trivially determine `base_sha` based on those, + # but need to find the paths and line numbers as of C and D. + # + # If the file was unchanged or newly created in A->B, the path as of D can be found + # by generating diff B->D ("head to head"), finding the diff file with + # `diff_file.old_path == position.new_path`, and taking `diff_file.new_path`. + # The path as of C can be found by taking diff C->D, finding the diff file + # with that same `new_path` and taking `diff_file.old_path`. + # The line number as of D can be found by using the LineMapper on diff B->D + # and providing the line number as of B. + # The line number as of C can be found by using the LineMapper on diff C->D + # and providing the line number as of D. + # + # If the file was deleted in A->B, the path as of C can be found + # by generating diff A->C ("base to base"), finding the diff file with + # `diff_file.old_path == position.old_path`, and taking `diff_file.new_path`. + # The path as of D can be found by taking diff C->D, finding the diff file + # with `old_path` set to that `diff_file.new_path` and taking `diff_file.new_path`. + # The line number as of C can be found by using the LineMapper on diff A->C + # and providing the line number as of A. + # The line number as of D can be found by using the LineMapper on diff C->D + # and providing the line number as of C. + + if position.added? + trace_added_line(position) + elsif position.removed? + trace_removed_line(position) + else # unchanged + trace_unchanged_line(position) + end + end + + private + + def trace_added_line(position) + b_path = position.new_path + b_line = position.new_line + + bd_diff = bd_diffs.diff_file_with_old_path(b_path) + + d_path = bd_diff&.new_path || b_path + d_line = LineMapper.new(bd_diff).old_to_new(b_line) + + if d_line + cd_diff = cd_diffs.diff_file_with_new_path(d_path) + + c_path = cd_diff&.old_path || d_path + c_line = LineMapper.new(cd_diff).new_to_old(d_line) + + if c_line + # If the line is still in D but also in C, it has turned from an + # added line into an unchanged one. + new_position = new_position(cd_diff, c_line, d_line) + if valid_position?(new_position) + # If the line is still in the MR, we don't treat this as outdated. + { position: new_position, outdated: false } + else + # If the line is no longer in the MR, we unfortunately cannot show + # the current state on the CD diff, so we treat it as outdated. + ac_diff = ac_diffs.diff_file_with_new_path(c_path) + + { position: new_position(ac_diff, nil, c_line), outdated: true } + end + else + # If the line is still in D and not in C, it is still added. + { position: new_position(cd_diff, nil, d_line), outdated: false } + end + else + # If the line is no longer in D, it has been removed from the MR. + { position: new_position(bd_diff, b_line, nil), outdated: true } + end + end + + def trace_removed_line(position) + a_path = position.old_path + a_line = position.old_line + + ac_diff = ac_diffs.diff_file_with_old_path(a_path) + + c_path = ac_diff&.new_path || a_path + c_line = LineMapper.new(ac_diff).old_to_new(a_line) + + if c_line + cd_diff = cd_diffs.diff_file_with_old_path(c_path) + + d_path = cd_diff&.new_path || c_path + d_line = LineMapper.new(cd_diff).old_to_new(c_line) + + if d_line + # If the line is still in C but also in D, it has turned from a + # removed line into an unchanged one. + bd_diff = bd_diffs.diff_file_with_new_path(d_path) + + { position: new_position(bd_diff, nil, d_line), outdated: true } + else + # If the line is still in C and not in D, it is still removed. + { position: new_position(cd_diff, c_line, nil), outdated: false } + end + else + # If the line is no longer in C, it has been removed outside of the MR. + { position: new_position(ac_diff, a_line, nil), outdated: true } + end + end + + def trace_unchanged_line(position) + a_path = position.old_path + a_line = position.old_line + b_path = position.new_path + b_line = position.new_line + + ac_diff = ac_diffs.diff_file_with_old_path(a_path) + + c_path = ac_diff&.new_path || a_path + c_line = LineMapper.new(ac_diff).old_to_new(a_line) + + bd_diff = bd_diffs.diff_file_with_old_path(b_path) + + d_line = LineMapper.new(bd_diff).old_to_new(b_line) + + cd_diff = cd_diffs.diff_file_with_old_path(c_path) + + if c_line && d_line + # If the line is still in C and D, it is still unchanged. + new_position = new_position(cd_diff, c_line, d_line) + if valid_position?(new_position) + # If the line is still in the MR, we don't treat this as outdated. + { position: new_position, outdated: false } + else + # If the line is no longer in the MR, we unfortunately cannot show + # the current state on the CD diff or any change on the BD diff, + # so we treat it as outdated. + { position: nil, outdated: true } + end + elsif d_line # && !c_line + # If the line is still in D but no longer in C, it has turned from + # an unchanged line into an added one. + # We don't treat this as outdated since the line is still in the MR. + { position: new_position(cd_diff, nil, d_line), outdated: false } + else # !d_line && (c_line || !c_line) + # If the line is no longer in D, it has turned from an unchanged line + # into a removed one. + { position: new_position(bd_diff, b_line, nil), outdated: true } + end + end + + def new_position(diff_file, old_line, new_line) + Position.new( + diff_file: diff_file, + old_line: old_line, + new_line: new_line + ) + end + + def valid_position?(position) + !!position.diff_line(project.repository) + end + end + end + end +end diff --git a/spec/lib/gitlab/diff/position_spec.rb b/spec/lib/gitlab/diff/position_spec.rb index aea02d21048..b755cd1aff0 100644 --- a/spec/lib/gitlab/diff/position_spec.rb +++ b/spec/lib/gitlab/diff/position_spec.rb @@ -610,4 +610,17 @@ describe Gitlab::Diff::Position do it_behaves_like "diff position json" end end + + describe "#file_hash" do + subject do + described_class.new( + old_path: "image.jpg", + new_path: "image.jpg" + ) + end + + it "returns SHA1 representation of the file_path" do + expect(subject.file_hash).to eq(Digest::SHA1.hexdigest(subject.file_path)) + end + end end diff --git a/spec/lib/gitlab/diff/position_tracer/image_strategy_spec.rb b/spec/lib/gitlab/diff/position_tracer/image_strategy_spec.rb new file mode 100644 index 00000000000..900816af53a --- /dev/null +++ b/spec/lib/gitlab/diff/position_tracer/image_strategy_spec.rb @@ -0,0 +1,238 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe Gitlab::Diff::PositionTracer::ImageStrategy do + include PositionTracerHelpers + + let(:project) { create(:project, :repository) } + let(:current_user) { project.owner } + 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_position) { position(old_path: file_name, new_path: file_name, position_type: 'image') } + + let(:tracer) do + Gitlab::Diff::PositionTracer.new( + project: project, + old_diff_refs: old_diff_refs, + new_diff_refs: new_diff_refs + ) + end + + let(:strategy) { described_class.new(tracer) } + + subject { strategy.trace(old_position) } + + let(:initial_commit) do + project.commit(create_branch(branch_name, 'master')[:branch].name) + end + + describe '#trace' do + describe 'diff scenarios' do + let(:create_file_commit) do + initial_commit + + create_file( + branch_name, + file_name, + Base64.encode64('content') + ) + end + + let(:update_file_commit) do + create_file_commit + + update_file( + branch_name, + file_name, + Base64.encode64('updatedcontent') + ) + end + + let(:update_file_again_commit) do + update_file_commit + + update_file( + branch_name, + file_name, + Base64.encode64('updatedcontentagain') + ) + end + + let(:delete_file_commit) do + create_file_commit + delete_file(branch_name, file_name) + end + + let(:rename_file_commit) do + delete_file_commit + + create_file( + branch_name, + new_file_name, + Base64.encode64('renamedcontent') + ) + end + + let(:create_second_file_commit) do + create_file_commit + + create_file( + branch_name, + second_file_name, + Base64.encode64('morecontent') + ) + end + + let(:create_another_file_commit) do + create_file( + branch_name, + second_file_name, + Base64.encode64('morecontent') + ) + end + + let(:update_another_file_commit) do + update_file( + branch_name, + second_file_name, + Base64.encode64('updatedmorecontent') + ) + end + + context 'when the file was created in the old diff' do + context 'when the file is 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) } + + it 'returns the new position' do + expect_new_position( + old_path: file_name, + new_path: file_name + ) + end + end + + context 'when the file was updated 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_file_commit) } + let(:change_diff_refs) { diff_refs(create_file_commit, update_file_commit) } + + it 'returns the position of the change' do + expect_change_position( + old_path: file_name, + new_path: file_name + ) + end + end + + context 'when the file was renamed in 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, rename_file_commit) } + let(:change_diff_refs) { diff_refs(create_file_commit, rename_file_commit) } + + it 'returns the position of the change' do + expect_change_position( + old_path: file_name, + new_path: file_name + ) + end + end + + context 'when the file was removed in 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, delete_file_commit) } + let(:change_diff_refs) { diff_refs(create_file_commit, delete_file_commit) } + + it 'returns the position of the change' do + expect_change_position( + old_path: file_name, + new_path: file_name + ) + end + end + + context 'when the file is unchanged in the new diff' do + let(:old_diff_refs) { diff_refs(initial_commit, create_file_commit) } + let(:new_diff_refs) { diff_refs(create_another_file_commit, update_another_file_commit) } + let(:change_diff_refs) { diff_refs(initial_commit, create_another_file_commit) } + + it 'returns the position of the change' do + expect_change_position( + old_path: file_name, + new_path: file_name + ) + end + end + end + + context 'when the file was changed in the old diff' do + context 'when the file is unchanged in between the old and the new diff' do + let(:old_diff_refs) { diff_refs(create_file_commit, update_file_commit) } + let(:new_diff_refs) { diff_refs(create_file_commit, create_second_file_commit) } + + it 'returns the new position' do + expect_new_position( + old_path: file_name, + new_path: file_name + ) + end + end + + context 'when the file was updated in between the old and the new diff' 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) } + let(:change_diff_refs) { diff_refs(update_file_commit, update_file_again_commit) } + + it 'returns the position of the change' do + expect_change_position( + old_path: file_name, + new_path: file_name + ) + end + end + + context 'when the file was renamed in between the old and the new diff' do + let(:old_diff_refs) { diff_refs(create_file_commit, update_file_commit) } + let(:new_diff_refs) { diff_refs(create_file_commit, rename_file_commit) } + let(:change_diff_refs) { diff_refs(update_file_commit, rename_file_commit) } + + it 'returns the position of the change' do + expect_change_position( + old_path: file_name, + new_path: file_name + ) + end + end + + context 'when the file was removed in between the old and the new diff' do + let(:old_diff_refs) { diff_refs(create_file_commit, update_file_commit) } + let(:new_diff_refs) { diff_refs(create_file_commit, delete_file_commit) } + let(:change_diff_refs) { diff_refs(update_file_commit, delete_file_commit) } + + it 'returns the position of the change' do + expect_change_position( + old_path: file_name, + new_path: file_name + ) + end + end + + context 'when the file is unchanged in the new diff' do + let(:old_diff_refs) { diff_refs(create_file_commit, update_file_commit) } + let(:new_diff_refs) { diff_refs(create_another_file_commit, update_another_file_commit) } + let(:change_diff_refs) { diff_refs(create_file_commit, create_another_file_commit) } + + it 'returns the position of the change' do + expect_change_position( + old_path: file_name, + new_path: file_name + ) + end + end + end + end + end +end diff --git a/spec/lib/gitlab/diff/position_tracer/line_strategy_spec.rb b/spec/lib/gitlab/diff/position_tracer/line_strategy_spec.rb new file mode 100644 index 00000000000..7f4902c5b86 --- /dev/null +++ b/spec/lib/gitlab/diff/position_tracer/line_strategy_spec.rb @@ -0,0 +1,1805 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe Gitlab::Diff::PositionTracer::LineStrategy 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 comparatively lackluster + # testing of other scenarios. + # + # I did still try to cover most of the obvious and potentially tricky + # scenarios, though. + + include RepoHelpers + include PositionTracerHelpers + + let(:project) { create(:project, :repository) } + 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(:change_diff_refs) { raise NotImplementedError } + let(:old_position) { raise NotImplementedError } + + let(:tracer) do + Gitlab::Diff::PositionTracer.new( + project: project, + old_diff_refs: old_diff_refs, + new_diff_refs: new_diff_refs + ) + end + + let(:strategy) { described_class.new(tracer) } + + subject { strategy.trace(old_position) } + + let(:initial_commit) do + project.commit(create_branch(branch_name, 'master')[: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(:change_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 + A + # 2 + BB + # 3 + C + + it "returns the position of the change" do + expect_change_position( + old_path: file_name, + new_path: file_name, + old_line: 2, + new_line: 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(:change_diff_refs) { diff_refs(update_line_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 the position of the change" do + expect_change_position( + old_path: file_name, + new_path: file_name, + old_line: 3, + new_line: 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(:change_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 the position of the change" do + expect_change_position( + old_path: file_name, + new_path: file_name, + old_line: 2, + new_line: 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(:change_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 the position of the change" do + expect_change_position( + old_path: file_name, + new_path: file_name, + old_line: 3, + new_line: 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(:change_diff_refs) { diff_refs(initial_commit, delete_line_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 position of the change" do + expect_change_position( + old_path: file_name, + new_path: file_name, + old_line: nil, + new_line: 2 + ) + 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(:change_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 the position of the change" do + expect_change_position( + old_path: file_name, + new_path: new_file_name, + old_line: 2, + new_line: 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(:change_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 the position of the change" do + expect_change_position( + old_path: file_name, + new_path: file_name, + old_line: 2, + new_line: 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(:change_diff_refs) { diff_refs(move_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 the position of the change" do + expect_change_position( + old_path: file_name, + new_path: file_name, + old_line: 2, + new_line: 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(:change_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 + BB + # 3 + C + # + # new diff: + # 1 - BB + # 2 - A + # 3 - C + + it "returns the position of the change" do + expect_change_position( + old_path: file_name, + new_path: file_name, + old_line: 2, + new_line: 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(:change_diff_refs) { diff_refs(create_file_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 the position of the change" do + expect_change_position( + old_path: file_name, + new_path: file_name, + old_line: 2, + new_line: 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(:change_diff_refs) { diff_refs(move_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 the position of the change" do + expect_change_position( + old_path: file_name, + new_path: file_name, + old_line: 3, + new_line: 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(:change_diff_refs) { diff_refs(initial_commit, create_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 the position of the change" do + expect_change_position( + old_path: file_name, + new_path: file_name, + old_line: nil, + new_line: 2 + ) + 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(:change_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 1 A + # 2 2 BB + # 3 3 C + + it "returns the position of the change" do + expect_change_position( + old_path: file_name, + new_path: file_name, + old_line: nil, + 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(initial_commit, update_line_commit) } + let(:new_diff_refs) { diff_refs(move_line_commit, move_second_file_line_commit) } + let(:change_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 1 BB + # 2 2 A + # 3 3 C + + it "returns the position of the change" do + expect_change_position( + old_path: file_name, + new_path: file_name, + old_line: nil, + 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(update_line_commit, update_second_file_line_commit) } + let(:change_diff_refs) { diff_refs(create_file_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 the position of the change" do + expect_change_position( + old_path: file_name, + new_path: file_name, + old_line: 2, + new_line: 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(:change_diff_refs) { diff_refs(move_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 the position of the change" do + expect_change_position( + old_path: file_name, + new_path: file_name, + old_line: 3, + new_line: 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(:change_diff_refs) { diff_refs(move_line_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 the position of the change" do + expect_change_position( + old_path: file_name, + new_path: file_name, + old_line: 1, + new_line: 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(:change_diff_refs) { diff_refs(create_file_commit, initial_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 the position of the change" do + expect_change_position( + old_path: file_name, + new_path: file_name, + old_line: 2, + new_line: 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(:change_diff_refs) { diff_refs(move_line_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 the position of the change" do + expect_change_position( + old_path: file_name, + new_path: file_name, + old_line: 3, + new_line: 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(:change_diff_refs) { diff_refs(move_line_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 the position of the change" do + expect_change_position( + old_path: file_name, + new_path: file_name, + old_line: 1, + new_line: 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 + + describe "typical use scenarios" do + let(:second_branch_name) { "#{branch_name}-2" } + + def expect_new_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| + strategy.trace(old_position) + end + + aggregate_failures do + new_positions.zip(new_attrs).each do |new_position, new_attrs| + if new_attrs&.delete(:change) + expect_change_position(new_attrs, new_position) + else + expect_new_position(new_attrs, new_position) + end + end + 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) } + let(:change_diff_refs) { diff_refs(update_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 }, + { new_path: file_name, new_line: 4, change: true }, + { new_path: file_name, old_line: 3, change: true }, + { old_path: file_name, new_path: file_name, old_line: 5, new_line: 5 }, + { new_path: file_name, old_line: 5, change: true }, + { new_path: file_name, new_line: 7 } + ] + + expect_new_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) } + let(:change_diff_refs) { diff_refs(update_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 }, + { new_path: file_name, new_line: 4, change: true }, + { old_path: file_name, old_line: 3, change: true }, + { old_path: file_name, new_path: file_name, old_line: 5, new_line: 5 }, + { old_path: file_name, old_line: 5, change: true }, + { new_path: file_name, new_line: 7 } + ] + + expect_new_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) } + let(:change_diff_refs) { diff_refs(update_file_again_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 }, + { old_path: file_name, old_line: 2, change: true }, + { old_path: file_name, new_path: file_name, old_line: 3, new_line: 2 }, + { old_path: file_name, old_line: 4, change: true }, + { old_path: file_name, new_path: file_name, old_line: 5, new_line: 4 }, + { new_path: file_name, new_line: 5, change: true }, + { old_path: file_name, old_line: 6, change: true }, + { new_path: file_name, new_line: 6 } + ] + + expect_new_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) } + let(:change_diff_refs) { diff_refs(update_file_again_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_new_positions(old_position_attrs, new_position_attrs) + end + end + + describe "merge of target branch" do + let(:merge_commit) do + second_create_file_commit + + merge_request = create(:merge_request, source_branch: second_branch_name, target_branch: branch_name, source_project: project) + + repository.merge(current_user, merge_request.diff_head_sha, merge_request, "Merge branches") + + 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) } + let(:change_diff_refs) { diff_refs(update_file_again_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_new_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) } + let(:change_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 + 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 }, + { old_path: file_name, old_line: 2, change: true }, + { 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_new_positions(old_position_attrs, new_position_attrs) + end + end + end + end +end diff --git a/spec/lib/gitlab/diff/position_tracer_spec.rb b/spec/lib/gitlab/diff/position_tracer_spec.rb index 866550753a8..79b33d4d276 100644 --- a/spec/lib/gitlab/diff/position_tracer_spec.rb +++ b/spec/lib/gitlab/diff/position_tracer_spec.rb @@ -1,1896 +1,98 @@ require 'spec_helper' describe Gitlab::Diff::PositionTracer 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 comparatively lackluster - # testing of other scenarios. - # - # I did still try to cover most of the obvious and potentially tricky - # scenarios, though. + include PositionTracerHelpers - include RepoHelpers - - let(:project) { create(:project, :repository) } - 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(:change_diff_refs) { raise NotImplementedError } - let(:old_position) { raise NotImplementedError } - - let(:position_tracer) { described_class.new(project: project, 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 text_position_attrs - [:old_line, :new_line] - end - - def position(attrs = {}) - attrs.reverse_merge!( - diff_refs: old_diff_refs + subject do + described_class.new( + project: project, + old_diff_refs: old_diff_refs, + new_diff_refs: new_diff_refs ) - Gitlab::Diff::Position.new(attrs) end - def expect_new_position(attrs, result = subject) - aggregate_failures("expect new position #{attrs.inspect}") do - if attrs.nil? - expect(result[:outdated]).to be_truthy - else - expect(result[:outdated]).to be_falsey + describe '#trace' do + let(:diff_refs) { double(complete?: true) } + let(:project) { double } + let(:old_diff_refs) { diff_refs } + let(:new_diff_refs) { diff_refs } + let(:position) { double(on_text?: on_text?, diff_refs: diff_refs) } + let(:tracer) { double } - new_position = result[:position] - expect(new_position).not_to be_nil + context 'position is on text' do + let(:on_text?) { true } - expect(new_position.diff_refs).to eq(new_diff_refs) + it 'calls LineStrategy#trace' do + expect(Gitlab::Diff::PositionTracer::LineStrategy) + .to receive(:new) + .with(subject) + .and_return(tracer) + expect(tracer).to receive(:trace).with(position) - attrs.each do |attr, value| - if text_position_attrs.include?(attr) - expect(new_position.formatter.send(attr)).to eq(value) - else - expect(new_position.send(attr)).to eq(value) - end - end + subject.trace(position) end end - end - - def expect_change_position(attrs, result = subject) - aggregate_failures("expect change position #{attrs.inspect}") do - expect(result[:outdated]).to be_truthy - - change_position = result[:position] - if attrs.nil? || attrs.empty? - expect(change_position).to be_nil - else - expect(change_position).not_to be_nil - - expect(change_position.diff_refs).to eq(change_diff_refs) - - attrs.each do |attr, value| - if text_position_attrs.include?(attr) - expect(change_position.formatter.send(attr)).to eq(value) - else - expect(change_position.send(attr)).to eq(value) - end - end - 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, - start_branch: branch_name, - branch_name: 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, - start_branch: branch_name, - branch_name: 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, - start_branch: branch_name, - branch_name: 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(:change_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 + A - # 2 + BB - # 3 + C - - it "returns the position of the change" do - expect_change_position( - old_path: file_name, - new_path: file_name, - old_line: 2, - new_line: 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(:change_diff_refs) { diff_refs(update_line_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 the position of the change" do - expect_change_position( - old_path: file_name, - new_path: file_name, - old_line: 3, - new_line: 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(:change_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 the position of the change" do - expect_change_position( - old_path: file_name, - new_path: file_name, - old_line: 2, - new_line: 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(:change_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 the position of the change" do - expect_change_position( - old_path: file_name, - new_path: file_name, - old_line: 3, - new_line: 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(:change_diff_refs) { diff_refs(initial_commit, delete_line_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 position of the change" do - expect_change_position( - old_path: file_name, - new_path: file_name, - old_line: nil, - new_line: 2 - ) - 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(:change_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 the position of the change" do - expect_change_position( - old_path: file_name, - new_path: new_file_name, - old_line: 2, - new_line: 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(:change_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 the position of the change" do - expect_change_position( - old_path: file_name, - new_path: file_name, - old_line: 2, - new_line: 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(:change_diff_refs) { diff_refs(move_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 the position of the change" do - expect_change_position( - old_path: file_name, - new_path: file_name, - old_line: 2, - new_line: 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(:change_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 + BB - # 3 + C - # - # new diff: - # 1 - BB - # 2 - A - # 3 - C - - it "returns the position of the change" do - expect_change_position( - old_path: file_name, - new_path: file_name, - old_line: 2, - new_line: 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(:change_diff_refs) { diff_refs(create_file_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 the position of the change" do - expect_change_position( - old_path: file_name, - new_path: file_name, - old_line: 2, - new_line: 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(:change_diff_refs) { diff_refs(move_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 + context 'position is not on text' do + let(:on_text?) { false } - it "returns the position of the change" do - expect_change_position( - old_path: file_name, - new_path: file_name, - old_line: 3, - new_line: nil - ) - end - end - end - end - end + it 'calls ImageStrategy#trace' do + expect(Gitlab::Diff::PositionTracer::ImageStrategy) + .to receive(:new) + .with(subject) + .and_return(tracer) + expect(tracer).to receive(:trace).with(position) - 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(:change_diff_refs) { diff_refs(initial_commit, create_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 the position of the change" do - expect_change_position( - old_path: file_name, - new_path: file_name, - old_line: nil, - new_line: 2 - ) - 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(:change_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 1 A - # 2 2 BB - # 3 3 C - - it "returns the position of the change" do - expect_change_position( - old_path: file_name, - new_path: file_name, - old_line: nil, - 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(initial_commit, update_line_commit) } - let(:new_diff_refs) { diff_refs(move_line_commit, move_second_file_line_commit) } - let(:change_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 1 BB - # 2 2 A - # 3 3 C - - it "returns the position of the change" do - expect_change_position( - old_path: file_name, - new_path: file_name, - old_line: nil, - 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(update_line_commit, update_second_file_line_commit) } - let(:change_diff_refs) { diff_refs(create_file_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 the position of the change" do - expect_change_position( - old_path: file_name, - new_path: file_name, - old_line: 2, - new_line: 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(:change_diff_refs) { diff_refs(move_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 the position of the change" do - expect_change_position( - old_path: file_name, - new_path: file_name, - old_line: 3, - new_line: 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(:change_diff_refs) { diff_refs(move_line_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 the position of the change" do - expect_change_position( - old_path: file_name, - new_path: file_name, - old_line: 1, - new_line: 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(:change_diff_refs) { diff_refs(create_file_commit, initial_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 the position of the change" do - expect_change_position( - old_path: file_name, - new_path: file_name, - old_line: 2, - new_line: 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(:change_diff_refs) { diff_refs(move_line_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 the position of the change" do - expect_change_position( - old_path: file_name, - new_path: file_name, - old_line: 3, - new_line: 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(:change_diff_refs) { diff_refs(move_line_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 the position of the change" do - expect_change_position( - old_path: file_name, - new_path: file_name, - old_line: 1, - new_line: 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 + subject.trace(position) end end end - describe "typical use scenarios" do - let(:second_branch_name) { "#{branch_name}-2" } - - def expect_new_positions(old_attrs, new_attrs) - old_positions = old_attrs.map do |old_attrs| - position(old_attrs) - end + describe 'diffs methods' do + let(:project) { create(:project, :repository) } + let(:current_user) { project.owner } - new_positions = old_positions.map do |old_position| - position_tracer.trace(old_position) - end - - aggregate_failures do - new_positions.zip(new_attrs).each do |new_position, new_attrs| - if new_attrs&.delete(:change) - expect_change_position(new_attrs, new_position) - else - expect_new_position(new_attrs, new_position) - end - end - 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 + let(:old_diff_refs) do + diff_refs( + project.commit(create_branch('new-branch', 'master')[:branch].name), + create_file('new-branch', 'file.md', '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 + let(:new_diff_refs) do + diff_refs( + create_file('new-branch', 'file.md', 'content'), + update_file('new-branch', 'file.md', 'updatedcontent') ) 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) } - let(:change_diff_refs) { diff_refs(update_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 }, - { new_path: file_name, new_line: 4, change: true }, - { new_path: file_name, old_line: 3, change: true }, - { old_path: file_name, new_path: file_name, old_line: 5, new_line: 5 }, - { new_path: file_name, old_line: 5, change: true }, - { new_path: file_name, new_line: 7 } - ] + describe '#ac_diffs' do + it 'returns the diffs between the base of old and new diff' do + diff_refs = subject.ac_diffs.diff_refs - expect_new_positions(old_position_attrs, new_position_attrs) + expect(diff_refs.base_sha).to eq(old_diff_refs.base_sha) + expect(diff_refs.start_sha).to eq(old_diff_refs.base_sha) + expect(diff_refs.head_sha).to eq(new_diff_refs.base_sha) end end - describe "force push to overwrite last commit" do - let(:second_create_file_commit) do - create_file_commit + describe '#bd_diffs' do + it 'returns the diffs between the HEAD of old and new diff' do + diff_refs = subject.bd_diffs.diff_refs - 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) } - let(:change_diff_refs) { diff_refs(update_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 }, - { new_path: file_name, new_line: 4, change: true }, - { old_path: file_name, old_line: 3, change: true }, - { old_path: file_name, new_path: file_name, old_line: 5, new_line: 5 }, - { old_path: file_name, old_line: 5, change: true }, - { new_path: file_name, new_line: 7 } - ] - - expect_new_positions(old_position_attrs, new_position_attrs) + expect(diff_refs.base_sha).to eq(old_diff_refs.head_sha) + expect(diff_refs.start_sha).to eq(old_diff_refs.head_sha) + expect(diff_refs.head_sha).to eq(new_diff_refs.head_sha) 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) } - let(:change_diff_refs) { diff_refs(update_file_again_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 }, - { old_path: file_name, old_line: 2, change: true }, - { old_path: file_name, new_path: file_name, old_line: 3, new_line: 2 }, - { old_path: file_name, old_line: 4, change: true }, - { old_path: file_name, new_path: file_name, old_line: 5, new_line: 4 }, - { new_path: file_name, new_line: 5, change: true }, - { old_path: file_name, old_line: 6, change: true }, - { new_path: file_name, new_line: 6 } - ] - - expect_new_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) } - let(:change_diff_refs) { diff_refs(update_file_again_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_new_positions(old_position_attrs, new_position_attrs) - end - end - - describe "merge of target branch" do - let(:merge_commit) do - second_create_file_commit - - merge_request = create(:merge_request, source_branch: second_branch_name, target_branch: branch_name, source_project: project) - - repository.merge(current_user, merge_request.diff_head_sha, merge_request, "Merge branches") - - 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) } - let(:change_diff_refs) { diff_refs(update_file_again_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_new_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) } - let(:change_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 + 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 }, - { old_path: file_name, old_line: 2, change: true }, - { 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 } - ] + describe '#cd_diffs' do + it 'returns the diffs in the new diff' do + diff_refs = subject.cd_diffs.diff_refs - expect_new_positions(old_position_attrs, new_position_attrs) + expect(diff_refs.base_sha).to eq(new_diff_refs.base_sha) + expect(diff_refs.start_sha).to eq(new_diff_refs.base_sha) + expect(diff_refs.head_sha).to eq(new_diff_refs.head_sha) end end end diff --git a/spec/services/system_note_service_spec.rb b/spec/services/system_note_service_spec.rb index 9f60e49290e..b934533b1ab 100644 --- a/spec/services/system_note_service_spec.rb +++ b/spec/services/system_note_service_spec.rb @@ -1175,16 +1175,30 @@ describe SystemNoteService do end it 'links to the diff in the system note' do - expect(subject.note).to include('version 1') - diff_id = merge_request.merge_request_diff.id line_code = change_position.line_code(project.repository) - expect(subject.note).to include(diffs_project_merge_request_path(project, merge_request, diff_id: diff_id, anchor: line_code)) + link = diffs_project_merge_request_path(project, merge_request, diff_id: diff_id, anchor: line_code) + + expect(subject.note).to eq("changed this line in [version 1 of the diff](#{link})") + end + + context 'discussion is on an image' do + let(:discussion) { create(:image_diff_note_on_merge_request, project: project).to_discussion } + + it 'links to the diff in the system note' do + diff_id = merge_request.merge_request_diff.id + file_hash = change_position.file_hash + link = diffs_project_merge_request_path(project, merge_request, diff_id: diff_id, anchor: file_hash) + + expect(subject.note).to eq("changed this file in [version 1 of the diff](#{link})") + end end end - context 'when the change_position is invalid for the discussion' do - let(:change_position) { project.commit(sample_commit.id) } + context 'when the change_position does not point to a valid version' do + before do + allow(merge_request).to receive(:version_params_for).and_return(nil) + end it 'creates a new note in the discussion' do # we need to completely rebuild the merge request object, or the `@discussions` on the merge request are not reloaded. diff --git a/spec/support/helpers/position_tracer_helpers.rb b/spec/support/helpers/position_tracer_helpers.rb new file mode 100644 index 00000000000..bbf6e06dd40 --- /dev/null +++ b/spec/support/helpers/position_tracer_helpers.rb @@ -0,0 +1,93 @@ +# frozen_string_literal: true + +module PositionTracerHelpers + 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, result = subject) + aggregate_failures("expect new position #{attrs.inspect}") do + if attrs.nil? + expect(result[:outdated]).to be_truthy + else + new_position = result[:position] + + expect(result[:outdated]).to be_falsey + 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 + end + + def expect_change_position(attrs, result = subject) + aggregate_failures("expect change position #{attrs.inspect}") do + change_position = result[:position] + + expect(result[:outdated]).to be_truthy + + if attrs.nil? || attrs.empty? + expect(change_position).to be_nil + else + expect(change_position).not_to be_nil + expect(change_position.diff_refs).to eq(change_diff_refs) + + attrs.each do |attr, value| + expect(change_position.send(attr)).to eq(value) + end + 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, + start_branch: branch_name, + branch_name: 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, + start_branch: branch_name, + branch_name: 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, + start_branch: branch_name, + branch_name: branch_name, + commit_message: "Delete file", + file_path: file_name + ).execute + project.commit(branch_name) + end +end |