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

@@ -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

go.mod

@@ -1,8 +1,8 @@
 module github.com/alecthomas/kong
 
 require (
-	github.com/alecthomas/assert/v2 v2.1.0
+	github.com/alecthomas/assert/v2 v2.4.1
-	github.com/alecthomas/repr v0.1.0
+	github.com/alecthomas/repr v0.3.0
 )
 
 require github.com/hexops/gotextdiff v1.0.3 // indirect

go.sum

@@ -1,6 +1,6 @@
-github.com/alecthomas/assert/v2 v2.1.0 h1:tbredtNcQnoSd3QBhQWI7QZ3XHOVkw1Moklp2ojoH/0=
+github.com/alecthomas/assert/v2 v2.4.1 h1:mwPZod/d35nlaCppr6sFP0rbCL05WH9fIo7lvsf47zo=
-github.com/alecthomas/assert/v2 v2.1.0/go.mod h1:b/+1DI2Q6NckYi+3mXyH3wFb8qG37K/DuK80n7WefXA=
+github.com/alecthomas/assert/v2 v2.4.1/go.mod h1:fw5suVxB+wfYJ3291t0hRTqtGzFYdSwstnRQdaQx2DM=
-github.com/alecthomas/repr v0.1.0 h1:ENn2e1+J3k09gyj2shc0dHr/yjaWSHRlrJ4DPMevDqE=
+github.com/alecthomas/repr v0.3.0 h1:NeYzUPfjjlqHY4KtzgKJiWd6sVq2eNUPTi34PiFGjY8=
-github.com/alecthomas/repr v0.1.0/go.mod h1:2kn6fqh/zIyPLmm3ugklbEi5hg5wS435eygvNfaDQL8=
+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=

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)

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:")
-		assert.Contains(t, err.Error(), "missing.txt: The system cannot find the file specified.")
+	assert.IsError(t, err, os.ErrNotExist)
-	} else {
-		assert.Contains(t, err.Error(), "missing.txt: no such file or directory")
-	}
 	_, 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")

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)