Skip to content

Improve tests #2309

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jun 21, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
52 changes: 13 additions & 39 deletions cmd/skaffold/app/cmd/find_configs_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ This helper function will generate the following file tree for testing purpose
├── valid-skaffold.yaml
└── invalid-skaffold.yaml
*/
func setUpTempFiles(tt *testutil.T, latestVersion, upgradeableVersion string) (*testutil.TempDir, *testutil.TempDir) {
func setUpTempFiles(t *testutil.T, latestVersion, upgradeableVersion string) (*testutil.TempDir, *testutil.TempDir) {
validYaml := fmt.Sprintf(`apiVersion: %s
kind: Config
build:
Expand All @@ -86,6 +86,7 @@ build:
docker:
dockerfile: dockerfile.test
`, latestVersion)

upgradeableYaml := fmt.Sprintf(`apiVersion: %s
kind: Config
build:
Expand All @@ -94,46 +95,19 @@ build:
docker:
dockerfile: dockerfile.test
`, upgradeableVersion)

invalidYaml := `This is invalid`

tmpDir1 := tt.NewTempDir()
tmpDir2 := tt.NewTempDir()

files := []struct {
fileName string
content string
tmpDir *testutil.TempDir
}{
{
fileName: invalidFileName,
content: invalidYaml,
tmpDir: tmpDir1,
},
{
fileName: validFileName,
content: validYaml,
tmpDir: tmpDir1,
},
{
fileName: upgradeableFileName,
content: upgradeableYaml,
tmpDir: tmpDir1,
},
{
fileName: invalidFileName,
content: invalidYaml,
tmpDir: tmpDir2,
},
{
fileName: validFileName,
content: validYaml,
tmpDir: tmpDir2,
},
}

for _, file := range files {
file.tmpDir.Write(file.fileName, file.content)
}
tmpDir1 := t.NewTempDir().WriteFiles(map[string]string{
invalidFileName: invalidYaml,
validFileName: validYaml,
upgradeableFileName: upgradeableYaml,
})

tmpDir2 := t.NewTempDir().WriteFiles(map[string]string{
invalidFileName: invalidYaml,
validFileName: validYaml,
})

return tmpDir1, tmpDir2
}
39 changes: 18 additions & 21 deletions cmd/skaffold/app/flags/build_output_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,23 +34,23 @@ func TestNewBuildOutputFlag(t *testing.T) {
func TestBuildOutputSet(t *testing.T) {
var tests = []struct {
description string
buildOutputBytes []byte
setValue string
files map[string]string
shouldErr bool
expectedBuildOutput BuildOutput
}{
{
description: "set returns correct build output format for json",
buildOutputBytes: []byte(`{
files: map[string]string{
"test.in": `{
"builds": [{
"imageName": "gcr.io/k8s/test1",
"tag": "sha256@foo"
}, {
"imageName": "gcr.io/k8s/test2",
"tag": "sha256@bar"
}]
}`),
setValue: "test.in",
}`,
},
expectedBuildOutput: BuildOutput{
Builds: []build.Artifact{{
ImageName: "gcr.io/k8s/test1",
Expand All @@ -62,33 +62,30 @@ func TestBuildOutputSet(t *testing.T) {
},
},
{
description: "set errors with in-correct build output format",
buildOutputBytes: []byte{},
setValue: "test.in",
shouldErr: true,
description: "set errors with in-correct build output format",
files: map[string]string{
"test.in": "",
},
shouldErr: true,
},
{
description: "set should error when file is not present with original flag value",
buildOutputBytes: nil,
setValue: "does_not_exist.in",
shouldErr: true,
description: "set should error when file is not present with original flag value",
files: map[string]string{},
shouldErr: true,
},
}
for _, test := range tests {
testutil.Run(t, test.description, func(t *testutil.T) {
dir := t.NewTempDir()
tmpDir := t.NewTempDir().
WriteFiles(test.files)

flag := NewBuildOutputFileFlag("")
if test.buildOutputBytes != nil {
dir.Write(test.setValue, string(test.buildOutputBytes))
}
err := flag.Set(tmpDir.Path("test.in"))

expectedFlag := &BuildOutputFileFlag{
filename: test.setValue,
filename: "test.in",
buildOutput: test.expectedBuildOutput,
}

err := flag.Set(dir.Path(test.setValue))

t.CheckErrorAndDeepEqual(test.shouldErr, err, expectedFlag.buildOutput, flag.buildOutput)
})
}
Expand Down
13 changes: 5 additions & 8 deletions pkg/skaffold/build/bazel/dependencies_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,23 +29,23 @@ func TestGetDependencies(t *testing.T) {
tests := []struct {
description string
target string
hasWorkspace bool
files map[string]string
expectedQuery string
output string
expected []string
}{
{
description: "with workspace",
target: "target",
hasWorkspace: true,
files: map[string]string{"WORKSPACE": "content is ignored"},
expectedQuery: "bazel query kind('source file', deps('target')) union buildfiles('target') --noimplicit_deps --order_output=no",
output: "@ignored\n//external/ignored\n\n//:dep1\n//:dep2\n",
expected: []string{"dep1", "dep2", "WORKSPACE"},
},
{
description: "without workspace",
target: "target2",
hasWorkspace: false,
files: map[string]string{},
expectedQuery: "bazel query kind('source file', deps('target2')) union buildfiles('target2') --noimplicit_deps --order_output=no",
output: "@ignored\n//external/ignored\n\n//:dep3\n",
expected: []string{"dep3"},
Expand All @@ -54,11 +54,8 @@ func TestGetDependencies(t *testing.T) {
for _, test := range tests {
testutil.Run(t, test.description, func(t *testutil.T) {
t.Override(&util.DefaultExecCommand, t.FakeRunOut(test.expectedQuery, test.output))

tmpDir := t.NewTempDir()
if test.hasWorkspace {
tmpDir.Write("WORKSPACE", "")
}
tmpDir := t.NewTempDir().
WriteFiles(test.files)

deps, err := GetDependencies(context.Background(), tmpDir.Root(), &latest.BazelArtifact{
BuildTarget: test.target,
Expand Down
24 changes: 12 additions & 12 deletions pkg/skaffold/build/cluster/types_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,22 +29,21 @@ import (
)

func TestNewBuilder(t *testing.T) {

tcs := []struct {
name string
tests := []struct {
description string
shouldErr bool
runCtx *runcontext.RunContext
expectedBuilder *Builder
}{
{
name: "failed to parse cluster build timeout",
description: "failed to parse cluster build timeout",
runCtx: stubRunContext(&latest.ClusterDetails{
Timeout: "illegal",
}, nil),
shouldErr: true,
},
{
name: "cluster builder inherits the config",
description: "cluster builder inherits the config",
runCtx: stubRunContext(&latest.ClusterDetails{
Timeout: "100s",
Namespace: "test-ns",
Expand All @@ -60,7 +59,7 @@ func TestNewBuilder(t *testing.T) {
},
},
{
name: "insecure registries are taken from the run context",
description: "insecure registries are taken from the run context",
runCtx: stubRunContext(&latest.ClusterDetails{
Timeout: "100s",
Namespace: "test-ns",
Expand All @@ -76,12 +75,13 @@ func TestNewBuilder(t *testing.T) {
},
},
}
for _, tc := range tcs {
testutil.Run(t, tc.name, func(t *testutil.T) {
builder, err := NewBuilder(tc.runCtx)
t.CheckError(tc.shouldErr, err)
if !tc.shouldErr {
t.CheckDeepEqual(tc.expectedBuilder, builder, cmp.AllowUnexported(Builder{}))
for _, test := range tests {
testutil.Run(t, test.description, func(t *testutil.T) {
builder, err := NewBuilder(test.runCtx)

t.CheckError(test.shouldErr, err)
if !test.shouldErr {
t.CheckDeepEqual(test.expectedBuilder, builder, cmp.AllowUnexported(Builder{}))
}
})
}
Expand Down
10 changes: 2 additions & 8 deletions pkg/skaffold/build/custom/dependencies_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,10 +36,7 @@ func TestGetDependenciesDockerfile(t *testing.T) {
// - baz
// file
// Dockerfile
tmpDir.Write("foo", "")
tmpDir.Write("bar", "")
tmpDir.Mkdir("baz")
tmpDir.Write("baz/file", "")
tmpDir.Touch("foo", "bar", "baz/file")
tmpDir.Write("Dockerfile", "FROM scratch \n ARG file \n COPY $file baz/file .")

customArtifact := &latest.CustomArtifact{
Expand Down Expand Up @@ -106,10 +103,7 @@ func TestGetDependenciesPaths(t *testing.T) {
// - baz
// file
tmpDir := t.NewTempDir().
Write("foo", "").
Write("bar", "").
Mkdir("baz").
Write("baz/file", "")
Touch("foo", "bar", "baz/file")

deps, err := GetDependencies(context.Background(), tmpDir.Root(), &latest.CustomArtifact{
Dependencies: &latest.CustomDependencies{
Expand Down
36 changes: 18 additions & 18 deletions pkg/skaffold/build/local/local_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,9 +45,6 @@ func (t testAuthHelper) GetAuthConfig(string) (types.AuthConfig, error) {
func (t testAuthHelper) GetAllAuthConfigs() (map[string]types.AuthConfig, error) { return nil, nil }

func TestLocalRun(t *testing.T) {
reset := testutil.Override(t, &docker.DefaultAuthHelper, testAuthHelper{})
defer reset()

var tests = []struct {
description string
api testutil.FakeAPIClient
Expand Down Expand Up @@ -221,6 +218,7 @@ func TestLocalRun(t *testing.T) {
}
for _, test := range tests {
testutil.Run(t, test.description, func(t *testutil.T) {
t.Override(&docker.DefaultAuthHelper, testAuthHelper{})
fakeWarner := &warnings.Collect{}
t.Override(&warnings.Printf, fakeWarner.Warnf)

Expand Down Expand Up @@ -259,23 +257,23 @@ func TestNewBuilder(t *testing.T) {

pFalse := false

tcs := []struct {
name string
tests := []struct {
description string
shouldErr bool
localBuild *latest.LocalBuild
expectedBuilder *Builder
localClusterFn func() (bool, error)
localDockerFn func(*runcontext.RunContext) (docker.LocalDaemon, error)
}{
{
name: "failed to get docker client",
description: "failed to get docker client",
localDockerFn: func(runContext *runcontext.RunContext) (daemon docker.LocalDaemon, e error) {
e = errors.New("dummy docker error")
return
},
shouldErr: true,
}, {
name: "pushImages becomes !localCluster when local:push is not defined",
description: "pushImages becomes !localCluster when local:push is not defined",
localDockerFn: func(runContext *runcontext.RunContext) (daemon docker.LocalDaemon, e error) {
daemon = dummyDaemon
return
Expand All @@ -297,7 +295,7 @@ func TestNewBuilder(t *testing.T) {
insecureRegistries: nil,
},
}, {
name: "pushImages defined in config (local:push)",
description: "pushImages defined in config (local:push)",
localDockerFn: func(runContext *runcontext.RunContext) (daemon docker.LocalDaemon, e error) {
daemon = dummyDaemon
return
Expand Down Expand Up @@ -325,18 +323,20 @@ func TestNewBuilder(t *testing.T) {
},
},
}
for _, tc := range tcs {
testutil.Run(t, tc.name, func(t *testutil.T) {
if tc.localDockerFn != nil {
t.Override(&getLocalDocker, tc.localDockerFn)
for _, test := range tests {
testutil.Run(t, test.description, func(t *testutil.T) {
if test.localDockerFn != nil {
t.Override(&getLocalDocker, test.localDockerFn)
}
if tc.localClusterFn != nil {
t.Override(&getLocalCluster, tc.localClusterFn)
if test.localClusterFn != nil {
t.Override(&getLocalCluster, test.localClusterFn)
}
builder, err := NewBuilder(stubRunContext(tc.localBuild))
t.CheckError(tc.shouldErr, err)
if !tc.shouldErr {
t.CheckDeepEqual(tc.expectedBuilder, builder, cmp.AllowUnexported(Builder{}, dummyDaemon))

builder, err := NewBuilder(stubRunContext(test.localBuild))

t.CheckError(test.shouldErr, err)
if !test.shouldErr {
t.CheckDeepEqual(test.expectedBuilder, builder, cmp.AllowUnexported(Builder{}, dummyDaemon))
}
})
}
Expand Down
8 changes: 3 additions & 5 deletions pkg/skaffold/deploy/helm_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ import (
schemautil "github.com/GoogleContainerTools/skaffold/pkg/skaffold/schema/util"
"github.com/GoogleContainerTools/skaffold/pkg/skaffold/util"
"github.com/GoogleContainerTools/skaffold/testutil"
"github.com/mitchellh/go-homedir"
homedir "github.com/mitchellh/go-homedir"
"github.com/sirupsen/logrus"
)

Expand Down Expand Up @@ -619,10 +619,8 @@ func TestHelmDependencies(t *testing.T) {
}
for _, test := range tests {
testutil.Run(t, test.description, func(t *testutil.T) {
tmpDir := t.NewTempDir()
for _, file := range test.files {
tmpDir.Write(file, "")
}
tmpDir := t.NewTempDir().
Touch(test.files...)

deployer := NewHelmDeployer(makeRunContext(&latest.HelmDeploy{
Releases: []latest.HelmRelease{
Expand Down
Loading