amadan
console: prefix the listen fatal, fix the notify test's false-skip risk
Fix round 1 review findings:
- cmd/console/main.go: log.Fatalf("console: listen %s: %v", addr, err)
instead of a bare log.Fatal(err), matching every other fatal in serve().
- cmd/console/sdnotify_test.go: build the unixgram socket under
os.MkdirTemp("", "n") with t.Cleanup, not t.TempDir — t.TempDir embeds
the test name in its path, and a long test name under a long TMPDIR can
push the socket path past sizeof(sockaddr_un.sun_path) (108 bytes on
Linux), which fails ListenPacket with "invalid argument" and was being
silently absorbed by a catch-all t.Skipf. Now only the genuinely
unsupported errors (EAFNOSUPPORT, EPERM) skip; anything else, including
a too-long path, fails the test loudly. Verified against a
reviewer-reported long TMPDIR: the old code skipped, the new code fails
with the real bind error instead — and passes clean under this
environment's actual (short) TMPDIR.
2 files changed,
+23
−4
cmd/console/main.go+1 −1cmd/console/sdnotify_test.go+22 −3
diff --git a/cmd/console/main.go b/cmd/console/main.go| index 38b55b902..9f860d956 100644 |
| --- a/cmd/console/main.go |
| +++ b/cmd/console/main.go |
| @@ -275,7 +275,7 @@ func serve() { |
| addr := envOr("CONSOLE_ADDR", "127.0.0.1:8090") |
| ln, err := net.Listen("tcp", addr) |
| if err != nil { |
| - log.Fatal(err) |
| + log.Fatalf("console: listen %s: %v", addr, err) |
| } |
| log.Printf("console: %s listening on %s", origin, addr) |
| // AFTER the listener is open and the store has migrated (openConsoleDB |
diff --git a/cmd/console/sdnotify_test.go b/cmd/console/sdnotify_test.go| index 7d269adee..968741c2e 100644 |
| --- a/cmd/console/sdnotify_test.go |
| +++ b/cmd/console/sdnotify_test.go |
| @@ -1,8 +1,11 @@ |
| package main |
| import ( |
| + "errors" |
| "net" |
| + "os" |
| "path/filepath" |
| + "syscall" |
| "testing" |
| "time" |
| ) |
| @@ -11,12 +14,28 @@ import ( |
| // process a unixgram socket path in NOTIFY_SOCKET and waits for READY=1 |
| // before calling the unit started — which is what turns "exec'd" into |
| // "actually serving" for the adopt pass's restart check. |
| +// |
| +// Uses os.MkdirTemp directly (NOT t.TempDir): t.TempDir embeds the test |
| +// name in the path it hands back, and AF_UNIX socket paths are capped at |
| +// sizeof(sockaddr_un.sun_path) — 108 bytes on Linux. A long test name under |
| +// a long TMPDIR pushes that over the limit and net.ListenPacket fails with |
| +// "invalid argument", which is a test-harness artifact, not evidence |
| +// unixgram sockets are unsupported here — so only the genuinely-unsupported |
| +// errors (EAFNOSUPPORT, EPERM) are allowed to skip the test; anything else, |
| +// including a too-long path, must fail loudly. |
| func TestNotifyReadyWritesToTheSocket(t *testing.T) { |
| - dir := t.TempDir() |
| - sock := filepath.Join(dir, "notify.sock") |
| + dir, err := os.MkdirTemp("", "n") |
| + if err != nil { |
| + t.Fatalf("MkdirTemp: %v", err) |
| + } |
| + t.Cleanup(func() { os.RemoveAll(dir) }) |
| + sock := filepath.Join(dir, "n.sock") |
| conn, err := net.ListenPacket("unixgram", sock) |
| if err != nil { |
| - t.Skipf("unixgram unavailable here: %v", err) |
| + if errors.Is(err, syscall.EAFNOSUPPORT) || errors.Is(err, syscall.EPERM) { |
| + t.Skipf("unixgram unavailable here: %v", err) |
| + } |
| + t.Fatalf("ListenPacket(unixgram, %s): %v", sock, err) |
| } |
| defer conn.Close() |
| t.Setenv("NOTIFY_SOCKET", sock) |