Merge pull request #5640 from jekyll/jekyll-utils-exec

Merge pull request 5640

features/support/helpers.rb

@@ -1,6 +1,5 @@
 require "fileutils"
 require "jekyll"
-require "open3"
 require "time"
 require "safe_yaml/load"
 
@@ -103,26 +102,18 @@ def run_jekyll(args)
 end
 
 #
-
-# rubocop:disable Metrics/AbcSize
 def run_in_shell(*args)
-  i, o, e, p = Open3.popen3(*args)
-  out = o.read.strip
-  err = e.read.strip
+  p, output = Jekyll::Utils::Exec.run(*args)
 
-  [i, o, e].each(&:close)
-
-  File.write(Paths.status_file, p.value.exitstatus)
+  File.write(Paths.status_file, p.exitstatus)
   File.open(Paths.output_file, "wb") do |f|
     f.puts "$ " << args.join(" ")
-    f.puts out
-    f.puts err
-    f.puts "EXIT STATUS: #{p.value.exitstatus}"
+    f.puts output
+    f.puts "EXIT STATUS: #{p.exitstatus}"
   end
 
-  p.value
+  p
 end
-# rubocop:enable Metrics/AbcSize
 
 #
 

lib/jekyll/commands/new.rb

@@ -125,23 +125,23 @@ RUBY
         # unless the user opts to generate a blank blog or skip 'bundle install'.
 
         def after_install(path, options = {})
-          Jekyll.logger.info "New jekyll site installed in #{path.cyan}."
-          Jekyll.logger.info "Bundle install skipped." if options["skip-bundle"]
-
           unless options["blank"] || options["skip-bundle"]
             bundle_install path
           end
+
+          Jekyll.logger.info "New jekyll site installed in #{path.cyan}."
+          Jekyll.logger.info "Bundle install skipped." if options["skip-bundle"]
         end
 
         def bundle_install(path)
           Jekyll::External.require_with_graceful_fail "bundler"
           Jekyll.logger.info "Running bundle install in #{path.cyan}..."
           Dir.chdir(path) do
-            if ENV["CI"]
-              system("bundle", "install", "--quiet")
-            else
-              system("bundle", "install")
-            end
+            process, output = Jekyll::Utils::Exec.run("bundle", "install")
+            output.to_s.each_line do |line|
+              Jekyll.logger.info("Bundler:".green, line.strip) unless line.to_s.empty?
+            end
+            raise SystemExit unless process.success?
           end
         end
       end

lib/jekyll/utils.rb

@@ -2,8 +2,9 @@
 module Jekyll
   module Utils
     extend self
-    autoload :Platforms, "jekyll/utils/platforms"
     autoload :Ansi, "jekyll/utils/ansi"
+    autoload :Exec, "jekyll/utils/exec"
+    autoload :Platforms, "jekyll/utils/platforms"
     autoload :WinTZ, "jekyll/utils/win_tz"
 
     # Constants for use in #slugify

lib/jekyll/utils/exec.rb

@@ -0,0 +1,25 @@
+require "open3"
+
+module Jekyll
+  module Utils
+    module Exec
+      extend self
+
+      # Runs a program in a sub-shell.
+      #
+      # *args - a list of strings containing the program name and arguments
+      #
+      # Returns a Process::Status and a String of output in an array in
+      # that order.
+      def run(*args)
+        stdin, stdout, stderr, process = Open3.popen3(*args)
+        out = stdout.read.strip
+        err = stderr.read.strip
+
+        [stdin, stdout, stderr].each(&:close)
+        [process.value, out + err]
+      end
+
+    end
+  end
+end

test/helper.rb

@@ -167,11 +167,11 @@ class JekyllUnitTest < Minitest::Test
   end
 
   def capture_output
-    stderr = StringIO.new
-    Jekyll.logger = Logger.new stderr
+    buffer = StringIO.new
+    Jekyll.logger = Logger.new(buffer)
     yield
-    stderr.rewind
-    return stderr.string.to_s
+    buffer.rewind
+    buffer.string.to_s
   end
   alias_method :capture_stdout, :capture_output
   alias_method :capture_stderr, :capture_output

