Permalink
Browse files

improve preserving message from failed pull request

 - If text editor aborts, message is discarded.
 - If the API request to create the PR fails, message is kept.
 - If the API request succeeds, message is discarded.
 - When kept message is reused for subsequent PR, only its title and
   body get inserted. The PR instructions and changelog still get
   regenerated.

Fixes #178
  • Loading branch information...
1 parent 85d4220 commit a53f02893a14378d9edf184f58b55dd8127539f5 @mislav mislav committed with mislav May 11, 2013
Showing with 123 additions and 21 deletions.
  1. +67 −0 features/pull_request.feature
  2. +15 −0 features/steps.rb
  3. +11 −0 features/support/env.rb
  4. +30 −21 lib/hub/commands.rb
View
67 features/pull_request.feature
@@ -80,3 +80,70 @@ Feature: hub pull-request
"""
When I successfully run `hub pull-request useragent`
Then the output should contain exactly "the://url\n"
+
+ Scenario: Text editor adds title and body
+ Given the "origin" remote has url "git://github.com/mislav/coral.git"
+ And the text editor adds:
+ """
+ This title comes from vim!
+
+ This body as well.
+ """
+ Given the GitHub API server:
+ """
+ post('/repos/mislav/coral/pulls') {
+ { :title => 'This title comes from vim!',
+ :body => 'This body as well.'
+ }.each do |param, value|
+ if params[param] != value
+ halt 422, json(
+ :message => "expected %s to be %s; got %s" % [
+ param.inspect,
+ value.inspect,
+ params[param].inspect
+ ]
+ )
+ end
+ end
+ json :html_url => "https://github.com/mislav/coral/pull/12"
+ }
+ """
+ When I successfully run `hub pull-request`
+ Then the output should contain exactly "https://github.com/mislav/coral/pull/12\n"
+ And the file ".git/PULLREQ_EDITMSG" should not exist
+
+ Scenario: Failed pull request preserves previous message
+ Given the "origin" remote has url "git://github.com/mislav/coral.git"
+ And the text editor adds:
+ """
+ This title will fail
+ """
+ Given the GitHub API server:
+ """
+ post('/repos/mislav/coral/pulls') {
+ halt 422 if params[:title].include?("fail")
+ halt 422 unless params[:body] == "This title will fail"
+ json :html_url => "https://github.com/mislav/coral/pull/12"
+ }
+ """
+ When I run `hub pull-request`
+ Then the exit status should be 1
+ And the stderr should contain exactly:
+ """
+ Error creating pull request: (HTTP 422)\n
+ """
+ Given the text editor adds:
+ """
+ But this title will prevail
+ """
+ When I successfully run `hub pull-request`
+ Then the file ".git/PULLREQ_EDITMSG" should not exist
+
+ Scenario: Text editor fails
+ Given the "origin" remote has url "git://github.com/mislav/coral.git"
+ And the text editor exits with error status
+ And an empty file named ".git/PULLREQ_EDITMSG"
+ When I run `hub pull-request`
+ Then the stderr should contain "error using text editor for pull request message"
+ And the exit status should be 1
+ And the file ".git/PULLREQ_EDITMSG" should not exist
View
15 features/steps.rb
@@ -159,3 +159,18 @@
Given /^the remote commit state of "(.*?)" "(.*?)" is nil$/ do |proj, ref|
step %{the remote commit states of "#{proj}" "#{ref}" are:}, "[ ]"
end
+
+Given /^the text editor exits with error status$/ do
+ text_editor_script "exit 1"
+end
+
+Given /^the text editor adds:$/ do |text|
+ text_editor_script <<-BASH
+ file="$3"
+ contents="$(cat "$file" 2>/dev/null || true)"
+ { echo "#{text}"
+ echo
+ echo "$contents"
+ } > "$file"
+ BASH
+end
View
11 features/support/env.rb
@@ -35,6 +35,8 @@ def close_streams
set_env 'HUB_TEST_HOST', '127.0.0.1:0'
# ensure we use fakebin `open` to test browsing
set_env 'BROWSER', 'open'
+ # ensure we use fakebin `vim` as text editor
+ set_env 'GIT_EDITOR', 'vim'
author_name = "Hub"
author_email = "hub@test.local"
@@ -56,6 +58,7 @@ def close_streams
After do
@server.stop if defined? @server and @server
processes.each {|_, p| p.close_streams }
+ FileUtils.rm_f("#{bin_dir}/vim")
end
RSpec::Matchers.define :be_successful_command do
@@ -124,6 +127,14 @@ def edit_hub_config
File.open(config, 'w') { |cfg| cfg << YAML.dump(data) }
end
+ define_method(:text_editor_script) do |bash_code|
+ File.open("#{bin_dir}/vim", 'w', 0755) do |exe|
+ exe.puts "#!/bin/bash"
+ exe.puts "set -e"
+ exe.puts bash_code
+ end
+ end
+
def run_silent cmd
in_current_dir do
command = SimpleCommand.run(cmd)
View
51 lib/hub/commands.rb
@@ -206,8 +206,9 @@ def pull_request(args)
[format, base_branch, remote_branch]
end
- options[:title], options[:body] = pullrequest_editmsg(commit_summary) { |msg|
- msg.puts default_message if default_message
+ options[:title], options[:body] = pullrequest_editmsg(commit_summary) { |msg, initial_message|
+ initial_message ||= default_message
+ msg.puts initial_message if initial_message
msg.puts ""
msg.puts "# Requesting a pull to #{base_project.owner}:#{options[:base]} from #{options[:head]}"
msg.puts "#"
@@ -218,8 +219,6 @@ def pull_request(args)
pull = api_client.create_pullrequest(options)
- delete_pullreq_editmsg_file
-
args.executable = 'echo'
args.replace [pull['html_url']]
rescue GitHubAPI::Exceptions
@@ -230,6 +229,8 @@ def pull_request(args)
warn "Are you sure that #{base_url} exists?"
end
exit 1
+ else
+ delete_editmsg
end
# $ hub clone rtomayko/tilt
@@ -997,29 +998,41 @@ def page_stdout
end
def pullrequest_editmsg(changes)
- message_file = pullreq_editmsg_file
-
- unless File.exists?(message_file)
- File.open(message_file, 'w') { |msg|
- yield msg
- if changes
- msg.puts "#\n# Changes:\n#"
- msg.puts changes.gsub(/^/, '# ').gsub(/ +$/, '')
- end
- }
+ message_file = pullrequest_editmsg_file
+
+ if File.exists?(message_file)
+ title, body = read_editmsg(message_file)
+ previous_message = [title, body].compact.join("\n\n") if title
end
+ File.open(message_file, 'w') { |msg|
+ yield msg, previous_message
+ if changes
+ msg.puts "#\n# Changes:\n#"
+ msg.puts changes.gsub(/^/, '# ').gsub(/ +$/, '')
+ end
+ }
+
edit_cmd = Array(git_editor).dup
edit_cmd << '-c' << 'set ft=gitcommit' if edit_cmd[0] =~ /^[mg]?vim$/
edit_cmd << message_file
system(*edit_cmd)
- abort "can't open text editor for pull request message" unless $?.success?
+
+ unless $?.success?
+ # writing was cancelled, or the editor never opened in the first place
+ delete_editmsg(message_file)
+ abort "error using text editor for pull request message"
+ end
title, body = read_editmsg(message_file)
abort "Aborting due to empty pull request title" unless title
[title, body]
end
+ def pullrequest_editmsg_file
+ File.join(git_dir, 'PULLREQ_EDITMSG')
+ end
+
def read_editmsg(file)
title, body = '', ''
File.open(file, 'r') { |msg|
@@ -1035,12 +1048,8 @@ def read_editmsg(file)
[title =~ /\S/ ? title : nil, body =~ /\S/ ? body : nil]
end
- def delete_pullreq_editmsg_file
- File.delete(pullreq_editmsg_file) if File.exists?(pullreq_editmsg_file)
- end
-
- def pullreq_editmsg_file
- File.join(git_dir, 'PULLREQ_EDITMSG')
+ def delete_editmsg(file = pullrequest_editmsg_file)
+ File.delete(file) if File.exist?(file)
end
def expand_alias(cmd)

0 comments on commit a53f028

Please sign in to comment.