Merge pull request #476 from baburdick/fix_missing_exit_status_code

Use Open3.capture2e instead of %x{} and $?

lib/teaspoon/coverage.rb

@@ -1,3 +1,5 @@
+require 'open3'
+
 module Teaspoon
   class Coverage
     def self.configuration(name = Teaspoon.configuration.use_coverage)
@@ -23,7 +25,7 @@ def generate_reports(&block)
         results = []
         @config.reports.each do |format|
           result = generate_report(input, format)
-          results << result if ["text", "text-summary"].include?(format.to_s)
+          results << result if ['text', 'text-summary'].include?(format.to_s)
         end
         block.call(results.join("\n\n")) unless results.blank?
       end
@@ -33,32 +35,35 @@ def check_thresholds(&block)
       args = threshold_args
       return if args.blank?
       input_path do |input|
-        result = %x{#{@executable} check-coverage #{args.join(" ")} #{input.shellescape} 2>&1}
-        return if $?.exitstatus == 0
-        result = result.scan(/ERROR: .*$/).join("\n").gsub("ERROR: ", "")
+        result, st = Open3.capture2e(@executable, 'check-coverage', *args, input.shellescape)
+        return if st.exitstatus.zero?
+        result = result.scan(/ERROR: .*$/).join("\n").gsub('ERROR: ', '')
         block.call(result) unless result.blank?
       end
     end
 
     private
 
     def self.normalize_config_name(name)
-      return "default" if name == true
+      return 'default' if name == true
       name.to_s
     end
 
     def input_path(&block)
       Dir.mktmpdir do |temp_path|
-        input_path = File.join(temp_path, "coverage.json")
-        File.open(input_path, "w") { |f| f.write(@data.to_json) }
+        input_path = File.join(temp_path, 'coverage.json')
+        File.open(input_path, 'w') { |f| f.write(@data.to_json) }
         block.call(input_path)
       end
     end
 
     def generate_report(input, format)
       output_path = File.join(@config.output_path, @suite_name)
-      result = %x{#{@executable} report --include=#{input.shellescape} --dir #{output_path} #{format} 2>&1}
-      return result.gsub("Done", "").gsub("Using reporter [#{format}]", "").strip if $?.exitstatus == 0
+      result, st =
+        Open3.capture2e(
+          @executable, 'report', "--include=#{input.shellescape}", "--dir #{output_path}", format
+        )
+      return result.gsub('Done', '').gsub("Using reporter [#{format}]", '').strip if st.exitstatus.zero?
       raise Teaspoon::DependencyError.new("Unable to generate #{format} coverage report:\n#{result}")
     end
 

spec/teaspoon/coverage_spec.rb

@@ -6,6 +6,9 @@
   let(:data) { { foo: "bar" } }
   let(:config) { double }
 
+  let(:exit_status_0) { OpenStruct.new exitstatus: 0 }
+  let(:exit_status_1) { OpenStruct.new exitstatus: 1 }
+
   before do
     allow(Teaspoon::Instrumentation).to receive(:executable).and_return("/path/to/executable")
     allow(Teaspoon.configuration).to receive(:use_coverage).and_return(true)
@@ -57,19 +60,20 @@
 
     it "generates reports using istanbul and passes them to the block provided" do
       stub_exit_code(ExitCodes::SUCCESS)
-      html_report = "/path/to/executable report --include=/temp_path/coverage.json --dir output/path/_suite_ html 2>&1"
-      text1_report = "/path/to/executable report --include=/temp_path/coverage.json --dir output/path/_suite_ text 2>&1"
-      text2_report = "/path/to/executable report --include=/temp_path/coverage.json --dir output/path/_suite_ text-summary 2>&1"
-      expect(subject).to receive(:`).with(html_report).and_return("_html_report_")
-      expect(subject).to receive(:`).with(text1_report).and_return("_text1_report_")
-      expect(subject).to receive(:`).with(text2_report).and_return("_text2_report_")
+      base_report  = %w[/path/to/executable report --include=/temp_path/coverage.json --dir\ output/path/_suite_]
+      html_report  = base_report + %w[html]
+      text1_report = base_report + %w[text]
+      text2_report = base_report + %w[text-summary]
+      expect(Open3).to receive(:capture2e).with(*html_report). and_return(["_html_report_",  exit_status_0])
+      expect(Open3).to receive(:capture2e).with(*text1_report).and_return(["_text1_report_", exit_status_0])
+      expect(Open3).to receive(:capture2e).with(*text2_report).and_return(["_text2_report_", exit_status_0])
       subject.generate_reports { |r| @result = r }
       expect(@result).to eq("_text1_report_\n\n_text2_report_")
     end
 
     it "raises an exception if the command doesn't exit cleanly" do
       stub_exit_code(ExitCodes::EXCEPTION)
-      allow(subject).to receive(:`).and_return("Results could not be generated.")
+      allow(Open3).to receive(:capture2e).and_return(["Results could not be generated.", exit_status_1])
 
       expect { subject.generate_reports }.to raise_error(
         Teaspoon::DependencyError,
@@ -89,16 +93,17 @@
 
     it "checks the coverage using istanbul and passes them to the block provided" do
       stub_exit_code(ExitCodes::EXCEPTION)
-      opts = "--statements=42 --functions=66.6 --branches=0 --lines=100"
-      expect(subject).to receive(:`).with("/path/to/executable check-coverage #{opts} /temp_path/coverage.json 2>&1").
-        and_return("some mumbo jumbo\nERROR: _failure1_\nmore garbage\nERROR: _failure2_")
+      opts = %w[--statements=42 --functions=66.6 --branches=0 --lines=100]
+      args = ["/path/to/executable", "check-coverage", *opts, "/temp_path/coverage.json"]
+      expect(Open3).to receive(:capture2e).with(*args).
+        and_return(["some mumbo jumbo\nERROR: _failure1_\nmore garbage\nERROR: _failure2_", exit_status_1])
       subject.check_thresholds { |r| @result = r }
       expect(@result).to eq("_failure1_\n_failure2_")
     end
 
     it "doesn't call the callback if the exit status is 0" do
       stub_exit_code(ExitCodes::SUCCESS)
-      expect(subject).to receive(:`).and_return("ERROR: _failure1_")
+      expect(Open3).to receive(:capture2e).and_return(["ERROR: _failure1_", exit_status_0])
       subject.check_thresholds { @called = true }
       expect(@called).to be_falsey
     end
@@ -114,7 +119,7 @@
       Teaspoon::Instrumentation.instance_variable_set(:@executable_checked, nil)
       expect(Teaspoon::Instrumentation).to receive(:executable).and_call_original
       expect(subject).to receive(:input_path).and_call_original
-      expect(subject).to receive(:`).and_call_original
+      expect(Open3).to receive(:capture2e).twice.and_call_original
 
       pending("needs istanbul to be installed") unless executable
       subject.instance_variable_set(:@executable, executable)
@@ -123,7 +128,7 @@
 
     it "generates coverage reports" do
       subject.generate_reports { |r| @report = r }
-      expect(@report).to eq <<-RESULT.strip_heredoc + "\n"
+      expect(@report).to eq <<-RESULT.strip_heredoc.chomp
         ---------------------|----------|----------|----------|----------|----------------|
         File                 |  % Stmts | % Branch |  % Funcs |  % Lines |Uncovered Lines |
         ---------------------|----------|----------|----------|----------|----------------|
@@ -133,6 +138,13 @@
         ---------------------|----------|----------|----------|----------|----------------|
         All files            |    90.91 |      100 |       75 |    90.91 |                |
         ---------------------|----------|----------|----------|----------|----------------|
+
+        =============================== Coverage summary ===============================
+        Statements   : 90.91% ( 10/11 )
+        Branches     : 100% ( 0/0 )
+        Functions    : 75% ( 3/4 )
+        Lines        : 90.91% ( 10/11 )
+        ================================================================================
       RESULT
     end
   end