test/test_new_command.rb

@@ -25,14 +25,14 @@ class TestNewCommand < JekyllUnitTest
 
     should "create a new directory" do
       refute_exist @full_path
-      Jekyll::Commands::New.process(@args)
+      capture_output { Jekyll::Commands::New.process(@args) }
       assert_exist @full_path
     end
 
     should "create a Gemfile" do
       gemfile = File.join(@full_path, "Gemfile")
       refute_exist @full_path
-      capture_stdout { Jekyll::Commands::New.process(@args) }
+      capture_output { Jekyll::Commands::New.process(@args) }
       assert_exist gemfile
       assert_match(%r!gem "jekyll", "#{Jekyll::VERSION}"!, File.read(gemfile))
       assert_match(%r!gem "github-pages"!, File.read(gemfile))
@@ -40,12 +40,11 @@ class TestNewCommand < JekyllUnitTest
 
     should "display a success message" do
       Jekyll::Commands::New.process(@args)
-      output = Jekyll.logger.messages[-3]
-      output_last = Jekyll.logger.messages.last
+      output = Jekyll.logger.messages
       success_message = "New jekyll site installed in #{@full_path.cyan}. "
       bundle_message = "Running bundle install in #{@full_path.cyan}... "
       assert_includes output, success_message
-      assert_includes output_last, bundle_message
+      assert_includes output, bundle_message
     end
 
     should "copy the static files in site template to the new directory" do
@@ -54,7 +53,7 @@ class TestNewCommand < JekyllUnitTest
       end
       static_template_files << "/Gemfile"
 
-      capture_stdout { Jekyll::Commands::New.process(@args) }
+      capture_output { Jekyll::Commands::New.process(@args) }
 
       new_site_files = dir_contents(@full_path).reject do |f|
         File.extname(f) == ".markdown"
@@ -76,7 +75,7 @@ class TestNewCommand < JekyllUnitTest
         f.gsub! "0000-00-00", stubbed_date
       end
 
-      capture_stdout { Jekyll::Commands::New.process(@args) }
+      capture_output { Jekyll::Commands::New.process(@args) }
 
       new_site_files = dir_contents(@full_path).select do |f|
         erb_template_files.include? f
@@ -87,7 +86,7 @@ class TestNewCommand < JekyllUnitTest
 
     should "create blank project" do
       blank_contents = %w(/_drafts /_layouts /_posts /index.html)
-      capture_stdout { Jekyll::Commands::New.process(@args, "--blank") }
+      capture_output { Jekyll::Commands::New.process(@args, "--blank") }
       output = Jekyll.logger.messages.last
       bundle_message = "Running bundle install in #{@full_path.cyan}..."
       assert_same_elements blank_contents, dir_contents(@full_path)
@@ -95,13 +94,13 @@ class TestNewCommand < JekyllUnitTest
     end
 
     should "force created folder" do
-      capture_stdout { Jekyll::Commands::New.process(@args) }
-      output = capture_stdout { Jekyll::Commands::New.process(@args, "--force") }
+      capture_output { Jekyll::Commands::New.process(@args) }
+      output = capture_output { Jekyll::Commands::New.process(@args, "--force") }
       assert_match(%r!New jekyll site installed in!, output)
     end
 
     should "skip bundle install when opted to" do
-      capture_stdout { Jekyll::Commands::New.process(@args, "--skip-bundle") }
+      capture_output { Jekyll::Commands::New.process(@args, "--skip-bundle") }
       output = Jekyll.logger.messages.last
       bundle_message = "Bundle install skipped."
       assert_includes output, bundle_message
@@ -120,7 +119,7 @@ class TestNewCommand < JekyllUnitTest
 
     should "create a new directory" do
       refute_exist @site_name_with_spaces
-      capture_stdout { Jekyll::Commands::New.process(@multiple_args) }
+      capture_output { Jekyll::Commands::New.process(@multiple_args) }
       assert_exist @site_name_with_spaces
     end
   end