diff options
author | Dmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com> | 2014-08-06 09:52:56 +0300 |
---|---|---|
committer | Dmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com> | 2014-08-06 09:52:56 +0300 |
commit | 8f1e60b05719bf8c5c7474ad228f925a0cab6b07 (patch) | |
tree | 8ef8467de3da53a99da2734c75105817cf07d392 | |
parent | b954f24420114a6acee9caf038e6dae6650bd066 (diff) | |
download | gitlab-ce-8f1e60b05719bf8c5c7474ad228f925a0cab6b07.tar.gz |
Fix satellite tests
Signed-off-by: Dmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com>
-rw-r--r-- | lib/gitlab/satellite/merge_action.rb | 15 | ||||
-rw-r--r-- | spec/factories/merge_requests.rb | 8 | ||||
-rw-r--r-- | spec/lib/gitlab/satellite/merge_action_spec.rb | 93 | ||||
-rw-r--r-- | spec/support/repo_helpers.rb | 11 |
4 files changed, 51 insertions, 76 deletions
diff --git a/lib/gitlab/satellite/merge_action.rb b/lib/gitlab/satellite/merge_action.rb index 6c32dfb3ad9..7c9b2294647 100644 --- a/lib/gitlab/satellite/merge_action.rb +++ b/lib/gitlab/satellite/merge_action.rb @@ -45,27 +45,30 @@ module Gitlab handle_exception(ex) end - # Get a raw diff of the source to the target def diff_in_satellite in_locked_and_timed_satellite do |merge_repo| prepare_satellite!(merge_repo) update_satellite_source_and_target!(merge_repo) - diff = merge_repo.git.native(:diff, default_options, "origin/#{merge_request.target_branch}", "source/#{merge_request.source_branch}") + + # Only show what is new in the source branch compared to the target branch, not the other way around. + # The line below with merge_base is equivalent to diff with three dots (git diff branch1...branch2) + # From the git documentation: "git diff A...B" is equivalent to "git diff $(git-merge-base A B) B" + common_commit = merge_repo.git.native(:merge_base, default_options, ["origin/#{merge_request.target_branch}", "source/#{merge_request.source_branch}"]).strip + merge_repo.git.native(:diff, default_options, common_commit, "source/#{merge_request.source_branch}") end rescue Grit::Git::CommandFailed => ex handle_exception(ex) end - # Only show what is new in the source branch compared to the target branch, not the other way around. - # The line below with merge_base is equivalent to diff with three dots (git diff branch1...branch2) - # From the git documentation: "git diff A...B" is equivalent to "git diff $(git-merge-base A B) B" def diffs_between_satellite in_locked_and_timed_satellite do |merge_repo| prepare_satellite!(merge_repo) update_satellite_source_and_target!(merge_repo) if merge_request.for_fork? + # Only show what is new in the source branch compared to the target branch, not the other way around. + # The line below with merge_base is equivalent to diff with three dots (git diff branch1...branch2) + # From the git documentation: "git diff A...B" is equivalent to "git diff $(git-merge-base A B) B" common_commit = merge_repo.git.native(:merge_base, default_options, ["origin/#{merge_request.target_branch}", "source/#{merge_request.source_branch}"]).strip - #this method doesn't take default options diffs = merge_repo.diff(common_commit, "source/#{merge_request.source_branch}") else raise "Attempt to determine diffs between for a non forked merge request in satellite MergeRequest.id:[#{merge_request.id}]" diff --git a/spec/factories/merge_requests.rb b/spec/factories/merge_requests.rb index 80dbd2f429b..3319262c015 100644 --- a/spec/factories/merge_requests.rb +++ b/spec/factories/merge_requests.rb @@ -11,6 +11,9 @@ FactoryGirl.define do # 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9 More submodules # d14d6c0abdd253381df51a723d58691b2ee1ab08 Remove ds_store files # c1acaa58bbcbc3eafe538cb8274ba387047b69f8 Ignore DS files + # + # See also RepoHelpers.sample_compare + # source_branch "master" target_branch "feature" @@ -19,6 +22,11 @@ FactoryGirl.define do trait :with_diffs do end + trait :conflict do + source_branch "feature_conflict" + target_branch "feature" + end + trait :closed do state :closed end diff --git a/spec/lib/gitlab/satellite/merge_action_spec.rb b/spec/lib/gitlab/satellite/merge_action_spec.rb index 41d3321b173..479a73a1081 100644 --- a/spec/lib/gitlab/satellite/merge_action_spec.rb +++ b/spec/lib/gitlab/satellite/merge_action_spec.rb @@ -1,22 +1,16 @@ require 'spec_helper' describe 'Gitlab::Satellite::MergeAction' do - before(:each) do - @master = ['master', '69b34b7e9ad9f496f0ad10250be37d6265a03bba'] - @one_after_stable = ['stable', '6ea87c47f0f8a24ae031c3fff17bc913889ecd00'] #this commit sha is one after stable - @wiki_branch = ['wiki', '635d3e09b72232b6e92a38de6cc184147e5bcb41'] #this is the commit sha where the wiki branch goes off from master - @conflicting_metior = ['metior', '313d96e42b313a0af5ab50fa233bf43e27118b3f'] #this branch conflicts with the wiki branch - - # these commits are quite close together, itended to make string diffs/format patches small - @close_commit1 = ['2_3_notes_fix', '8470d70da67355c9c009e4401746b1d5410af2e3'] - @close_commit2 = ['scss_refactoring', 'f0f14c8eaba69ebddd766498a9d0b0e79becd633'] - end + include RepoHelpers let(:project) { create(:project, namespace: create(:group)) } let(:fork_project) { create(:project, namespace: create(:group), forked_from_project: project) } let(:merge_request) { create(:merge_request, source_project: project, target_project: project) } let(:merge_request_fork) { create(:merge_request, source_project: fork_project, target_project: project) } + let(:merge_request_with_conflict) { create(:merge_request, :conflict, source_project: project, target_project: project) } + let(:merge_request_fork_with_conflict) { create(:merge_request, :conflict, source_project: project, target_project: project) } + describe '#commits_between' do def verify_commits(commits, first_commit_sha, last_commit_sha) commits.each { |commit| commit.class.should == Gitlab::Git::Commit } @@ -26,51 +20,27 @@ describe 'Gitlab::Satellite::MergeAction' do context 'on fork' do it 'should get proper commits between' do - merge_request_fork.target_branch = @one_after_stable[0] - merge_request_fork.source_branch = @master[0] - commits = Gitlab::Satellite::MergeAction.new(merge_request_fork.author, merge_request_fork).commits_between - verify_commits(commits, @one_after_stable[1], @master[1]) - - merge_request_fork.target_branch = @wiki_branch[0] - merge_request_fork.source_branch = @master[0] commits = Gitlab::Satellite::MergeAction.new(merge_request_fork.author, merge_request_fork).commits_between - verify_commits(commits, @wiki_branch[1], @master[1]) + verify_commits(commits, sample_compare.commits.first, sample_compare.commits.last) end end context 'between branches' do it 'should raise exception -- not expected to be used by non forks' do - merge_request.target_branch = @one_after_stable[0] - merge_request.source_branch = @master[0] - expect {Gitlab::Satellite::MergeAction.new(merge_request.author, merge_request).commits_between}.to raise_error - - merge_request.target_branch = @wiki_branch[0] - merge_request.source_branch = @master[0] - expect {Gitlab::Satellite::MergeAction.new(merge_request.author, merge_request).commits_between}.to raise_error + expect { Gitlab::Satellite::MergeAction.new(merge_request.author, merge_request).commits_between }.to raise_error end end end describe '#format_patch' do - let(:target_commit) {['artiom-config-examples','9edbac5ac88ffa1ec9dad0097226b51e29ebc9ac']} - let(:source_commit) {['metior', '313d96e42b313a0af5ab50fa233bf43e27118b3f']} - def verify_content(patch) - (patch.include? source_commit[1]).should be_true - (patch.include? '635d3e09b72232b6e92a38de6cc184147e5bcb41').should be_true - (patch.include? '2bb2dee057327c81978ed0aa99904bd7ff5e6105').should be_true - (patch.include? '2e83de1924ad3429b812d17498b009a8b924795d').should be_true - (patch.include? 'ee45a49c57a362305431cbf004e4590b713c910e').should be_true - (patch.include? 'a6870dd08f8f274d9a6b899f638c0c26fefaa690').should be_true - - (patch.include? 'e74fae147abc7d2ffbf93d363dbbe45b87751f6f').should be_false - (patch.include? '86f76b11c670425bbab465087f25172378d76147').should be_false + sample_compare.commits.each do |commit| + patch.include?(commit).should be_true + end end context 'on fork' do it 'should build a format patch' do - merge_request_fork.target_branch = target_commit[0] - merge_request_fork.source_branch = source_commit[0] patch = Gitlab::Satellite::MergeAction.new(merge_request_fork.author, merge_request_fork).format_patch verify_content(patch) end @@ -78,8 +48,6 @@ describe 'Gitlab::Satellite::MergeAction' do context 'between branches' do it 'should build a format patch' do - merge_request.target_branch = target_commit[0] - merge_request.source_branch = source_commit[0] patch = Gitlab::Satellite::MergeAction.new(merge_request_fork.author, merge_request).format_patch verify_content(patch) end @@ -87,7 +55,6 @@ describe 'Gitlab::Satellite::MergeAction' do end describe '#diffs_between_satellite tested against diff_in_satellite' do - def is_a_matching_diff(diff, diffs) diff_count = diff.scan('diff --git').size diff_count.should >= 1 @@ -100,50 +67,38 @@ describe 'Gitlab::Satellite::MergeAction' do context 'on fork' do it 'should get proper diffs' do - merge_request_fork.target_branch = @close_commit1[0] - merge_request_fork.source_branch = @master[0] diffs = Gitlab::Satellite::MergeAction.new(merge_request_fork.author, merge_request_fork).diffs_between_satellite - - merge_request_fork.target_branch = @close_commit1[0] - merge_request_fork.source_branch = @master[0] diff = Gitlab::Satellite::MergeAction.new(merge_request.author, merge_request_fork).diff_in_satellite - is_a_matching_diff(diff, diffs) end end context 'between branches' do it 'should get proper diffs' do - merge_request.target_branch = @close_commit1[0] - merge_request.source_branch = @master[0] - expect{Gitlab::Satellite::MergeAction.new(merge_request.author, merge_request).diffs_between_satellite}.to raise_error + expect{ Gitlab::Satellite::MergeAction.new(merge_request.author, merge_request).diffs_between_satellite }.to raise_error end end end describe '#can_be_merged?' do context 'on fork' do - it 'return true or false depending on if something is mergable' do - merge_request_fork.target_branch = @one_after_stable[0] - merge_request_fork.source_branch = @master[0] - Gitlab::Satellite::MergeAction.new(merge_request_fork.author, merge_request_fork).can_be_merged?.should be_true - - merge_request_fork.target_branch = @conflicting_metior[0] - merge_request_fork.source_branch = @wiki_branch[0] - Gitlab::Satellite::MergeAction.new(merge_request_fork.author, merge_request_fork).can_be_merged?.should be_false - end + it { Gitlab::Satellite::MergeAction.new( + merge_request_fork.author, + merge_request_fork).can_be_merged?.should be_true } + + it { Gitlab::Satellite::MergeAction.new( + merge_request_fork_with_conflict.author, + merge_request_fork_with_conflict).can_be_merged?.should be_false } end context 'between branches' do - it 'return true or false depending on if something is mergable' do - merge_request.target_branch = @one_after_stable[0] - merge_request.source_branch = @master[0] - Gitlab::Satellite::MergeAction.new(merge_request.author, merge_request).can_be_merged?.should be_true - - merge_request.target_branch = @conflicting_metior[0] - merge_request.source_branch = @wiki_branch[0] - Gitlab::Satellite::MergeAction.new(merge_request.author, merge_request).can_be_merged?.should be_false - end + it { Gitlab::Satellite::MergeAction.new( + merge_request.author, + merge_request).can_be_merged?.should be_true } + + it { Gitlab::Satellite::MergeAction.new( + merge_request_with_conflict.author, + merge_request_with_conflict).can_be_merged?.should be_false } end end end diff --git a/spec/support/repo_helpers.rb b/spec/support/repo_helpers.rb index f19c8ebf844..4c4775da692 100644 --- a/spec/support/repo_helpers.rb +++ b/spec/support/repo_helpers.rb @@ -81,10 +81,19 @@ eos } ] + commits = [ + '5937ac0a7beb003549fc5fd26fc247adbce4a52e', + '570e7b2abdd848b95f2f578043fc23bd6f6fd24d', + '6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9', + 'd14d6c0abdd253381df51a723d58691b2ee1ab08', + 'c1acaa58bbcbc3eafe538cb8274ba387047b69f8', + ].reverse # last commit is recent one + OpenStruct.new( source_branch: 'master', target_branch: 'feature', - changes: changes + changes: changes, + commits: commits ) end end |