From 145e2cd68483e3c4407909c838fd18aebef854eb Mon Sep 17 00:00:00 2001 From: Jahvon Dockery Date: Thu, 27 Aug 2026 02:03:15 -0400 Subject: [PATCH] fix(test): make container and store tests platform-aware on Windows Four specs asserted Unix-only behavior and failed on Windows. All have been latent for a while: windows-ci only runs behind the test:windows label, and no PR had carried it since the code that introduced them. expandVolumeHost gates on filepath.IsAbs, which rejects "/opt/data" on Windows where an absolute path needs a drive letter. Two specs hardcoded that path as the host side of a volume. They now build the host path for the platform, while the container side stays Unix - container paths are always Linux paths, and ExecContainerVolume.Parts already handles the drive-letter colon. writeEnvFile's 0600 assertion read back as 0666: Go maps Unix permission bits onto ACLs on Windows, so the mode is not expressible there. The assertion is skipped on Windows and still enforced everywhere it means something; the content assertion is unchanged. The store suite derived its database filename from the spec name. One spec is named "... (running -> terminal)", and ">" is not a legal Windows filename character, so every spec in the suite failed in BeforeEach with ERROR_INVALID_NAME. TempDir is already unique per spec, so the suffix bought nothing and is dropped. Only test code changes. The production paths were already Windows-capable, so this fixes assertions rather than behavior. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01R328pa3FUUfga4gYah1iQi --- internal/runner/exec/exec_test.go | 23 +++++++++++++++++++---- internal/services/run/container_test.go | 8 +++++++- pkg/store/store_test.go | 6 ++++-- 3 files changed, 30 insertions(+), 7 deletions(-) diff --git a/internal/runner/exec/exec_test.go b/internal/runner/exec/exec_test.go index c4ababe2..3f94d022 100644 --- a/internal/runner/exec/exec_test.go +++ b/internal/runner/exec/exec_test.go @@ -5,6 +5,7 @@ import ( "errors" "os" "path/filepath" + "runtime" "testing" tuikitIO "github.com/flowexec/tuikit/io" @@ -27,6 +28,17 @@ type runCall struct { mode tuikitIO.LogMode } +// absHostVolumePath is an absolute host path valid on the current platform. +// expandVolumeHost gates on filepath.IsAbs, which rejects a Unix-style path on +// Windows - there an absolute path needs a drive letter. The container side of a +// volume stays Unix, since container paths are always Linux paths. +func absHostVolumePath() string { + if runtime.GOOS == "windows" { + return `C:\opt\data` + } + return "/opt/data" +} + func TestExec(t *testing.T) { RegisterFailHandler(Fail) RunSpecs(t, "Exec Suite") @@ -253,8 +265,11 @@ var _ = Describe("Exec Runner", func() { It("expands and mounts workspace-relative and absolute volumes", func() { c := &executable.ExecContainer{ - Image: "alpine:3", - Volumes: []executable.ExecContainerVolume{"//cache:/cache", "/opt/data:/data:ro"}, + Image: "alpine:3", + Volumes: []executable.ExecContainerVolume{ + "//cache:/cache", + executable.ExecContainerVolume(absHostVolumePath() + ":/data:ro"), + }, } e := newContainerExec(c, executable.Directory(wsPath)) Expect(execRnr.Exec(ctx.Ctx, e, mockEngine, map[string]string{}, nil)).To(Succeed()) @@ -263,7 +278,7 @@ var _ = Describe("Exec Runner", func() { // mounts[0] is the workspace; the two user volumes follow in order. Expect(mounts[len(mounts)-2].HostPath).To(Equal(filepath.Join(wsPath, "cache"))) Expect(mounts[len(mounts)-2].ContainerPath).To(Equal("/cache")) - Expect(mounts[len(mounts)-1].HostPath).To(Equal("/opt/data")) + Expect(mounts[len(mounts)-1].HostPath).To(Equal(absHostVolumePath())) Expect(mounts[len(mounts)-1].ContainerPath).To(Equal("/data")) Expect(mounts[len(mounts)-1].Options).To(Equal("ro")) }) @@ -306,7 +321,7 @@ var _ = Describe("Exec Runner", func() { Expect(got).To(HaveSuffix(wantSuffix)) }, Entry("workspace-relative", "//sub/dir", false, filepath.Join("/ws/root", "sub", "dir")), - Entry("absolute", "/opt/data", false, "/opt/data"), + Entry("absolute", absHostVolumePath(), false, absHostVolumePath()), Entry("home-relative", "~/thing", false, "thing"), Entry("cwd-relative", "./local", false, "local"), Entry("bare relative is rejected", "relative/path", true, ""), diff --git a/internal/services/run/container_test.go b/internal/services/run/container_test.go index 68d2b6a4..b295cd73 100644 --- a/internal/services/run/container_test.go +++ b/internal/services/run/container_test.go @@ -3,6 +3,7 @@ package run_test import ( "errors" "os" + "runtime" "strings" . "github.com/onsi/ginkgo/v2" @@ -132,7 +133,12 @@ var _ = Describe("Container backend", func() { info, err := os.Stat(path) Expect(err).NotTo(HaveOccurred()) - Expect(info.Mode().Perm()).To(Equal(os.FileMode(0600))) + // Unix permission bits are not expressible on Windows: Go maps them + // onto ACLs and the file reads back as 0666. The env file still holds + // secrets, so the mode is asserted everywhere it means something. + if runtime.GOOS != "windows" { + Expect(info.Mode().Perm()).To(Equal(os.FileMode(0600))) + } content, err := os.ReadFile(path) Expect(err).NotTo(HaveOccurred()) diff --git a/pkg/store/store_test.go b/pkg/store/store_test.go index 69a831cf..d52dba4e 100644 --- a/pkg/store/store_test.go +++ b/pkg/store/store_test.go @@ -1,7 +1,6 @@ package store_test import ( - "fmt" "path/filepath" "testing" "time" @@ -22,7 +21,10 @@ var _ = Describe("BoltDataStore", func() { var err error BeforeEach(func() { - path := filepath.Join(GinkgoT().TempDir(), fmt.Sprintf("test_%s.db", GinkgoT().Name())) + // TempDir is already unique per spec, so the file needs no disambiguating + // suffix - and deriving one from the spec name broke on Windows, where a + // name containing "->" yields an illegal filename (ERROR_INVALID_NAME). + path := filepath.Join(GinkgoT().TempDir(), "test.db") ds, err = store.NewDataStore(path) Expect(err).NotTo(HaveOccurred()) Expect(ds).NotTo(BeNil())