Use Process::Status rather than an integer

However keep backward compatibility

lib/gitlab/popen.rb

@@ -11,7 +11,7 @@ module Gitlab
     def popen(cmd, path = nil, vars = {}, &block)
       result = popen_with_detail(cmd, path, vars, &block)
 
-      [result.stdout << result.stderr, result.status]
+      [result.stdout << result.stderr, result.status&.exitstatus]
     end
 
     # Returns Result
@@ -30,7 +30,7 @@ module Gitlab
 
       cmd_stdout = ''
       cmd_stderr = ''
-      cmd_status = 0
+      cmd_status = nil
       start = Time.now
 
       Open3.popen3(vars, *cmd, options) do |stdin, stdout, stderr, wait_thr|
@@ -39,7 +39,7 @@ module Gitlab
 
         cmd_stdout = stdout.read
         cmd_stderr = stderr.read
-        cmd_status = wait_thr.value.exitstatus
+        cmd_status = wait_thr.value
       end
 
       Result.new(cmd, cmd_stdout, cmd_stderr, cmd_status, Time.now - start)

lib/gitlab/popen/runner.rb

@@ -20,12 +20,12 @@ module Gitlab
         end
       end
 
-      def all_good?
-        all_status_zero? && all_stderr_empty?
+      def all_success_and_clean?
+        all_success? && all_stderr_empty?
       end
 
-      def all_status_zero?
-        results.all? { |result| result.status.zero? }
+      def all_success?
+        results.all? { |result| result.status.success? }
       end
 
       def all_stderr_empty?
@@ -33,12 +33,12 @@ module Gitlab
       end
 
       def failed_results
-        results.select { |result| result.status.nonzero? }
+        results.reject { |result| result.status.success? }
       end
 
       def warned_results
         results.select do |result|
-          result.status.zero? && !result.stderr.empty?
+          result.status.success? && !result.stderr.empty?
         end
       end
     end

scripts/static-analysis

@@ -57,9 +57,9 @@ puts '==================================================='
 puts
 puts
 
-if static_analysis.all_good?
+if static_analysis.all_success_and_clean?
   puts 'All static analyses passed successfully.'
-elsif static_analysis.all_status_zero?
+elsif static_analysis.all_success?
   puts 'All static analyses passed successfully, but we have warnings:'
   puts
 

spec/lib/gitlab/popen/runner_spec.rb

@@ -11,43 +11,43 @@ describe Gitlab::Popen::Runner do
     end
   end
 
-  describe '#all_good?' do
+  describe '#all_success_and_clean?' do
     it 'returns true when exit status is 0 and stderr is empty' do
       run_command
 
-      expect(subject).to be_all_good
+      expect(subject).to be_all_success_and_clean
     end
 
     it 'returns false when exit status is not 0' do
       run_command(exitstatus: 1)
 
-      expect(subject).not_to be_all_good
+      expect(subject).not_to be_all_success_and_clean
     end
 
     it 'returns false when exit stderr has something' do
       run_command(stderr: 'stderr')
 
-      expect(subject).not_to be_all_good
+      expect(subject).not_to be_all_success_and_clean
     end
   end
 
-  describe '#all_status_zero?' do
+  describe '#all_success?' do
     it 'returns true when exit status is 0' do
       run_command
 
-      expect(subject).to be_all_status_zero
+      expect(subject).to be_all_success
     end
 
     it 'returns false when exit status is not 0' do
       run_command(exitstatus: 1)
 
-      expect(subject).not_to be_all_status_zero
+      expect(subject).not_to be_all_success
     end
 
     it 'returns true' do
       run_command(stderr: 'stderr')
 
-      expect(subject).to be_all_status_zero
+      expect(subject).to be_all_success
     end
   end
 

spec/lib/gitlab/popen_spec.rb

@@ -16,7 +16,7 @@ describe Gitlab::Popen do
     it { expect(subject.cmd).to eq(cmd) }
     it { expect(subject.stdout).to eq("1\n") }
     it { expect(subject.stderr).to eq("2\n") }
-    it { expect(subject.status).to eq(3) }
+    it { expect(subject.status.exitstatus).to eq(3) }
     it { expect(subject.duration).to be_kind_of(Numeric) }
   end