Repository: alecthomas/kong
Pull request #394 (2023-12-10 02:43:35)

Fix Windows tests, verify in CI
This adds testing against Windows to CI.
Hermit doesn't support Windows,
so the setup for that is done with GitHub Actions.

The following additional fixes were needed:

- filecontent mapper returned ioutil.ReadAll error as-is.
  This is less informative on Windows when the source is a directory:
  "Invalid function."
- mapper_windows_test made use of a non-existent assert.NotNil function.
- mapper_test looked for Unix-specific "no such file or directory" messages.
  To fix this, we now assert that errors match `os.ErrNotExist`.
  This handles system-specific variations in the error message.


alecthomas: Annoying that it takes 1m just to run a test that takes 1s, but that's Windows life for you I guess.

Patch:
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)


Test patch:
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)