From 7d75e42bc9d0f19f8829b65634495965f02e89a3 Mon Sep 17 00:00:00 2001 From: sakeven Date: Fri, 26 Aug 2016 17:06:07 +0800 Subject: [PATCH] fmt dockerfile error message Signed-off-by: sakeven --- builder/dockerfile/dispatchers.go | 24 +++++++------- builder/dockerfile/dispatchers_test.go | 46 +++++++++++++++++++++----- builder/dockerfile/evaluator_test.go | 2 +- 3 files changed, 50 insertions(+), 22 deletions(-) diff --git a/builder/dockerfile/dispatchers.go b/builder/dockerfile/dispatchers.go index dc5b7ef0b8..d54299ff40 100644 --- a/builder/dockerfile/dispatchers.go +++ b/builder/dockerfile/dispatchers.go @@ -68,7 +68,7 @@ func env(b *Builder, args []string, attributes map[string]bool, original string) // value ==> args[j+1] if len(args[j]) == 0 { - return fmt.Errorf("ENV names can not be blank") + return errBlankCommandNames("ENV") } newVar := args[j] + "=" + args[j+1] + "" @@ -136,7 +136,7 @@ func label(b *Builder, args []string, attributes map[string]bool, original strin // value ==> args[j+1] if len(args[j]) == 0 { - return fmt.Errorf("LABEL names can not be blank") + return errBlankCommandNames("LABEL") } newVar := args[j] + "=" + args[j+1] + "" @@ -455,7 +455,7 @@ func parseOptInterval(f *Flag) (time.Duration, error) { // func healthcheck(b *Builder, args []string, attributes map[string]bool, original string) error { if len(args) == 0 { - return fmt.Errorf("HEALTHCHECK requires an argument") + return errAtLeastOneArgument("HEALTHCHECK") } typ := strings.ToUpper(args[0]) args = args[1:] @@ -529,11 +529,7 @@ func healthcheck(b *Builder, args []string, attributes map[string]bool, original b.runConfig.Healthcheck = &healthcheck } - if err := b.commit("", b.runConfig.Cmd, fmt.Sprintf("HEALTHCHECK %q", b.runConfig.Healthcheck)); err != nil { - return err - } - - return nil + return b.commit("", b.runConfig.Cmd, fmt.Sprintf("HEALTHCHECK %q", b.runConfig.Healthcheck)) } // ENTRYPOINT /usr/sbin/nginx @@ -654,7 +650,7 @@ func volume(b *Builder, args []string, attributes map[string]bool, original stri for _, v := range args { v = strings.TrimSpace(v) if v == "" { - return fmt.Errorf("Volume specified can not be an empty string") + return fmt.Errorf("VOLUME specified can not be an empty string") } b.runConfig.Volumes[v] = struct{}{} } @@ -669,7 +665,7 @@ func volume(b *Builder, args []string, attributes map[string]bool, original stri // Set the signal that will be used to kill the container. func stopSignal(b *Builder, args []string, attributes map[string]bool, original string) error { if len(args) != 1 { - return fmt.Errorf("STOPSIGNAL requires exactly one argument") + return errExactlyOneArgument("STOPSIGNAL") } sig := args[0] @@ -689,7 +685,7 @@ func stopSignal(b *Builder, args []string, attributes map[string]bool, original // Dockerfile author may optionally set a default value of this variable. func arg(b *Builder, args []string, attributes map[string]bool, original string) error { if len(args) != 1 { - return fmt.Errorf("ARG requires exactly one argument definition") + return errExactlyOneArgument("ARG") } var ( @@ -707,7 +703,7 @@ func arg(b *Builder, args []string, attributes map[string]bool, original string) if strings.Contains(arg, "=") { parts := strings.SplitN(arg, "=", 2) if len(parts[0]) == 0 { - return fmt.Errorf("ARG names can not be blank") + return errBlankCommandNames("ARG") } name = parts[0] @@ -764,6 +760,10 @@ func errAtLeastTwoArguments(command string) error { return fmt.Errorf("%s requires at least two arguments", command) } +func errBlankCommandNames(command string) error { + return fmt.Errorf("%s names can not be blank", command) +} + func errTooManyArguments(command string) error { return fmt.Errorf("Bad input to %s, too many arguments", command) } diff --git a/builder/dockerfile/dispatchers_test.go b/builder/dockerfile/dispatchers_test.go index 22ff66a68c..c094ed99a5 100644 --- a/builder/dockerfile/dispatchers_test.go +++ b/builder/dockerfile/dispatchers_test.go @@ -22,7 +22,8 @@ func TestCommandsExactlyOneArgument(t *testing.T) { {"MAINTAINER", func(args []string) error { return maintainer(nil, args, nil, "") }}, {"FROM", func(args []string) error { return from(nil, args, nil, "") }}, {"WORKDIR", func(args []string) error { return workdir(nil, args, nil, "") }}, - {"USER", func(args []string) error { return user(nil, args, nil, "") }}} + {"USER", func(args []string) error { return user(nil, args, nil, "") }}, + {"STOPSIGNAL", func(args []string) error { return stopSignal(nil, args, nil, "") }}} for _, command := range commands { err := command.function([]string{}) @@ -31,9 +32,9 @@ func TestCommandsExactlyOneArgument(t *testing.T) { t.Fatalf("Error should be present for %s command", command.name) } - expectedError := fmt.Sprintf("%s requires exactly one argument", command.name) + expectedError := errExactlyOneArgument(command.name) - if err.Error() != expectedError { + if err.Error() != expectedError.Error() { t.Fatalf("Wrong error message for %s. Got: %s. Should be: %s", command.name, err.Error(), expectedError) } } @@ -44,6 +45,7 @@ func TestCommandsAtLeastOneArgument(t *testing.T) { {"ENV", func(args []string) error { return env(nil, args, nil, "") }}, {"LABEL", func(args []string) error { return label(nil, args, nil, "") }}, {"ONBUILD", func(args []string) error { return onbuild(nil, args, nil, "") }}, + {"HEALTHCHECK", func(args []string) error { return healthcheck(nil, args, nil, "") }}, {"EXPOSE", func(args []string) error { return expose(nil, args, nil, "") }}, {"VOLUME", func(args []string) error { return volume(nil, args, nil, "") }}} @@ -54,9 +56,9 @@ func TestCommandsAtLeastOneArgument(t *testing.T) { t.Fatalf("Error should be present for %s command", command.name) } - expectedError := fmt.Sprintf("%s requires at least one argument", command.name) + expectedError := errAtLeastOneArgument(command.name) - if err.Error() != expectedError { + if err.Error() != expectedError.Error() { t.Fatalf("Wrong error message for %s. Got: %s. Should be: %s", command.name, err.Error(), expectedError) } } @@ -74,9 +76,9 @@ func TestCommandsAtLeastTwoArguments(t *testing.T) { t.Fatalf("Error should be present for %s command", command.name) } - expectedError := fmt.Sprintf("%s requires at least two arguments", command.name) + expectedError := errAtLeastTwoArguments(command.name) - if err.Error() != expectedError { + if err.Error() != expectedError.Error() { t.Fatalf("Wrong error message for %s. Got: %s. Should be: %s", command.name, err.Error(), expectedError) } } @@ -94,9 +96,35 @@ func TestCommandsTooManyArguments(t *testing.T) { t.Fatalf("Error should be present for %s command", command.name) } - expectedError := fmt.Sprintf("Bad input to %s, too many arguments", command.name) + expectedError := errTooManyArguments(command.name) - if err.Error() != expectedError { + if err.Error() != expectedError.Error() { + t.Fatalf("Wrong error message for %s. Got: %s. Should be: %s", command.name, err.Error(), expectedError) + } + } +} + +func TestCommandseBlankNames(t *testing.T) { + bflags := &BFlags{} + config := &container.Config{} + + b := &Builder{flags: bflags, runConfig: config, disableCommit: true} + + commands := []commandWithFunction{ + {"ENV", func(args []string) error { return env(b, args, nil, "") }}, + {"LABEL", func(args []string) error { return label(b, args, nil, "") }}, + } + + for _, command := range commands { + err := command.function([]string{"", ""}) + + if err == nil { + t.Fatalf("Error should be present for %s command", command.name) + } + + expectedError := errBlankCommandNames(command.name) + + if err.Error() != expectedError.Error() { t.Fatalf("Wrong error message for %s. Got: %s. Should be: %s", command.name, err.Error(), expectedError) } } diff --git a/builder/dockerfile/evaluator_test.go b/builder/dockerfile/evaluator_test.go index acc8eccea9..ac066d7f64 100644 --- a/builder/dockerfile/evaluator_test.go +++ b/builder/dockerfile/evaluator_test.go @@ -45,7 +45,7 @@ func initDispatchTestCases() []dispatchTestCase { { name: "ARG two arguments", dockerfile: "ARG foo bar", - expectedError: "ARG requires exactly one argument definition", + expectedError: "ARG requires exactly one argument", files: nil, }, {