From 8eafa5adfe9a107afc3d2a0e247b760bb45e7c97 Mon Sep 17 00:00:00 2001 From: Christopher Allen Lane Date: Sat, 14 Feb 2026 21:31:26 -0500 Subject: [PATCH] fix: cross-platform CI test fixes and parse bug fix - Add .gitattributes to force LF in mock files (Windows autocrlf) - Fix parse.go: detect line endings from content instead of runtime.GOOS - Add fail-fast: false to CI matrix; trigger on all branch pushes - Skip chmod-based tests on Windows (permissions work differently) - Use filepath.Join for expected paths in Windows path tests - Use platform-appropriate invalid paths in error tests - Add Windows absolute path test case for ValidateSheetName - Skip Unix-specific integration tests on Windows Co-Authored-By: Claude Opus 4.6 --- .gitattributes | 3 ++ .github/workflows/build.yml | 4 +-- cmd/cheat/path_traversal_integration_test.go | 9 ++++++ internal/cheatpath/validate_test.go | 11 +++++-- internal/config/config_extended_test.go | 5 ++++ internal/config/config_test.go | 4 +++ internal/config/init_test.go | 11 +++++-- internal/config/paths_test.go | 16 +++++++--- internal/installer/run_test.go | 31 +++++++++++++------- internal/repo/clone_test.go | 4 +++ internal/sheet/copy_error_test.go | 5 ++++ internal/sheet/parse.go | 5 ++-- internal/sheet/parse_extended_test.go | 6 ---- 13 files changed, 83 insertions(+), 31 deletions(-) create mode 100644 .gitattributes diff --git a/.gitattributes b/.gitattributes new file mode 100644 index 0000000..c1acb74 --- /dev/null +++ b/.gitattributes @@ -0,0 +1,3 @@ +# Force LF line endings for mock/test data files to ensure consistent +# behavior across platforms (Windows git autocrlf converts to CRLF otherwise) +mocks/** text eol=lf diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 698c262..833d68a 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -3,9 +3,6 @@ name: CI on: push: - branches: [master] - pull_request: - branches: [master] jobs: lint: @@ -26,6 +23,7 @@ jobs: test: strategy: + fail-fast: false matrix: os: [ubuntu-latest, macos-latest, windows-latest] runs-on: ${{ matrix.os }} diff --git a/cmd/cheat/path_traversal_integration_test.go b/cmd/cheat/path_traversal_integration_test.go index 49dd583..4650f3e 100644 --- a/cmd/cheat/path_traversal_integration_test.go +++ b/cmd/cheat/path_traversal_integration_test.go @@ -5,6 +5,7 @@ import ( "os" "os/exec" "path/filepath" + "runtime" "strings" "testing" ) @@ -12,6 +13,10 @@ import ( // TestPathTraversalIntegration tests that the cheat binary properly blocks // path traversal attempts when invoked as a subprocess. func TestPathTraversalIntegration(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("integration test uses Unix-specific env and tools") + } + // Build the cheat binary binPath := filepath.Join(t.TempDir(), "cheat_test") if output, err := exec.Command("go", "build", "-o", binPath, ".").CombinedOutput(); err != nil { @@ -146,6 +151,10 @@ cheatpaths: // TestPathTraversalRealWorld tests with more realistic scenarios func TestPathTraversalRealWorld(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("integration test uses Unix-specific env and tools") + } + // This test ensures our protection works with actual file operations // Build cheat diff --git a/internal/cheatpath/validate_test.go b/internal/cheatpath/validate_test.go index 2ac3fd6..93b7820 100644 --- a/internal/cheatpath/validate_test.go +++ b/internal/cheatpath/validate_test.go @@ -1,6 +1,7 @@ package cheatpath import ( + "runtime" "strings" "testing" ) @@ -53,9 +54,15 @@ func TestValidateSheetName(t *testing.T) { errMsg: "'..'", }, { - name: "absolute path", + name: "absolute path unix", input: "/etc/passwd", - wantErr: true, + wantErr: runtime.GOOS != "windows", // /etc/passwd is not absolute on Windows + errMsg: "absolute", + }, + { + name: "absolute path windows", + input: `C:\evil`, + wantErr: runtime.GOOS == "windows", // C:\evil is not absolute on Unix errMsg: "absolute", }, { diff --git a/internal/config/config_extended_test.go b/internal/config/config_extended_test.go index ac82cb7..ef09da0 100644 --- a/internal/config/config_extended_test.go +++ b/internal/config/config_extended_test.go @@ -3,6 +3,7 @@ package config import ( "os" "path/filepath" + "runtime" "testing" "github.com/cheat/cheat/internal/mock" @@ -226,6 +227,10 @@ cheatpaths: // TestConfigGetCwdError tests error handling when os.Getwd fails func TestConfigGetCwdError(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("Windows does not allow removing the current directory") + } + // This is difficult to test without being able to break os.Getwd // We'll create a scenario where the current directory is removed diff --git a/internal/config/config_test.go b/internal/config/config_test.go index 9bb7f31..fc56a38 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -4,6 +4,7 @@ import ( "os" "path/filepath" "reflect" + "runtime" "testing" "github.com/davecgh/go-spew/spew" @@ -78,6 +79,9 @@ func TestConfigFailure(t *testing.T) { // TestEmptyEditor asserts that envvars are respected if an editor is not // specified in the configs func TestEmptyEditor(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("Editor() returns notepad on Windows before checking env vars") + } // clear the environment variables os.Setenv("VISUAL", "") diff --git a/internal/config/init_test.go b/internal/config/init_test.go index 1ebab5e..8017f23 100644 --- a/internal/config/init_test.go +++ b/internal/config/init_test.go @@ -3,6 +3,7 @@ package config import ( "os" "path/filepath" + "runtime" "strings" "testing" ) @@ -74,12 +75,18 @@ func TestInitCreateDirectory(t *testing.T) { // TestInitWriteError tests error handling when file write fails func TestInitWriteError(t *testing.T) { // Skip this test if running as root (can write anywhere) - if os.Getuid() == 0 { + if runtime.GOOS != "windows" && os.Getuid() == 0 { t.Skip("Cannot test write errors as root") } + // Use a platform-appropriate invalid path + invalidPath := "/dev/null/impossible/path/conf.yml" + if runtime.GOOS == "windows" { + invalidPath = `NUL\impossible\path\conf.yml` + } + // Try to write to a read-only directory - err := Init("/dev/null/impossible/path/conf.yml", "test") + err := Init(invalidPath, "test") if err == nil { t.Error("expected error when writing to invalid path, got nil") } diff --git a/internal/config/paths_test.go b/internal/config/paths_test.go index 7590042..130ebf2 100644 --- a/internal/config/paths_test.go +++ b/internal/config/paths_test.go @@ -1,7 +1,9 @@ package config import ( + "path/filepath" "reflect" + "runtime" "testing" "github.com/davecgh/go-spew/spew" @@ -10,6 +12,9 @@ import ( // TestValidatePathsNix asserts that the proper config paths are returned on // *nix platforms func TestValidatePathsNix(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("filepath.Join uses backslashes on Windows") + } // mock the user's home directory home := "/home/foo" @@ -57,6 +62,9 @@ func TestValidatePathsNix(t *testing.T) { // TestValidatePathsNixNoXDG asserts that the proper config paths are returned // on *nix platforms when `XDG_CONFIG_HOME is not set func TestValidatePathsNixNoXDG(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("filepath.Join uses backslashes on Windows") + } // mock the user's home directory home := "/home/foo" @@ -106,8 +114,8 @@ func TestValidatePathsWindows(t *testing.T) { // mock some envvars envvars := map[string]string{ - "APPDATA": "/apps", - "PROGRAMDATA": "/programs", + "APPDATA": filepath.Join("C:", "apps"), + "PROGRAMDATA": filepath.Join("C:", "programs"), } // get the paths for the platform @@ -118,8 +126,8 @@ func TestValidatePathsWindows(t *testing.T) { // specify the expected output want := []string{ - "/apps/cheat/conf.yml", - "/programs/cheat/conf.yml", + filepath.Join("C:", "apps", "cheat", "conf.yml"), + filepath.Join("C:", "programs", "cheat", "conf.yml"), } // assert that output matches expectations diff --git a/internal/installer/run_test.go b/internal/installer/run_test.go index 889333d..9cbf688 100644 --- a/internal/installer/run_test.go +++ b/internal/installer/run_test.go @@ -5,6 +5,7 @@ import ( "io" "os" "path/filepath" + "runtime" "strings" "testing" ) @@ -69,23 +70,31 @@ cheatpaths: wantInErr: "failed to clone cheatsheets", }, { - name: "invalid config path", - configs: "test", - confpath: "/nonexistent/path/conf.yml", + name: "invalid config path", + configs: "test", + // /dev/null/... is truly uncreatable on Unix; + // NUL\... is uncreatable on Windows + confpath: func() string { + if runtime.GOOS == "windows" { + return `NUL\impossible\conf.yml` + } + return "/dev/null/impossible/conf.yml" + }(), userInput: "n\n", wantErr: true, - wantInErr: "failed to create config file", + wantInErr: "failed to create", }, } - // Pre-create a non-empty community dir so PlainClone fails reliably - // (otherwise, on CI runners with network access, the clone succeeds) - cloneBlocker := filepath.Join(tempDir, "conf2", "cheatsheets", "community") - if err := os.MkdirAll(cloneBlocker, 0755); err != nil { - t.Fatalf("failed to create clone blocker dir: %v", err) + // Pre-create a .git dir inside the community path so go-git's PlainClone + // returns ErrRepositoryAlreadyExists (otherwise, on CI runners with + // network access, the real clone succeeds and the test fails) + fakeGitDir := filepath.Join(tempDir, "conf2", "cheatsheets", "community", ".git") + if err := os.MkdirAll(fakeGitDir, 0755); err != nil { + t.Fatalf("failed to create fake .git dir: %v", err) } - if err := os.WriteFile(filepath.Join(cloneBlocker, ".gitkeep"), []byte(""), 0644); err != nil { - t.Fatalf("failed to write clone blocker file: %v", err) + if err := os.WriteFile(filepath.Join(fakeGitDir, "HEAD"), []byte("ref: refs/heads/main\n"), 0644); err != nil { + t.Fatalf("failed to write fake HEAD: %v", err) } for _, tt := range tests { diff --git a/internal/repo/clone_test.go b/internal/repo/clone_test.go index 5f5a14a..98e9b1a 100644 --- a/internal/repo/clone_test.go +++ b/internal/repo/clone_test.go @@ -3,6 +3,7 @@ package repo import ( "os" "path/filepath" + "runtime" "testing" ) @@ -12,6 +13,9 @@ func TestClone(t *testing.T) { // that don't require actual cloning t.Run("clone to read-only directory", func(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("chmod does not restrict writes on Windows") + } if os.Getuid() == 0 { t.Skip("Cannot test read-only directory as root") } diff --git a/internal/sheet/copy_error_test.go b/internal/sheet/copy_error_test.go index c9719d1..ea0d001 100644 --- a/internal/sheet/copy_error_test.go +++ b/internal/sheet/copy_error_test.go @@ -3,6 +3,7 @@ package sheet import ( "os" "path/filepath" + "runtime" "testing" ) @@ -130,6 +131,10 @@ func TestCopyIOError(t *testing.T) { // TestCopyCleanupOnError verifies that partially written files are cleaned up on error func TestCopyCleanupOnError(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("chmod does not restrict reads on Windows") + } + // Create a source file that we'll make unreadable after opening src, err := os.CreateTemp("", "copy-test-cleanup-*") if err != nil { diff --git a/internal/sheet/parse.go b/internal/sheet/parse.go index d2abdc1..16547ea 100644 --- a/internal/sheet/parse.go +++ b/internal/sheet/parse.go @@ -2,7 +2,6 @@ package sheet import ( "fmt" - "runtime" "strings" "gopkg.in/yaml.v3" @@ -11,9 +10,9 @@ import ( // Parse parses cheatsheet frontmatter func parse(markdown string) (frontmatter, string, error) { - // determine the appropriate line-break for the platform + // detect the line-break style used in the content linebreak := "\n" - if runtime.GOOS == "windows" { + if strings.Contains(markdown, "\r\n") { linebreak = "\r\n" } diff --git a/internal/sheet/parse_extended_test.go b/internal/sheet/parse_extended_test.go index cf55dfd..e0962ab 100644 --- a/internal/sheet/parse_extended_test.go +++ b/internal/sheet/parse_extended_test.go @@ -1,17 +1,11 @@ package sheet import ( - "runtime" "testing" ) // TestParseWindowsLineEndings tests parsing with Windows line endings func TestParseWindowsLineEndings(t *testing.T) { - // Only test Windows line endings on Windows - if runtime.GOOS != "windows" { - t.Skip("Skipping Windows line ending test on non-Windows platform") - } - // stub our cheatsheet content with Windows line endings markdown := "---\r\nsyntax: go\r\ntags: [ test ]\r\n---\r\nTo foo the bar: baz"