Merge pull request #19502 from jimeh/fix-brew-services-list

fix(services/list): correctly handle services with an error code

Library/Homebrew/services/formula_wrapper.rb

@@ -161,14 +161,14 @@ module Homebrew
 
       sig { returns(T::Boolean) }
       def pid?
-        pid.present? && !pid.zero?
+        pid.present? && pid.positive?
       end
 
       sig { returns(T::Boolean) }
       def error?
         return false if pid?
 
-        exit_code.present? && exit_code.nonzero?
+        exit_code.present? && !exit_code.zero?
       end
 
       sig { returns(T::Boolean) }

Library/Homebrew/test/services/formula_wrapper_spec.rb

@@ -218,10 +218,26 @@ RSpec.describe Homebrew::Services::FormulaWrapper do
   end
 
   describe "#pid?" do
-    it "outputs false because there is not pid" do
+    it "outputs false because there is not PID" do
       allow(service).to receive(:pid).and_return(nil)
       expect(service.pid?).to be(false)
     end
+
+    it "outputs false because there is a PID and it is zero" do
+      allow(service).to receive(:pid).and_return(0)
+      expect(service.pid?).to be(false)
+    end
+
+    it "outputs true because there is a PID and it is positive" do
+      allow(service).to receive(:pid).and_return(12)
+      expect(service.pid?).to be(true)
+    end
+
+    # This should never happen in practice, as PIDs cannot be negative.
+    it "outputs false because there is a PID and it is negative" do
+      allow(service).to receive(:pid).and_return(-1)
+      expect(service.pid?).to be(false)
+    end
   end
 
   describe "#pid", :needs_systemd do
@@ -230,9 +246,9 @@ RSpec.describe Homebrew::Services::FormulaWrapper do
     end
   end
 
-  describe "#error?", :needs_systemd do
+  describe "#error?" do
-    it "outputs false because there a no PID" do
+    it "outputs false because there is no PID or exit code" do
-      allow(service).to receive(:pid).and_return(nil)
+      allow(service).to receive_messages(pid: nil, exit_code: nil)
       expect(service.error?).to be(false)
     end
 
@@ -240,6 +256,32 @@ RSpec.describe Homebrew::Services::FormulaWrapper do
       allow(service).to receive_messages(pid: 12, exit_code: nil)
       expect(service.error?).to be(false)
     end
+
+    it "outputs false because there is a PID and a zero exit code" do
+      allow(service).to receive_messages(pid: 12, exit_code: 0)
+      expect(service.error?).to be(false)
+    end
+
+    it "outputs false because there is a PID and a positive exit code" do
+      allow(service).to receive_messages(pid: 12, exit_code: 1)
+      expect(service.error?).to be(false)
+    end
+
+    it "outputs false because there is no PID and a zero exit code" do
+      allow(service).to receive_messages(pid: nil, exit_code: 0)
+      expect(service.error?).to be(false)
+    end
+
+    it "outputs true because there is no PID and a positive exit code" do
+      allow(service).to receive_messages(pid: nil, exit_code: 1)
+      expect(service.error?).to be(true)
+    end
+
+    # This should never happen in practice, as exit codes cannot be negative.
+    it "outputs true because there is no PID and a negative exit code" do
+      allow(service).to receive_messages(pid: nil, exit_code: -1)
+      expect(service.error?).to be(true)
+    end
   end
 
   describe "#exit_code", :needs_systemd do