Skip to content
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

Improve code coverage #2242

Merged
merged 9 commits into from
Jun 10, 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
4 changes: 2 additions & 2 deletions cmd/skaffold/app/cmd/config/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -59,8 +59,8 @@ func TestReadConfig(t *testing.T) {
}
for _, test := range tests {
testutil.Run(t, test.description, func(t *testutil.T) {
tmpDir := t.NewTempDir()
t.Chdir(tmpDir.Root())
tmpDir := t.NewTempDir().
Chdir()

if test.filename != "" {
c, _ := yaml.Marshal(*baseConfig)
Expand Down
14 changes: 8 additions & 6 deletions cmd/skaffold/app/cmd/config/util.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,15 +38,17 @@ func resolveKubectlContext() {
return
}

context, err := context.CurrentContext()
if err != nil {
logrus.Warn(errors.Wrap(err, "retrieving current kubectl context"))
}
if context == "" {
config, err := context.CurrentConfig()
switch {
case err != nil:
logrus.Warn("unable to retrieve current kubectl context, using global values")
global = true
case config.CurrentContext == "":
logrus.Infof("no kubectl context currently set, using global values")
global = true
default:
kubecontext = config.CurrentContext
}
kubecontext = context
}

func resolveConfigFile() error {
Expand Down
6 changes: 3 additions & 3 deletions cmd/skaffold/app/cmd/runner_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -81,9 +81,9 @@ func TestNewRunner(t *testing.T) {
}
for _, test := range tests {
testutil.Run(t, test.description, func(t *testutil.T) {
tmpDir := t.NewTempDir().
Write("skaffold.yaml", fmt.Sprintf("apiVersion: %s\nkind: Config\n%s", latest.Version, test.config))
t.Chdir(tmpDir.Root())
t.NewTempDir().
Write("skaffold.yaml", fmt.Sprintf("apiVersion: %s\nkind: Config\n%s", latest.Version, test.config)).
Chdir()

_, _, err := newRunner(test.options)

Expand Down
24 changes: 7 additions & 17 deletions cmd/skaffold/man/man.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,27 +27,15 @@ import (
)

func main() {
if err := printMan(os.Stdout, os.Stderr); err != nil {
panic(err)
}
printMan(os.Stdout, os.Stderr)
}

func printMan(stdout, stderr io.Writer) error {
func printMan(stdout, stderr io.Writer) {
command := cmd.NewSkaffoldCommand(stdout, stderr)
return printCommand(stdout, command)
}

func printSubCommands(out io.Writer, command *cobra.Command) error {
for _, subCommand := range command.Commands() {
if err := printCommand(out, subCommand); err != nil {
return err
}
}

return nil
printCommand(stdout, command)
}

func printCommand(out io.Writer, command *cobra.Command) error {
func printCommand(out io.Writer, command *cobra.Command) {
command.DisableFlagsInUseLine = true

fmt.Fprintf(out, "\n### %s\n", command.CommandPath())
Expand All @@ -62,5 +50,7 @@ func printCommand(out io.Writer, command *cobra.Command) error {
})
}

return printSubCommands(out, command)
for _, subCommand := range command.Commands() {
printCommand(out, subCommand)
}
}
3 changes: 1 addition & 2 deletions cmd/skaffold/man/man_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,11 +29,10 @@ func TestPrintMan(t *testing.T) {
var stdout bytes.Buffer
var stderr bytes.Buffer

err := printMan(&stdout, &stderr)
printMan(&stdout, &stderr)
output := stdout.String()

// Sanity checks
testutil.CheckError(t, false, err)
testutil.CheckDeepEqual(t, "", stderr.String())
testutil.CheckContains(t, "skaffold build", output)
testutil.CheckContains(t, "skaffold run", output)
Expand Down
15 changes: 7 additions & 8 deletions hack/schemas/main_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,28 +39,27 @@ func TestSchemas(t *testing.T) {
}

func TestGenerators(t *testing.T) {
tcs := []struct {
tests := []struct {
name string
}{
{name: "inline"},
{name: "inline-anyof"},
{name: "inline-hybrid"},
}

for _, tc := range tcs {
t.Run(tc.name, func(t *testing.T) {
input := filepath.Join("testdata", tc.name, "input.go")
expectedOutput := filepath.Join("testdata", tc.name, "output.json")
for _, test := range tests {
testutil.Run(t, test.name, func(t *testutil.T) {
input := filepath.Join("testdata", test.name, "input.go")
expectedOutput := filepath.Join("testdata", test.name, "output.json")

generator := schemaGenerator{
strict: false,
}

actual, err := generator.Apply(input)
testutil.CheckError(t, false, err)
t.CheckNoError(err)

expected, err := ioutil.ReadFile(expectedOutput)
testutil.CheckError(t, false, err)
t.CheckNoError(err)

expected = bytes.Replace(expected, []byte("\r\n"), []byte("\n"), -1)

Expand Down
12 changes: 6 additions & 6 deletions pkg/skaffold/deploy/kubectl_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -149,10 +149,10 @@ func TestKubectlDeploy(t *testing.T) {
}
for _, test := range tests {
testutil.Run(t, test.description, func(t *testutil.T) {
tmpDir := t.NewTempDir().
t.NewTempDir().
Write("deployment.yaml", deploymentWebYAML).
Write("empty.ignored", "")
t.Chdir(tmpDir.Root())
Write("empty.ignored", "").
Chdir()

t.Override(&util.DefaultExecCommand, test.command)

Expand Down Expand Up @@ -221,9 +221,9 @@ func TestKubectlCleanup(t *testing.T) {
}
for _, test := range tests {
testutil.Run(t, test.description, func(t *testutil.T) {
tmpDir := t.NewTempDir().
Write("deployment.yaml", deploymentWebYAML)
t.Chdir(tmpDir.Root())
t.NewTempDir().
Write("deployment.yaml", deploymentWebYAML).
Chdir()

t.Override(&util.DefaultExecCommand, test.command)

Expand Down
4 changes: 2 additions & 2 deletions pkg/skaffold/deploy/kustomize_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -66,8 +66,8 @@ func TestKustomizeDeploy(t *testing.T) {
}
for _, test := range tests {
testutil.Run(t, test.description, func(t *testutil.T) {
tmpDir := t.NewTempDir()
t.Chdir(tmpDir.Root())
t.NewTempDir().
Chdir()

t.Override(&util.DefaultExecCommand, test.command)

Expand Down
4 changes: 2 additions & 2 deletions pkg/skaffold/docker/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,13 +49,13 @@ var (
// NewAPIClient guesses the docker client to use based on current kubernetes context.
func NewAPIClient(forceRemove bool, insecureRegistries map[string]bool) (LocalDaemon, error) {
dockerAPIClientOnce.Do(func() {
kubeContext, err := kubectx.CurrentContext()
kubeConfig, err := kubectx.CurrentConfig()
if err != nil {
dockerAPIClientErr = errors.Wrap(err, "getting current cluster context")
return
}

env, apiClient, err := newAPIClient(kubeContext)
env, apiClient, err := newAPIClient(kubeConfig.CurrentContext)
dockerAPIClient = NewLocalDaemon(apiClient, env, forceRemove, insecureRegistries)
dockerAPIClientErr = err
})
Expand Down
6 changes: 3 additions & 3 deletions pkg/skaffold/docker/context_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,14 +29,14 @@ import (
func TestDockerContext(t *testing.T) {
for _, dir := range []string{".", "sub"} {
testutil.Run(t, dir, func(t *testutil.T) {
tmpDir := t.NewTempDir().
t.NewTempDir().
Write(dir+"/files/ignored.txt", "").
Write(dir+"/files/included.txt", "").
Write(dir+"/.dockerignore", "**/ignored.txt\nalsoignored.txt").
Write(dir+"/Dockerfile", "FROM alpine\nCOPY ./files /files").
Write(dir+"/ignored.txt", "").
Write(dir+"/alsoignored.txt", "")
t.Chdir(tmpDir.Root())
Write(dir+"/alsoignored.txt", "").
Chdir()

imageFetcher := fakeImageFetcher{}
t.Override(&RetrieveImage, imageFetcher.fetch)
Expand Down
13 changes: 5 additions & 8 deletions pkg/skaffold/kubernetes/context/context.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,11 @@ var (
currentConfigErr error
)

// ResetCurrentConfig is used by tests
func ResetCurrentConfig() {
currentConfigOnce = sync.Once{}
}

func CurrentConfig() (clientcmdapi.Config, error) {
currentConfigOnce.Do(func() {
loadingRules := clientcmd.NewDefaultClientConfigLoadingRules()
Expand All @@ -43,11 +48,3 @@ func CurrentConfig() (clientcmdapi.Config, error) {
})
return currentConfig, currentConfigErr
}

func CurrentContext() (string, error) {
cfg, err := CurrentConfig()
if err != nil {
return "", err
}
return cfg.CurrentContext, nil
}
23 changes: 18 additions & 5 deletions pkg/skaffold/kubernetes/context/context_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,16 +20,29 @@ import (
"testing"

"github.com/GoogleContainerTools/skaffold/testutil"
"k8s.io/client-go/tools/clientcmd/api"
)

func TestCurrentContext(t *testing.T) {
testutil.Run(t, "", func(t *testutil.T) {
t.SetupFakeKubernetesContext(api.Config{CurrentContext: "cluster1"})
testutil.Run(t, "valid context", func(t *testutil.T) {
resetKubeConfig(t, "apiVersion: v1\nkind: Config\ncurrent-context: cluster1\n")

context, err := CurrentContext()
config, err := CurrentConfig()

t.CheckNoError(err)
t.CheckDeepEqual("cluster1", context)
t.CheckDeepEqual("cluster1", config.CurrentContext)
})

testutil.Run(t, "invalid context", func(t *testutil.T) {
resetKubeConfig(t, "invalid")

_, err := CurrentConfig()

t.CheckError(true, err)
})
}

func resetKubeConfig(t *testutil.T, content string) {
kubeConfig := t.TempFile("config", []byte(content))
t.SetEnvs(map[string]string{"KUBECONFIG": kubeConfig})
ResetCurrentConfig()
}
3 changes: 2 additions & 1 deletion pkg/skaffold/runner/context/context.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,10 +42,11 @@ type RunContext struct {
}

func GetRunContext(opts *config.SkaffoldOptions, cfg *latest.Pipeline) (*RunContext, error) {
kubeContext, err := kubectx.CurrentContext()
kubeConfig, err := kubectx.CurrentConfig()
if err != nil {
return nil, errors.Wrap(err, "getting current cluster context")
}
kubeContext := kubeConfig.CurrentContext
logrus.Infof("Using kubectl context: %s", kubeContext)

// TODO(dgageot): this should be the folder containing skaffold.yaml. Should also be moved elsewhere.
Expand Down
4 changes: 2 additions & 2 deletions pkg/skaffold/schema/profiles.go
Original file line number Diff line number Diff line change
Expand Up @@ -112,12 +112,12 @@ func isKubeContext(kubeContext string) (bool, error) {
return true, nil
}

currentKubeContext, err := kubectx.CurrentContext()
currentKubeConfig, err := kubectx.CurrentConfig()
if err != nil {
return false, errors.Wrap(err, "getting current cluster context")
}

return satisfies(kubeContext, currentKubeContext), nil
return satisfies(kubeContext, currentKubeConfig.CurrentContext), nil
}

func satisfies(expected, actual string) bool {
Expand Down
16 changes: 7 additions & 9 deletions pkg/skaffold/util/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,10 +25,9 @@ import (

func TestReadConfiguration(t *testing.T) {
testutil.Run(t, "", func(t *testutil.T) {
tmpDir := t.NewTempDir()
t.Chdir(tmpDir.Root())

tmpDir.Write("skaffold.yaml", "some yaml")
t.NewTempDir().
Write("skaffold.yaml", "some yaml").
Chdir()

content, err := ReadConfiguration("skaffold.yaml")

Expand All @@ -39,11 +38,10 @@ func TestReadConfiguration(t *testing.T) {

func TestReadConfigurationFallback(t *testing.T) {
testutil.Run(t, "", func(t *testutil.T) {
tmpDir := t.NewTempDir()
t.Chdir(tmpDir.Root())

// skaffold.yaml doesn't exist but .yml does
tmpDir.Write("skaffold.yml", "some yaml")
t.NewTempDir().
// skaffold.yaml doesn't exist but .yml does
Write("skaffold.yml", "some yaml").
Chdir()

content, err := ReadConfiguration("skaffold.yaml")

Expand Down
Loading