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

The `brew services list` command was not correctly handling services
that had an error code status.

While the `#zero?` method returns a boolean, the `#nonzero?` method
confusingly returns self or nil. Hence a negated `#zero?` call to check
for a non-zero exit code fixes the error.

While here, `#pid?` method uses a negated `#zero?`, which is not
accurate, as a negative PID value would not be a valid PID. Hence I
changed it to use `#positive?` instead.

The tests for the `#error?` method were marked as needing systemd, but I
saw no obvious reason for that due to how they all use mocked values, so
I removed the systemd requirement.
This commit is contained in:
Jim Myhrberg 2025-03-15 19:42:33 +00:00
parent 4d55e48a16
commit f969c05b20
No known key found for this signature in database
GPG Key ID: B85A9E6D6BBB670E
2 changed files with 48 additions and 6 deletions

View File

@ -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) }

View File

@ -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
it "outputs false because there a no PID" do
allow(service).to receive(:pid).and_return(nil)
describe "#error?" do
it "outputs false because there is no PID or exit code" do
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