From 728dfbe2c8dc0b387b5e6671ff0fe4e4cfbed2f8 Mon Sep 17 00:00:00 2001 From: Martin Polden Date: Wed, 29 Sep 2021 09:06:30 +0200 Subject: Fix copyright headers --- client/go/cmd/query.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'client/go/cmd/query.go') diff --git a/client/go/cmd/query.go b/client/go/cmd/query.go index f05914eb9a7..7de0addb9b8 100644 --- a/client/go/cmd/query.go +++ b/client/go/cmd/query.go @@ -1,4 +1,4 @@ -// Copyright Verizon Media. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. // vespa query command // author: bratseth -- cgit v1.2.3 From 16977143c48117d254c548e7f94968976f54a72c Mon Sep 17 00:00:00 2001 From: Martin Polden Date: Tue, 28 Sep 2021 15:17:50 +0200 Subject: Print errors to stderr --- client/go/cmd/api_key_test.go | 4 ++-- client/go/cmd/cert_test.go | 10 +++++----- client/go/cmd/command_tester.go | 25 ++++++++++++++----------- client/go/cmd/config_test.go | 11 ++++++++--- client/go/cmd/deploy_test.go | 15 +++++++++------ client/go/cmd/document.go | 17 +++++++++++------ client/go/cmd/document_test.go | 20 ++++++++++++-------- client/go/cmd/helpers.go | 12 +++++------- client/go/cmd/log_test.go | 4 ++-- client/go/cmd/query.go | 8 +++----- client/go/cmd/query_test.go | 6 ++++-- client/go/cmd/status_test.go | 7 +++++-- 12 files changed, 80 insertions(+), 59 deletions(-) (limited to 'client/go/cmd/query.go') diff --git a/client/go/cmd/api_key_test.go b/client/go/cmd/api_key_test.go index 1deb628c21e..b08758ae21d 100644 --- a/client/go/cmd/api_key_test.go +++ b/client/go/cmd/api_key_test.go @@ -17,7 +17,7 @@ func TestAPIKey(t *testing.T) { out, _ := execute(command{args: []string{"api-key", "-a", "t1.a1.i1"}, homeDir: homeDir}, t, nil) assert.Contains(t, out, "Success: API private key written to "+keyFile+"\n") - out, _ = execute(command{args: []string{"api-key", "-a", "t1.a1.i1"}, homeDir: homeDir}, t, nil) - assert.Contains(t, out, "Error: File "+keyFile+" already exists\nHint: Use -f to overwrite it\n") + out, outErr := execute(command{args: []string{"api-key", "-a", "t1.a1.i1"}, homeDir: homeDir}, t, nil) + assert.Contains(t, outErr, "Error: File "+keyFile+" already exists\nHint: Use -f to overwrite it\n") assert.Contains(t, out, "This is your public key") } diff --git a/client/go/cmd/cert_test.go b/client/go/cmd/cert_test.go index cd5f88764b9..96b626b5c98 100644 --- a/client/go/cmd/cert_test.go +++ b/client/go/cmd/cert_test.go @@ -28,8 +28,8 @@ func TestCert(t *testing.T) { assert.Equal(t, fmt.Sprintf("Success: Certificate written to %s\nSuccess: Certificate written to %s\nSuccess: Private key written to %s\n", pkgCertificate, certificate, privateKey), out) - out, _ = execute(command{args: []string{"cert", "-a", "t1.a1.i1", pkgDir}, homeDir: homeDir}, t, nil) - assert.Contains(t, out, fmt.Sprintf("Error: Application package %s already contains a certificate", appDir)) + _, outErr := execute(command{args: []string{"cert", "-a", "t1.a1.i1", pkgDir}, homeDir: homeDir}, t, nil) + assert.Contains(t, outErr, fmt.Sprintf("Error: Application package %s already contains a certificate", appDir)) } func TestCertCompressedPackage(t *testing.T) { @@ -41,13 +41,13 @@ func TestCertCompressedPackage(t *testing.T) { _, err = os.Create(zipFile) assert.Nil(t, err) - out, _ := execute(command{args: []string{"cert", "-a", "t1.a1.i1", pkgDir}, homeDir: homeDir}, t, nil) - assert.Contains(t, out, "Error: Cannot add certificate to compressed application package") + _, outErr := execute(command{args: []string{"cert", "-a", "t1.a1.i1", pkgDir}, homeDir: homeDir}, t, nil) + assert.Contains(t, outErr, "Error: Cannot add certificate to compressed application package") err = os.Remove(zipFile) assert.Nil(t, err) - out, _ = execute(command{args: []string{"cert", "-f", "-a", "t1.a1.i1", pkgDir}, homeDir: homeDir}, t, nil) + out, _ := execute(command{args: []string{"cert", "-f", "-a", "t1.a1.i1", pkgDir}, homeDir: homeDir}, t, nil) assert.Contains(t, out, "Success: Certificate written to") assert.Contains(t, out, "Success: Private key written to") } diff --git a/client/go/cmd/command_tester.go b/client/go/cmd/command_tester.go index 6929b59decb..8eaf6be2c22 100644 --- a/client/go/cmd/command_tester.go +++ b/client/go/cmd/command_tester.go @@ -27,6 +27,18 @@ type command struct { moreArgs []string } +func resetFlag(f *pflag.Flag) { + switch v := f.Value.(type) { + case pflag.SliceValue: + _ = v.Replace([]string{}) + default: + switch v.Type() { + case "bool", "string", "int": + _ = v.Set(f.DefValue) + } + } +} + func execute(cmd command, t *testing.T, client *mockHttpClient) (string, string) { if client != nil { util.ActiveHttpClient = client @@ -44,17 +56,8 @@ func execute(cmd command, t *testing.T, client *mockHttpClient) (string, string) os.Setenv("VESPA_CLI_CACHE_DIR", cmd.cacheDir) // Reset flags to their default value - persistent flags in Cobra persists over tests - rootCmd.Flags().VisitAll(func(f *pflag.Flag) { - switch v := f.Value.(type) { - case pflag.SliceValue: - _ = v.Replace([]string{}) - default: - switch v.Type() { - case "bool", "string", "int": - _ = v.Set(f.DefValue) - } - } - }) + rootCmd.Flags().VisitAll(resetFlag) + documentCmd.Flags().VisitAll(resetFlag) // Do not exit in tests exitFunc = func(code int) {} diff --git a/client/go/cmd/config_test.go b/client/go/cmd/config_test.go index 223c8a396c3..7f7bdbb3edd 100644 --- a/client/go/cmd/config_test.go +++ b/client/go/cmd/config_test.go @@ -10,7 +10,7 @@ import ( func TestConfig(t *testing.T) { homeDir := filepath.Join(t.TempDir(), ".vespa") - assertConfigCommand(t, "invalid option or value: \"foo\": \"bar\"\n", homeDir, "config", "set", "foo", "bar") + assertConfigCommandErr(t, "invalid option or value: \"foo\": \"bar\"\n", homeDir, "config", "set", "foo", "bar") assertConfigCommand(t, "foo = \n", homeDir, "config", "get", "foo") assertConfigCommand(t, "target = local\n", homeDir, "config", "get", "target") assertConfigCommand(t, "", homeDir, "config", "set", "target", "cloud") @@ -19,7 +19,7 @@ func TestConfig(t *testing.T) { assertConfigCommand(t, "", homeDir, "config", "set", "target", "https://127.0.0.1") assertConfigCommand(t, "target = https://127.0.0.1\n", homeDir, "config", "get", "target") - assertConfigCommand(t, "invalid application: \"foo\"\n", homeDir, "config", "set", "application", "foo") + assertConfigCommandErr(t, "invalid application: \"foo\"\n", homeDir, "config", "set", "application", "foo") assertConfigCommand(t, "application = \n", homeDir, "config", "get", "application") assertConfigCommand(t, "", homeDir, "config", "set", "application", "t1.a1.i1") assertConfigCommand(t, "application = t1.a1.i1\n", homeDir, "config", "get", "application") @@ -27,7 +27,7 @@ func TestConfig(t *testing.T) { assertConfigCommand(t, "application = t1.a1.i1\ncolor = auto\ntarget = https://127.0.0.1\nwait = 0\n", homeDir, "config", "get") assertConfigCommand(t, "", homeDir, "config", "set", "wait", "60") - assertConfigCommand(t, "wait option must be an integer >= 0, got \"foo\"\n", homeDir, "config", "set", "wait", "foo") + assertConfigCommandErr(t, "wait option must be an integer >= 0, got \"foo\"\n", homeDir, "config", "set", "wait", "foo") assertConfigCommand(t, "wait = 60\n", homeDir, "config", "get", "wait") } @@ -35,3 +35,8 @@ func assertConfigCommand(t *testing.T, expected, homeDir string, args ...string) out, _ := execute(command{homeDir: homeDir, args: args}, t, nil) assert.Equal(t, expected, out) } + +func assertConfigCommandErr(t *testing.T, expected, homeDir string, args ...string) { + _, outErr := execute(command{homeDir: homeDir, args: args}, t, nil) + assert.Equal(t, expected, outErr) +} diff --git a/client/go/cmd/deploy_test.go b/client/go/cmd/deploy_test.go index 9614806b968..5bb45e70fad 100644 --- a/client/go/cmd/deploy_test.go +++ b/client/go/cmd/deploy_test.go @@ -61,9 +61,10 @@ func TestDeployApplicationDirectoryWithPomAndTarget(t *testing.T) { func TestDeployApplicationDirectoryWithPomAndEmptyTarget(t *testing.T) { client := &mockHttpClient{} + _, outErr := execute(command{args: []string{"deploy", "testdata/applications/withEmptyTarget"}}, t, client) assert.Equal(t, "Error: pom.xml exists but no target/application.zip. Run mvn package first\n", - executeCommand(t, client, []string{"deploy", "testdata/applications/withEmptyTarget"}, []string{})) + outErr) } func TestDeployApplicationPackageErrorWithUnexpectedNonJson(t *testing.T) { @@ -85,7 +86,7 @@ func TestDeployApplicationPackageErrorWithExpectedFormat(t *testing.T) { "Invalid XML, error in services.xml:\nelement \"nosuch\" not allowed here", `{ "error-code": "INVALID_APPLICATION_PACKAGE", - "message": "Invalid XML, error in services.xml: element \"nosuch\" not allowed here\n" + "message": "Invalid XML, error in services.xml: element \"nosuch\" not allowed here" }`) } @@ -94,7 +95,7 @@ func TestPrepareApplicationPackageErrorWithExpectedFormat(t *testing.T) { "Invalid XML, error in services.xml:\nelement \"nosuch\" not allowed here", `{ "error-code": "INVALID_APPLICATION_PACKAGE", - "message": "Invalid XML, error in services.xml: element \"nosuch\" not allowed here\n" + "message": "Invalid XML, error in services.xml: element \"nosuch\" not allowed here" }`) } @@ -158,18 +159,20 @@ func assertDeployRequestMade(target string, client *mockHttpClient, t *testing.T assertPackageUpload(-1, target+"/application/v2/tenant/default/prepareandactivate", client, t) } -func assertApplicationPackageError(t *testing.T, command string, status int, expectedMessage string, returnBody string) { +func assertApplicationPackageError(t *testing.T, cmd string, status int, expectedMessage string, returnBody string) { client := &mockHttpClient{} client.NextResponse(status, returnBody) + _, outErr := execute(command{args: []string{cmd, "testdata/applications/withTarget/target/application.zip"}}, t, client) assert.Equal(t, "Error: Invalid application package (Status "+strconv.Itoa(status)+")\n\n"+expectedMessage+"\n", - executeCommand(t, client, []string{command, "testdata/applications/withTarget/target/application.zip"}, []string{})) + outErr) } func assertDeployServerError(t *testing.T, status int, errorMessage string) { client := &mockHttpClient{} client.NextResponse(status, errorMessage) + _, outErr := execute(command{args: []string{"deploy", "testdata/applications/withTarget/target/application.zip"}}, t, client) assert.Equal(t, "Error: Error from deploy service at 127.0.0.1:19071 (Status "+strconv.Itoa(status)+"):\n"+errorMessage+"\n", - executeCommand(t, client, []string{"deploy", "testdata/applications/withTarget/target/application.zip"}, []string{})) + outErr) } diff --git a/client/go/cmd/document.go b/client/go/cmd/document.go index cc5fb948e3b..7f2c9b8a916 100644 --- a/client/go/cmd/document.go +++ b/client/go/cmd/document.go @@ -5,9 +5,9 @@ package cmd import ( + "fmt" "io" "io/ioutil" - "log" "strings" "github.com/spf13/cobra" @@ -123,20 +123,25 @@ func curlOutput() io.Writer { } func printResult(result util.OperationResult, payloadOnlyOnSuccess bool) { + out := stdout if !result.Success { - log.Print(color.Red("Error: "), result.Message) + out = stderr + } + + if !result.Success { + fmt.Fprintln(out, color.Red("Error:"), result.Message) } else if !(payloadOnlyOnSuccess && result.Payload != "") { - log.Print(color.Green("Success: "), result.Message) + fmt.Fprintln(out, color.Green("Success:"), result.Message) } if result.Detail != "" { - log.Print(color.Yellow(result.Detail)) + fmt.Fprintln(out, color.Yellow(result.Detail)) } if result.Payload != "" { if !payloadOnlyOnSuccess { - log.Println("") + fmt.Fprintln(out) } - log.Print(result.Payload) + fmt.Fprintln(out, result.Payload) } } diff --git a/client/go/cmd/document_test.go b/client/go/cmd/document_test.go index 1f82b85f915..649aca8703a 100644 --- a/client/go/cmd/document_test.go +++ b/client/go/cmd/document_test.go @@ -67,17 +67,19 @@ func TestDocumentRemoveWithoutIdArg(t *testing.T) { func TestDocumentSendMissingId(t *testing.T) { arguments := []string{"document", "put", "testdata/A-Head-Full-of-Dreams-Without-Operation.json"} client := &mockHttpClient{} + _, outErr := execute(command{args: arguments}, t, client) assert.Equal(t, "Error: No document id given neither as argument or as a 'put' key in the json file\n", - executeCommand(t, client, arguments, []string{})) + outErr) } func TestDocumentSendWithDisagreeingOperations(t *testing.T) { arguments := []string{"document", "update", "testdata/A-Head-Full-of-Dreams-Put.json"} client := &mockHttpClient{} + _, outErr := execute(command{args: arguments}, t, client) assert.Equal(t, "Error: Wanted document operation is update but the JSON file specifies put\n", - executeCommand(t, client, arguments, []string{})) + outErr) } func TestDocumentPutDocumentError(t *testing.T) { @@ -139,21 +141,23 @@ func assertDocumentGet(arguments []string, documentId string, t *testing.T) { func assertDocumentError(t *testing.T, status int, errorMessage string) { client := &mockHttpClient{} client.NextResponse(status, errorMessage) + _, outErr := execute(command{args: []string{"document", "put", + "id:mynamespace:music::a-head-full-of-dreams", + "testdata/A-Head-Full-of-Dreams-Put.json"}}, t, client) assert.Equal(t, "Error: Invalid document operation: Status "+strconv.Itoa(status)+"\n\n"+errorMessage+"\n", - executeCommand(t, client, []string{"document", "put", - "id:mynamespace:music::a-head-full-of-dreams", - "testdata/A-Head-Full-of-Dreams-Put.json"}, []string{})) + outErr) } func assertDocumentServerError(t *testing.T, status int, errorMessage string) { client := &mockHttpClient{} client.NextResponse(status, errorMessage) + _, outErr := execute(command{args: []string{"document", "put", + "id:mynamespace:music::a-head-full-of-dreams", + "testdata/A-Head-Full-of-Dreams-Put.json"}}, t, client) assert.Equal(t, "Error: Container (document API) at 127.0.0.1:8080: Status "+strconv.Itoa(status)+"\n\n"+errorMessage+"\n", - executeCommand(t, client, []string{"document", "put", - "id:mynamespace:music::a-head-full-of-dreams", - "testdata/A-Head-Full-of-Dreams-Put.json"}, []string{})) + outErr) } func documentServiceURL(client *mockHttpClient) string { diff --git a/client/go/cmd/helpers.go b/client/go/cmd/helpers.go index 98d6814d16f..b5525cf11fe 100644 --- a/client/go/cmd/helpers.go +++ b/client/go/cmd/helpers.go @@ -32,16 +32,16 @@ func fatalErr(err error, msg ...interface{}) { func printErrHint(err error, hints ...string) { printErr(nil, err.Error()) for _, hint := range hints { - log.Print(color.Cyan("Hint: "), hint) + fmt.Fprintln(stderr, color.Cyan("Hint:"), hint) } } func printErr(err error, msg ...interface{}) { if len(msg) > 0 { - log.Print(color.Red("Error: "), fmt.Sprint(msg...)) + fmt.Fprintln(stderr, color.Red("Error:"), fmt.Sprint(msg...)) } if err != nil { - log.Print(color.Yellow(err)) + fmt.Fprintln(stderr, color.Yellow(err)) } } @@ -215,11 +215,9 @@ func waitForService(service string, sessionOrRunID int64) { if status/100 == 2 { log.Print(s.Description(), " at ", color.Cyan(s.BaseURL), " is ", color.Green("ready")) } else { - log.Print(s.Description(), " at ", color.Cyan(s.BaseURL), " is ", color.Red("not ready")) if err == nil { - log.Print(color.Yellow(fmt.Sprintf("Status %d", status))) - } else { - log.Print(color.Yellow(err)) + err = fmt.Errorf("Status %d", status) } + fatalErr(err, s.Description(), " at ", color.Cyan(s.BaseURL), " is ", color.Red("not ready")) } } diff --git a/client/go/cmd/log_test.go b/client/go/cmd/log_test.go index 7808d429e4f..f239bebc488 100644 --- a/client/go/cmd/log_test.go +++ b/client/go/cmd/log_test.go @@ -23,6 +23,6 @@ func TestLog(t *testing.T) { expected := "[2021-09-27 10:31:30.905535] host1a.dev.aws-us-east-1c info logserver-container Container.com.yahoo.container.jdisc.ConfiguredApplication Switching to the latest deployed set of configurations and components. Application config generation: 52532\n" assert.Equal(t, expected, out) - out, _ = execute(command{homeDir: homeDir, args: []string{"log", "--from", "2021-09-27T13:12:49Z", "--to", "2021-09-27T13:15:00", "1h"}}, t, httpClient) - assert.Equal(t, "Error: Invalid period\ncannot combine --from/--to with relative value: 1h\n", out) + _, errOut := execute(command{homeDir: homeDir, args: []string{"log", "--from", "2021-09-27T13:12:49Z", "--to", "2021-09-27T13:15:00", "1h"}}, t, httpClient) + assert.Equal(t, "Error: Invalid period\ncannot combine --from/--to with relative value: 1h\n", errOut) } diff --git a/client/go/cmd/query.go b/client/go/cmd/query.go index 7de0addb9b8..5e2b268865d 100644 --- a/client/go/cmd/query.go +++ b/client/go/cmd/query.go @@ -47,7 +47,7 @@ func query(arguments []string) { response, err := service.Do(&http.Request{URL: url}, time.Second*10) if err != nil { - log.Print(color.Red("Error: "), "Request failed: ", err) + printErr(nil, "Request failed: ", err) return } defer response.Body.Close() @@ -55,11 +55,9 @@ func query(arguments []string) { if response.StatusCode == 200 { log.Print(util.ReaderToJSON(response.Body)) } else if response.StatusCode/100 == 4 { - log.Print(color.Red("Error: "), "Invalid query: ", response.Status, "\n") - log.Print(util.ReaderToJSON(response.Body)) + printErr(nil, "Invalid query: ", response.Status, "\n", util.ReaderToJSON(response.Body)) } else { - log.Print(color.Red("Error: "), response.Status, " from container at ", color.Cyan(url.Host), "\n") - log.Print(util.ReaderToJSON(response.Body)) + printErr(nil, response.Status, " from container at ", color.Cyan(url.Host), "\n", util.ReaderToJSON(response.Body)) } } diff --git a/client/go/cmd/query_test.go b/client/go/cmd/query_test.go index b4ae63d777f..81dc03766be 100644 --- a/client/go/cmd/query_test.go +++ b/client/go/cmd/query_test.go @@ -57,18 +57,20 @@ func assertQuery(t *testing.T, expectedQuery string, query ...string) { func assertQueryError(t *testing.T, status int, errorMessage string) { client := &mockHttpClient{} client.NextResponse(status, errorMessage) + _, outErr := execute(command{args: []string{"query", "yql=select from sources * where title contains 'foo'"}}, t, client) assert.Equal(t, "Error: Invalid query: Status "+strconv.Itoa(status)+"\n"+errorMessage+"\n", - executeCommand(t, client, []string{"query"}, []string{"yql=select from sources * where title contains 'foo'"}), + outErr, "error output") } func assertQueryServiceError(t *testing.T, status int, errorMessage string) { client := &mockHttpClient{} client.NextResponse(status, errorMessage) + _, outErr := execute(command{args: []string{"query", "yql=select from sources * where title contains 'foo'"}}, t, client) assert.Equal(t, "Error: Status "+strconv.Itoa(status)+" from container at 127.0.0.1:8080\n"+errorMessage+"\n", - executeCommand(t, client, []string{"query"}, []string{"yql=select from sources * where title contains 'foo'"}), + outErr, "error output") } diff --git a/client/go/cmd/status_test.go b/client/go/cmd/status_test.go index fc2ba3d4586..757ef5f3b06 100644 --- a/client/go/cmd/status_test.go +++ b/client/go/cmd/status_test.go @@ -78,8 +78,11 @@ func assertDocumentStatus(target string, args []string, t *testing.T) { func assertQueryStatusError(target string, args []string, t *testing.T) { client := &mockHttpClient{} client.NextStatus(500) + cmd := []string{"status", "container"} + cmd = append(cmd, args...) + _, outErr := execute(command{args: cmd}, t, client) assert.Equal(t, - "Container (query API) at "+target+" is not ready\nStatus 500\n", - executeCommand(t, client, []string{"status", "container"}, args), + "Error: Container (query API) at "+target+" is not ready\nStatus 500\n", + outErr, "vespa status container") } -- cgit v1.2.3