From 533e96a605276a1cd8b1745466337fd6fe9a828f Mon Sep 17 00:00:00 2001 From: gabemontero Date: Fri, 26 Jan 2018 11:33:39 -0500 Subject: [PATCH] adjust newapp/newbuild error messages (arg classification vs. actual processing) --- pkg/generate/app/errors.go | 4 ++-- pkg/oc/cli/cmd/newapp.go | 18 +++++++----------- pkg/oc/generate/app/cmd/newapp.go | 9 ++++++++- test/cmd/newapp.sh | 6 +++--- test/integration/newapp_test.go | 2 +- 5 files changed, 21 insertions(+), 18 deletions(-) diff --git a/pkg/generate/app/errors.go b/pkg/generate/app/errors.go index 1e38fd825a56..bf08869d7f26 100644 --- a/pkg/generate/app/errors.go +++ b/pkg/generate/app/errors.go @@ -17,9 +17,9 @@ type ErrNoMatch struct { func (e ErrNoMatch) Error() string { if len(e.Qualifier) != 0 { - return fmt.Sprintf("no match for %q: %s", e.Value, e.Qualifier) + return fmt.Sprintf("unable to locate resource for %q: %s", e.Value, e.Qualifier) } - return fmt.Sprintf("no match for %q", e.Value) + return fmt.Sprintf("unable to locate resource for %q", e.Value) } // Suggestion is the usage error message returned when no match is found. diff --git a/pkg/oc/cli/cmd/newapp.go b/pkg/oc/cli/cmd/newapp.go index b8988bb73cea..f2c6d637fcb3 100644 --- a/pkg/oc/cli/cmd/newapp.go +++ b/pkg/oc/cli/cmd/newapp.go @@ -714,17 +714,6 @@ func handleError(err error, baseName, commandName, commandPath string, config *n transformError(err, baseName, commandName, commandPath, groups) } buf := &bytes.Buffer{} - if len(config.ArgumentClassificationErrors) > 0 { - fmt.Fprintf(buf, "Errors occurred while determining argument types:\n") - for _, classErr := range config.ArgumentClassificationErrors { - fmt.Fprintf(buf, fmt.Sprintf("\n%s: %v\n", classErr.Key, classErr.Value)) - } - fmt.Fprint(buf, "\n") - // this print serves as a header for the printing of the errorGroups, but - // only print it if we precede with classification errors, to help distinguish - // between the two - fmt.Fprintln(buf, "Errors occurred during resource creation:") - } for _, group := range groups { fmt.Fprint(buf, kcmdutil.MultipleErrors("error: ", group.errs)) if len(group.suggestion) > 0 { @@ -732,6 +721,13 @@ func handleError(err error, baseName, commandName, commandPath string, config *n } fmt.Fprint(buf, group.suggestion) } + if len(config.ArgumentClassificationErrors) > 0 && len(groups) > 0 { + fmt.Fprintf(buf, "\nFor further assistance, %s classification of the argument types resulted in the following:\n", commandName) + for _, classErr := range config.ArgumentClassificationErrors { + fmt.Fprintf(buf, fmt.Sprintf("\n%s: %v\n", classErr.Key, classErr.Value)) + } + fmt.Fprintf(buf, fmt.Sprintf("\n%s", config.ArgumentClassificationWinner)) + } return fmt.Errorf(buf.String()) } diff --git a/pkg/oc/generate/app/cmd/newapp.go b/pkg/oc/generate/app/cmd/newapp.go index 0319737cb00f..4c083c067212 100644 --- a/pkg/oc/generate/app/cmd/newapp.go +++ b/pkg/oc/generate/app/cmd/newapp.go @@ -132,6 +132,7 @@ type AppConfig struct { OriginNamespace string ArgumentClassificationErrors []ArgumentClassificationError + ArgumentClassificationWinner string } type ArgumentClassificationError struct { @@ -312,6 +313,7 @@ func (c *AppConfig) tryToAddTemplateArguments(s string) bool { // AddArguments converts command line arguments into the appropriate bucket based on what they look like func (c *AppConfig) AddArguments(args []string) []string { unknown := []string{} + winnerFmt := "The argument %s passed the %s classification tests" c.ArgumentClassificationErrors = []ArgumentClassificationError{} for _, s := range args { if len(s) == 0 { @@ -320,9 +322,14 @@ func (c *AppConfig) AddArguments(args []string) []string { switch { case c.tryToAddEnvironmentArguments(s): + c.ArgumentClassificationWinner = fmt.Sprintf(winnerFmt, s, "environment") case c.tryToAddSourceArguments(s): - case c.tryToAddComponentArguments(s): + c.ArgumentClassificationWinner = fmt.Sprintf(winnerFmt, s, "source") case c.tryToAddTemplateArguments(s): + c.ArgumentClassificationWinner = fmt.Sprintf(winnerFmt, s, "template") + case c.tryToAddComponentArguments(s): + // NOTE, component argument classification currently is the most lenient, so we save it for the end + c.ArgumentClassificationWinner = fmt.Sprintf(winnerFmt, s, "component") default: glog.V(2).Infof("treating %s as unknown\n", s) unknown = append(unknown, s) diff --git a/test/cmd/newapp.sh b/test/cmd/newapp.sh index d5958eb3c828..76302b47bf54 100755 --- a/test/cmd/newapp.sh +++ b/test/cmd/newapp.sh @@ -346,8 +346,8 @@ os::cmd::expect_success_and_text 'oc new-app --dry-run --docker-image=mysql' 'Th os::cmd::expect_success_and_text 'oc new-app --dry-run --docker-image=mysql' "WARNING: Image \"mysql\" runs as the 'root' user" # verify multiple errors are displayed together, a nested error is returned, and that the usage message is displayed -os::cmd::expect_failure_and_text 'oc new-app --dry-run __template_fail __templatefile_fail' 'error: no match for "__template_fail"' -os::cmd::expect_failure_and_text 'oc new-app --dry-run __template_fail __templatefile_fail' 'error: no match for "__templatefile_fail"' +os::cmd::expect_failure_and_text 'oc new-app --dry-run __template_fail __templatefile_fail' 'error: unable to locate resource for "__template_fail"' +os::cmd::expect_failure_and_text 'oc new-app --dry-run __template_fail __templatefile_fail' 'error: unable to locate resource for "__templatefile_fail"' os::cmd::expect_failure_and_text 'oc new-app --dry-run __template_fail __templatefile_fail' 'error: unable to find the specified template file' os::cmd::expect_failure_and_text 'oc new-app --dry-run __template_fail __templatefile_fail' "The 'oc new-app' command will match arguments" @@ -448,7 +448,7 @@ os::cmd::expect_success_and_text 'oc new-app -f test/testdata/circular.yaml' 'sh os::cmd::expect_success_and_not_text 'oc new-app -f test/testdata/bc-from-imagestreamimage.json --dry-run' 'Unable to follow reference type' # do not allow use of non-existent image (should fail) -os::cmd::expect_failure_and_text 'oc new-app openshift/bogusimage https://github.com/openshift/ruby-hello-world.git -o yaml' "no match for" +os::cmd::expect_failure_and_text 'oc new-app openshift/bogusimage https://github.com/openshift/ruby-hello-world.git -o yaml' "unable to locate resource for" # allow use of non-existent image (should succeed) os::cmd::expect_success 'oc new-app openshift/bogusimage https://github.com/openshift/ruby-hello-world.git -o yaml --allow-missing-images' diff --git a/test/integration/newapp_test.go b/test/integration/newapp_test.go index 2587052df26f..273f2b4fc07b 100644 --- a/test/integration/newapp_test.go +++ b/test/integration/newapp_test.go @@ -154,7 +154,7 @@ func TestNewAppResolve(t *testing.T) { }, }, })}, - expectedErr: `no match for "mysql:invalid`, + expectedErr: `unable to locate resource for "mysql:invalid`, }, { name: "Successful mysql builder",