From fb53a5dc13f895a93c3f8866592ed6030d9a3be1 Mon Sep 17 00:00:00 2001 From: Andrew Keesler Date: Wed, 28 Oct 2020 09:43:27 -0400 Subject: [PATCH] Fix unit tests on Windows and add related workflow --- .github/workflows/go.yml | 47 +++++++++++++++++++ .github/workflows/unit-tests.yml | 47 +++++++++++++++++++ cmd/pinniped/cmd/get_kubeconfig_test.go | 15 ++++-- cmd/pinniped/cmd/login_oidc_test.go | 18 ++++++- .../supervisorconfig/jwks_writer_test.go | 1 + internal/downward/downward_test.go | 14 +++++- .../oidcclient/filesession/cachefile_test.go | 9 +++- .../filesession/filesession_test.go | 27 ++++++++--- 8 files changed, 164 insertions(+), 14 deletions(-) create mode 100644 .github/workflows/go.yml create mode 100644 .github/workflows/unit-tests.yml diff --git a/.github/workflows/go.yml b/.github/workflows/go.yml new file mode 100644 index 00000000..d30519b2 --- /dev/null +++ b/.github/workflows/go.yml @@ -0,0 +1,47 @@ +name: unit-tests +on: workflow_dispatch +jobs: + macos-unit-tests: + name: macOS Unit Tests + runs-on: macos-latest + steps: + - name: Set up Go + uses: actions/setup-go@v2 + with: + go-version: ^1.15 + id: go + - name: Checkout + uses: actions/checkout@v2 + - name: Cache Dependencies + uses: actions/cache@v2 + with: + path: ~/go/pkg/mod + key: ${{ runner.os }}-go-${{ hashFiles('**/go.sum') }} + restore-keys: | + ${{ runner.os }}-go- + - name: Build + run: go build -v ./... + - name: Test + run: go test -short -race -v ./... + windows-unit-tests: + name: Windows Unit Tests + runs-on: windows-latest + steps: + - name: Set up Go + uses: actions/setup-go@v2 + with: + go-version: ^1.15 + id: go + - name: Checkout + uses: actions/checkout@v2 + - name: Cache Dependencies + uses: actions/cache@v2 + with: + path: ~/go/pkg/mod + key: ${{ runner.os }}-go-${{ hashFiles('**/go.sum') }} + restore-keys: | + ${{ runner.os }}-go- + - name: Build + run: go build -v ./... + - name: Test + run: go test -short -race -v ./... diff --git a/.github/workflows/unit-tests.yml b/.github/workflows/unit-tests.yml new file mode 100644 index 00000000..d30519b2 --- /dev/null +++ b/.github/workflows/unit-tests.yml @@ -0,0 +1,47 @@ +name: unit-tests +on: workflow_dispatch +jobs: + macos-unit-tests: + name: macOS Unit Tests + runs-on: macos-latest + steps: + - name: Set up Go + uses: actions/setup-go@v2 + with: + go-version: ^1.15 + id: go + - name: Checkout + uses: actions/checkout@v2 + - name: Cache Dependencies + uses: actions/cache@v2 + with: + path: ~/go/pkg/mod + key: ${{ runner.os }}-go-${{ hashFiles('**/go.sum') }} + restore-keys: | + ${{ runner.os }}-go- + - name: Build + run: go build -v ./... + - name: Test + run: go test -short -race -v ./... + windows-unit-tests: + name: Windows Unit Tests + runs-on: windows-latest + steps: + - name: Set up Go + uses: actions/setup-go@v2 + with: + go-version: ^1.15 + id: go + - name: Checkout + uses: actions/checkout@v2 + - name: Cache Dependencies + uses: actions/cache@v2 + with: + path: ~/go/pkg/mod + key: ${{ runner.os }}-go-${{ hashFiles('**/go.sum') }} + restore-keys: | + ${{ runner.os }}-go- + - name: Build + run: go build -v ./... + - name: Test + run: go test -short -race -v ./... diff --git a/cmd/pinniped/cmd/get_kubeconfig_test.go b/cmd/pinniped/cmd/get_kubeconfig_test.go index 7834e1b4..944213a8 100644 --- a/cmd/pinniped/cmd/get_kubeconfig_test.go +++ b/cmd/pinniped/cmd/get_kubeconfig_test.go @@ -7,13 +7,14 @@ import ( "bytes" "encoding/base64" "fmt" + "runtime" "strings" "testing" "github.com/spf13/cobra" "github.com/stretchr/testify/require" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/runtime" + k8sruntime "k8s.io/apimachinery/pkg/runtime" "k8s.io/client-go/rest" coretesting "k8s.io/client-go/testing" @@ -191,6 +192,12 @@ func newCredentialIssuerConfig(name, namespace, server, certificateAuthorityData func TestRun(t *testing.T) { t.Parallel() + + fileDoesNotExistError := "stat ./testdata/does-not-exist.yaml: no such file or directory" + if runtime.GOOS == "windows" { + fileDoesNotExistError = "CreateFile ./testdata/does-not-exist.yaml: The system cannot find the file specified." + } + tests := []struct { name string mocks func(*getKubeConfigCommand) @@ -212,7 +219,7 @@ func TestRun(t *testing.T) { mocks: func(cmd *getKubeConfigCommand) { cmd.flags.kubeconfig = "./testdata/does-not-exist.yaml" }, - wantError: "stat ./testdata/does-not-exist.yaml: no such file or directory", + wantError: fileDoesNotExistError, }, { name: "fail to get client", @@ -229,7 +236,7 @@ func TestRun(t *testing.T) { cmd.flags.idpName = "" cmd.flags.idpType = "" clientset := pinnipedfake.NewSimpleClientset() - clientset.PrependReactor("*", "*", func(_ coretesting.Action) (bool, runtime.Object, error) { + clientset.PrependReactor("*", "*", func(_ coretesting.Action) (bool, k8sruntime.Object, error) { return true, nil, fmt.Errorf("some error getting IDPs") }) cmd.kubeClientCreator = func(_ *rest.Config) (pinnipedclientset.Interface, error) { @@ -267,7 +274,7 @@ func TestRun(t *testing.T) { name: "fail to get CredentialIssuerConfigs", mocks: func(cmd *getKubeConfigCommand) { clientset := pinnipedfake.NewSimpleClientset() - clientset.PrependReactor("*", "*", func(_ coretesting.Action) (bool, runtime.Object, error) { + clientset.PrependReactor("*", "*", func(_ coretesting.Action) (bool, k8sruntime.Object, error) { return true, nil, fmt.Errorf("some error getting CredentialIssuerConfigs") }) cmd.kubeClientCreator = func(_ *rest.Config) (pinnipedclientset.Interface, error) { diff --git a/cmd/pinniped/cmd/login_oidc_test.go b/cmd/pinniped/cmd/login_oidc_test.go index af288e3e..d56157f7 100644 --- a/cmd/pinniped/cmd/login_oidc_test.go +++ b/cmd/pinniped/cmd/login_oidc_test.go @@ -5,6 +5,8 @@ package cmd import ( "bytes" + "path/filepath" + "strings" "testing" "time" @@ -47,7 +49,7 @@ func TestLoginOIDCCommand(t *testing.T) { --issuer string OpenID Connect issuer URL. --listen-port uint16 TCP port for localhost listener (authorization code flow only). --scopes strings OIDC scopes to request during login. (default [offline_access,openid,email,profile]) - --session-cache string Path to session cache file. (default "` + cfgDir + `/sessions.yaml") + --session-cache string Path to session cache file. (default "` + windowsSafeJoin(cfgDir, "sessions.yaml") + `") --skip-browser Skip opening the browser (just print the URL). `), }, @@ -125,3 +127,17 @@ func TestLoginOIDCCommand(t *testing.T) { }) } } + +// windowsSafeJoin is a function to help us get around a weird double-slash behavior in our help +// test. It should not affect the behavior of a path using '/' separators. +// +// When we create a flag with this help text +// C:\some\path\to\file.yaml +// then it shows up on the command line as this +// C:\\some\\path\\to\\file.yaml +// because (I think) the cobra library is doing some backslash escaping. +func windowsSafeJoin(s ...string) string { + joined := filepath.Join(s...) + joined = strings.ReplaceAll(joined, `\`, `\\`) + return joined +} diff --git a/internal/controller/supervisorconfig/jwks_writer_test.go b/internal/controller/supervisorconfig/jwks_writer_test.go index 39006054..5edd2174 100644 --- a/internal/controller/supervisorconfig/jwks_writer_test.go +++ b/internal/controller/supervisorconfig/jwks_writer_test.go @@ -702,6 +702,7 @@ func readJWKJSON(t *testing.T, path string) []byte { // our implementation. data = bytes.ReplaceAll(data, []byte(" "), []byte{}) data = bytes.ReplaceAll(data, []byte("\n"), []byte{}) + data = bytes.ReplaceAll(data, []byte("\r"), []byte{}) return data } diff --git a/internal/downward/downward_test.go b/internal/downward/downward_test.go index 9bc954d5..4a345ec6 100644 --- a/internal/downward/downward_test.go +++ b/internal/downward/downward_test.go @@ -4,6 +4,8 @@ package downward import ( + "path/filepath" + "runtime" "testing" "github.com/stretchr/testify/require" @@ -11,6 +13,14 @@ import ( func TestLoad(t *testing.T) { t.Parallel() + + fileDoesNotExistError := "no such file or directory" + directoryDoesNotExistError := "no such file or directory" + if runtime.GOOS == "windows" { + fileDoesNotExistError = "The system cannot find the file specified." + directoryDoesNotExistError = "The system cannot find the path specified." + } + tests := []struct { name string inputDir string @@ -20,12 +30,12 @@ func TestLoad(t *testing.T) { { name: "missing directory", inputDir: "./testdata/no-such-directory", - wantErr: "could not load namespace: open testdata/no-such-directory/namespace: no such file or directory", + wantErr: "could not load namespace: open " + filepath.Join("testdata", "no-such-directory", "namespace") + ": " + directoryDoesNotExistError, }, { name: "missing labels file", inputDir: "./testdata/missinglabels", - wantErr: "could not load labels: open testdata/missinglabels/labels: no such file or directory", + wantErr: "could not load labels: open " + filepath.Join("testdata", "missinglabels", "labels") + ": " + fileDoesNotExistError, }, { name: "invalid labels file", diff --git a/internal/oidcclient/filesession/cachefile_test.go b/internal/oidcclient/filesession/cachefile_test.go index 24bce561..ff0c4b35 100644 --- a/internal/oidcclient/filesession/cachefile_test.go +++ b/internal/oidcclient/filesession/cachefile_test.go @@ -5,6 +5,7 @@ package filesession import ( "os" + "runtime" "testing" "time" @@ -47,6 +48,12 @@ var validSession = sessionCache{ func TestReadSessionCache(t *testing.T) { t.Parallel() + + directoryError := "is a directory" + if runtime.GOOS == "windows" { + directoryError = "The handle is invalid." + } + tests := []struct { name string path string @@ -64,7 +71,7 @@ func TestReadSessionCache(t *testing.T) { { name: "other file error", path: "./testdata/", - wantErr: "could not read session file: read ./testdata/: is a directory", + wantErr: "could not read session file: read ./testdata/: " + directoryError, }, { name: "invalid YAML", diff --git a/internal/oidcclient/filesession/filesession_test.go b/internal/oidcclient/filesession/filesession_test.go index 8dd07bc9..c0791130 100644 --- a/internal/oidcclient/filesession/filesession_test.go +++ b/internal/oidcclient/filesession/filesession_test.go @@ -8,6 +8,7 @@ import ( "io/ioutil" "os" "path/filepath" + "runtime" "strings" "testing" "time" @@ -20,7 +21,7 @@ import ( func TestNew(t *testing.T) { t.Parallel() - tmp := t.TempDir() + "/sessions.yaml" + tmp := filepath.Join(t.TempDir(), "sessions.yaml") c := New(tmp) require.NotNil(t, c) require.Equal(t, tmp, c.path) @@ -30,6 +31,12 @@ func TestNew(t *testing.T) { func TestGetToken(t *testing.T) { t.Parallel() + + isADirectoryError := "is a directory" + if runtime.GOOS == "windows" { + isADirectoryError = "The handle is invalid." + } + now := time.Now().Round(1 * time.Second) tests := []struct { name string @@ -81,7 +88,7 @@ func TestGetToken(t *testing.T) { }, key: oidcclient.SessionCacheKey{}, wantErrors: []string{ - "failed to read cache, resetting: could not read session file: read TEMPFILE: is a directory", + "failed to read cache, resetting: could not read session file: read TEMPFILE: " + isADirectoryError, "could not write session cache: open TEMPFILE: is a directory", }, }, @@ -186,7 +193,7 @@ func TestGetToken(t *testing.T) { tt := tt t.Run(tt.name, func(t *testing.T) { t.Parallel() - tmp := t.TempDir() + "/sessions.yaml" + tmp := filepath.Join(t.TempDir(), "sessions.yaml") if tt.makeTestFile != nil { tt.makeTestFile(t, tmp) } @@ -213,6 +220,14 @@ func TestGetToken(t *testing.T) { func TestPutToken(t *testing.T) { t.Parallel() + + notADirectoryError := "not a directory" + isADirectoryError := "is a directory" + if runtime.GOOS == "windows" { + notADirectoryError = "The system cannot find the path specified." + isADirectoryError = "The handle is invalid." + } + now := time.Now().Round(1 * time.Second) tests := []struct { name string @@ -228,7 +243,7 @@ func TestPutToken(t *testing.T) { require.NoError(t, ioutil.WriteFile(filepath.Dir(tmp), []byte{}, 0600)) }, wantErrors: []string{ - "could not create session cache directory: mkdir TEMPDIR: not a directory", + "could not create session cache directory: mkdir TEMPDIR: " + notADirectoryError, }, }, { @@ -403,7 +418,7 @@ func TestPutToken(t *testing.T) { }, }, wantErrors: []string{ - "failed to read cache, resetting: could not read session file: read TEMPFILE: is a directory", + "failed to read cache, resetting: could not read session file: read TEMPFILE: " + isADirectoryError, "could not write session cache: open TEMPFILE: is a directory", }, wantTestFile: func(t *testing.T, tmp string) { @@ -417,7 +432,7 @@ func TestPutToken(t *testing.T) { tt := tt t.Run(tt.name, func(t *testing.T) { t.Parallel() - tmp := t.TempDir() + "/sessiondir/sessions.yaml" + tmp := filepath.Join(t.TempDir(), "sessiondir", "sessions.yaml") if tt.makeTestFile != nil { tt.makeTestFile(t, tmp) }