Merge "Increase testing of request_type"

android/bazel_handler.go

@@ -317,7 +317,7 @@ func (bazelCtx *bazelContext) GetPythonBinary(label string, cfgKey configKey) (s
 func (bazelCtx *bazelContext) GetApexInfo(label string, cfgKey configKey) (cquery.ApexInfo, error) {
 	key := makeCqueryKey(label, cquery.GetApexInfo, cfgKey)
 	if rawString, ok := bazelCtx.results[key]; ok {
-		return cquery.GetApexInfo.ParseResult(strings.TrimSpace(rawString)), nil
+		return cquery.GetApexInfo.ParseResult(strings.TrimSpace(rawString))
 	}
 	return cquery.ApexInfo{}, fmt.Errorf("no bazel response found for %v", key)
 }
@@ -325,7 +325,7 @@ func (bazelCtx *bazelContext) GetApexInfo(label string, cfgKey configKey) (cquer
 func (bazelCtx *bazelContext) GetCcUnstrippedInfo(label string, cfgKey configKey) (cquery.CcUnstrippedInfo, error) {
 	key := makeCqueryKey(label, cquery.GetCcUnstrippedInfo, cfgKey)
 	if rawString, ok := bazelCtx.results[key]; ok {
-		return cquery.GetCcUnstrippedInfo.ParseResult(strings.TrimSpace(rawString)), nil
+		return cquery.GetCcUnstrippedInfo.ParseResult(strings.TrimSpace(rawString))
 	}
 	return cquery.CcUnstrippedInfo{}, fmt.Errorf("no bazel response for %s", key)
 }

bazel/cquery/request_type.go

@@ -194,7 +194,9 @@ return json_encode({
 // Starlark given in StarlarkFunctionBody.
 func (g getCcInfoType) ParseResult(rawString string) (CcInfo, error) {
 	var ccInfo CcInfo
-	parseJson(rawString, &ccInfo)
+	if err := parseJson(rawString, &ccInfo); err != nil {
+		return ccInfo, err
+	}
 	return ccInfo, nil
 }
 
@@ -250,10 +252,10 @@ type ApexInfo struct {
 // ParseResult returns a value obtained by parsing the result of the request's Starlark function.
 // The given rawString must correspond to the string output which was created by evaluating the
 // Starlark given in StarlarkFunctionBody.
-func (g getApexInfoType) ParseResult(rawString string) ApexInfo {
+func (g getApexInfoType) ParseResult(rawString string) (ApexInfo, error) {
 	var info ApexInfo
-	parseJson(rawString, &info)
+	err := parseJson(rawString, &info)
-	return info
+	return info, err
 }
 
 // getCcUnstrippedInfoType implements cqueryRequest interface. It handles the
@@ -282,10 +284,10 @@ return json_encode({
 // ParseResult returns a value obtained by parsing the result of the request's Starlark function.
 // The given rawString must correspond to the string output which was created by evaluating the
 // Starlark given in StarlarkFunctionBody.
-func (g getCcUnstippedInfoType) ParseResult(rawString string) CcUnstrippedInfo {
+func (g getCcUnstippedInfoType) ParseResult(rawString string) (CcUnstrippedInfo, error) {
 	var info CcUnstrippedInfo
-	parseJson(rawString, &info)
+	err := parseJson(rawString, &info)
-	return info
+	return info, err
 }
 
 type CcUnstrippedInfo struct {
@@ -305,10 +307,12 @@ func splitOrEmpty(s string, sep string) []string {
 
 // parseJson decodes json string into the fields of the receiver.
 // Unknown attribute name causes panic.
-func parseJson(jsonString string, info interface{}) {
+func parseJson(jsonString string, info interface{}) error {
 	decoder := json.NewDecoder(strings.NewReader(jsonString))
 	decoder.DisallowUnknownFields() //useful to detect typos, e.g. in unit tests
-	if err := decoder.Decode(info); err != nil {
+	err := decoder.Decode(info)
-		panic(fmt.Errorf("cannot parse cquery result '%s': %s", jsonString, err))
+	if err != nil {
+		return fmt.Errorf("cannot parse cquery result '%s': %s", jsonString, err)
 	}
+	return nil
 }

bazel/cquery/request_type_test.go

@@ -3,10 +3,12 @@ package cquery
 import (
 	"encoding/json"
 	"reflect"
+	"strings"
 	"testing"
 )
 
 func TestGetOutputFilesParseResults(t *testing.T) {
+	t.Parallel()
 	testCases := []struct {
 		description    string
 		input          string
@@ -29,14 +31,17 @@ func TestGetOutputFilesParseResults(t *testing.T) {
 		},
 	}
 	for _, tc := range testCases {
+		t.Run(tc.description, func(t *testing.T) {
 			actualOutput := GetOutputFiles.ParseResult(tc.input)
 			if !reflect.DeepEqual(tc.expectedOutput, actualOutput) {
-			t.Errorf("%q: expected %#v != actual %#v", tc.description, tc.expectedOutput, actualOutput)
+				t.Errorf("expected %#v != actual %#v", tc.expectedOutput, actualOutput)
 			}
+		})
 	}
 }
 
 func TestGetPythonBinaryParseResults(t *testing.T) {
+	t.Parallel()
 	testCases := []struct {
 		description    string
 		input          string
@@ -54,14 +59,17 @@ func TestGetPythonBinaryParseResults(t *testing.T) {
 		},
 	}
 	for _, tc := range testCases {
+		t.Run(tc.description, func(t *testing.T) {
 			actualOutput := GetPythonBinary.ParseResult(tc.input)
 			if !reflect.DeepEqual(tc.expectedOutput, actualOutput) {
-			t.Errorf("%q: expected %#v != actual %#v", tc.description, tc.expectedOutput, actualOutput)
+				t.Errorf("expected %#v != actual %#v", tc.expectedOutput, actualOutput)
 			}
+		})
 	}
 }
 
 func TestGetCcInfoParseResults(t *testing.T) {
+	t.Parallel()
 	testCases := []struct {
 		description    string
 		inputCcInfo    CcInfo
@@ -72,24 +80,6 @@ func TestGetCcInfoParseResults(t *testing.T) {
 			inputCcInfo:    CcInfo{},
 			expectedOutput: CcInfo{},
 		},
-		{
-			description: "only output",
-			inputCcInfo: CcInfo{
-				OutputFiles: []string{"test", "test3"},
-			},
-			expectedOutput: CcInfo{
-				OutputFiles: []string{"test", "test3"},
-			},
-		},
-		{
-			description: "only ToC",
-			inputCcInfo: CcInfo{
-				TocFile: "test",
-			},
-			expectedOutput: CcInfo{
-				TocFile: "test",
-			},
-		},
 		{
 			description: "all items set",
 			inputCcInfo: CcInfo{
@@ -119,17 +109,51 @@ func TestGetCcInfoParseResults(t *testing.T) {
 		},
 	}
 	for _, tc := range testCases {
+		t.Run(tc.description, func(t *testing.T) {
 			jsonInput, _ := json.Marshal(tc.inputCcInfo)
 			actualOutput, err := GetCcInfo.ParseResult(string(jsonInput))
 			if err != nil {
-			t.Errorf("%q:\n test case get error: %q", tc.description, err)
+				t.Errorf("error parsing result: %q", err)
 			} else if err == nil && !reflect.DeepEqual(tc.expectedOutput, actualOutput) {
-			t.Errorf("%q:\n expected %#v\n!= actual %#v", tc.description, tc.expectedOutput, actualOutput)
+				t.Errorf("expected %#v\n!= actual %#v", tc.expectedOutput, actualOutput)
 			}
+		})
+	}
+}
+
+func TestGetCcInfoParseResultsError(t *testing.T) {
+	t.Parallel()
+	testCases := []struct {
+		description   string
+		input         string
+		expectedError string
+	}{
+		{
+			description:   "not json",
+			input:         ``,
+			expectedError: `cannot parse cquery result '': EOF`,
+		},
+		{
+			description: "invalid field",
+			input: `{
+	"toc_file": "dir/file.so.toc"
+}`,
+			expectedError: `json: unknown field "toc_file"`,
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.description, func(t *testing.T) {
+			_, err := GetCcInfo.ParseResult(tc.input)
+			if !strings.Contains(err.Error(), tc.expectedError) {
+				t.Errorf("expected string %q in error message, got %q", tc.expectedError, err)
+			}
+		})
 	}
 }
 
 func TestGetApexInfoParseResults(t *testing.T) {
+	t.Parallel()
 	testCases := []struct {
 		description    string
 		input          string
@@ -169,14 +193,51 @@ func TestGetApexInfoParseResults(t *testing.T) {
 		},
 	}
 	for _, tc := range testCases {
-		actualOutput := GetApexInfo.ParseResult(tc.input)
+		t.Run(tc.description, func(t *testing.T) {
-		if !reflect.DeepEqual(tc.expectedOutput, actualOutput) {
+			actualOutput, err := GetApexInfo.ParseResult(tc.input)
-			t.Errorf("%q: expected %#v != actual %#v", tc.description, tc.expectedOutput, actualOutput)
+			if err != nil {
+				t.Errorf("Unexpected error %q", err)
 			}
+			if !reflect.DeepEqual(tc.expectedOutput, actualOutput) {
+				t.Errorf("expected %#v != actual %#v", tc.expectedOutput, actualOutput)
+			}
+		})
+	}
+}
+
+func TestGetApexInfoParseResultsError(t *testing.T) {
+	t.Parallel()
+	testCases := []struct {
+		description   string
+		input         string
+		expectedError string
+	}{
+		{
+			description:   "not json",
+			input:         ``,
+			expectedError: `cannot parse cquery result '': EOF`,
+		},
+		{
+			description: "invalid field",
+			input: `{
+	"fake_field": "path/to/file"
+}`,
+			expectedError: `json: unknown field "fake_field"`,
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.description, func(t *testing.T) {
+			_, err := GetApexInfo.ParseResult(tc.input)
+			if !strings.Contains(err.Error(), tc.expectedError) {
+				t.Errorf("expected string %q in error message, got %q", tc.expectedError, err)
+			}
+		})
 	}
 }
 
 func TestGetCcUnstrippedParseResults(t *testing.T) {
+	t.Parallel()
 	testCases := []struct {
 		description    string
 		input          string
@@ -197,9 +258,45 @@ func TestGetCcUnstrippedParseResults(t *testing.T) {
 		},
 	}
 	for _, tc := range testCases {
-		actualOutput := GetCcUnstrippedInfo.ParseResult(tc.input)
+		t.Run(tc.description, func(t *testing.T) {
-		if !reflect.DeepEqual(tc.expectedOutput, actualOutput) {
+			actualOutput, err := GetCcUnstrippedInfo.ParseResult(tc.input)
-			t.Errorf("%q: expected %#v != actual %#v", tc.description, tc.expectedOutput, actualOutput)
+			if err != nil {
+				t.Errorf("Unexpected error %q", err)
 			}
+			if !reflect.DeepEqual(tc.expectedOutput, actualOutput) {
+				t.Errorf("expected %#v != actual %#v", tc.expectedOutput, actualOutput)
+			}
+		})
+	}
+}
+
+func TestGetCcUnstrippedParseResultsErrors(t *testing.T) {
+	t.Parallel()
+	testCases := []struct {
+		description   string
+		input         string
+		expectedError string
+	}{
+		{
+			description:   "not json",
+			input:         ``,
+			expectedError: `cannot parse cquery result '': EOF`,
+		},
+		{
+			description: "invalid field",
+			input: `{
+	"fake_field": "path/to/file"
+}`,
+			expectedError: `json: unknown field "fake_field"`,
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.description, func(t *testing.T) {
+			_, err := GetCcUnstrippedInfo.ParseResult(tc.input)
+			if !strings.Contains(err.Error(), tc.expectedError) {
+				t.Errorf("expected string %q in error message, got %q", tc.expectedError, err)
+			}
+		})
 	}
 }