summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorPatrick Bajao <ebajao@gitlab.com>2019-07-05 11:03:47 +0000
committerDouwe Maan <douwe@gitlab.com>2019-07-05 11:03:47 +0000
commit6ef6693e3767d480362ce528dd0beff159c895ff (patch)
treed2f55dee037e7258ce8eec829bca4bb77f34a745
parenta1edc5a5cb3e740b0c215c54ce282535b143a9db (diff)
downloadgitlab-ce-6ef6693e3767d480362ce528dd0beff159c895ff.tar.gz
Refactor PositionTracer to support different types
This is to prepare for supporing image type position tracing
-rw-r--r--app/services/discussions/update_diff_position_service.rb3
-rw-r--r--app/services/system_note_service.rb8
-rw-r--r--changelogs/unreleased/57793-fix-line-age.yml5
-rw-r--r--lib/gitlab/diff/position.rb4
-rw-r--r--lib/gitlab/diff/position_tracer.rb192
-rw-r--r--lib/gitlab/diff/position_tracer/base_strategy.rb26
-rw-r--r--lib/gitlab/diff/position_tracer/image_strategy.rb50
-rw-r--r--lib/gitlab/diff/position_tracer/line_strategy.rb201
-rw-r--r--spec/lib/gitlab/diff/position_spec.rb13
-rw-r--r--spec/lib/gitlab/diff/position_tracer/image_strategy_spec.rb238
-rw-r--r--spec/lib/gitlab/diff/position_tracer/line_strategy_spec.rb1805
-rw-r--r--spec/lib/gitlab/diff/position_tracer_spec.rb1918
-rw-r--r--spec/services/system_note_service_spec.rb24
-rw-r--r--spec/support/helpers/position_tracer_helpers.rb93
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