From 7d71fa551a4b92f320878e1b1d8891268db62673 Mon Sep 17 00:00:00 2001 From: Malo Bourgon Date: Fri, 14 Aug 2026 14:56:30 -0700 Subject: [PATCH] test: isolate tests from ambient XDG and GOG_* path variables Tests isolate storage via per-test HOME/t.TempDir sandboxes, but the layout resolver (internal/config/layout.go) honors GOG_HOME, the GOG_{CONFIG,DATA,STATE,CACHE}_DIR overrides, and the XDG base directory variables ahead of HOME-derived defaults. On machines that export any of them, tests resolve the developer's real gogcli directories: with XDG_DATA_HOME/XDG_STATE_HOME exported, 19 failures across internal/cmd, internal/config, and internal/secrets from cross-test contamination (the exact split depends on preexisting state and platform); with GOG_HOME exported, 77+ failures. In every case test fixtures (service-account stubs, tracking.json, gmail-watch state, file-keyring entries) leak into the real directories, clobbering any real file-keyring, tracking, or watch state. CI never sees this because GitHub runners export none of these variables. Unset the GOG_* path overrides plus XDG data/state/cache in internal/cmd's TestMain (which already redirects HOME and XDG_CONFIG_HOME to a temp root), add equivalent TestMains to internal/secrets and internal/config, and clear the GOG_* overrides in the internal/googleapi test that deliberately writes to the ambient layout (with GOG_HOME exported it previously stayed green while writing into the real directory). Unsetting rather than redirecting matters: a single shared override directory still cross-contaminates tests; unsetting lets each test's own sandbox take effect, matching CI behavior. Co-Authored-By: Claude Fable 5 --- internal/cmd/testmain_test.go | 21 +++++++++++++++++++ internal/config/testmain_test.go | 24 ++++++++++++++++++++++ internal/googleapi/service_account_test.go | 7 +++++++ internal/secrets/testmain_test.go | 24 ++++++++++++++++++++++ 4 files changed, 76 insertions(+) create mode 100644 internal/config/testmain_test.go create mode 100644 internal/secrets/testmain_test.go diff --git a/internal/cmd/testmain_test.go b/internal/cmd/testmain_test.go index df4088bc0..17d493fae 100644 --- a/internal/cmd/testmain_test.go +++ b/internal/cmd/testmain_test.go @@ -26,6 +26,23 @@ func TestMain(m *testing.M) { _ = os.Setenv("HOME", home) _ = os.Setenv("XDG_CONFIG_HOME", xdg) + // Ambient GOG_* path overrides and XDG data/state/cache directories escape + // this sandbox entirely: the layout resolver (internal/config/layout.go) + // honors them ahead of the HOME- and XDG_CONFIG_HOME-derived defaults set + // above, pointing tests at shared real directories. Unset rather than + // redirect: a single shared override directory still cross-contaminates + // tests. Per-test t.Setenv values are unaffected. + oldPathEnv := map[string]string{} + for _, name := range []string{ + "GOG_HOME", "GOG_CONFIG_DIR", "GOG_DATA_DIR", "GOG_STATE_DIR", "GOG_CACHE_DIR", + "XDG_DATA_HOME", "XDG_STATE_HOME", "XDG_CACHE_HOME", + } { + if value, ok := os.LookupEnv(name); ok { + oldPathEnv[name] = value + } + _ = os.Unsetenv(name) + } + code := m.Run() if oldHome == "" { @@ -38,6 +55,10 @@ func TestMain(m *testing.M) { } else { _ = os.Setenv("XDG_CONFIG_HOME", oldXDG) } + + for name, value := range oldPathEnv { + _ = os.Setenv(name, value) + } _ = os.RemoveAll(root) os.Exit(code) } diff --git a/internal/config/testmain_test.go b/internal/config/testmain_test.go new file mode 100644 index 000000000..e617d6d9e --- /dev/null +++ b/internal/config/testmain_test.go @@ -0,0 +1,24 @@ +package config + +import ( + "os" + "testing" +) + +func TestMain(m *testing.M) { + // Tests isolate storage via per-test temp directories (HOME by default), + // but the layout resolver (internal/config/layout.go) honors ambient GOG_* + // path overrides and XDG base directories ahead of HOME-derived defaults, + // leaking state across tests and into the developer's real gogcli + // directories. Unset rather than redirect: a single shared override + // directory still cross-contaminates tests. Per-test t.Setenv values are + // unaffected. + for _, name := range []string{ + "GOG_HOME", "GOG_CONFIG_DIR", "GOG_DATA_DIR", "GOG_STATE_DIR", "GOG_CACHE_DIR", + "XDG_CONFIG_HOME", "XDG_DATA_HOME", "XDG_STATE_HOME", "XDG_CACHE_HOME", + } { + _ = os.Unsetenv(name) + } + + os.Exit(m.Run()) +} diff --git a/internal/googleapi/service_account_test.go b/internal/googleapi/service_account_test.go index 45b4720ae..de0a89412 100644 --- a/internal/googleapi/service_account_test.go +++ b/internal/googleapi/service_account_test.go @@ -43,6 +43,13 @@ func TestTokenSourceForServiceAccountScopesUsesInjectedStore(t *testing.T) { t.Setenv("XDG_CONFIG_HOME", filepath.Join(ambientHome, "xdg-config")) t.Setenv("XDG_DATA_HOME", filepath.Join(ambientHome, "xdg-data")) + // GOG_* path overrides outrank HOME and XDG in the layout resolver; clear + // them so the ambient file below lands in this test's sandbox rather than + // a real gogcli data directory. + for _, name := range []string{"GOG_HOME", "GOG_CONFIG_DIR", "GOG_DATA_DIR", "GOG_STATE_DIR", "GOG_CACHE_DIR"} { + t.Setenv(name, "") + } + ambientLayout, err := config.NewSystemResolver("").Resolve(config.PathKindData) if err != nil { t.Fatalf("resolve ambient layout: %v", err) diff --git a/internal/secrets/testmain_test.go b/internal/secrets/testmain_test.go new file mode 100644 index 000000000..f1d5c8777 --- /dev/null +++ b/internal/secrets/testmain_test.go @@ -0,0 +1,24 @@ +package secrets + +import ( + "os" + "testing" +) + +func TestMain(m *testing.M) { + // Tests isolate storage via per-test temp directories (HOME by default), + // but the layout resolver (internal/config/layout.go) honors ambient GOG_* + // path overrides and XDG base directories ahead of HOME-derived defaults, + // leaking state across tests and into the developer's real gogcli + // directories. Unset rather than redirect: a single shared override + // directory still cross-contaminates tests. Per-test t.Setenv values are + // unaffected. + for _, name := range []string{ + "GOG_HOME", "GOG_CONFIG_DIR", "GOG_DATA_DIR", "GOG_STATE_DIR", "GOG_CACHE_DIR", + "XDG_CONFIG_HOME", "XDG_DATA_HOME", "XDG_STATE_HOME", "XDG_CACHE_HOME", + } { + _ = os.Unsetenv(name) + } + + os.Exit(m.Run()) +}