Skip to content

Fix: Follow-up review findings from #1076/#1080 #1107

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
huang195 merged 1 commit into from
Sep 23, 2026
Merged

Fix: Follow-up review findings from #1076/#1080 #1107

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 21 additions & 3 deletions authbridge/cmd/abctl/cmd_service_systemd_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -348,13 +348,16 @@ echo "unit not loaded" >&2
exit 1
`)
err := unloadService("linux", p)
if err == nil || !strings.Contains(err.Error(), "disable") || !strings.Contains(err.Error(), "unit not loaded") {
t.Errorf("err = %v, want it to name disable --now and the underlying reason", err)
if err == nil ||
!strings.Contains(err.Error(), "--user disable --now cortex.service") ||
!strings.Contains(err.Error(), "unit not loaded") {
t.Errorf("err = %v, want it to name the exact systemctl invocation and the underlying reason", err)
}
})
}

func TestLingerEnabled(t *testing.T) {
uid := strconv.Itoa(os.Getuid())
cases := []struct {
name string
body string
Expand All @@ -370,11 +373,26 @@ func TestLingerEnabled(t *testing.T) {
for _, tc := range cases {
t.Run(tc.name, func(t *testing.T) {
fakeLoginctl(t, tc.body)
if got := lingerEnabled("501"); got != tc.want {
if got := lingerEnabled(uid); got != tc.want {
t.Errorf("lingerEnabled() = %v, want %v", got, tc.want)
}
})
}

// The cases above only check the parsed result — a wrong property flag or a
// wrong uid would leave all four green, since fakeLoginctl answers the same
// way regardless of what it was actually asked. This pins the invocation
// itself, closing that the same way callLog does everywhere else in this file.
t.Run("invokes show-user with the exact uid and property", func(t *testing.T) {
logPath, logLine := callLog(t)
fakeLoginctl(t, "#!/bin/sh\n"+logLine+"\necho 'Linger=yes'\nexit 0\n")
lingerEnabled(uid)

@huang195 huang195 Sep 23, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: the return value is discarded here, so the stub's echo 'Linger=yes' is inert — this subtest passes identically with a bare exit 0 body. The sibling invocation-pinning subtests in this file (TestControlService_Linux, the tc.action loop) do check what came back.

A one-liner would keep the canned output load-bearing and match that style:

if !lingerEnabled(uid) {
    t.Error("lingerEnabled() = false, want true for Linger=yes")
}

The explicit yes table case already covers the parse, so this is belt-and-braces — take it or leave it.

calls := readCallLog(t, logPath)
want := "show-user " + uid + " --property=Linger"
if len(calls) != 1 || calls[0] != want {
t.Errorf("loginctl calls = %v, want exactly [%s]", calls, want)
}
})
}

func TestControlService_Linux(t *testing.T) {
Expand Down
4 changes: 2 additions & 2 deletions authbridge/docs/superpowers/specs/2026-09-16-linux-systemd-lifecycle-design.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -128,8 +128,8 @@ of systemd's own 90s default (`systemd.system.conf(5)`) exceeding 15s.
**Closed by #1079** — `renderUnitFor("linux", ...)` now sets `TimeoutStopSec=20`
explicitly on `main`, matching the macOS supervisor's 20s headroom over the same
15s drain, with a subtest in `cmd_service_test.go` asserting the line is present.
Not yet on this branch's own tree until it merges — cited by symbol, not line,
since #1079 landed in a sibling PR.
Cited by symbol, not line, since #1079 landed in a sibling PR and this file's own
history spans branches where it wasn't there yet.

### 8. Works under user systemd, and states what happens where systemd is absent
The best-handled bullet: `loadService` gives a clear, actionable message when
Expand Down
Loading