From 3263463a7ed23ea303fd80b3964529933ccd2cce Mon Sep 17 00:00:00 2001 From: Abhinav Gupta Date: Sat, 9 Dec 2023 20:42:50 -0800 Subject: [PATCH] Fix Windows tests, verify in CI (#394) * ci: Test on Windows too Adds a Windows test run to CI. Go setup relies on GHA for this because Hermit doesn't yet support Windows. * fix(mapper_windows_test): assert.NotNil => assert.True assert.NotNil does not exist. * filecontent mapper: Handle error from directory If we couldn't read because the source is a directory, override the original error message. * fix(test): Handle platform-specific "file not found" messages --- .github/workflows/ci.yml | 18 ++++++++++++++++-- go.mod | 4 ++-- go.sum | 8 ++++---- mapper.go | 3 +++ mapper_test.go | 41 ++++++++++++++++++++++------------------ mapper_windows_test.go | 5 +++-- 6 files changed, 51 insertions(+), 28 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 334898c..0d34ae4 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -12,9 +12,23 @@ jobs: - name: Checkout code uses: actions/checkout@v2 - name: Init Hermit - run: ./bin/hermit env -r >> $GITHUB_ENV + run: ./bin/hermit env -r >> "$GITHUB_ENV" - name: Test run: go test ./... + + test-windows: + name: Test / Windows + runs-on: windows-latest + steps: + - name: Checkout code + uses: actions/checkout@v2 + - name: Setup Go + uses: actions/setup-go@v4 + with: + go-version: 1.21 + - name: Test + run: go test ./... + lint: name: Lint runs-on: ubuntu-latest @@ -22,6 +36,6 @@ jobs: - name: Checkout code uses: actions/checkout@v2 - name: Init Hermit - run: ./bin/hermit env -r >> $GITHUB_ENV + run: ./bin/hermit env -r >> "$GITHUB_ENV" - name: golangci-lint run: golangci-lint run diff --git a/go.mod b/go.mod index 159f5f7..ad2dc66 100644 --- a/go.mod +++ b/go.mod @@ -1,8 +1,8 @@ module github.com/alecthomas/kong require ( - github.com/alecthomas/assert/v2 v2.1.0 - github.com/alecthomas/repr v0.1.0 + github.com/alecthomas/assert/v2 v2.4.1 + github.com/alecthomas/repr v0.3.0 ) require github.com/hexops/gotextdiff v1.0.3 // indirect diff --git a/go.sum b/go.sum index f5d4fd4..5a217a1 100644 --- a/go.sum +++ b/go.sum @@ -1,6 +1,6 @@ -github.com/alecthomas/assert/v2 v2.1.0 h1:tbredtNcQnoSd3QBhQWI7QZ3XHOVkw1Moklp2ojoH/0= -github.com/alecthomas/assert/v2 v2.1.0/go.mod h1:b/+1DI2Q6NckYi+3mXyH3wFb8qG37K/DuK80n7WefXA= -github.com/alecthomas/repr v0.1.0 h1:ENn2e1+J3k09gyj2shc0dHr/yjaWSHRlrJ4DPMevDqE= -github.com/alecthomas/repr v0.1.0/go.mod h1:2kn6fqh/zIyPLmm3ugklbEi5hg5wS435eygvNfaDQL8= +github.com/alecthomas/assert/v2 v2.4.1 h1:mwPZod/d35nlaCppr6sFP0rbCL05WH9fIo7lvsf47zo= +github.com/alecthomas/assert/v2 v2.4.1/go.mod h1:fw5suVxB+wfYJ3291t0hRTqtGzFYdSwstnRQdaQx2DM= +github.com/alecthomas/repr v0.3.0 h1:NeYzUPfjjlqHY4KtzgKJiWd6sVq2eNUPTi34PiFGjY8= +github.com/alecthomas/repr v0.3.0/go.mod h1:Fr0507jx4eOXV7AlPV6AVZLYrLIuIeSOWtW57eE/O/4= github.com/hexops/gotextdiff v1.0.3 h1:gitA9+qJrrTCsiCl7+kh75nPqQt1cx4ZkudSTLoUqJM= github.com/hexops/gotextdiff v1.0.3/go.mod h1:pSWU5MAI3yDq+fZBTazCSJysOMbxWL1BSow5/V2vxeg= diff --git a/mapper.go b/mapper.go index c332cce..f9610aa 100644 --- a/mapper.go +++ b/mapper.go @@ -717,6 +717,9 @@ func fileContentMapper(r *Registry) MapperFunc { data, err = ioutil.ReadAll(os.Stdin) } if err != nil { + if info, statErr := os.Stat(path); statErr == nil && info.IsDir() { + return fmt.Errorf("%q exists but is a directory: %w", path, err) + } return err } target.SetBytes(data) diff --git a/mapper_test.go b/mapper_test.go index 81818fc..fd917e3 100644 --- a/mapper_test.go +++ b/mapper_test.go @@ -8,8 +8,8 @@ import ( "math" "net/url" "os" + "path/filepath" "reflect" - "runtime" "strings" "testing" "time" @@ -460,15 +460,13 @@ func TestFileMapper(t *testing.T) { _ = cli.File.Close() _, err = p.Parse([]string{"testdata/missing.txt"}) assert.Error(t, err) - if runtime.GOOS == "windows" { - assert.Contains(t, err.Error(), "missing.txt: The system cannot find the file specified.") - } else { - assert.Contains(t, err.Error(), "missing.txt: no such file or directory") - } + assert.Contains(t, err.Error(), "missing.txt:") + assert.IsError(t, err, os.ErrNotExist) _, err = p.Parse([]string{"-"}) assert.NoError(t, err) assert.Equal(t, os.Stdin, cli.File) } + func TestFileContentMapper(t *testing.T) { type CLI struct { File []byte `type:"filecontent"` @@ -481,9 +479,11 @@ func TestFileContentMapper(t *testing.T) { p = mustNew(t, &cli) _, err = p.Parse([]string{"--file", "testdata/missing.txt"}) assert.Error(t, err) - assert.Contains(t, err.Error(), "missing.txt: no such file or directory") + assert.Contains(t, err.Error(), "missing.txt:") + assert.IsError(t, err, os.ErrNotExist) p = mustNew(t, &cli) - _, err = p.Parse([]string{"--file", "testdata/"}) + + _, err = p.Parse([]string{"--file", "testdata"}) assert.Error(t, err) assert.Contains(t, err.Error(), "is a directory") } @@ -501,7 +501,8 @@ func TestExistingFileMapper(t *testing.T) { p = mustNew(t, &cli) _, err = p.Parse([]string{"--file", "testdata/missing.txt"}) assert.Error(t, err) - assert.Contains(t, err.Error(), "missing.txt: no such file or directory") + assert.Contains(t, err.Error(), "missing.txt:") + assert.IsError(t, err, os.ErrNotExist) p = mustNew(t, &cli) _, err = p.Parse([]string{"--file", "testdata/"}) assert.Error(t, err) @@ -514,7 +515,7 @@ func TestExistingFileMapperDefaultMissing(t *testing.T) { } var cli CLI p := mustNew(t, &cli) - file := "testdata/file.txt" + file := filepath.Join("testdata", "file.txt") _, err := p.Parse([]string{"--file", file}) assert.NoError(t, err) assert.NotZero(t, cli.File) @@ -522,7 +523,8 @@ func TestExistingFileMapperDefaultMissing(t *testing.T) { p = mustNew(t, &cli) _, err = p.Parse([]string{}) assert.Error(t, err) - assert.Contains(t, err.Error(), "missing.txt: no such file or directory") + assert.Contains(t, err.Error(), "missing.txt:") + assert.IsError(t, err, os.ErrNotExist) } func TestExistingFileMapperDefaultMissingCmds(t *testing.T) { @@ -536,7 +538,7 @@ func TestExistingFileMapperDefaultMissingCmds(t *testing.T) { } `cmd:""` } var cli CLI - file := "testdata/file.txt" + file := filepath.Join("testdata", "file.txt") p := mustNew(t, &cli) _, err := p.Parse([]string{"cmd-a", "--file-a", file, "--file-b", file}) assert.NoError(t, err) @@ -547,7 +549,8 @@ func TestExistingFileMapperDefaultMissingCmds(t *testing.T) { p = mustNew(t, &cli) _, err = p.Parse([]string{"cmd-a", "--file-a", file}) assert.Error(t, err) - assert.Contains(t, err.Error(), "bbb-missing.txt: no such file or directory") + assert.Contains(t, err.Error(), "bbb-missing.txt:") + assert.IsError(t, err, os.ErrNotExist) } //nolint:dupl @@ -563,7 +566,8 @@ func TestExistingDirMapper(t *testing.T) { p = mustNew(t, &cli) _, err = p.Parse([]string{"--dir", "missingdata/"}) assert.Error(t, err) - assert.Contains(t, err.Error(), "missingdata: no such file or directory") + assert.Contains(t, err.Error(), "missingdata:") + assert.IsError(t, err, os.ErrNotExist) p = mustNew(t, &cli) _, err = p.Parse([]string{"--dir", "testdata/file.txt"}) assert.Error(t, err) @@ -584,7 +588,8 @@ func TestExistingDirMapperDefaultMissing(t *testing.T) { p = mustNew(t, &cli) _, err = p.Parse([]string{}) assert.Error(t, err) - assert.Contains(t, err.Error(), "missing-dir: no such file or directory") + assert.Contains(t, err.Error(), "missing-dir:") + assert.IsError(t, err, os.ErrNotExist) } func TestExistingDirMapperDefaultMissingCmds(t *testing.T) { @@ -609,7 +614,8 @@ func TestExistingDirMapperDefaultMissingCmds(t *testing.T) { p = mustNew(t, &cli) _, err = p.Parse([]string{"cmd-a", "--dir-a", dir}) assert.Error(t, err) - assert.Contains(t, err.Error(), "bbb-missing-dir: no such file or directory") + assert.Contains(t, err.Error(), "bbb-missing-dir:") + assert.IsError(t, err, os.ErrNotExist) } func TestMapperPlaceHolder(t *testing.T) { @@ -632,8 +638,7 @@ func TestMapperPlaceHolder(t *testing.T) { assert.Contains(t, b.String(), "--flag=/a/b/c") } -type testMapperWithPlaceHolder struct { -} +type testMapperWithPlaceHolder struct{} func (t testMapperWithPlaceHolder) Decode(ctx *kong.DecodeContext, target reflect.Value) error { target.SetString("hi") diff --git a/mapper_windows_test.go b/mapper_windows_test.go index 2a51fee..94958ec 100644 --- a/mapper_windows_test.go +++ b/mapper_windows_test.go @@ -33,11 +33,12 @@ func TestWindowsFileMapper(t *testing.T) { p := mustNew(t, &cli) _, err := p.Parse([]string{"testdata\\file.txt"}) assert.NoError(t, err) - assert.NotNil(t, cli.File) + assert.True(t, cli.File != nil, "File should not be nil") _ = cli.File.Close() _, err = p.Parse([]string{"testdata\\missing.txt"}) assert.Error(t, err) - assert.Contains(t, err.Error(), "missing.txt: The system cannot find the file specified.") + assert.Contains(t, err.Error(), "missing.txt:") + assert.IsError(t, err, os.ErrNotExist) _, err = p.Parse([]string{"-"}) assert.NoError(t, err) assert.Equal(t, os.Stdin, cli.File)