Permalink
Browse files

Make accept check if the branch can be added cleanly to master

Accept now will try to rebase the reviewed code onto the remote
master. This will check for conflicts and ensure that the
non-meta-data commits from the review branch can be cherry-picked onto
the master branch

Signed-off-by: Ryan Burrows <rhburrows@gmail.com>
  • Loading branch information...
1 parent 6b14dff commit 3d3b3cada77c69ee71d2d9f2222818a2fb08bfbd @rhburrows committed Apr 21, 2010
@@ -11,3 +11,9 @@ Feature: Code Reviewer accepts changes
Scenario: Reviewr fetches remote branch
When I run "reviewr accept review_12345678"
Then reviewr should fetch the branch "review_12345678"
+
+ Scenario: Reviewr rejects changes if they don't apply cleanly
+ Given remote branch "review_12345678" won't apply cleanly
+ When I run "reviewr accept review_12345678"
+ Then reviewr should abort the acceptance
+ And I should see "Branch 'review_12345678' won't merge cleanly"
@@ -33,6 +33,20 @@
"#{commit} refs/heads/master")
end
+Given /^remote branch "([^\"]*)" won't apply cleanly$/ do |branch|
+ mock_git("git rebase origin/master #{branch}",
+ [
+ "CONFLICT (content): Merge conflict in blah.txt",
+ "Failed to merge in the changes.",
+ "Patch failed at 0002.",
+ "",
+ "",
+ "When you have resolved this problem run \"git rebase --continue\".",
+ "If you would prefer to skip this patch, instead run \"git rebase --skip\".",
+ "To restore the original branch and stop rebasing run \"git rebase --abort\"."
+ ].join("\n"))
+end
+
Then /^reviewr should fetch the branch "([^\"]*)"$/ do |branch|
git_executed?("git fetch origin #{branch}").should be_true
end
@@ -3,7 +3,7 @@
end
Then /^reviewr should create a new branch called "([^\"]*)"$/ do |name|
- git_executed?("git checkout -b #{name}").should be_true
+ git_executed?("git branch #{name} master").should be_true
end
Then /^reviewr should create a commit with message:$/ do |msg|
@@ -19,6 +19,10 @@
Pony.sent[:body].should == body
end
+Then /^reviewr should abort the acceptance$/ do
+ git_executed?("git rebase --abort").should be_true
+end
+
def git_executed?(cmd)
Reviewr::Git.instance.commands.include?(cmd)
end
@@ -5,6 +5,11 @@ def execute
project.review_branch = arguments.first
prompt_for_user
project.fetch_review_branch
+ project.fetch_master
+ project.create_review_branch("origin/#{arguments.first}")
+ unless project.rebase_review
+ output.print "Branch '#{arguments.first}' won't merge cleanly"
+ end
end
end
end
View
@@ -20,8 +20,18 @@ def last_commit
execute('git show --pretty=format:"%H" HEAD').split("\n")[0]
end
- def create_branch(branch_name)
- execute("git checkout -b #{branch_name}")
+ def rebase(base, branch)
+ conflict = execute("git rebase #{base} #{branch}").to_s.
+ include?("CONFLICT")
+ if conflict
+ execute('git rebase --abort')
+ end
+ !conflict
+ end
+
+ def create_branch(branch_name, base)
+ execute("git branch #{branch_name} #{base}")
+ execute("git checkout #{branch_name}")
end
def commit(msg)
@@ -15,14 +15,22 @@ def initialize(git = Git.instance)
@git = git
end
- def create_review_branch
- git.create_branch(review_branch)
+ def create_review_branch(base = 'master')
+ git.create_branch(review_branch, base)
+ end
+
+ def rebase_review
+ git.rebase("#{remote_repo}/master", review_branch)
end
def fetch_review_branch
git.fetch(review_branch)
end
+ def fetch_master
+ git.fetch('master')
+ end
+
def create_review_commit(msg)
git.commit(msg)
end
@@ -10,6 +10,7 @@ module CLI
before do
accept.stub(:prompt_for_user)
accept.arguments = []
+ accept.output = double("Output").as_null_object
end
it "sets the review_branch" do
@@ -27,6 +28,22 @@ module CLI
project.should_receive(:fetch_review_branch)
accept.call
end
+
+ it "fetches the remote master" do
+ project.should_receive(:fetch_master)
+ accept.call
+ end
+
+ it "creates a working branch for the reviewed code" do
+ project.should_receive(:create_review_branch).with("origin/review")
+ accept.arguments = ["review"]
+ accept.call
+ end
+
+ it "rebases the review branch on the master branch" do
+ project.should_receive(:rebase_review)
+ accept.call
+ end
end
end
end
@@ -4,6 +4,10 @@ module Reviewr
describe Git do
let(:git) { Git.new }
+ before do
+ git.stub(:execute)
+ end
+
describe "#last_commit" do
it "shells out to git to the get the hash of the last commit" do
git.stub!(:execute).and_return("")
@@ -19,8 +23,36 @@ module Reviewr
describe "#create_branch" do
it "creates a new branch through git" do
- git.should_receive(:execute).with('git checkout -b branch_name')
- git.create_branch("branch_name")
+ git.should_receive(:execute).with('git branch branch_name base')
+ git.create_branch("branch_name", "base")
+ end
+
+ it "checks out the newly created branch" do
+ git.should_receive(:execute).with('git checkout branch_name')
+ git.create_branch('branch_name', 'base')
+ end
+ end
+
+ describe "#rebase" do
+ it "runs rebase through git" do
+ git.should_receive(:execute).with('git rebase base branch')
+ git.rebase('base', 'branch')
+ end
+
+ it "returns true if the rebase happens cleanly" do
+ git.stub(:execute).and_return("")
+ git.rebase('base', 'branch').should be_true
+ end
+
+ it "returns false if the rebase has a conflict" do
+ git.stub(:execute).and_return("CONFLICT")
+ git.rebase('base', 'branch').should_not be_true
+ end
+
+ it "aborts the rebase if there is a conflict" do
+ git.stub(:execute).and_return("CONFLICT")
+ git.should_receive(:execute).with('git rebase --abort')
+ git.rebase('base', 'branch')
end
end
@@ -33,6 +33,15 @@ module Reviewr
end
end
+ describe "#rebase_review" do
+ it "rebases the review branch on the remote master" do
+ git.stub(:remote_repo).and_return('remote')
+ git.should_receive(:rebase).with('remote/master', 'review')
+ project.stub(:review_branch).and_return('review')
+ project.rebase_review
+ end
+ end
+
describe "#fetch_review_branch" do
it "fetches the branch with the name from #review_branch" do
project.stub(:review_branch).and_return('branch')
@@ -41,10 +50,29 @@ module Reviewr
end
end
+ describe "#fetch_master" do
+ it "fetches the master" do
+ git.should_receive(:fetch).with('master')
+ project.fetch_master
+ end
+ end
+
describe "#create_review_branch" do
it "creates the branch with the name from #review_branch" do
project.stub(:review_branch).and_return('branch')
- git.should_receive(:create_branch).with('branch')
+ git.should_receive(:create_branch).with('branch', anything)
+ project.create_review_branch
+ end
+
+ it "bases the branch on the parameter if specified" do
+ project.stub(:review_branch)
+ git.should_receive(:create_branch).with(anything, 'base')
+ project.create_review_branch('base')
+ end
+
+ it "bases the branch on the master by default" do
+ project.stub(:review_branch)
+ git.should_receive(:create_branch).with(anything, 'master')
project.create_review_branch
end
end

0 comments on commit 3d3b3ca

Please sign in to comment.