Merge pull request #1251 from nkubala/skaffold_fix_tests

Don't lose test configuration when running skaffold fix

cmd/skaffold/app/cmd/fix_test.go

@@ -0,0 +1,113 @@
+/*
+Copyright 2018 The Skaffold Authors
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package cmd
+
+import (
+	"bytes"
+	"fmt"
+	"testing"
+
+	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/schema"
+	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/schema/latest"
+	"github.com/GoogleContainerTools/skaffold/testutil"
+)
+
+func TestFix(t *testing.T) {
+	tests := []struct {
+		name       string
+		inputYaml  string
+		outputYaml string
+		shouldErr  bool
+	}{
+		{
+			name: "v1alpha4 to latest",
+			inputYaml: `apiVersion: skaffold/v1alpha4
+kind: Config
+build:
+  artifacts:
+  - image: docker/image
+    docker:
+      dockerfile: dockerfile.test
+test:
+- image: docker/image
+  structureTests:
+  - ./test/*
+deploy:
+  kubectl:
+    manifests:
+    - k8s/deployment.yaml
+`,
+			outputYaml: fmt.Sprintf(`apiVersion: %s
+kind: Config
+build:
+  artifacts:
+  - image: docker/image
+    docker:
+      dockerfile: dockerfile.test
+test:
+- image: docker/image
+  structureTests:
+  - ./test/*
+deploy:
+  kubectl:
+    manifests:
+    - k8s/deployment.yaml
+`, latest.Version),
+		},
+		{
+			name: "v1alpha1 to latest",
+			inputYaml: `apiVersion: skaffold/v1alpha1
+kind: Config
+build:
+  artifacts:
+  - imageName: docker/image
+    dockerfilePath: dockerfile.test
+deploy:
+  kubectl:
+    manifests:
+    - paths:
+      - k8s/deployment.yaml
+`,
+			outputYaml: fmt.Sprintf(`apiVersion: %s
+kind: Config
+build:
+  artifacts:
+  - image: docker/image
+    docker:
+      dockerfile: dockerfile.test
+deploy:
+  kubectl:
+    manifests:
+    - k8s/deployment.yaml
+`, latest.Version),
+		},
+	}
+
+	for _, tt := range tests {
+		cfgFile, teardown := testutil.TempFile(t, "config", []byte(tt.inputYaml))
+		defer teardown()
+
+		cfg, err := schema.ParseConfig(cfgFile, false)
+		if err != nil {
+			t.Fatalf(err.Error())
+		}
+		var b bytes.Buffer
+		err = runFix(&b, cfg)
+
+		testutil.CheckErrorAndDeepEqual(t, tt.shouldErr, err, tt.outputYaml, b.String())
+	}
+}

pkg/skaffold/runner/runner.go

@@ -145,7 +145,7 @@ func getBuilder(cfg *latest.BuildConfig, kubeContext string) (build.Builder, err
 	}
 }
 
-func getTester(cfg *[]*latest.TestCase) (test.Tester, error) {
+func getTester(cfg *latest.TestConfig) (test.Tester, error) {
 	return test.NewTester(cfg)
 }
 

pkg/skaffold/schema/latest/config.go

@@ -34,7 +34,7 @@ type SkaffoldPipeline struct {
 	Kind       string `yaml:"kind"`
 
 	Build    BuildConfig  `yaml:"build,omitempty"`
-	Test     []*TestCase  `yaml:"test,omitempty"`
+	Test     TestConfig   `yaml:"test,omitempty"`
 	Deploy   DeployConfig `yaml:"deploy,omitempty"`
 	Profiles []Profile    `yaml:"profiles,omitempty"`
 }
@@ -133,6 +133,8 @@ type AzureContainerBuild struct {
 	TenantID       string `yaml:"tenantId,omitempty"`
 }
 
+type TestConfig []*TestCase
+
 // TestCase is a struct containing all the specified test
 // configuration for an image.
 type TestCase struct {
@@ -237,7 +239,7 @@ type Artifact struct {
 type Profile struct {
 	Name   string       `yaml:"name,omitempty"`
 	Build  BuildConfig  `yaml:"build,omitempty"`
-	Test   []*TestCase  `yaml:"test,omitempty"`
+	Test   TestConfig   `yaml:"test,omitempty"`
 	Deploy DeployConfig `yaml:"deploy,omitempty"`
 }
 

pkg/skaffold/schema/profiles.go

@@ -55,7 +55,7 @@ func applyProfile(config *latest.SkaffoldPipeline, profile latest.Profile) {
 		Kind:       config.Kind,
 		Build:      overlayProfileField(config.Build, profile.Build).(latest.BuildConfig),
 		Deploy:     overlayProfileField(config.Deploy, profile.Deploy).(latest.DeployConfig),
-		Test:       overlayProfileField(config.Test, profile.Test).([]*latest.TestCase),
+		Test:       overlayProfileField(config.Test, profile.Test).(latest.TestConfig),
 	}
 }
 

pkg/skaffold/schema/v1alpha4/config.go

@@ -34,11 +34,13 @@ type SkaffoldPipeline struct {
 	Kind       string `yaml:"kind"`
 
 	Build    BuildConfig  `yaml:"build,omitempty"`
-	Test     []TestCase   `yaml:"test,omitempty"`
+	Test     TestConfig   `yaml:"test,omitempty"`
 	Deploy   DeployConfig `yaml:"deploy,omitempty"`
 	Profiles []Profile    `yaml:"profiles,omitempty"`
 }
 
+type TestConfig []TestCase
+
 func (c *SkaffoldPipeline) GetVersion() string {
 	return c.APIVersion
 }

pkg/skaffold/schema/v1alpha4/upgrade.go

@@ -46,11 +46,18 @@ func (config *SkaffoldPipeline) Upgrade() (util.VersionedConfig, error) {
 		return nil, errors.Wrap(err, "converting new build")
 	}
 
+	// convert Test (should be the same)
+	var newTest next.TestConfig
+	if err := convert(config.Test, &newTest); err != nil {
+		return nil, errors.Wrap(err, "converting new test")
+	}
+
 	return &next.SkaffoldPipeline{
 		APIVersion: next.Version,
 		Kind:       config.Kind,
-		Deploy:     newDeploy,
 		Build:      newBuild,
+		Test:       newTest,
+		Deploy:     newDeploy,
 		Profiles:   newProfiles,
 	}, nil
 }

pkg/skaffold/test/test.go

@@ -32,7 +32,7 @@ import (
 // NewTester parses the provided test cases from the Skaffold config,
 // and returns a Tester instance with all the necessary test runners
 // to run all specified tests.
-func NewTester(testCases *[]*latest.TestCase) (Tester, error) {
+func NewTester(testCases *latest.TestConfig) (Tester, error) {
 	// TODO(nkubala): copied this from runner.getDeployer(), this should be moved somewhere else
 	cwd, err := os.Getwd()
 	if err != nil {

pkg/skaffold/test/types.go

@@ -42,7 +42,7 @@ type Tester interface {
 // FullTester should always be the ONLY implementation of the Tester interface;
 // newly added testing implementations should implement the Runner interface.
 type FullTester struct {
-	testCases  *[]*latest.TestCase
+	testCases  *latest.TestConfig
 	workingDir string
 }