Improve test output

6 files changed, 157 insertions, 71 deletions

diff --git a/client/go/cmd/test.go b/client/go/cmd/test.go
index e9d5e571845..c2aaab68a0c 100644
--- a/client/go/cmd/test.go
+++ b/client/go/cmd/test.go
@@ -119,7 +119,7 @@ func runTest(testPath string, target vespa.Target, dryRun bool) string {
 		testName = filepath.Base(testPath)
 	}
 	if !dryRun {
-		fmt.Fprintf(stdout, "Running %s:", color.Cyan(testName))
+		fmt.Fprintf(stdout, "%s:", testName)
 	}
 
 	defaultParameters, err := getParameters(test.Defaults.ParametersRaw, path.Dir(testPath))
@@ -133,9 +133,9 @@ func runTest(testPath string, target vespa.Target, dryRun bool) string {
 		fatalErrHint(fmt.Errorf("a test must have at least one step, but none were found in %s", testPath), "See https://cloud.vespa.ai/en/reference/testing")
 	}
 	for i, step := range test.Steps {
-		stepName := step.Name
-		if stepName == "" {
-			stepName = fmt.Sprintf("step %d", i+1)
+		stepName := fmt.Sprintf("Step %d", i+1)
+		if step.Name != "" {
+			stepName = fmt.Sprintf("Step: %s", step.Name)
 		}
 		failure, longFailure, err := verify(step, path.Dir(testPath), test.Defaults.Cluster, defaultParameters, target, dryRun)
 		if err != nil {
@@ -144,13 +144,13 @@ func runTest(testPath string, target vespa.Target, dryRun bool) string {
 		}
 		if !dryRun {
 			if failure != "" {
-				fmt.Fprintf(stdout, " %s %s:\n%s\n", color.Red("Failed"), color.Cyan(stepName), longFailure)
+				fmt.Fprintf(stdout, " %s\n%s:\n%s\n", color.Red("failed"), stepName, longFailure)
 				return fmt.Sprintf("%s: %s: %s", testName, stepName, failure)
 			}
 			if i == 0 {
 				fmt.Fprintf(stdout, " ")
 			}
-			fmt.Fprint(stdout, color.Green("."))
+			fmt.Fprint(stdout, ".")
 		}
 	}
 	if !dryRun {
@@ -204,7 +204,7 @@ func verify(step step, testsPath string, defaultCluster string, defaultParameter
 	}
 	externalEndpoint := requestUrl.IsAbs()
 	if !externalEndpoint {
-		baseURL := "http://dummy/"
+		baseURL := ""
 		if service != nil {
 			baseURL = service.BaseURL
 		}
@@ -264,8 +264,13 @@ func verify(step step, testsPath string, defaultCluster string, defaultParameter
 	defer response.Body.Close()
 
 	if statusCode != response.StatusCode {
-		failure := fmt.Sprintf("Unexpected %s: %s", "status code", color.Red(response.StatusCode))
-		return failure, fmt.Sprintf("%s\nExpected: %s\nActual response:\n%s", failure, color.Cyan(statusCode), util.ReaderToJSON(response.Body)), nil
+		return fmt.Sprintf("Unexpected status code: %d", color.Red(response.StatusCode)),
+			fmt.Sprintf("Unexpected status code\nExpected: %d\nActual:   %d\nRequested: %s at %s\nResponse:\n%s",
+				color.Cyan(statusCode),
+				color.Red(response.StatusCode),
+				color.Cyan(method),
+				color.Cyan(requestUrl),
+				util.ReaderToJSON(response.Body)), nil
 	}
 
 	if responseBodySpec == nil {
@@ -282,20 +287,24 @@ func verify(step step, testsPath string, defaultCluster string, defaultParameter
 		return "", "", fmt.Errorf("got non-JSON response; %w:\n%s", err, string(responseBodyBytes))
 	}
 
-	failure, expected, err := compare(responseBodySpec, responseBody, "")
+	failure, expected, actual, err := compare(responseBodySpec, responseBody, "")
 	if failure != "" {
 		responsePretty, _ := json.MarshalIndent(responseBody, "", "  ")
 		longFailure := failure
 		if expected != "" {
-			longFailure += "\n" + expected
+			longFailure += "\nExpected: " + expected
 		}
-		longFailure += "\nActual response:\n" + string(responsePretty)
+		if actual != "" {
+			failure += ": " + actual
+			longFailure += "\nActual:   " + actual
+		}
+		longFailure += fmt.Sprintf("\nRequested: %s at %s\nResponse:\n%s", color.Cyan(method), color.Cyan(requestUrl), string(responsePretty))
 		return failure, longFailure, err
 	}
 	return "", "", err
 }
 
-func compare(expected interface{}, actual interface{}, path string) (string, string, error) {
+func compare(expected interface{}, actual interface{}, path string) (string, string, string, error) {
 	typeMatch := false
 	valueMatch := false
 	switch u := expected.(type) {
@@ -320,18 +329,15 @@ func compare(expected interface{}, actual interface{}, path string) (string, str
 		if ok {
 			if len(u) == len(v) {
 				for i, e := range u {
-					failure, expected, err := compare(e, v[i], fmt.Sprintf("%s/%d", path, i))
-					if failure != "" || err != nil {
-						return failure, expected, err
+					if failure, expected, actual, err := compare(e, v[i], fmt.Sprintf("%s/%d", path, i)); failure != "" || err != nil {
+						return failure, expected, actual, err
 					}
 				}
 				valueMatch = true
 			} else {
-				return fmt.Sprintf("Unexpected %s at %s: %d",
-						"number of elements",
-						color.Cyan(path),
-						color.Red(len(v))),
-					fmt.Sprintf("Expected: %d", color.Cyan(len(u))),
+				return fmt.Sprintf("Unexpected number of elements at %s", color.Cyan(path)),
+					fmt.Sprintf("%d", color.Cyan(len(u))),
+					fmt.Sprintf("%d", color.Red(len(v))),
 					nil
 			}
 		}
@@ -343,17 +349,16 @@ func compare(expected interface{}, actual interface{}, path string) (string, str
 				childPath := fmt.Sprintf("%s/%s", path, strings.ReplaceAll(strings.ReplaceAll(n, "~", "~0"), "/", "~1"))
 				f, ok := v[n]
 				if !ok {
-					return fmt.Sprintf("Missing expected field at %s", color.Red(childPath)), "", nil
+					return fmt.Sprintf("Missing expected field at %s", color.Red(childPath)), "", "", nil
 				}
-				failure, expected, err := compare(e, f, childPath)
-				if failure != "" || err != nil {
-					return failure, expected, err
+				if failure, expected, actual, err := compare(e, f, childPath); failure != "" || err != nil {
+					return failure, expected, actual, err
 				}
 			}
 			valueMatch = true
 		}
 	default:
-		return "", "", fmt.Errorf("unexpected JSON type for value '%v'", expected)
+		return "", "", "", fmt.Errorf("unexpected JSON type for value '%v'", expected)
 	}
 
 	if !valueMatch {
@@ -366,14 +371,12 @@ func compare(expected interface{}, actual interface{}, path string) (string, str
 		}
 		expectedJson, _ := json.Marshal(expected)
 		actualJson, _ := json.Marshal(actual)
-		return fmt.Sprintf("Unexpected %s at %s: %s",
-				mismatched,
-				color.Cyan(path),
-				color.Red(actualJson)),
-			fmt.Sprintf("Expected: %s", color.Cyan(expectedJson)),
+		return fmt.Sprintf("Unexpected %s at %s", mismatched, color.Cyan(path)),
+			fmt.Sprintf("%s", color.Cyan(expectedJson)),
+			fmt.Sprintf("%s", color.Red(actualJson)),
 			nil
 	}
-	return "", "", nil
+	return "", "", "", nil
 }
 
 func getParameters(parametersRaw []byte, testsPath string) (map[string]string, error) {
diff --git a/client/go/cmd/test_test.go b/client/go/cmd/test_test.go
index 4c5e4c3f1e5..10a62e0495d 100644
--- a/client/go/cmd/test_test.go
+++ b/client/go/cmd/test_test.go
@@ -5,6 +5,7 @@
 package cmd
 
 import (
+	"fmt"
 	"github.com/vespa-engine/vespa/client/go/util"
 	"github.com/vespa-engine/vespa/client/go/vespa"
 	"io/ioutil"
@@ -23,29 +24,32 @@ func TestSuite(t *testing.T) {
 	searchResponse, _ := ioutil.ReadFile("testdata/tests/response.json")
 	client.NextStatus(200)
 	client.NextStatus(200)
-	for i := 0; i < 10; i++ {
+	for i := 0; i < 11; i++ {
 		client.NextResponse(200, string(searchResponse))
 	}
 
 	expectedBytes, _ := ioutil.ReadFile("testdata/tests/expected-suite.out")
 	outBytes, errBytes := execute(command{args: []string{"test", "testdata/tests/system-test"}}, t, client)
-	assert.Equal(t, string(expectedBytes), outBytes)
-	assert.Equal(t, "", errBytes)
 
 	baseUrl := "http://127.0.0.1:8080"
 	urlWithQuery := baseUrl + "/search/?presentation.timing=true&query=artist%3A+foo&timeout=3.4s"
 	requests := []*http.Request{createFeedRequest(baseUrl), createFeedRequest(baseUrl), createSearchRequest(urlWithQuery), createSearchRequest(urlWithQuery)}
-	for i := 0; i < 8; i++ {
+	requests = append(requests, createSearchRequest(baseUrl+"/search/"))
+	requests = append(requests, createSearchRequest(baseUrl+"/search/?foo=%2F"))
+	for i := 0; i < 7; i++ {
 		requests = append(requests, createSearchRequest(baseUrl+"/search/"))
 	}
 	assertRequests(requests, client, t)
+	fmt.Println(outBytes)
+	assert.Equal(t, string(expectedBytes), outBytes)
+	assert.Equal(t, "", errBytes)
 }
 
 func TestProductionTest(t *testing.T) {
 	client := &mockHttpClient{}
 	client.NextStatus(200)
 	outBytes, errBytes := execute(command{args: []string{"test", "testdata/tests/production-test/external.json"}}, t, client)
-	assert.Equal(t, "Running external.json: . OK\n\nSuccess: 1 test OK\n", outBytes)
+	assert.Equal(t, "external.json: . OK\n\nSuccess: 1 test OK\n", outBytes)
 	assert.Equal(t, "", errBytes)
 	assertRequests([]*http.Request{createRequest("GET", "https://my.service:123/path?query=wohoo", "")}, client, t)
 }
diff --git a/client/go/cmd/testdata/tests/expected-suite.out b/client/go/cmd/testdata/tests/expected-suite.out
index 963889b8019..df916f50a95 100644
--- a/client/go/cmd/testdata/tests/expected-suite.out
+++ b/client/go/cmd/testdata/tests/expected-suite.out
@@ -1,8 +1,11 @@
-Running my test: .... OK
-Running wrong-bool-value.json: Failed step 1:
-Unexpected value at /root/coverage/full: true
+My test: .... OK
+wrong-bool-value.json: failed
+Step 1:
+Unexpected value at /root/coverage/full
 Expected: false
-Actual response:
+Actual:   true
+Requested: GET at http://127.0.0.1:8080/search/
+Response:
 {
   "root": {
     "children": [
@@ -38,10 +41,55 @@ Actual response:
   }
 }
 
-Running wrong-element-count.json: Failed step 1:
-Unexpected number of elements at /root/children: 1
+wrong-code.json: failed
+Step 1:
+Unexpected status code
+Expected: 123
+Actual:   200
+Requested: GET at http://127.0.0.1:8080/search/?foo=%2F
+Response:
+{
+    "root": {
+        "children": [
+            {
+                "fields": {
+                    "artist": "Foo Fighters",
+                    "documentid": "id:test:music::doc",
+                    "sddocname": "music"
+                },
+                "id": "id:test:music::doc",
+                "relevance": 0.38186238359951247,
+                "source": "music"
+            }
+        ],
+        "coverage": {
+            "coverage": 100,
+            "documents": 1,
+            "full": true,
+            "nodes": 1,
+            "results": 1,
+            "resultsFull": 1
+        },
+        "fields": {
+            "totalCount": 1
+        },
+        "id": "toplevel",
+        "relevance": 1
+    },
+    "timing": {
+        "querytime": 0.003,
+        "searchtime": 0.004,
+        "summaryfetchtime": 0
+    }
+}
+
+wrong-element-count.json: failed
+Step 1:
+Unexpected number of elements at /root/children
 Expected: 0
-Actual response:
+Actual:   1
+Requested: GET at http://127.0.0.1:8080/search/
+Response:
 {
   "root": {
     "children": [
@@ -77,9 +125,11 @@ Actual response:
   }
 }
 
-Running wrong-field-name.json: Failed step 1:
+wrong-field-name.json: failed
+Step 1:
 Missing expected field at /root/fields/totalCountDracula
-Actual response:
+Requested: GET at http://127.0.0.1:8080/search/
+Response:
 {
   "root": {
     "children": [
@@ -115,10 +165,13 @@ Actual response:
   }
 }
 
-Running wrong-float-value.json: Failed step 1:
-Unexpected value at /root/children/0/relevance: 0.38186238359951247
+wrong-float-value.json: failed
+Step 1:
+Unexpected value at /root/children/0/relevance
 Expected: 0.381862373599
-Actual response:
+Actual:   0.38186238359951247
+Requested: GET at http://127.0.0.1:8080/search/
+Response:
 {
   "root": {
     "children": [
@@ -154,10 +207,13 @@ Actual response:
   }
 }
 
-Running wrong-int-value.json: Failed step 1:
-Unexpected value at /root/fields/totalCount: 1
+wrong-int-value.json: failed
+Step 1:
+Unexpected value at /root/fields/totalCount
 Expected: 2
-Actual response:
+Actual:   1
+Requested: GET at http://127.0.0.1:8080/search/
+Response:
 {
   "root": {
     "children": [
@@ -193,9 +249,11 @@ Actual response:
   }
 }
 
-Running wrong-null-value.json: Failed step 1:
+wrong-null-value.json: failed
+Step 1:
 Missing expected field at /boot
-Actual response:
+Requested: GET at http://127.0.0.1:8080/search/
+Response:
 {
   "root": {
     "children": [
@@ -231,10 +289,13 @@ Actual response:
   }
 }
 
-Running wrong-string-value.json: Failed step 1:
-Unexpected value at /root/children/0/fields/artist: "Foo Fighters"
+wrong-string-value.json: failed
+Step 1:
+Unexpected value at /root/children/0/fields/artist
 Expected: "Boo Fighters"
-Actual response:
+Actual:   "Foo Fighters"
+Requested: GET at http://127.0.0.1:8080/search/
+Response:
 {
   "root": {
     "children": [
@@ -270,10 +331,13 @@ Actual response:
   }
 }
 
-Running wrong-type.json: Failed step 1:
-Unexpected type at /root/fields/totalCount: 1
+wrong-type.json: failed
+Step 1:
+Unexpected type at /root/fields/totalCount
 Expected: "1"
-Actual response:
+Actual:   1
+Requested: GET at http://127.0.0.1:8080/search/
+Response:
 {
   "root": {
     "children": [
@@ -309,12 +373,13 @@ Actual response:
   }
 }
 
-Failure: 8 of 9 tests failed:
-wrong-bool-value.json: step 1: Unexpected value at /root/coverage/full: true
-wrong-element-count.json: step 1: Unexpected number of elements at /root/children: 1
-wrong-field-name.json: step 1: Missing expected field at /root/fields/totalCountDracula
-wrong-float-value.json: step 1: Unexpected value at /root/children/0/relevance: 0.38186238359951247
-wrong-int-value.json: step 1: Unexpected value at /root/fields/totalCount: 1
-wrong-null-value.json: step 1: Missing expected field at /boot
-wrong-string-value.json: step 1: Unexpected value at /root/children/0/fields/artist: "Foo Fighters"
-wrong-type.json: step 1: Unexpected type at /root/fields/totalCount: 1
+Failure: 9 of 10 tests failed:
+wrong-bool-value.json: Step 1: Unexpected value at /root/coverage/full: true
+wrong-code.json: Step 1: Unexpected status code: 200
+wrong-element-count.json: Step 1: Unexpected number of elements at /root/children: 1
+wrong-field-name.json: Step 1: Missing expected field at /root/fields/totalCountDracula
+wrong-float-value.json: Step 1: Unexpected value at /root/children/0/relevance: 0.38186238359951247
+wrong-int-value.json: Step 1: Unexpected value at /root/fields/totalCount: 1
+wrong-null-value.json: Step 1: Missing expected field at /boot
+wrong-string-value.json: Step 1: Unexpected value at /root/children/0/fields/artist: "Foo Fighters"
+wrong-type.json: Step 1: Unexpected type at /root/fields/totalCount: 1
diff --git a/client/go/cmd/testdata/tests/expected.out b/client/go/cmd/testdata/tests/expected.out
index 084fb10f72a..2ca35fe6a37 100644
--- a/client/go/cmd/testdata/tests/expected.out
+++ b/client/go/cmd/testdata/tests/expected.out
@@ -1,3 +1,3 @@
-Running my test: .... OK
+My test: .... OK
 
 Success: 1 test OK
diff --git a/client/go/cmd/testdata/tests/system-test/test.json b/client/go/cmd/testdata/tests/system-test/test.json
index f53df929dbd..2e327b5e5df 100644
--- a/client/go/cmd/testdata/tests/system-test/test.json
+++ b/client/go/cmd/testdata/tests/system-test/test.json
@@ -1,5 +1,5 @@
 {
-    "name": "my test",
+    "name": "My test",
     "defaults": {
         "cluster": "container",
         "parameters": {
diff --git a/client/go/cmd/testdata/tests/system-test/wrong-code.json b/client/go/cmd/testdata/tests/system-test/wrong-code.json
new file mode 100644
index 00000000000..c325054faa1
--- /dev/null
+++ b/client/go/cmd/testdata/tests/system-test/wrong-code.json
@@ -0,0 +1,14 @@
+{
+    "steps": [
+        {
+            "request": {
+                "parameters": {
+                    "foo": "/"
+                }
+            },
+            "response": {
+                "code": 123
+            }
+        }
+    ]
+}