diff options
author | Oswaldo Ferreira <oswaldo@gitlab.com> | 2018-10-16 13:21:16 -0300 |
---|---|---|
committer | Oswaldo Ferreira <oswaldo@gitlab.com> | 2018-11-09 16:56:48 -0200 |
commit | f9b4130bb75adf33fbf2f74fb2662f09d073bd6f (patch) | |
tree | e6734e431b82d518b271218e620c6efec1b4c9bf /spec | |
parent | 5b270431399ca14198d7684c1855de04ee8aec5e (diff) | |
download | gitlab-ce-f9b4130bb75adf33fbf2f74fb2662f09d073bd6f.tar.gz |
Comment on any expanded diff line on MRsosw-comment-on-any-line-on-diffs-w-feature-flag
Diffstat (limited to 'spec')
-rw-r--r-- | spec/controllers/projects/blob_controller_spec.rb | 26 | ||||
-rw-r--r-- | spec/features/merge_request/user_posts_diff_notes_spec.rb | 13 | ||||
-rw-r--r-- | spec/javascripts/diffs/store/mutations_spec.js | 4 | ||||
-rw-r--r-- | spec/lib/gitlab/diff/file_spec.rb | 46 | ||||
-rw-r--r-- | spec/lib/gitlab/diff/lines_unfolder_spec.rb | 750 | ||||
-rw-r--r-- | spec/services/merge_requests/reload_diffs_service_spec.rb | 21 | ||||
-rw-r--r-- | spec/services/notes/create_service_spec.rb | 51 | ||||
-rw-r--r-- | spec/services/notes/destroy_service_spec.rb | 33 |
8 files changed, 913 insertions, 31 deletions
diff --git a/spec/controllers/projects/blob_controller_spec.rb b/spec/controllers/projects/blob_controller_spec.rb index 28f7e4634a5..74771abde71 100644 --- a/spec/controllers/projects/blob_controller_spec.rb +++ b/spec/controllers/projects/blob_controller_spec.rb @@ -141,6 +141,28 @@ describe Projects::BlobController do expect(lines.first).to have_key('rich_text') end + context 'comment in any diff line feature flag' do + it 'renders context lines when feature disabled' do + stub_feature_flags(comment_in_any_diff_line: false) + + do_get(since: 1, to: 5, offset: 10, from_merge_request: true) + lines = JSON.parse(response.body) + all_context = lines.all? { |line| line['type'] == 'context' } + + expect(all_context).to be(true) + end + + it 'renders unchanged lines when feature enabled' do + stub_feature_flags(comment_in_any_diff_line: true) + + do_get(since: 1, to: 5, offset: 10, from_merge_request: true) + lines = JSON.parse(response.body) + all_unchanged = lines.all? { |line| line['type'].nil? } + + expect(all_unchanged).to be(true) + end + end + context 'when rendering match lines' do it 'adds top match line when "since" is less than 1' do do_get(since: 5, to: 10, offset: 10, from_merge_request: true) @@ -157,7 +179,7 @@ describe Projects::BlobController do match_line = JSON.parse(response.body).first - expect(match_line['type']).to eq('context') + expect(match_line['type']).to be_nil end it 'adds bottom match line when "t"o is less than blob size' do @@ -177,7 +199,7 @@ describe Projects::BlobController do match_line = JSON.parse(response.body).last - expect(match_line['type']).to eq('context') + expect(match_line['type']).to be_nil end end end diff --git a/spec/features/merge_request/user_posts_diff_notes_spec.rb b/spec/features/merge_request/user_posts_diff_notes_spec.rb index fa148715855..51b78d3e7d1 100644 --- a/spec/features/merge_request/user_posts_diff_notes_spec.rb +++ b/spec/features/merge_request/user_posts_diff_notes_spec.rb @@ -85,12 +85,13 @@ describe 'Merge request > User posts diff notes', :js do # `.line_holder` will be an unfolded line. let(:line_holder) { first('#a5cc2925ca8258af241be7e5b0381edf30266302 .line_holder') } - it 'does not allow commenting on the left side' do - should_not_allow_commenting(line_holder, 'left') + it 'allows commenting on the left side' do + should_allow_commenting(line_holder, 'left') end - it 'does not allow commenting on the right side' do - should_not_allow_commenting(line_holder, 'right') + it 'allows commenting on the right side' do + # Automatically shifts comment box to left side. + should_allow_commenting(line_holder, 'right') end end end @@ -147,8 +148,8 @@ describe 'Merge request > User posts diff notes', :js do # `.line_holder` will be an unfolded line. let(:line_holder) { first('.line_holder[id="a5cc2925ca8258af241be7e5b0381edf30266302_1_1"]') } - it 'does not allow commenting' do - should_not_allow_commenting line_holder + it 'allows commenting' do + should_allow_commenting line_holder end end diff --git a/spec/javascripts/diffs/store/mutations_spec.js b/spec/javascripts/diffs/store/mutations_spec.js index fed04cbaed8..8821cde76f4 100644 --- a/spec/javascripts/diffs/store/mutations_spec.js +++ b/spec/javascripts/diffs/store/mutations_spec.js @@ -98,7 +98,7 @@ describe('DiffsStoreMutations', () => { it('should call utils.addContextLines with proper params', () => { const options = { lineNumbers: { oldLineNumber: 1, newLineNumber: 2 }, - contextLines: [{ oldLine: 1 }], + contextLines: [{ oldLine: 1, newLine: 1, lineCode: 'ff9200_1_1', discussions: [] }], fileHash: 'ff9200', params: { bottom: true, @@ -110,7 +110,7 @@ describe('DiffsStoreMutations', () => { parallelDiffLines: [], }; const state = { diffFiles: [diffFile] }; - const lines = [{ oldLine: 1 }]; + const lines = [{ oldLine: 1, newLine: 1 }]; const findDiffFileSpy = spyOnDependency(mutations, 'findDiffFile').and.returnValue(diffFile); const removeMatchLineSpy = spyOnDependency(mutations, 'removeMatchLine'); diff --git a/spec/lib/gitlab/diff/file_spec.rb b/spec/lib/gitlab/diff/file_spec.rb index 2f51642b58e..3417896e259 100644 --- a/spec/lib/gitlab/diff/file_spec.rb +++ b/spec/lib/gitlab/diff/file_spec.rb @@ -41,6 +41,52 @@ describe Gitlab::Diff::File do end end + describe '#unfold_diff_lines' do + let(:unfolded_lines) { double('expanded-lines') } + let(:unfolder) { instance_double(Gitlab::Diff::LinesUnfolder) } + let(:position) { instance_double(Gitlab::Diff::Position, old_line: 10) } + + before do + allow(Gitlab::Diff::LinesUnfolder).to receive(:new) { unfolder } + end + + context 'when unfold required' do + before do + allow(unfolder).to receive(:unfold_required?) { true } + allow(unfolder).to receive(:unfolded_diff_lines) { unfolded_lines } + end + + it 'changes @unfolded to true' do + diff_file.unfold_diff_lines(position) + + expect(diff_file).to be_unfolded + end + + it 'updates @diff_lines' do + diff_file.unfold_diff_lines(position) + + expect(diff_file.diff_lines).to eq(unfolded_lines) + end + end + + context 'when unfold not required' do + before do + allow(unfolder).to receive(:unfold_required?) { false } + end + + it 'keeps @unfolded false' do + diff_file.unfold_diff_lines(position) + + expect(diff_file).not_to be_unfolded + end + + it 'does not update @diff_lines' do + expect { diff_file.unfold_diff_lines(position) } + .not_to change(diff_file, :diff_lines) + end + end + end + describe '#mode_changed?' do it { expect(diff_file.mode_changed?).to be_falsey } end diff --git a/spec/lib/gitlab/diff/lines_unfolder_spec.rb b/spec/lib/gitlab/diff/lines_unfolder_spec.rb new file mode 100644 index 00000000000..8e00c8e0e30 --- /dev/null +++ b/spec/lib/gitlab/diff/lines_unfolder_spec.rb @@ -0,0 +1,750 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe Gitlab::Diff::LinesUnfolder do + let(:raw_diff) do + <<-DIFF.strip_heredoc + @@ -7,9 +7,6 @@ + "tags": ["devel", "development", "nightly"], + "desktop-file-name-prefix": "(Development) ", + "finish-args": [ + - "--share=ipc", "--socket=x11", + - "--socket=wayland", + - "--talk-name=org.gnome.OnlineAccounts", + "--talk-name=org.freedesktop.Tracker1", + "--filesystem=home", + "--talk-name=org.gtk.vfs", "--talk-name=org.gtk.vfs.*", + @@ -62,7 +59,7 @@ + }, + { + "name": "gnome-desktop", + - "config-opts": ["--disable-debug-tools", "--disable-udev"], + + "config-opts": ["--disable-debug-tools", "--disable-"], + "sources": [ + { + "type": "git", + @@ -83,11 +80,6 @@ + "buildsystem": "meson", + "builddir": true, + "name": "nautilus", + - "config-opts": [ + - "-Denable-desktop=false", + - "-Denable-selinux=false", + - "--libdir=/app/lib" + - ], + "sources": [ + { + "type": "git", + DIFF + end + + let(:raw_old_blob) do + <<-BLOB.strip_heredoc + { + "app-id": "org.gnome.Nautilus", + "runtime": "org.gnome.Platform", + "runtime-version": "master", + "sdk": "org.gnome.Sdk", + "command": "nautilus", + "tags": ["devel", "development", "nightly"], + "desktop-file-name-prefix": "(Development) ", + "finish-args": [ + "--share=ipc", "--socket=x11", + "--socket=wayland", + "--talk-name=org.gnome.OnlineAccounts", + "--talk-name=org.freedesktop.Tracker1", + "--filesystem=home", + "--talk-name=org.gtk.vfs", "--talk-name=org.gtk.vfs.*", + "--filesystem=xdg-run/dconf", "--filesystem=~/.config/dconf:ro", + "--talk-name=ca.desrt.dconf", "--env=DCONF_USER_CONFIG_DIR=.config/dconf" + ], + "cleanup": [ "/include", "/share/bash-completion" ], + "modules": [ + { + "name": "exiv2", + "sources": [ + { + "type": "archive", + "url": "http://exiv2.org/builds/exiv2-0.26-trunk.tar.gz", + "sha256": "c75e3c4a0811bf700d92c82319373b7a825a2331c12b8b37d41eb58e4f18eafb" + }, + { + "type": "shell", + "commands": [ + "cp -f /usr/share/gnu-config/config.sub ./config/", + "cp -f /usr/share/gnu-config/config.guess ./config/" + ] + } + ] + }, + { + "name": "gexiv2", + "config-opts": [ "--disable-introspection" ], + "sources": [ + { + "type": "git", + "url": "https://git.gnome.org/browse/gexiv2" + } + ] + }, + { + "name": "tracker", + "cleanup": [ "/bin", "/etc", "/libexec" ], + "config-opts": [ "--disable-miner-apps", "--disable-static", + "--disable-tracker-extract", "--disable-tracker-needle", + "--disable-tracker-preferences", "--disable-artwork", + "--disable-tracker-writeback", "--disable-miner-user-guides", + "--with-bash-completion-dir=no" ], + "sources": [ + { + "type": "git", + "url": "https://git.gnome.org/browse/tracker" + } + ] + }, + { + "name": "gnome-desktop", + "config-opts": ["--disable-debug-tools", "--disable-udev"], + "sources": [ + { + "type": "git", + "url": "https://git.gnome.org/browse/gnome-desktop" + } + ] + }, + { + "name": "gnome-autoar", + "sources": [ + { + "type": "git", + "url": "https://git.gnome.org/browse/gnome-autoar" + } + ] + }, + { + "buildsystem": "meson", + "builddir": true, + "name": "nautilus", + "config-opts": [ + "-Denable-desktop=false", + "-Denable-selinux=false", + "--libdir=/app/lib" + ], + "sources": [ + { + "type": "git", + "url": "https://gitlab.gnome.org/GNOME/nautilus.git" + } + ] + } + ] + }, + { + "app-id": "foo", + "runtime": "foo", + "runtime-version": "foo", + "sdk": "foo", + "command": "foo", + "tags": ["foo", "bar", "kux"], + "desktop-file-name-prefix": "(Foo) ", + { + "buildsystem": "meson", + "builddir": true, + "name": "nautilus", + "sources": [ + { + "type": "git", + "url": "https://gitlab.gnome.org/GNOME/nautilus.git" + } + ] + } + }, + { + "app-id": "foo", + "runtime": "foo", + "runtime-version": "foo", + "sdk": "foo", + "command": "foo", + "tags": ["foo", "bar", "kux"], + "desktop-file-name-prefix": "(Foo) ", + { + "buildsystem": "meson", + "builddir": true, + "name": "nautilus", + "sources": [ + { + "type": "git", + "url": "https://gitlab.gnome.org/GNOME/nautilus.git" + } + ] + } + } + BLOB + end + + let(:project) { create(:project) } + + let(:old_blob) { Gitlab::Git::Blob.new(data: raw_old_blob) } + + let(:diff) do + Gitlab::Git::Diff.new(diff: raw_diff, + new_path: "build-aux/flatpak/org.gnome.Nautilus.json", + old_path: "build-aux/flatpak/org.gnome.Nautilus.json", + a_mode: "100644", + b_mode: "100644", + new_file: false, + renamed_file: false, + deleted_file: false, + too_large: false) + end + + let(:diff_file) do + Gitlab::Diff::File.new(diff, repository: project.repository) + end + + before do + allow(old_blob).to receive(:load_all_data!) + allow(diff_file).to receive(:old_blob) { old_blob } + end + + subject { described_class.new(diff_file, position) } + + context 'position requires a middle expansion and new match lines' do + let(:position) do + Gitlab::Diff::Position.new(base_sha: "1c59dfa64afbea8c721bb09a06a9d326c952ea19", + start_sha: "1c59dfa64afbea8c721bb09a06a9d326c952ea19", + head_sha: "1487062132228de836236c522fe52fed4980a46c", + old_path: "build-aux/flatpak/org.gnome.Nautilus.json", + new_path: "build-aux/flatpak/org.gnome.Nautilus.json", + position_type: "text", + old_line: 43, + new_line: 40) + end + + context 'blob lines' do + let(:expected_blob_lines) do + [[40, 40, " \"config-opts\": [ \"--disable-introspection\" ],"], + [41, 41, " \"sources\": ["], + [42, 42, " {"], + [43, 43, " \"type\": \"git\","], + [44, 44, " \"url\": \"https://git.gnome.org/browse/gexiv2\""], + [45, 45, " }"], + [46, 46, " ]"]] + end + + it 'returns the extracted blob lines correctly' do + extracted_lines = subject.blob_lines + + expect(extracted_lines.size).to eq(7) + + extracted_lines.each_with_index do |line, i| + expect([line.old_line, line.new_line, line.text]).to eq(expected_blob_lines[i]) + end + end + end + + context 'diff lines' do + let(:expected_diff_lines) do + [[7, 7, "@@ -7,9 +7,6 @@"], + [7, 7, " \"tags\": [\"devel\", \"development\", \"nightly\"],"], + [8, 8, " \"desktop-file-name-prefix\": \"(Development) \","], + [9, 9, " \"finish-args\": ["], + [10, 10, "- \"--share=ipc\", \"--socket=x11\","], + [11, 10, "- \"--socket=wayland\","], + [12, 10, "- \"--talk-name=org.gnome.OnlineAccounts\","], + [13, 10, " \"--talk-name=org.freedesktop.Tracker1\","], + [14, 11, " \"--filesystem=home\","], + [15, 12, " \"--talk-name=org.gtk.vfs\", \"--talk-name=org.gtk.vfs.*\","], + + # New match line + [40, 37, "@@ -40,7+37,7 @@"], + + # Injected blob lines + [40, 37, " \"config-opts\": [ \"--disable-introspection\" ],"], + [41, 38, " \"sources\": ["], + [42, 39, " {"], + [43, 40, " \"type\": \"git\","], # comment + [44, 41, " \"url\": \"https://git.gnome.org/browse/gexiv2\""], + [45, 42, " }"], + [46, 43, " ]"], + # end + + # Second match line + [62, 59, "@@ -62,7+59,7 @@"], + + [62, 59, " },"], + [63, 60, " {"], + [64, 61, " \"name\": \"gnome-desktop\","], + [65, 62, "- \"config-opts\": [\"--disable-debug-tools\", \"--disable-udev\"],"], + [66, 62, "+ \"config-opts\": [\"--disable-debug-tools\", \"--disable-\"],"], + [66, 63, " \"sources\": ["], + [67, 64, " {"], + [68, 65, " \"type\": \"git\","], + [83, 80, "@@ -83,11 +80,6 @@"], + [83, 80, " \"buildsystem\": \"meson\","], + [84, 81, " \"builddir\": true,"], + [85, 82, " \"name\": \"nautilus\","], + [86, 83, "- \"config-opts\": ["], + [87, 83, "- \"-Denable-desktop=false\","], + [88, 83, "- \"-Denable-selinux=false\","], + [89, 83, "- \"--libdir=/app/lib\""], + [90, 83, "- ],"], + [91, 83, " \"sources\": ["], + [92, 84, " {"], + [93, 85, " \"type\": \"git\","]] + end + + it 'return merge of blob lines with diff lines correctly' do + new_diff_lines = subject.unfolded_diff_lines + + expected_diff_lines.each_with_index do |expected_line, i| + line = new_diff_lines[i] + + expect([line.old_pos, line.new_pos, line.text]) + .to eq([expected_line[0], expected_line[1], expected_line[2]]) + end + end + + it 'merged lines have correct line codes' do + new_diff_lines = subject.unfolded_diff_lines + + new_diff_lines.each_with_index do |line, i| + old_pos, new_pos = expected_diff_lines[i][0], expected_diff_lines[i][1] + + unless line.type == 'match' + expect(line.line_code).to eq(Gitlab::Git.diff_line_code(diff_file.file_path, new_pos, old_pos)) + end + end + end + end + end + + context 'position requires a middle expansion and no top match line' do + let(:position) do + Gitlab::Diff::Position.new(base_sha: "1c59dfa64afbea8c721bb09a06a9d326c952ea19", + start_sha: "1c59dfa64afbea8c721bb09a06a9d326c952ea19", + head_sha: "1487062132228de836236c522fe52fed4980a46c", + old_path: "build-aux/flatpak/org.gnome.Nautilus.json", + new_path: "build-aux/flatpak/org.gnome.Nautilus.json", + position_type: "text", + old_line: 16, + new_line: 17) + end + + context 'blob lines' do + let(:expected_blob_lines) do + [[16, 16, " \"--filesystem=xdg-run/dconf\", \"--filesystem=~/.config/dconf:ro\","], + [17, 17, " \"--talk-name=ca.desrt.dconf\", \"--env=DCONF_USER_CONFIG_DIR=.config/dconf\""], + [18, 18, " ],"], + [19, 19, " \"cleanup\": [ \"/include\", \"/share/bash-completion\" ],"]] + end + + it 'returns the extracted blob lines correctly' do + extracted_lines = subject.blob_lines + + expect(extracted_lines.size).to eq(4) + + extracted_lines.each_with_index do |line, i| + expect([line.old_line, line.new_line, line.text]).to eq(expected_blob_lines[i]) + end + end + end + + context 'diff lines' do + let(:expected_diff_lines) do + [[7, 7, "@@ -7,9 +7,6 @@"], + [7, 7, " \"tags\": [\"devel\", \"development\", \"nightly\"],"], + [8, 8, " \"desktop-file-name-prefix\": \"(Development) \","], + [9, 9, " \"finish-args\": ["], + [10, 10, "- \"--share=ipc\", \"--socket=x11\","], + [11, 10, "- \"--socket=wayland\","], + [12, 10, "- \"--talk-name=org.gnome.OnlineAccounts\","], + [13, 10, " \"--talk-name=org.freedesktop.Tracker1\","], + [14, 11, " \"--filesystem=home\","], + [15, 12, " \"--talk-name=org.gtk.vfs\", \"--talk-name=org.gtk.vfs.*\","], + # No new match needed + + # Injected blob lines + [16, 13, " \"--filesystem=xdg-run/dconf\", \"--filesystem=~/.config/dconf:ro\","], + [17, 14, " \"--talk-name=ca.desrt.dconf\", \"--env=DCONF_USER_CONFIG_DIR=.config/dconf\""], + [18, 15, " ],"], + [19, 16, " \"cleanup\": [ \"/include\", \"/share/bash-completion\" ],"], + # end + + # Second match line + [62, 59, "@@ -62,4+59,4 @@"], + + [62, 59, " },"], + [63, 60, " {"], + [64, 61, " \"name\": \"gnome-desktop\","], + [65, 62, "- \"config-opts\": [\"--disable-debug-tools\", \"--disable-udev\"],"], + [66, 62, "+ \"config-opts\": [\"--disable-debug-tools\", \"--disable-\"],"], + [66, 63, " \"sources\": ["], + [67, 64, " {"], + [68, 65, " \"type\": \"git\","], + [83, 80, "@@ -83,11 +80,6 @@"], + [83, 80, " \"buildsystem\": \"meson\","], + [84, 81, " \"builddir\": true,"], + [85, 82, " \"name\": \"nautilus\","], + [86, 83, "- \"config-opts\": ["], + [87, 83, "- \"-Denable-desktop=false\","], + [88, 83, "- \"-Denable-selinux=false\","], + [89, 83, "- \"--libdir=/app/lib\""], + [90, 83, "- ],"], + [91, 83, " \"sources\": ["], + [92, 84, " {"], + [93, 85, " \"type\": \"git\","]] + end + + it 'return merge of blob lines with diff lines correctly' do + new_diff_lines = subject.unfolded_diff_lines + + expected_diff_lines.each_with_index do |expected_line, i| + line = new_diff_lines[i] + + expect([line.old_pos, line.new_pos, line.text]) + .to eq([expected_line[0], expected_line[1], expected_line[2]]) + end + end + + it 'merged lines have correct line codes' do + new_diff_lines = subject.unfolded_diff_lines + + new_diff_lines.each_with_index do |line, i| + old_pos, new_pos = expected_diff_lines[i][0], expected_diff_lines[i][1] + + unless line.type == 'match' + expect(line.line_code).to eq(Gitlab::Git.diff_line_code(diff_file.file_path, new_pos, old_pos)) + end + end + end + end + end + + context 'position requires a middle expansion and no bottom match line' do + let(:position) do + Gitlab::Diff::Position.new(base_sha: "1c59dfa64afbea8c721bb09a06a9d326c952ea19", + start_sha: "1c59dfa64afbea8c721bb09a06a9d326c952ea19", + head_sha: "1487062132228de836236c522fe52fed4980a46c", + old_path: "build-aux/flatpak/org.gnome.Nautilus.json", + new_path: "build-aux/flatpak/org.gnome.Nautilus.json", + position_type: "text", + old_line: 82, + new_line: 79) + end + + context 'blob lines' do + let(:expected_blob_lines) do + [[79, 79, " }"], + [80, 80, " ]"], + [81, 81, " },"], + [82, 82, " {"]] + end + + it 'returns the extracted blob lines correctly' do + extracted_lines = subject.blob_lines + + expect(extracted_lines.size).to eq(4) + + extracted_lines.each_with_index do |line, i| + expect([line.old_line, line.new_line, line.text]).to eq(expected_blob_lines[i]) + end + end + end + + context 'diff lines' do + let(:expected_diff_lines) do + [[7, 7, "@@ -7,9 +7,6 @@"], + [7, 7, " \"tags\": [\"devel\", \"development\", \"nightly\"],"], + [8, 8, " \"desktop-file-name-prefix\": \"(Development) \","], + [9, 9, " \"finish-args\": ["], + [10, 10, "- \"--share=ipc\", \"--socket=x11\","], + [11, 10, "- \"--socket=wayland\","], + [12, 10, "- \"--talk-name=org.gnome.OnlineAccounts\","], + [13, 10, " \"--talk-name=org.freedesktop.Tracker1\","], + [14, 11, " \"--filesystem=home\","], + [15, 12, " \"--talk-name=org.gtk.vfs\", \"--talk-name=org.gtk.vfs.*\","], + [62, 59, "@@ -62,7 +59,7 @@"], + [62, 59, " },"], + [63, 60, " {"], + [64, 61, " \"name\": \"gnome-desktop\","], + [65, 62, "- \"config-opts\": [\"--disable-debug-tools\", \"--disable-udev\"],"], + [66, 62, "+ \"config-opts\": [\"--disable-debug-tools\", \"--disable-\"],"], + [66, 63, " \"sources\": ["], + [67, 64, " {"], + [68, 65, " \"type\": \"git\","], + + # New top match line + [79, 76, "@@ -79,4+76,4 @@"], + + # Injected blob lines + [79, 76, " }"], + [80, 77, " ]"], + [81, 78, " },"], + [82, 79, " {"], + # end + + # No new second match line + [83, 80, " \"buildsystem\": \"meson\","], + [84, 81, " \"builddir\": true,"], + [85, 82, " \"name\": \"nautilus\","], + [86, 83, "- \"config-opts\": ["], + [87, 83, "- \"-Denable-desktop=false\","], + [88, 83, "- \"-Denable-selinux=false\","], + [89, 83, "- \"--libdir=/app/lib\""], + [90, 83, "- ],"], + [91, 83, " \"sources\": ["], + [92, 84, " {"], + [93, 85, " \"type\": \"git\","]] + end + + it 'return merge of blob lines with diff lines correctly' do + new_diff_lines = subject.unfolded_diff_lines + + expected_diff_lines.each_with_index do |expected_line, i| + line = new_diff_lines[i] + + expect([line.old_pos, line.new_pos, line.text]) + .to eq([expected_line[0], expected_line[1], expected_line[2]]) + end + end + + it 'merged lines have correct line codes' do + new_diff_lines = subject.unfolded_diff_lines + + new_diff_lines.each_with_index do |line, i| + old_pos, new_pos = expected_diff_lines[i][0], expected_diff_lines[i][1] + + unless line.type == 'match' + expect(line.line_code).to eq(Gitlab::Git.diff_line_code(diff_file.file_path, new_pos, old_pos)) + end + end + end + end + end + + context 'position requires a short top expansion' do + let(:position) do + Gitlab::Diff::Position.new(base_sha: "1c59dfa64afbea8c721bb09a06a9d326c952ea19", + start_sha: "1c59dfa64afbea8c721bb09a06a9d326c952ea19", + head_sha: "1487062132228de836236c522fe52fed4980a46c", + old_path: "build-aux/flatpak/org.gnome.Nautilus.json", + new_path: "build-aux/flatpak/org.gnome.Nautilus.json", + position_type: "text", + old_line: 6, + new_line: 6) + end + + context 'blob lines' do + let(:expected_blob_lines) do + [[3, 3, " \"runtime\": \"org.gnome.Platform\","], + [4, 4, " \"runtime-version\": \"master\","], + [5, 5, " \"sdk\": \"org.gnome.Sdk\","], + [6, 6, " \"command\": \"nautilus\","]] + end + + it 'returns the extracted blob lines correctly' do + extracted_lines = subject.blob_lines + + expect(extracted_lines.size).to eq(4) + + extracted_lines.each_with_index do |line, i| + expect([line.old_line, line.new_line, line.text]).to eq(expected_blob_lines[i]) + end + end + end + + context 'diff lines' do + let(:expected_diff_lines) do + # New match line + [[3, 3, "@@ -3,4+3,4 @@"], + + # Injected blob lines + [3, 3, " \"runtime\": \"org.gnome.Platform\","], + [4, 4, " \"runtime-version\": \"master\","], + [5, 5, " \"sdk\": \"org.gnome.Sdk\","], + [6, 6, " \"command\": \"nautilus\","], + # end + [7, 7, " \"tags\": [\"devel\", \"development\", \"nightly\"],"], + [8, 8, " \"desktop-file-name-prefix\": \"(Development) \","], + [9, 9, " \"finish-args\": ["], + [10, 10, "- \"--share=ipc\", \"--socket=x11\","], + [11, 10, "- \"--socket=wayland\","], + [12, 10, "- \"--talk-name=org.gnome.OnlineAccounts\","], + [13, 10, " \"--talk-name=org.freedesktop.Tracker1\","], + [14, 11, " \"--filesystem=home\","], + [15, 12, " \"--talk-name=org.gtk.vfs\", \"--talk-name=org.gtk.vfs.*\","], + [62, 59, "@@ -62,7 +59,7 @@"], + [62, 59, " },"], + [63, 60, " {"], + [64, 61, " \"name\": \"gnome-desktop\","], + [65, 62, "- \"config-opts\": [\"--disable-debug-tools\", \"--disable-udev\"],"], + [66, 62, "+ \"config-opts\": [\"--disable-debug-tools\", \"--disable-\"],"], + [66, 63, " \"sources\": ["], + [67, 64, " {"], + [68, 65, " \"type\": \"git\","], + [83, 80, "@@ -83,11 +80,6 @@"], + [83, 80, " \"buildsystem\": \"meson\","], + [84, 81, " \"builddir\": true,"], + [85, 82, " \"name\": \"nautilus\","], + [86, 83, "- \"config-opts\": ["], + [87, 83, "- \"-Denable-desktop=false\","], + [88, 83, "- \"-Denable-selinux=false\","], + [89, 83, "- \"--libdir=/app/lib\""], + [90, 83, "- ],"], + [91, 83, " \"sources\": ["], + [92, 84, " {"], + [93, 85, " \"type\": \"git\","]] + end + + it 'return merge of blob lines with diff lines correctly' do + new_diff_lines = subject.unfolded_diff_lines + + expected_diff_lines.each_with_index do |expected_line, i| + line = new_diff_lines[i] + + expect([line.old_pos, line.new_pos, line.text]) + .to eq([expected_line[0], expected_line[1], expected_line[2]]) + end + end + + it 'merged lines have correct line codes' do + new_diff_lines = subject.unfolded_diff_lines + + new_diff_lines.each_with_index do |line, i| + old_pos, new_pos = expected_diff_lines[i][0], expected_diff_lines[i][1] + + unless line.type == 'match' + expect(line.line_code).to eq(Gitlab::Git.diff_line_code(diff_file.file_path, new_pos, old_pos)) + end + end + end + end + end + + context 'position sits between two match lines (no expasion needed)' do + let(:position) do + Gitlab::Diff::Position.new(base_sha: "1c59dfa64afbea8c721bb09a06a9d326c952ea19", + start_sha: "1c59dfa64afbea8c721bb09a06a9d326c952ea19", + head_sha: "1487062132228de836236c522fe52fed4980a46c", + old_path: "build-aux/flatpak/org.gnome.Nautilus.json", + new_path: "build-aux/flatpak/org.gnome.Nautilus.json", + position_type: "text", + old_line: 64, + new_line: 61) + end + + context 'diff lines' do + it 'returns nil' do + expect(subject.unfolded_diff_lines).to be_nil + end + end + end + + context 'position requires bottom expansion and new match lines' do + let(:position) do + Gitlab::Diff::Position.new(base_sha: "1c59dfa64afbea8c721bb09a06a9d326c952ea19", + start_sha: "1c59dfa64afbea8c721bb09a06a9d326c952ea19", + head_sha: "1487062132228de836236c522fe52fed4980a46c", + old_path: "build-aux/flatpak/org.gnome.Nautilus.json", + new_path: "build-aux/flatpak/org.gnome.Nautilus.json", + position_type: "text", + old_line: 107, + new_line: 99) + end + + context 'blob lines' do + let(:expected_blob_lines) do + [[104, 104, " \"sdk\": \"foo\","], + [105, 105, " \"command\": \"foo\","], + [106, 106, " \"tags\": [\"foo\", \"bar\", \"kux\"],"], + [107, 107, " \"desktop-file-name-prefix\": \"(Foo) \","], + [108, 108, " {"], + [109, 109, " \"buildsystem\": \"meson\","], + [110, 110, " \"builddir\": true,"]] + end + + it 'returns the extracted blob lines correctly' do + extracted_lines = subject.blob_lines + + expect(extracted_lines.size).to eq(7) + + extracted_lines.each_with_index do |line, i| + expect([line.old_line, line.new_line, line.text]).to eq(expected_blob_lines[i]) + end + end + end + + context 'diff lines' do + let(:expected_diff_lines) do + [[7, 7, "@@ -7,9 +7,6 @@"], + [7, 7, " \"tags\": [\"devel\", \"development\", \"nightly\"],"], + [8, 8, " \"desktop-file-name-prefix\": \"(Development) \","], + [9, 9, " \"finish-args\": ["], + [10, 10, "- \"--share=ipc\", \"--socket=x11\","], + [11, 10, "- \"--socket=wayland\","], + [12, 10, "- \"--talk-name=org.gnome.OnlineAccounts\","], + [13, 10, " \"--talk-name=org.freedesktop.Tracker1\","], + [14, 11, " \"--filesystem=home\","], + [15, 12, " \"--talk-name=org.gtk.vfs\", \"--talk-name=org.gtk.vfs.*\","], + [62, 59, "@@ -62,7 +59,7 @@"], + [62, 59, " },"], + [63, 60, " {"], + [64, 61, " \"name\": \"gnome-desktop\","], + [65, 62, "- \"config-opts\": [\"--disable-debug-tools\", \"--disable-udev\"],"], + [66, 62, "+ \"config-opts\": [\"--disable-debug-tools\", \"--disable-\"],"], + [66, 63, " \"sources\": ["], + [67, 64, " {"], + [68, 65, " \"type\": \"git\","], + [83, 80, "@@ -83,11 +80,6 @@"], + [83, 80, " \"buildsystem\": \"meson\","], + [84, 81, " \"builddir\": true,"], + [85, 82, " \"name\": \"nautilus\","], + [86, 83, "- \"config-opts\": ["], + [87, 83, "- \"-Denable-desktop=false\","], + [88, 83, "- \"-Denable-selinux=false\","], + [89, 83, "- \"--libdir=/app/lib\""], + [90, 83, "- ],"], + [91, 83, " \"sources\": ["], + [92, 84, " {"], + [93, 85, " \"type\": \"git\","], + # New match line + [104, 96, "@@ -104,7+96,7 @@"], + + # Injected blob lines + [104, 96, " \"sdk\": \"foo\","], + [105, 97, " \"command\": \"foo\","], + [106, 98, " \"tags\": [\"foo\", \"bar\", \"kux\"],"], + [107, 99, " \"desktop-file-name-prefix\": \"(Foo) \","], + [108, 100, " {"], + [109, 101, " \"buildsystem\": \"meson\","], + [110, 102, " \"builddir\": true,"]] + # end + end + + it 'return merge of blob lines with diff lines correctly' do + new_diff_lines = subject.unfolded_diff_lines + + expected_diff_lines.each_with_index do |expected_line, i| + line = new_diff_lines[i] + + expect([line.old_pos, line.new_pos, line.text]) + .to eq([expected_line[0], expected_line[1], expected_line[2]]) + end + end + + it 'merged lines have correct line codes' do + new_diff_lines = subject.unfolded_diff_lines + + new_diff_lines.each_with_index do |line, i| + old_pos, new_pos = expected_diff_lines[i][0], expected_diff_lines[i][1] + + unless line.type == 'match' + expect(line.line_code).to eq(Gitlab::Git.diff_line_code(diff_file.file_path, new_pos, old_pos)) + end + end + end + end + end +end diff --git a/spec/services/merge_requests/reload_diffs_service_spec.rb b/spec/services/merge_requests/reload_diffs_service_spec.rb index 546c9f277c5..5acd01828cb 100644 --- a/spec/services/merge_requests/reload_diffs_service_spec.rb +++ b/spec/services/merge_requests/reload_diffs_service_spec.rb @@ -31,32 +31,11 @@ describe MergeRequests::ReloadDiffsService, :use_clean_rails_memory_store_cachin end context 'cache clearing' do - before do - allow_any_instance_of(Gitlab::Diff::File).to receive(:text?).and_return(true) - allow_any_instance_of(Gitlab::Diff::File).to receive(:diffable?).and_return(true) - end - - it 'retrieves the diff files to cache the highlighted result' do - new_diff = merge_request.create_merge_request_diff - cache_key = new_diff.diffs_collection.cache_key - - expect(merge_request).to receive(:create_merge_request_diff).and_return(new_diff) - expect(Rails.cache).to receive(:read).with(cache_key).and_call_original - expect(Rails.cache).to receive(:write).with(cache_key, anything, anything).and_call_original - - subject.execute - end - it 'clears the cache for older diffs on the merge request' do old_diff = merge_request.merge_request_diff old_cache_key = old_diff.diffs_collection.cache_key - new_diff = merge_request.create_merge_request_diff - new_cache_key = new_diff.diffs_collection.cache_key - expect(merge_request).to receive(:create_merge_request_diff).and_return(new_diff) expect(Rails.cache).to receive(:delete).with(old_cache_key).and_call_original - expect(Rails.cache).to receive(:read).with(new_cache_key).and_call_original - expect(Rails.cache).to receive(:write).with(new_cache_key, anything, anything).and_call_original subject.execute end diff --git a/spec/services/notes/create_service_spec.rb b/spec/services/notes/create_service_spec.rb index b1290fd0d47..80b015d4cd0 100644 --- a/spec/services/notes/create_service_spec.rb +++ b/spec/services/notes/create_service_spec.rb @@ -57,6 +57,57 @@ describe Notes::CreateService do end end + context 'noteable highlight cache clearing' do + let(:project_with_repo) { create(:project, :repository) } + let(:merge_request) do + create(:merge_request, source_project: project_with_repo, + target_project: project_with_repo) + end + + let(:position) do + Gitlab::Diff::Position.new(old_path: "files/ruby/popen.rb", + new_path: "files/ruby/popen.rb", + old_line: nil, + new_line: 14, + diff_refs: merge_request.diff_refs) + end + + let(:new_opts) do + opts.merge(in_reply_to_discussion_id: nil, + type: 'DiffNote', + noteable_type: 'MergeRequest', + noteable_id: merge_request.id, + position: position.to_h) + end + + before do + allow_any_instance_of(Gitlab::Diff::Position) + .to receive(:unfolded_diff?) { true } + end + + it 'clears noteable diff cache when it was unfolded for the note position' do + expect_any_instance_of(Gitlab::Diff::HighlightCache).to receive(:clear) + + described_class.new(project_with_repo, user, new_opts).execute + end + + it 'does not clear cache when note is not the first of the discussion' do + prev_note = + create(:diff_note_on_merge_request, noteable: merge_request, + project: project_with_repo) + reply_opts = + opts.merge(in_reply_to_discussion_id: prev_note.discussion_id, + type: 'DiffNote', + noteable_type: 'MergeRequest', + noteable_id: merge_request.id, + position: position.to_h) + + expect(merge_request).not_to receive(:diffs) + + described_class.new(project_with_repo, user, reply_opts).execute + end + end + context 'note diff file' do let(:project_with_repo) { create(:project, :repository) } let(:merge_request) do diff --git a/spec/services/notes/destroy_service_spec.rb b/spec/services/notes/destroy_service_spec.rb index 64445be560e..b1f4e87e8ea 100644 --- a/spec/services/notes/destroy_service_spec.rb +++ b/spec/services/notes/destroy_service_spec.rb @@ -21,5 +21,38 @@ describe Notes::DestroyService do expect { described_class.new(project, user).execute(note) } .to change { user.todos_pending_count }.from(1).to(0) end + + context 'noteable highlight cache clearing' do + let(:repo_project) { create(:project, :repository) } + let(:merge_request) do + create(:merge_request, source_project: repo_project, + target_project: repo_project) + end + + let(:note) do + create(:diff_note_on_merge_request, project: repo_project, + noteable: merge_request) + end + + before do + allow(note.position).to receive(:unfolded_diff?) { true } + end + + it 'clears noteable diff cache when it was unfolded for the note position' do + expect(merge_request).to receive_message_chain(:diffs, :clear_cache) + + described_class.new(repo_project, user).execute(note) + end + + it 'does not clear cache when note is not the first of the discussion' do + reply_note = create(:diff_note_on_merge_request, in_reply_to: note, + project: repo_project, + noteable: merge_request) + + expect(merge_request).not_to receive(:diffs) + + described_class.new(repo_project, user).execute(reply_note) + end + end end end |