2016-06-28 12:25:32 -04:00
|
|
|
require 'spec_helper'
|
|
|
|
|
2017-07-10 10:24:02 -04:00
|
|
|
describe MergeRequestDiff do
|
2017-11-21 11:58:08 -05:00
|
|
|
let(:diff_with_commits) { create(:merge_request).merge_request_diff }
|
|
|
|
|
2019-03-04 13:36:34 -05:00
|
|
|
describe 'validations' do
|
|
|
|
subject { diff_with_commits }
|
|
|
|
|
|
|
|
it 'checks sha format of base_commit_sha, head_commit_sha and start_commit_sha' do
|
|
|
|
subject.base_commit_sha = subject.head_commit_sha = subject.start_commit_sha = 'foobar'
|
|
|
|
|
|
|
|
expect(subject.valid?).to be false
|
|
|
|
expect(subject.errors.count).to eq 3
|
|
|
|
expect(subject.errors).to all(include('is not a valid SHA'))
|
|
|
|
end
|
|
|
|
end
|
|
|
|
|
2016-07-28 16:40:13 -04:00
|
|
|
describe 'create new record' do
|
2017-11-21 11:58:08 -05:00
|
|
|
subject { diff_with_commits }
|
2016-07-28 16:40:13 -04:00
|
|
|
|
|
|
|
it { expect(subject).to be_valid }
|
2016-08-01 11:41:21 -04:00
|
|
|
it { expect(subject).to be_persisted }
|
2016-10-03 08:11:16 -04:00
|
|
|
it { expect(subject.commits.count).to eq(29) }
|
|
|
|
it { expect(subject.diffs.count).to eq(20) }
|
|
|
|
it { expect(subject.head_commit_sha).to eq('b83d6e391c22777fca1ed3012fce84f633d7fed0') }
|
2016-08-02 08:38:03 -04:00
|
|
|
it { expect(subject.base_commit_sha).to eq('ae73cb07c9eeaf35924a10f713b364d32b2dd34f') }
|
|
|
|
it { expect(subject.start_commit_sha).to eq('0b4bc9a49b562e85de7cc9e834518ea6828729b9') }
|
2016-07-28 16:40:13 -04:00
|
|
|
end
|
|
|
|
|
2018-01-12 15:38:36 -05:00
|
|
|
describe '.by_commit_sha' do
|
|
|
|
subject(:by_commit_sha) { described_class.by_commit_sha(sha) }
|
|
|
|
|
|
|
|
let!(:merge_request) { create(:merge_request, :with_diffs) }
|
|
|
|
|
|
|
|
context 'with sha contained in' do
|
|
|
|
let(:sha) { 'b83d6e391c22777fca1ed3012fce84f633d7fed0' }
|
|
|
|
|
|
|
|
it 'returns merge request diffs' do
|
|
|
|
expect(by_commit_sha).to eq([merge_request.merge_request_diff])
|
|
|
|
end
|
|
|
|
end
|
|
|
|
|
|
|
|
context 'with sha not contained in' do
|
|
|
|
let(:sha) { 'b83d6e3' }
|
|
|
|
|
|
|
|
it 'returns empty result' do
|
|
|
|
expect(by_commit_sha).to be_empty
|
|
|
|
end
|
|
|
|
end
|
|
|
|
end
|
|
|
|
|
2016-08-25 03:59:30 -04:00
|
|
|
describe '#latest' do
|
|
|
|
let!(:mr) { create(:merge_request, :with_diffs) }
|
|
|
|
let!(:first_diff) { mr.merge_request_diff }
|
|
|
|
let!(:last_diff) { mr.create_merge_request_diff }
|
|
|
|
|
Use latest_merge_request_diff association
Compared to the merge_request_diff association:
1. It's simpler to query. The query uses a foreign key to the
merge_request_diffs table, so no ordering is necessary.
2. It's faster for preloading. The merge_request_diff association has to load
every diff for the MRs in the set, then discard all but the most recent for
each. This association means that Rails can just query for N diffs from N
MRs.
3. It's more complicated to update. This is a bidirectional foreign key, so we
need to update two tables when adding a diff record. This also means we need
to handle this as a special case when importing a GitLab project.
There is some juggling with this association in the merge request model:
* `MergeRequest#latest_merge_request_diff` is _always_ the latest diff.
* `MergeRequest#merge_request_diff` reuses
`MergeRequest#latest_merge_request_diff` unless:
* Arguments are passed. These are typically to force-reload the association.
* It doesn't exist. That means we might be trying to implicitly create a
diff. This only seems to happen in specs.
* The association is already loaded. This is important for the reasons
explained in the comment, which I'll reiterate here: if we a) load a
non-latest diff, then b) get its `merge_request`, then c) get that MR's
`merge_request_diff`, we should get the diff we loaded in c), even though
that's not the latest diff.
Basically, `MergeRequest#merge_request_diff` is the latest diff in most cases,
but not quite all.
2017-11-15 12:22:18 -05:00
|
|
|
it { expect(last_diff.reload).to be_latest }
|
|
|
|
it { expect(first_diff.reload).not_to be_latest }
|
2016-08-25 03:59:30 -04:00
|
|
|
end
|
|
|
|
|
2019-01-09 12:01:28 -05:00
|
|
|
shared_examples_for 'merge request diffs' do
|
2018-06-11 16:45:16 -04:00
|
|
|
let(:merge_request) { create(:merge_request, :with_diffs) }
|
|
|
|
let!(:diff) { merge_request.merge_request_diff.reload }
|
|
|
|
|
|
|
|
context 'when it was not cleaned by the system' do
|
|
|
|
it 'returns persisted diffs' do
|
2018-11-01 12:51:42 -04:00
|
|
|
expect(diff).to receive(:load_diffs).and_call_original
|
2018-06-11 16:45:16 -04:00
|
|
|
|
2018-11-01 12:51:42 -04:00
|
|
|
diff.diffs.diff_files
|
2018-06-11 16:45:16 -04:00
|
|
|
end
|
|
|
|
end
|
|
|
|
|
|
|
|
context 'when diff was cleaned by the system' do
|
|
|
|
before do
|
|
|
|
diff.clean!
|
|
|
|
end
|
|
|
|
|
|
|
|
it 'returns diffs from repository if can compare with current diff refs' do
|
|
|
|
expect(diff).not_to receive(:load_diffs)
|
|
|
|
|
|
|
|
expect(Compare)
|
|
|
|
.to receive(:new)
|
|
|
|
.with(instance_of(Gitlab::Git::Compare), merge_request.target_project,
|
|
|
|
base_sha: diff.base_commit_sha, straight: false)
|
|
|
|
.and_call_original
|
|
|
|
|
|
|
|
diff.diffs
|
|
|
|
end
|
|
|
|
|
|
|
|
it 'returns persisted diffs if cannot compare with diff refs' do
|
2018-11-01 12:51:42 -04:00
|
|
|
expect(diff).to receive(:load_diffs).and_call_original
|
2018-06-11 16:45:16 -04:00
|
|
|
|
2019-03-04 13:36:34 -05:00
|
|
|
diff.update!(head_commit_sha: Digest::SHA1.hexdigest(SecureRandom.hex))
|
2018-06-11 16:45:16 -04:00
|
|
|
|
2018-11-01 12:51:42 -04:00
|
|
|
diff.diffs.diff_files
|
2018-06-11 16:45:16 -04:00
|
|
|
end
|
2018-07-17 16:19:22 -04:00
|
|
|
|
|
|
|
it 'returns persisted diffs if diff refs does not exist' do
|
2018-11-01 12:51:42 -04:00
|
|
|
expect(diff).to receive(:load_diffs).and_call_original
|
2018-07-17 16:19:22 -04:00
|
|
|
|
|
|
|
diff.update!(start_commit_sha: nil, base_commit_sha: nil)
|
|
|
|
|
2018-11-01 12:51:42 -04:00
|
|
|
diff.diffs.diff_files
|
2018-07-17 16:19:22 -04:00
|
|
|
end
|
2018-06-11 16:45:16 -04:00
|
|
|
end
|
|
|
|
|
2019-01-09 12:01:28 -05:00
|
|
|
describe '#raw_diffs' do
|
|
|
|
context 'when the :ignore_whitespace_change option is set' do
|
|
|
|
it 'creates a new compare object instead of using preprocessed data' do
|
|
|
|
expect(diff_with_commits).not_to receive(:load_diffs)
|
|
|
|
expect(diff_with_commits.compare).to receive(:diffs).and_call_original
|
2016-06-28 12:25:32 -04:00
|
|
|
|
2019-01-09 12:01:28 -05:00
|
|
|
diff_with_commits.raw_diffs(ignore_whitespace_change: true)
|
|
|
|
end
|
2016-06-28 12:25:32 -04:00
|
|
|
end
|
|
|
|
|
2019-01-09 12:01:28 -05:00
|
|
|
context 'when the raw diffs are empty' do
|
|
|
|
before do
|
|
|
|
MergeRequestDiffFile.where(merge_request_diff_id: diff_with_commits.id).delete_all
|
|
|
|
end
|
2016-10-08 01:48:23 -04:00
|
|
|
|
2019-01-09 12:01:28 -05:00
|
|
|
it 'returns an empty DiffCollection' do
|
|
|
|
expect(diff_with_commits.raw_diffs).to be_a(Gitlab::Git::DiffCollection)
|
|
|
|
expect(diff_with_commits.raw_diffs).to be_empty
|
|
|
|
end
|
2016-10-08 01:48:23 -04:00
|
|
|
end
|
|
|
|
|
2019-01-09 12:01:28 -05:00
|
|
|
context 'when the raw diffs exist' do
|
|
|
|
it 'returns the diffs' do
|
|
|
|
expect(diff_with_commits.raw_diffs).to be_a(Gitlab::Git::DiffCollection)
|
|
|
|
expect(diff_with_commits.raw_diffs).not_to be_empty
|
|
|
|
end
|
2016-06-28 12:25:32 -04:00
|
|
|
|
2019-01-09 12:01:28 -05:00
|
|
|
context 'when the :paths option is set' do
|
|
|
|
let(:diffs) { diff_with_commits.raw_diffs(paths: ['files/ruby/popen.rb', 'files/ruby/popen.rb']) }
|
2016-06-28 12:25:32 -04:00
|
|
|
|
2019-01-09 12:01:28 -05:00
|
|
|
it 'only returns diffs that match the (old path, new path) given' do
|
|
|
|
expect(diffs.map(&:new_path)).to contain_exactly('files/ruby/popen.rb')
|
|
|
|
end
|
2016-06-28 12:25:32 -04:00
|
|
|
|
2019-01-09 12:01:28 -05:00
|
|
|
it 'only serializes diff files found by query' do
|
|
|
|
expect(diff_with_commits.merge_request_diff_files.count).to be > 10
|
|
|
|
expect_any_instance_of(MergeRequestDiffFile).to receive(:to_hash).once
|
2018-07-31 11:47:22 -04:00
|
|
|
|
2019-01-09 12:01:28 -05:00
|
|
|
diffs
|
|
|
|
end
|
2018-07-31 11:47:22 -04:00
|
|
|
|
2019-01-09 12:01:28 -05:00
|
|
|
it 'uses the preprocessed diffs' do
|
|
|
|
expect(diff_with_commits).to receive(:load_diffs)
|
2016-06-28 12:25:32 -04:00
|
|
|
|
2019-01-09 12:01:28 -05:00
|
|
|
diffs
|
|
|
|
end
|
2016-06-28 12:25:32 -04:00
|
|
|
end
|
|
|
|
end
|
|
|
|
end
|
2016-09-22 05:34:46 -04:00
|
|
|
|
2019-01-09 12:01:28 -05:00
|
|
|
describe '#save_diffs' do
|
|
|
|
it 'saves collected state' do
|
|
|
|
mr_diff = create(:merge_request).merge_request_diff
|
2016-12-30 14:16:25 -05:00
|
|
|
|
2019-01-09 12:01:28 -05:00
|
|
|
expect(mr_diff.collected?).to be_truthy
|
|
|
|
end
|
2016-12-30 14:16:25 -05:00
|
|
|
|
2019-01-09 12:01:28 -05:00
|
|
|
it 'saves overflow state' do
|
|
|
|
allow(Commit).to receive(:max_diff_options)
|
|
|
|
.and_return(max_lines: 0, max_files: 0)
|
2016-12-30 14:16:25 -05:00
|
|
|
|
2019-01-09 12:01:28 -05:00
|
|
|
mr_diff = create(:merge_request).merge_request_diff
|
2016-12-30 14:16:25 -05:00
|
|
|
|
2019-01-09 12:01:28 -05:00
|
|
|
expect(mr_diff.overflow?).to be_truthy
|
|
|
|
end
|
2016-12-30 14:16:25 -05:00
|
|
|
|
2019-01-09 12:01:28 -05:00
|
|
|
it 'saves empty state' do
|
|
|
|
allow_any_instance_of(described_class).to receive_message_chain(:compare, :commits)
|
|
|
|
.and_return([])
|
2016-12-30 14:16:25 -05:00
|
|
|
|
2019-01-09 12:01:28 -05:00
|
|
|
mr_diff = create(:merge_request).merge_request_diff
|
2016-12-30 14:16:25 -05:00
|
|
|
|
2019-01-09 12:01:28 -05:00
|
|
|
expect(mr_diff.empty?).to be_truthy
|
|
|
|
end
|
2017-07-25 12:57:02 -04:00
|
|
|
|
2019-01-09 12:01:28 -05:00
|
|
|
it 'expands collapsed diffs before saving' do
|
|
|
|
mr_diff = create(:merge_request, source_branch: 'expand-collapse-lines', target_branch: 'master').merge_request_diff
|
|
|
|
diff_file = mr_diff.merge_request_diff_files.find_by(new_path: 'expand-collapse/file-5.txt')
|
2018-06-29 09:25:35 -04:00
|
|
|
|
2019-01-09 12:01:28 -05:00
|
|
|
expect(diff_file.diff).not_to be_empty
|
|
|
|
end
|
|
|
|
|
|
|
|
it 'saves binary diffs correctly' do
|
|
|
|
path = 'files/images/icn-time-tracking.pdf'
|
|
|
|
mr_diff = create(:merge_request, source_branch: 'add-pdf-text-binary', target_branch: 'master').merge_request_diff
|
|
|
|
diff_file = mr_diff.merge_request_diff_files.find_by(new_path: path)
|
|
|
|
|
|
|
|
expect(diff_file).to be_binary
|
|
|
|
expect(diff_file.diff).to eq(mr_diff.compare.diffs(paths: [path]).to_a.first.diff)
|
|
|
|
end
|
2018-06-29 09:25:35 -04:00
|
|
|
end
|
2019-01-09 12:01:28 -05:00
|
|
|
end
|
2018-06-29 09:25:35 -04:00
|
|
|
|
2019-01-09 12:01:28 -05:00
|
|
|
describe 'internal diffs configured' do
|
|
|
|
include_examples 'merge request diffs'
|
|
|
|
end
|
2017-07-25 12:57:02 -04:00
|
|
|
|
2019-01-09 12:01:28 -05:00
|
|
|
describe 'external diffs configured' do
|
|
|
|
before do
|
|
|
|
stub_external_diffs_setting(enabled: true)
|
2017-07-25 12:57:02 -04:00
|
|
|
end
|
2019-01-09 12:01:28 -05:00
|
|
|
|
|
|
|
include_examples 'merge request diffs'
|
2016-12-30 14:16:25 -05:00
|
|
|
end
|
|
|
|
|
2017-06-16 10:00:58 -04:00
|
|
|
describe '#commit_shas' do
|
2017-11-21 11:58:08 -05:00
|
|
|
it 'returns all commit SHAs using commits from the DB' do
|
|
|
|
expect(diff_with_commits.commit_shas).not_to be_empty
|
|
|
|
expect(diff_with_commits.commit_shas).to all(match(/\h{40}/))
|
2016-09-29 10:46:47 -04:00
|
|
|
end
|
|
|
|
end
|
|
|
|
|
|
|
|
describe '#compare_with' do
|
|
|
|
it 'delegates compare to the service' do
|
|
|
|
expect(CompareService).to receive(:new).and_call_original
|
|
|
|
|
2017-11-21 11:58:08 -05:00
|
|
|
diff_with_commits.compare_with(nil)
|
2016-09-29 11:02:06 -04:00
|
|
|
end
|
|
|
|
|
|
|
|
it 'uses git diff A..B approach by default' do
|
2017-11-21 11:58:08 -05:00
|
|
|
diffs = diff_with_commits.compare_with('0b4bc9a49b562e85de7cc9e834518ea6828729b9').diffs
|
2016-09-29 11:02:06 -04:00
|
|
|
|
2017-11-21 11:58:08 -05:00
|
|
|
expect(diffs.size).to eq(21)
|
2016-09-22 05:34:46 -04:00
|
|
|
end
|
2016-06-28 12:25:32 -04:00
|
|
|
end
|
2016-12-01 06:17:30 -05:00
|
|
|
|
|
|
|
describe '#commits_count' do
|
|
|
|
it 'returns number of commits using serialized commits' do
|
2017-11-21 11:58:08 -05:00
|
|
|
expect(diff_with_commits.commits_count).to eq(29)
|
2017-06-06 12:40:07 -04:00
|
|
|
end
|
|
|
|
end
|
2018-11-02 14:27:01 -04:00
|
|
|
|
|
|
|
describe '#commits_by_shas' do
|
|
|
|
let(:commit_shas) { diff_with_commits.commit_shas }
|
|
|
|
|
|
|
|
it 'returns empty if no SHAs were provided' do
|
|
|
|
expect(diff_with_commits.commits_by_shas([])).to be_empty
|
|
|
|
end
|
|
|
|
|
|
|
|
it 'returns one SHA' do
|
|
|
|
commits = diff_with_commits.commits_by_shas([commit_shas.first, Gitlab::Git::BLANK_SHA])
|
|
|
|
|
|
|
|
expect(commits.count).to eq(1)
|
|
|
|
end
|
|
|
|
|
|
|
|
it 'returns all matching SHAs' do
|
|
|
|
commits = diff_with_commits.commits_by_shas(commit_shas)
|
|
|
|
|
|
|
|
expect(commits.count).to eq(commit_shas.count)
|
|
|
|
expect(commits.map(&:sha)).to match_array(commit_shas)
|
|
|
|
end
|
|
|
|
end
|
2018-10-25 09:46:04 -04:00
|
|
|
|
|
|
|
describe '#modified_paths' do
|
|
|
|
subject do
|
|
|
|
diff = create(:merge_request_diff)
|
|
|
|
create(:merge_request_diff_file, :new_file, merge_request_diff: diff)
|
|
|
|
create(:merge_request_diff_file, :renamed_file, merge_request_diff: diff)
|
|
|
|
diff
|
|
|
|
end
|
|
|
|
|
|
|
|
it 'returns affected file paths' do
|
|
|
|
expect(subject.modified_paths).to eq(%w{foo bar baz})
|
|
|
|
end
|
|
|
|
end
|
2019-01-09 12:01:28 -05:00
|
|
|
|
|
|
|
describe '#opening_external_diff' do
|
|
|
|
subject(:diff) { diff_with_commits }
|
|
|
|
|
|
|
|
context 'external diffs disabled' do
|
|
|
|
it { expect(diff.external_diff).not_to be_exists }
|
|
|
|
|
|
|
|
it 'yields nil' do
|
|
|
|
expect { |b| diff.opening_external_diff(&b) }.to yield_with_args(nil)
|
|
|
|
end
|
|
|
|
end
|
|
|
|
|
|
|
|
context 'external diffs enabled' do
|
|
|
|
let(:test_dir) { 'tmp/tests/external-diffs' }
|
|
|
|
|
|
|
|
around do |example|
|
|
|
|
FileUtils.mkdir_p(test_dir)
|
|
|
|
|
|
|
|
begin
|
|
|
|
example.run
|
|
|
|
ensure
|
|
|
|
FileUtils.rm_rf(test_dir)
|
|
|
|
end
|
|
|
|
end
|
|
|
|
|
|
|
|
before do
|
|
|
|
stub_external_diffs_setting(enabled: true, storage_path: test_dir)
|
|
|
|
end
|
|
|
|
|
|
|
|
it { expect(diff.external_diff).to be_exists }
|
|
|
|
|
|
|
|
it 'yields an open file' do
|
|
|
|
expect { |b| diff.opening_external_diff(&b) }.to yield_with_args(File)
|
|
|
|
end
|
|
|
|
|
|
|
|
it 'is re-entrant' do
|
|
|
|
outer_file_a =
|
|
|
|
diff.opening_external_diff do |outer_file|
|
|
|
|
diff.opening_external_diff do |inner_file|
|
|
|
|
expect(outer_file).to eq(inner_file)
|
|
|
|
end
|
|
|
|
|
|
|
|
outer_file
|
|
|
|
end
|
|
|
|
|
|
|
|
diff.opening_external_diff do |outer_file_b|
|
|
|
|
expect(outer_file_a).not_to eq(outer_file_b)
|
|
|
|
end
|
|
|
|
end
|
|
|
|
end
|
|
|
|
end
|
2016-06-28 12:25:32 -04:00
|
|
|
end
|