diff --git a/README.rdoc b/README.rdoc index c016ec9..fc088aa 100644 --- a/README.rdoc +++ b/README.rdoc @@ -205,9 +205,9 @@ You kick butt. You've got your code reviewed and now you're ready to merge it do Reflow's +deliver+ requires you to have a pull request, so you'll be protected on those mornings when the coffee isn't working yet. We built this to get in your way and make you follow the process. If you don't like it, do it by hand. You already know what you're doing. -You'll be presented with a prefilled commit message based on the body of your pull request which includes the text Closes #XX that will automatically close the associated pull request on github when deliver completes. +You'll be presented with a prefilled commit message based on the body of your pull request which includes the text Merges #XX that will automatically merge the associated pull request on github when deliver completes. -Make a mistake and deliver too early? It happens. You'll be prompted after you edit your commit message if you want to push your updated +master+ to github. If you answer 'n', then you'll want to do git reset --hard origin/master and checkout your branch again. +Want to clean up your feature branch afterwards? You'll be prompted after you edit your commit message if you want to clean up your +feature_branch+ on github. If you answer 'n', then your feature_branch will exist for you to clean it up later. This is what it looks like: $ git reflow deliver @@ -276,25 +276,17 @@ This is what we do behind the scenes when you do +deliver+ If the review is done, it's time to merge. Here's what happens: -First, update our local +master+ so we don't get conflicts - git checkout master - git pull origin master +First, we call the github_api gem to merge the pull request -Next, squash merge our feature branch - git merge --squash nh-branch-name +Next, we prompt you if you want to deploy and cleanup + Would you like cleanup your feature branch? -Now, we'll apply a little magic so we have a great commit message by default. Your editor will open and you'll see a nice default for your commit message based on the pull request body. - -Once you've saved the commit, prompt the user to see if we should continue - Merge complete! - Would you like to push this branch to your remote repo and cleanup your feature branch? - -If 'y', then we'll push to +master+ and delete the feature branch - git push origin master - git push origin :nh-branch-name - git branch -D nh-branch-name +If 'y', then we'll update the +master+ and delete the feature branch + git pull origin #{base_branch} + git push origin :#{feature_branch} + git branch -D #{feature_branch} -If 'n', then just stop here. The user can reset his local +master+. +If 'n', then just stop here. The user can clean up his branch locally. And we're done. diff --git a/lib/git_reflow.rb b/lib/git_reflow.rb index 569ad4e..507d93a 100644 --- a/lib/git_reflow.rb +++ b/lib/git_reflow.rb @@ -107,10 +107,6 @@ def deliver(options = {}) feature_branch = current_branch base_branch = options['base'] || 'master' - update_current_branch - fetch_destination(base_branch) - update_destination(feature_branch) - begin existing_pull_request = git_server.find_open_pull_request( :from => current_branch, :to => base_branch ) @@ -127,23 +123,29 @@ def deliver(options = {}) if existing_pull_request.good_to_merge?(force: options['skip_lgtm']) puts "Merging pull request ##{existing_pull_request.number}: '#{existing_pull_request.title}', from '#{existing_pull_request.feature_branch_name}' into '#{existing_pull_request.base_branch_name}'" - update_destination(base_branch) - merge_feature_branch(feature_branch, - :destination_branch => base_branch, - :pull_request_number => existing_pull_request.number, - :lgtm_authors => existing_pull_request.approvals, - :message => commit_message) - committed = run_command_with_label 'git commit', with_system: true - - if committed - say "Merge complete!", :success + begin + res = existing_pull_request.merge_feature_branch( + feature_branch, + :destination_branch => base_branch, + :pull_request_number => existing_pull_request.number, + :lgtm_authors => existing_pull_request.approvals, + :title => existing_pull_request.title, + :message => commit_message, + :head => existing_pull_request.head + ) + rescue StandardError => error + say error.inspect, :deliver_halted + else + # if committed + say "Pull Request successfully merged.", :success # check if user always wants to push and cleanup, otherwise ask always_deploy_and_cleanup = GitReflow::Config.get('reflow.always-deploy-and-cleanup') == "true" - deploy_and_cleanup = always_deploy_and_cleanup || (ask "Would you like to push this branch to your remote repo and cleanup your feature branch? ") =~ /^y/i + deploy_and_cleanup = always_deploy_and_cleanup || (ask "Would you like to cleanup your feature branch? ") =~ /^y/i if deploy_and_cleanup - run_command_with_label "git push origin #{base_branch}" + # Pulls merged changes from remote base_branch + run_command_with_label "git pull origin #{base_branch}" run_command_with_label "git push origin :#{feature_branch}" run_command_with_label "git branch -D #{feature_branch}" puts "Nice job buddy." @@ -151,8 +153,6 @@ def deliver(options = {}) puts "Cleanup halted. Local changes were not pushed to remote repo.".colorize(:red) puts "To reset and go back to your branch run \`git reset --hard origin/#{base_branch} && git checkout #{feature_branch}\`" end - else - say "There were problems commiting your feature... please check the errors above and try again.", :error end else say existing_pull_request.rejection_message, :deliver_halted @@ -185,7 +185,7 @@ def deploy(destination_server) end def git_server - @git_server ||= GitServer.connect provider: GitReflow::Config.get('reflow.git-server').strip, silent: true + @git_server ||= GitServer.connect provider: "#{GitReflow::Config.get('reflow.git-server')}".strip, silent: true end end diff --git a/lib/git_reflow/commands/setup.rb b/lib/git_reflow/commands/setup.rb index deb6990..d497963 100644 --- a/lib/git_reflow/commands/setup.rb +++ b/lib/git_reflow/commands/setup.rb @@ -25,5 +25,12 @@ GitReflow::Config.add "constants.minimumApprovals", ask("Set the minimum number of approvals (leaving blank will require approval from all commenters): "), local: reflow_options[:project_only] GitReflow::Config.add "constants.approvalRegex", GitReflow::GitServer::PullRequest::DEFAULT_APPROVAL_REGEX, local: reflow_options[:project_only] + say("Enter the follow information associated with your remote repository: (The information is used to run 'git reflow deliver'.") + GitReflow::Config.add "github.owner", ask("Enter the owner of the remote repository: "), local: reflow_options[:project_only] + GitReflow::Config.add "github.repo", ask("Enter the name of the remote repository: "), local: reflow_options[:project_only] + + # Sets the user's github auth token + GitReflow::Config.set "github.oauth-token", ask("Set the your local github authorization token: (You cannot run 'git reflow deliver' without it!) "), local: reflow_options[:project_only] + say("Thanks! Your settings are saved to #{GitReflow::Config::CONFIG_FILE_PATH}.") end end diff --git a/lib/git_reflow/git_server/bit_bucket/pull_request.rb b/lib/git_reflow/git_server/bit_bucket/pull_request.rb index 50b7f25..c5bd96a 100644 --- a/lib/git_reflow/git_server/bit_bucket/pull_request.rb +++ b/lib/git_reflow/git_server/bit_bucket/pull_request.rb @@ -77,7 +77,6 @@ def approvals approved end - end end end diff --git a/lib/git_reflow/git_server/git_hub/pull_request.rb b/lib/git_reflow/git_server/git_hub/pull_request.rb index 00c10e0..d9dfb4e 100644 --- a/lib/git_reflow/git_server/git_hub/pull_request.rb +++ b/lib/git_reflow/git_server/git_hub/pull_request.rb @@ -78,6 +78,46 @@ def approved? end end + def merge_feature_branch(feature_branch_name, options = {}) + # binding.pry + options[:destination_branch] ||= 'master' + + message = "#{options[:message]}" + + if "#{options[:pull_request_number]}".length > 0 + message << "\nMerges ##{options[:pull_request_number]}\n" + end + + if lgtm_authors = Array(options[:lgtm_authors]) and lgtm_authors.any? + message << "\nLGTM given by: @#{lgtm_authors.join(', @')}\n" + end + + data = { + "commit_title" => options[:title], + "commit_message" => options[:message], + "sha" => options[:head].sha, + "squash" => true + } + + res = GitReflow::GitServer::GitHub.connection.pull_requests.merge( + "#{GitReflow::Config.get('github.owner')}", + "#{GitReflow::Config.get('github.repo')}", + "#{options[:pull_request_number]}", + data + ) + + if res.code == "405" + raise StandardError, "Pull Request is not mergeable. Try 'git reflow refresh' first." + elsif res.code == "409" + raise StandardError, "Head branch was modified. Review and try the merge again. Try 'git reflow refresh' first." + elsif res.code != "200" + raise StandardError, "There was another error. Please review the pull request and try again." + else + append_to_squashed_commit_message(message) if message.length > 0 + return res + end + end + def build github_build_status = GitReflow.git_server.get_build_status(self.head.sha) build_status_object = Struct.new(:state, :description, :url) diff --git a/lib/git_reflow/git_server/pull_request.rb b/lib/git_reflow/git_server/pull_request.rb index bf7da52..8294e27 100644 --- a/lib/git_reflow/git_server/pull_request.rb +++ b/lib/git_reflow/git_server/pull_request.rb @@ -87,6 +87,7 @@ def all_comments_addressed? end def good_to_merge?(force: false) + return true if force (build_status.nil? or build_status == "success") and approved? @@ -140,6 +141,25 @@ def method_missing(method_sym, *arguments, &block) super end end + + def merge_feature_branch(feature_branch_name, options = {}) + options[:destination_branch] ||= 'master' + + message = "#{options[:message]}" + + if "#{options[:pull_request_number]}".length > 0 + message << "\nMerges ##{options[:pull_request_number]}\n" + end + + if lgtm_authors = Array(options[:lgtm_authors]) and lgtm_authors.any? + message << "\nLGTM given by: @#{lgtm_authors.join(', @')}\n" + end + + run_command_with_label "git checkout #{options[:destination_branch]}" + run_command_with_label "git merge --squash #{feature_branch_name}" + + append_to_squashed_commit_message(message) if message.length > 0 + end end end end diff --git a/spec/git_reflow_spec.rb b/spec/git_reflow_spec.rb index b29a10d..07b90af 100644 --- a/spec/git_reflow_spec.rb +++ b/spec/git_reflow_spec.rb @@ -22,6 +22,8 @@ # Stubbing out numlgtm value to test all reviewers in gitconfig file allow(GitReflow::Config).to receive(:get).with("constants.minimumApprovals").and_return('') allow(GitReflow::Config).to receive(:get).and_call_original + allow(GitReflow::GitServer::GitHub).to receive_message_chain(:connection, :pull_requests, :merge).and_return({}) + allow_any_instance_of(Hash).to receive(:code).and_return("200") allow_any_instance_of(HighLine).to receive(:ask) do |terminal, question| values = { @@ -29,7 +31,7 @@ "Please enter your GitHub password (we do NOT store this): " => password, "Please enter your Enterprise site URL (e.g. https://github.company.com):" => enterprise_site, "Please enter your Enterprise API endpoint (e.g. https://github.company.com/api/v3):" => enterprise_api, - "Would you like to push this branch to your remote repo and cleanup your feature branch? " => 'yes', + "Would you like to cleanup your feature branch? " => 'yes', "Would you like to open it in your browser?" => 'n' } return_value = values[question] || values[terminal] @@ -174,7 +176,12 @@ context :deliver do let(:branch) { 'new-feature' } - let(:inputs) { {} } + let(:inputs) { + { + "title" => "new-feature", + "head" => "reenhanced:new-feature", + } + } let!(:github) do allow_any_instance_of(GitReflow::GitServer::GitHub::PullRequest).to receive(:build).and_return(Struct.new(:state, :description, :url).new) stub_github_with({ @@ -188,7 +195,11 @@ before do - allow(GitReflow).to receive(:append_to_squashed_commit_message).and_return(true) + allow(existing_pull_request).to receive(:append_to_squashed_commit_message).and_return(true) + + # Stubs out the http response to github api + allow(GitReflow::GitServer::GitHub).to receive_message_chain(:connection, :pull_requests, :merge).and_return({}) + allow_any_instance_of(Hash).to receive(:code).and_return("200"); module Kernel def system(cmd) @@ -199,16 +210,6 @@ def system(cmd) subject { GitReflow.deliver inputs } - it "fetches the latest changes to the current branch" do - expect(GitReflow).to receive(:update_current_branch) - subject - end - - it "fetches the latest changes to the destination branch" do - expect(GitReflow).to receive(:fetch_destination).with('master') - subject - end - it "looks for a pull request matching the feature branch and destination branch" do expect(github).to receive(:find_open_pull_request).with(from: branch, to: 'master') subject @@ -219,6 +220,7 @@ def system(cmd) allow(github).to receive(:build_status).and_return(build_status) allow(github).to receive(:find_open_pull_request).and_return(existing_pull_request) allow(existing_pull_request).to receive(:has_comments?).and_return(true) + allow(github).to receive(:reviewers).and_return(['codenamev']) end @@ -237,19 +239,22 @@ def system(cmd) context 'and build status is nil' do let(:build_status) { nil } - let(:inputs) {{ skip_lgtm: true }} + let(:inputs) {{ 'skip_lgtm': false }} before do # stubbing unrelated results so we can just test that it made it insdide the conditional block allow(existing_pull_request).to receive(:has_comments?).and_return(true) allow(existing_pull_request).to receive(:reviewers).and_return([]) - allow(GitReflow).to receive(:update_destination).and_return(true) - allow(GitReflow).to receive(:merge_feature_branch).and_return(true) - allow(GitReflow).to receive(:append_to_squashed_commit_message).and_return(true) + allow(existing_pull_request).to receive(:reviewers_pending_response).and_return([]) + allow(existing_pull_request).to receive(:approvals).and_return(['simonzhu24']) + # Stubs out the http response to github api + allow(GitReflow::GitServer::GitHub).to receive_message_chain(:connection, :pull_requests, :merge).and_return({}) + allow_any_instance_of(Hash).to receive(:code).and_return("200"); + allow(existing_pull_request).to receive(:append_to_squashed_commit_message).and_return(true) end it "ignores build status when not setup" do - expect { subject }.to have_said "Merge complete!", :success + expect { subject }.to have_said "Pull Request successfully merged.", :success end end @@ -269,8 +274,8 @@ def system(cmd) end it "includes the pull request body in the commit message" do - squash_message = "#{existing_pull_request.body}\nCloses ##{existing_pull_request.number}\n\nLGTM given by: @nhance\n" - expect(GitReflow).to receive(:append_to_squashed_commit_message).with(squash_message) + squash_message = "#{existing_pull_request.body}\nMerges ##{existing_pull_request.number}\n\nLGTM given by: @nhance\n" + expect(existing_pull_request).to receive(:append_to_squashed_commit_message).with(squash_message) subject end @@ -298,7 +303,7 @@ def system(cmd) end it "commits the changes if the build status is nil but has comments/approvals and no pending response" do - expect{ subject }.to have_said 'Merge complete!', :success + expect{ subject }.to have_said 'Pull Request successfully merged.', :success end end @@ -313,30 +318,30 @@ def system(cmd) end it "includes the first commit message for the new branch in the commit message of the merge" do - squash_message = "#{first_commit_message}\nCloses ##{existing_pull_request.number}\n\nLGTM given by: @nhance\n" - expect(GitReflow).to receive(:append_to_squashed_commit_message).with(squash_message) + squash_message = "#{first_commit_message}\nMerges ##{existing_pull_request.number}\n\nLGTM given by: @nhance\n" + expect(existing_pull_request).to receive(:append_to_squashed_commit_message).with(squash_message) subject end end it "notifies user of the merge and performs it" do - expect(GitReflow).to receive(:merge_feature_branch).with('new-feature', { + expect(existing_pull_request).to receive(:merge_feature_branch).with('new-feature', { destination_branch: 'master', pull_request_number: existing_pull_request.number, lgtm_authors: ['nhance'], - message: existing_pull_request.body - }) + title: existing_pull_request.title, + message: existing_pull_request.body, + head: existing_pull_request.head + }).and_return({}) - expect { subject }.to have_output "Merging pull request ##{existing_pull_request.number}: '#{existing_pull_request.title}', from '#{existing_pull_request.head.label}' into '#{existing_pull_request.base.label}'" - end + # Stubs out the http response to github api + allow_any_instance_of(Hash).to receive(:code).and_return("200") - it "updates the destination brnach" do - expect(GitReflow).to receive(:update_destination).with('master') - subject + expect { subject }.to have_output "Merging pull request ##{existing_pull_request.number}: '#{existing_pull_request.title}', from '#{existing_pull_request.head.label}' into '#{existing_pull_request.base.label}'" end it "commits the changes for the squash merge" do - expect{ subject }.to have_said 'Merge complete!', :success + expect{ subject }.to have_said 'Pull Request successfully merged.', :success end context "and cleaning up feature branch" do @@ -347,7 +352,7 @@ def system(cmd) "Please enter your GitHub password (we do NOT store this): " => password, "Please enter your Enterprise site URL (e.g. https://github.company.com):" => enterprise_site, "Please enter your Enterprise API endpoint (e.g. https://github.company.com/api/v3):" => enterprise_api, - "Would you like to push this branch to your remote repo and cleanup your feature branch? " => 'yes', + "Would you like to cleanup your feature branch? " => 'yes', "Would you like to open it in your browser?" => 'no' } return_value = values[question] || values[terminal] @@ -362,8 +367,8 @@ def system(cmd) allow(GitReflow::Config).to receive(:get).and_call_original end - it "pushes local squash merged base branch to remote repo" do - expect { subject }.to have_run_command("git push origin master") + it "pulls changes from remote repo to local branch" do + expect { subject }.to have_run_command("git pull origin master") end it "deletes the remote feature branch" do @@ -381,8 +386,8 @@ def system(cmd) allow(GitReflow::Config).to receive(:get).and_call_original end - it "pushes local squash merged base branch to remote repo" do - expect { subject }.to have_run_command("git push origin master") + it "pulls changes from remote repo to local branch" do + expect { subject }.to have_run_command("git pull origin master") end it "deletes the remote feature branch" do @@ -404,7 +409,7 @@ def system(cmd) "Please enter your GitHub password (we do NOT store this): " => password, "Please enter your Enterprise site URL (e.g. https://github.company.com):" => enterprise_site, "Please enter your Enterprise API endpoint (e.g. https://github.company.com/api/v3):" => enterprise_api, - "Would you like to push this branch to your remote repo and cleanup your feature branch? " => 'no', + "Would you like to cleanup your feature branch? " => 'no', "Would you like to open it in your browser?" => 'no' } return_value = values[question] || values[terminal] @@ -430,10 +435,24 @@ def system(cmd) end end - context "and there were issues commiting the squash merge to the base branch" do - before { stub_with_fallback(GitReflow, :run_command_with_label).with('git commit', {with_system: true}).and_return false } - it "notifies user of issues commiting the squash merge of the feature branch" do - expect { subject }.to have_said("There were problems commiting your feature... please check the errors above and try again.", :error) + context "and the pull request is not mergeable" do + before { allow_any_instance_of(Hash).to receive(:code).and_return "405" } + it "notifies user to try refresh first" do + expect { subject }.to have_said("#", :deliver_halted) + end + end + + context "and the head branch was modified" do + before { allow_any_instance_of(Hash).to receive(:code).and_return "409" } + it "notifies user to try refresh first" do + expect { subject }.to have_said("#", :deliver_halted) + end + end + + context "and there was another error" do + before { allow_any_instance_of(Hash).to receive(:code).and_return "500" } + it "notifies user to try refresh first" do + expect { subject }.to have_said("#", :deliver_halted) end end @@ -461,16 +480,18 @@ def system(cmd) end it "successfully finds a pull request for the current feature branch" do + allow(existing_pull_request).to receive(:good_to_merge?).and_return(true) + allow(existing_pull_request).to receive(:approvals).and_return(["Simon"]) + allow(existing_pull_request).to receive(:title).and_return(inputs['title']) expect { subject }.to have_output "Merging pull request #1: 'new-feature', from 'new-feature' into 'master'" end - it "checks out the destination branch and updates any remote changes" do - expect(GitReflow).to receive(:update_destination) - subject - end - it "merges and squashes the feature branch into the master branch" do - expect(GitReflow).to receive(:merge_feature_branch) + allow(existing_pull_request).to receive(:good_to_merge?).and_return(true) + allow(existing_pull_request).to receive(:approvals).and_return(["Simon"]) + allow(existing_pull_request).to receive(:title).and_return(inputs['title']) + expect(existing_pull_request).to receive(:merge_feature_branch).and_return({}) + allow_any_instance_of(Hash).to receive(:code).and_return("200") subject end end diff --git a/spec/lgtm_git_reflow_spec.rb b/spec/lgtm_git_reflow_spec.rb index adf8bbd..0a69bbc 100644 --- a/spec/lgtm_git_reflow_spec.rb +++ b/spec/lgtm_git_reflow_spec.rb @@ -40,7 +40,7 @@ context :deliver do let(:branch) { 'new-feature' } - let(:inputs) { {} } + let(:inputs) { { "title" => "new-feature", "head" => "reenhanced:new-feature" } } let!(:github) do allow_any_instance_of(GitReflow::GitServer::GitHub::PullRequest).to receive(:build).and_return(Struct.new(:state, :description, :url).new) stub_github_with({ @@ -54,7 +54,11 @@ before do - allow(GitReflow).to receive(:append_to_squashed_commit_message).and_return(true) + allow(existing_pull_request).to receive(:append_to_squashed_commit_message).and_return(true) + + # Stubs out the http response to github api + allow(GitReflow::GitServer::GitHub).to receive_message_chain(:connection, :pull_requests, :merge).and_return({}) + allow_any_instance_of(Hash).to receive(:code).and_return("200"); module Kernel def system(cmd) @@ -65,11 +69,6 @@ def system(cmd) subject { GitReflow.deliver inputs } - it "fetches the latest changes to the destination branch" do - expect(GitReflow).to receive(:fetch_destination).with('master') - subject - end - it "looks for a pull request matching the feature branch and destination branch" do expect(github).to receive(:find_open_pull_request).with(from: branch, to: 'master') subject @@ -81,7 +80,6 @@ def system(cmd) expect(github).to receive(:find_open_pull_request).and_return(existing_pull_request) allow(existing_pull_request).to receive(:has_comments?).and_return(true) allow(github).to receive(:reviewers).and_return(['codenamev']) - allow(existing_pull_request).to receive(:approvals).and_return(["Simon", "John"]) allow(existing_pull_request).to receive_message_chain(:last_comment, :match).and_return(true) end @@ -101,19 +99,22 @@ def system(cmd) context 'and build status is nil' do let(:build_status) { nil } - let(:inputs) {{ 'skip_lgtm' => true }} + let(:inputs) {{ 'skip_lgtm' => false }} before do # stubbing unrelated results so we can just test that it made it insdide the conditional block allow(existing_pull_request).to receive(:has_comments?).and_return(true) allow(existing_pull_request).to receive(:reviewers).and_return([]) - allow(GitReflow).to receive(:update_destination).and_return(true) - allow(GitReflow).to receive(:merge_feature_branch).and_return(true) - allow(GitReflow).to receive(:append_to_squashed_commit_message).and_return(true) + allow(existing_pull_request).to receive(:reviewers_pending_response).and_return([]) + allow(existing_pull_request).to receive(:approvals).and_return(['simonzhu24']) + # Stubs out the http response to github api + allow(GitReflow::GitServer::GitHub).to receive_message_chain(:connection, :pull_requests, :merge).and_return({}) + allow_any_instance_of(Hash).to receive(:code).and_return("200"); + allow(existing_pull_request).to receive(:append_to_squashed_commit_message).and_return(true) end it "ignores build status when not setup" do - expect { subject }.to have_said "Merge complete!", :success + expect { subject }.to have_said "Pull Request successfully merged.", :success end end @@ -136,8 +137,8 @@ def system(cmd) end it "doesn't include the pull request body in the commit message" do - squash_message = "#{existing_pull_request.body}\nCloses ##{existing_pull_request.number}\n\nLGTM given by: @nhance, @Simon\n" - expect(GitReflow).to receive(:append_to_squashed_commit_message).never.with(squash_message) + squash_message = "#{existing_pull_request.body}\nMerges ##{existing_pull_request.number}\n\nLGTM given by: @nhance, @Simon\n" + expect(existing_pull_request).to receive(:append_to_squashed_commit_message).never.with(squash_message) subject end @@ -152,19 +153,24 @@ def system(cmd) end it "doesn't include the first commit message for the new branch in the commit message of the merge" do - squash_message = "#{first_commit_message}\nCloses ##{existing_pull_request.number}\n\nLGTM given by: @nhance, @Simon\n" - expect(GitReflow).to receive(:append_to_squashed_commit_message).never.with(squash_message) + squash_message = "#{first_commit_message}\nMerges ##{existing_pull_request.number}\n\nLGTM given by: @nhance, @Simon\n" + expect(existing_pull_request).to receive(:append_to_squashed_commit_message).never.with(squash_message) subject end end it "doesn't notify user of the merge and performs it" do - expect(GitReflow).to receive(:merge_feature_branch).never.with('new-feature', { + expect(existing_pull_request).to receive(:merge_feature_branch).never.with('new-feature', { destination_branch: 'master', pull_request_number: existing_pull_request.number, lgtm_authors: ['nhance', 'Simon'], - message: existing_pull_request.body + title: existing_pull_request.title, + message: existing_pull_request.body, + head: existing_pull_request.head }) + # Stubs out the http response to github api + allow(GitReflow::GitServer::GitHub).to receive_message_chain(:connection, :pull_requests, :merge).and_return({}) + allow_any_instance_of(Hash).to receive(:code).and_return("200"); expect { subject }.to_not have_output "Merging pull request ##{existing_pull_request.number}: '#{existing_pull_request.title}', from '#{existing_pull_request.head.label}' into '#{existing_pull_request.base.label}'" end @@ -280,8 +286,8 @@ def system(cmd) end it "includes the pull request body in the commit message" do - squash_message = "#{existing_pull_request.body}\nCloses ##{existing_pull_request.number}\n\nLGTM given by: @nhance, @Simon\n" - expect(GitReflow).to receive(:append_to_squashed_commit_message).with(squash_message) + squash_message = "#{existing_pull_request.body}\nMerges ##{existing_pull_request.number}\n\nLGTM given by: @nhance, @Simon\n" + expect(existing_pull_request).to receive(:append_to_squashed_commit_message).with(squash_message) subject end @@ -309,7 +315,7 @@ def system(cmd) end it "commits the changes if the build status is nil but has comments/approvals and no pending response" do - expect{ subject }.to have_said 'Merge complete!', :success + expect{ subject }.to have_said 'Pull Request successfully merged.', :success end end @@ -324,30 +330,30 @@ def system(cmd) end it "includes the first commit message for the new branch in the commit message of the merge" do - squash_message = "#{first_commit_message}\nCloses ##{existing_pull_request.number}\n\nLGTM given by: @nhance, @Simon\n" - expect(GitReflow).to receive(:append_to_squashed_commit_message).with(squash_message) + squash_message = "#{first_commit_message}\nMerges ##{existing_pull_request.number}\n\nLGTM given by: @nhance, @Simon\n" + expect(existing_pull_request).to receive(:append_to_squashed_commit_message).with(squash_message) subject end end it "notifies user of the merge and performs it" do - expect(GitReflow).to receive(:merge_feature_branch).with('new-feature', { + expect(existing_pull_request).to receive(:merge_feature_branch).with('new-feature', { destination_branch: 'master', pull_request_number: existing_pull_request.number, lgtm_authors: ['nhance', 'Simon'], - message: existing_pull_request.body - }) + title: existing_pull_request.title, + message: existing_pull_request.body, + head: existing_pull_request.head + }).and_return({}) - expect { subject }.to have_output "Merging pull request ##{existing_pull_request.number}: '#{existing_pull_request.title}', from '#{existing_pull_request.head.label}' into '#{existing_pull_request.base.label}'" - end + # Stubs out the http response to github api + allow_any_instance_of(Hash).to receive(:code).and_return("200"); - it "updates the destination branch" do - expect(GitReflow).to receive(:update_destination).with('master') - subject + expect { subject }.to have_output "Merging pull request ##{existing_pull_request.number}: '#{existing_pull_request.title}', from '#{existing_pull_request.head.label}' into '#{existing_pull_request.base.label}'" end it "commits the changes for the squash merge" do - expect{ subject }.to have_said 'Merge complete!', :success + expect{ subject }.to have_said 'Pull Request successfully merged.', :success end context "and cleaning up feature branch" do @@ -358,7 +364,7 @@ def system(cmd) "Please enter your GitHub password (we do NOT store this): " => password, "Please enter your Enterprise site URL (e.g. https://github.company.com):" => enterprise_site, "Please enter your Enterprise API endpoint (e.g. https://github.company.com/api/v3):" => enterprise_api, - "Would you like to push this branch to your remote repo and cleanup your feature branch? " => 'yes', + "Would you like to cleanup your feature branch? " => 'yes', "Would you like to open it in your browser?" => 'no' } return_value = values[question] || values[terminal] @@ -372,8 +378,8 @@ def system(cmd) allow(GitReflow::Config).to receive(:get) { "false" } end - it "pushes local squash merged base branch to remote repo" do - expect { subject }.to have_run_command("git push origin master") + it "pulls changes from remote repo to local branch" do + expect { subject }.to have_run_command("git pull origin master") end it "deletes the remote feature branch" do @@ -390,8 +396,8 @@ def system(cmd) allow(GitReflow::Config).to receive(:get) { "true" } end - it "pushes local squash merged base branch to remote repo" do - expect { subject }.to have_run_command("git push origin master") + it "pulls changes from remote repo to local branch" do + expect { subject }.to have_run_command("git pull origin master") end it "deletes the remote feature branch" do @@ -438,10 +444,24 @@ def system(cmd) end end - context "and there were issues commiting the squash merge to the base branch" do - before { stub_with_fallback(GitReflow, :run_command_with_label).with('git commit', {with_system: true}).and_return false } - it "notifies user of issues commiting the squash merge of the feature branch" do - expect { subject }.to have_said("There were problems commiting your feature... please check the errors above and try again.", :error) + context "and the pull request is not mergeable" do + before { allow_any_instance_of(Hash).to receive(:code).and_return "405" } + it "notifies user to try refresh first" do + expect { subject }.to have_said("#", :deliver_halted) + end + end + + context "and the head branch was modified" do + before { allow_any_instance_of(Hash).to receive(:code).and_return "409" } + it "notifies user to try refresh first" do + expect { subject }.to have_said("#", :deliver_halted) + end + end + + context "and there was another error" do + before { allow_any_instance_of(Hash).to receive(:code).and_return "500" } + it "notifies user to try refresh first" do + expect { subject }.to have_said("#", :deliver_halted) end end @@ -465,21 +485,23 @@ def system(cmd) end it "notifies the user to get their code reviewed" do - expect { subject }.to have_said "Merge complete!", :success + expect { subject }.to have_said "Pull Request successfully merged.", :success end end it "successfully finds a pull request for the current feature branch" do + allow(existing_pull_request).to receive(:good_to_merge?).and_return(true) + allow(existing_pull_request).to receive(:approvals).and_return(["Simon"]) + allow(existing_pull_request).to receive(:title).and_return(inputs['title']) expect { subject }.to have_output "Merging pull request #1: 'new-feature', from 'new-feature' into 'master'" end - it "checks out the destination branch and updates any remote changes" do - expect(GitReflow).to receive(:update_destination) - subject - end - it "merges and squashes the feature branch into the master branch" do - expect(GitReflow).to receive(:merge_feature_branch) + allow(existing_pull_request).to receive(:good_to_merge?).and_return(true) + allow(existing_pull_request).to receive(:approvals).and_return(["Simon"]) + allow(existing_pull_request).to receive(:title).and_return(inputs['title']) + expect(existing_pull_request).to receive(:merge_feature_branch).and_return({}) + allow_any_instance_of(Hash).to receive(:code).and_return("200") subject end end