Merge branch 'changelog-code-style' into 'master'

Improve shelling out in bin/changelog

See merge request gitlab-org/gitlab-ce!18977

bin/changelog

@@ -19,7 +19,24 @@ Options = Struct.new(
 )
 INVALID_TYPE = -1
 
+module ChangelogHelpers
+  Abort = Class.new(StandardError)
+  Done = Class.new(StandardError)
+
+  def capture_stdout(cmd)
+    output = IO.popen(cmd, &:read)
+    fail_with "command failed: #{cmd.join(' ')}" unless $?.success?
+    output
+  end
+
+  def fail_with(message)
+    raise Abort, "\e[31merror\e[0m #{message}"
+  end
+end
+
 class ChangelogOptionParser
+  extend ChangelogHelpers
+
   Type = Struct.new(:name, :description)
   TYPES = [
     Type.new('added', 'New feature'),
@@ -68,7 +85,7 @@ class ChangelogOptionParser
 
         opts.on('-h', '--help', 'Print help message') do
           $stdout.puts opts
-          exit
+          raise Done.new
         end
       end
 
@@ -108,18 +125,19 @@ class ChangelogOptionParser
 
     def assert_valid_type!(type)
       unless type
-        $stderr.puts "Invalid category index, please select an index between 1 and #{TYPES.length}"
-        exit 1
+        raise Abort, "Invalid category index, please select an index between 1 and #{TYPES.length}"
       end
     end
 
     def git_user_name
-      %x{git config user.name}.strip
+      capture_stdout(%w[git config user.name]).strip
     end
   end
 end
 
 class ChangelogEntry
+  include ChangelogHelpers
+
   attr_reader :options
 
   def initialize(options)
@@ -159,13 +177,9 @@ class ChangelogEntry
   end
 
   def amend_commit
-    %x{git add #{file_path}}
-    exec("git commit --amend")
-  end
+    fail_with "git add failed" unless system(*%W[git add #{file_path}])
 
-  def fail_with(message)
-    $stderr.puts "\e[31merror\e[0m #{message}"
-    exit 1
+    Kernel.exec(*%w[git commit --amend])
   end
 
   def assert_feature_branch!
@@ -203,7 +217,7 @@ class ChangelogEntry
   end
 
   def last_commit_subject
-    %x{git log --format="%s" -1}.strip
+    capture_stdout(%w[git log --format=%s -1]).strip
   end
 
   def file_path
@@ -225,7 +239,7 @@ class ChangelogEntry
   end
 
   def branch_name
-    @branch_name ||= %x{git symbolic-ref --short HEAD}.strip
+    @branch_name ||= capture_stdout(%w[git symbolic-ref --short HEAD]).strip
   end
 
   def remove_trailing_whitespace(yaml_content)
@@ -234,8 +248,15 @@ class ChangelogEntry
 end
 
 if $0 == __FILE__
+  begin
     options = ChangelogOptionParser.parse(ARGV)
     ChangelogEntry.new(options)
+  rescue ChangelogHelpers::Abort => ex
+    $stderr.puts ex.message
+    exit 1
+  rescue ChangelogHelpers::Done
+    exit
+  end
 end
 
 # vim: ft=ruby

spec/bin/changelog_spec.rb

@@ -56,11 +56,11 @@ describe 'bin/changelog' do
       it 'parses -h' do
         expect do
           expect { described_class.parse(%w[foo -h bar]) }.to output.to_stdout
-        end.to raise_error(SystemExit)
+        end.to raise_error(ChangelogHelpers::Done)
       end
 
       it 'assigns title' do
-        options = described_class.parse(%W[foo -m 1 bar\n -u baz\r\n --amend])
+        options = described_class.parse(%W[foo -m 1 bar\n baz\r\n --amend])
 
         expect(options.title).to eq 'foo bar baz'
       end
@@ -82,9 +82,10 @@ describe 'bin/changelog' do
         it 'shows error message and exits the program' do
           allow($stdin).to receive(:getc).and_return(type)
           expect do
-            expect do
-              expect { described_class.read_type }.to raise_error(SystemExit)
-            end.to output("Invalid category index, please select an index between 1 and 8\n").to_stderr
+            expect { described_class.read_type }.to raise_error(
+              ChangelogHelpers::Abort,
+              'Invalid category index, please select an index between 1 and 8'
+            )
           end.to output.to_stdout
         end
       end