diff --git a/internal/selfupdate/updater.go b/internal/selfupdate/updater.go index c2a5c680b..d9cb5ab97 100644 --- a/internal/selfupdate/updater.go +++ b/internal/selfupdate/updater.go @@ -17,6 +17,13 @@ import ( "github.com/larksuite/cli/internal/vfs" ) +// execLookPath is the LookPath implementation used by VerifyBinary. +// It defaults to the standard library exec.LookPath but is swapped in tests +// via lookPathMock to provide controlled binary resolution. +// +// Tests that mutate execLookPath must not call t.Parallel(). +var execLookPath = exec.LookPath + // InstallMethod describes how the CLI was installed. type InstallMethod int @@ -186,13 +193,13 @@ func (u *Updater) VerifyBinary(expectedVersion string) error { if u.VerifyOverride != nil { return u.VerifyOverride(expectedVersion) } - // Prefer the current executable path (what the user actually launched). - // Use Executable() directly without EvalSymlinks — after npm install the - // symlink target may have changed, but the path itself is still valid for - // execution. Fall back to LookPath only if Executable() fails entirely. - exe, err := vfs.Executable() + // Prefer PATH resolution so npm global bin symlinks pick up the newly + // installed binary (#836). If `lark-cli` is not on PATH (e.g. the user + // invoked this process by absolute path), fall back to the running + // executable — same as the pre-#836 secondary resolution path. + exe, err := execLookPath("lark-cli") if err != nil { - exe, err = exec.LookPath("lark-cli") + exe, err = vfs.Executable() if err != nil { return fmt.Errorf("cannot locate binary: %w", err) } diff --git a/internal/selfupdate/updater_test.go b/internal/selfupdate/updater_test.go index a46b51cb0..f13c80b65 100644 --- a/internal/selfupdate/updater_test.go +++ b/internal/selfupdate/updater_test.go @@ -4,6 +4,7 @@ package selfupdate import ( + "fmt" "os" "path/filepath" "runtime" @@ -12,6 +13,7 @@ import ( "github.com/larksuite/cli/internal/vfs" ) +// executableTestFS mocks vfs for tests that still need vfs.Executable. type executableTestFS struct { vfs.OsFs exe string @@ -19,6 +21,28 @@ type executableTestFS struct { func (f executableTestFS) Executable() (string, error) { return f.exe, nil } +// lookPathMock patches execLookPath within VerifyBinary for controlled testing. +// Do not use t.Parallel() in tests that install this mock — it mutates a package-level var. +type lookPathMock struct { + oldLookPath func(string) (string, error) + result string + resultErr error +} + +func (m *lookPathMock) install(bin string) { + m.oldLookPath = execLookPath + execLookPath = func(name string) (string, error) { + if name == bin { + return m.result, m.resultErr + } + return m.oldLookPath(name) + } +} + +func (m *lookPathMock) restore() { + execLookPath = m.oldLookPath +} + func TestResolveExe(t *testing.T) { u := New() p, err := u.resolveExe() @@ -44,46 +68,101 @@ func TestCleanupStaleFiles_NoPanic(t *testing.T) { u.CleanupStaleFiles() } -func TestVerifyBinaryChecksVersion(t *testing.T) { +func TestVerifyBinaryLookPath(t *testing.T) { + t.Setenv("LARKSUITE_CLI_CONFIG_DIR", t.TempDir()) if runtime.GOOS == "windows" { t.Skip("uses a POSIX shell script") } dir := t.TempDir() - exe := filepath.Join(dir, "lark-cli") - // Script prints version string matching real CLI format when --version is passed. - script := "#!/bin/sh\nif [ \"$1\" = \"--version\" ]; then echo \"lark-cli version 2.0.0\"; exit 0; fi\nexit 12\n" - if err := os.WriteFile(exe, []byte(script), 0755); err != nil { + bin := filepath.Join(dir, "lark-cli") + script := "#!/bin/sh\nif [ \"$1\" = \"--version\" ]; then echo \"lark-cli version 2.1.0\"; exit 0; fi\nexit 12\n" + if err := os.WriteFile(bin, []byte(script), 0755); err != nil { t.Fatalf("write test binary: %v", err) } - // Mock vfs.Executable to return our test script, matching VerifyBinary's - // primary lookup path. Also prepend to PATH for the LookPath fallback. - origFS := vfs.DefaultFS - vfs.DefaultFS = executableTestFS{OsFs: vfs.OsFs{}, exe: exe} - t.Cleanup(func() { vfs.DefaultFS = origFS }) + mock := &lookPathMock{result: bin} + mock.install("lark-cli") + t.Cleanup(mock.restore) - origPath := os.Getenv("PATH") - t.Setenv("PATH", dir+string(os.PathListSeparator)+origPath) - - // Matching version → success. - if err := New().VerifyBinary("2.0.0"); err != nil { - t.Fatalf("VerifyBinary(matching) error = %v, want nil", err) + if err := New().VerifyBinary("2.1.0"); err != nil { + t.Fatalf("VerifyBinary(2.1.0) error = %v, want nil", err) } - // Mismatched version → error. if err := New().VerifyBinary("3.0.0"); err == nil { t.Fatal("VerifyBinary(mismatched) expected error, got nil") } - // Substring of actual version must not match (e.g. "0.0" is in "2.0.0"). + // Regression: version must match exactly (not substring / prefix). if err := New().VerifyBinary("0.0"); err == nil { - t.Fatal("VerifyBinary(substring) expected error, got nil") + t.Fatal("VerifyBinary(substring-style mismatch) expected error, got nil") } - - // Version that is a prefix of actual must not match (e.g. "2.0.0" in "12.0.0"). - // Binary reports "2.0.0", asking for "12.0.0" must fail. - if err := New().VerifyBinary("12.0.0"); err == nil { - t.Fatal("VerifyBinary(prefix-mismatch) expected error, got nil") + if err := New().VerifyBinary("12.1.0"); err == nil { + t.Fatal("VerifyBinary(prefix-style mismatch) expected error, got nil") + } +} + +func TestVerifyBinaryLookPathNotFound(t *testing.T) { + t.Setenv("LARKSUITE_CLI_CONFIG_DIR", t.TempDir()) + mock := &lookPathMock{result: "", resultErr: fmt.Errorf("not found")} + mock.install("lark-cli") + t.Cleanup(mock.restore) + + oldFS := vfs.DefaultFS + t.Cleanup(func() { vfs.DefaultFS = oldFS }) + // Without this, VerifyBinary would fall back to the real test binary, which + // is not a lark-cli --version implementation. + vfs.DefaultFS = executableTestFS{exe: filepath.Join(t.TempDir(), "missing-lark-cli")} + + if err := New().VerifyBinary("2.0.0"); err == nil { + t.Fatal("VerifyBinary(not-found) expected error, got nil") + } +} + +func TestVerifyBinaryFallbackExecutableWhenNotOnPath(t *testing.T) { + t.Setenv("LARKSUITE_CLI_CONFIG_DIR", t.TempDir()) + if runtime.GOOS == "windows" { + t.Skip("uses a POSIX shell script") + } + + dir := t.TempDir() + bin := filepath.Join(dir, "lark-cli-abs") + script := "#!/bin/sh\nif [ \"$1\" = \"--version\" ]; then echo \"lark-cli version 2.1.0\"; exit 0; fi\nexit 12\n" + if err := os.WriteFile(bin, []byte(script), 0o755); err != nil { + t.Fatalf("write test binary: %v", err) + } + + mock := &lookPathMock{result: "", resultErr: fmt.Errorf("not on PATH")} + mock.install("lark-cli") + t.Cleanup(mock.restore) + + oldFS := vfs.DefaultFS + t.Cleanup(func() { vfs.DefaultFS = oldFS }) + vfs.DefaultFS = executableTestFS{exe: bin} + + if err := New().VerifyBinary("2.1.0"); err != nil { + t.Fatalf("VerifyBinary(fallback executable) error = %v, want nil", err) + } +} + +func TestVerifyBinaryEmptyOutput(t *testing.T) { + t.Setenv("LARKSUITE_CLI_CONFIG_DIR", t.TempDir()) + if runtime.GOOS == "windows" { + t.Skip("uses a POSIX shell script") + } + + dir := t.TempDir() + bin := filepath.Join(dir, "lark-cli") + script := "#!/bin/sh\necho\nexit 0\n" + if err := os.WriteFile(bin, []byte(script), 0755); err != nil { + t.Fatalf("write test binary: %v", err) + } + + mock := &lookPathMock{result: bin} + mock.install("lark-cli") + t.Cleanup(mock.restore) + + if err := New().VerifyBinary("2.0.0"); err == nil { + t.Fatal("VerifyBinary(empty output) expected error, got nil") } }