diff --git a/android/bazel_handler.go b/android/bazel_handler.go index d014fc62b..eec78d255 100644 --- a/android/bazel_handler.go +++ b/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) } diff --git a/bazel/cquery/request_type.go b/bazel/cquery/request_type.go index 5bf9fb255..eb44dd18d 100644 --- a/bazel/cquery/request_type.go +++ b/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) - return info + err := parseJson(rawString, &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) - return info + err := parseJson(rawString, &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 { - panic(fmt.Errorf("cannot parse cquery result '%s': %s", jsonString, err)) + err := decoder.Decode(info) + if err != nil { + return fmt.Errorf("cannot parse cquery result '%s': %s", jsonString, err) } + return nil } diff --git a/bazel/cquery/request_type_test.go b/bazel/cquery/request_type_test.go index 86f128e15..a0a993f6c 100644 --- a/bazel/cquery/request_type_test.go +++ b/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 { - actualOutput := GetOutputFiles.ParseResult(tc.input) - if !reflect.DeepEqual(tc.expectedOutput, actualOutput) { - t.Errorf("%q: expected %#v != actual %#v", tc.description, tc.expectedOutput, actualOutput) - } + t.Run(tc.description, func(t *testing.T) { + actualOutput := GetOutputFiles.ParseResult(tc.input) + if !reflect.DeepEqual(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 { - actualOutput := GetPythonBinary.ParseResult(tc.input) - if !reflect.DeepEqual(tc.expectedOutput, actualOutput) { - t.Errorf("%q: expected %#v != actual %#v", tc.description, tc.expectedOutput, actualOutput) - } + t.Run(tc.description, func(t *testing.T) { + actualOutput := GetPythonBinary.ParseResult(tc.input) + if !reflect.DeepEqual(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 { - 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) - } else if err == nil && !reflect.DeepEqual(tc.expectedOutput, actualOutput) { - t.Errorf("%q:\n expected %#v\n!= actual %#v", tc.description, tc.expectedOutput, actualOutput) - } + t.Run(tc.description, func(t *testing.T) { + jsonInput, _ := json.Marshal(tc.inputCcInfo) + actualOutput, err := GetCcInfo.ParseResult(string(jsonInput)) + if err != nil { + t.Errorf("error parsing result: %q", err) + } else if err == nil && !reflect.DeepEqual(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) - if !reflect.DeepEqual(tc.expectedOutput, actualOutput) { - t.Errorf("%q: expected %#v != actual %#v", tc.description, tc.expectedOutput, actualOutput) - } + t.Run(tc.description, func(t *testing.T) { + actualOutput, err := GetApexInfo.ParseResult(tc.input) + 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) - if !reflect.DeepEqual(tc.expectedOutput, actualOutput) { - t.Errorf("%q: expected %#v != actual %#v", tc.description, tc.expectedOutput, actualOutput) - } + t.Run(tc.description, func(t *testing.T) { + actualOutput, err := GetCcUnstrippedInfo.ParseResult(tc.input) + 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) + } + }) } }