Merge "Add tests for request_type ParseResult functions." am: 6bd0a19b9d am: e41f177874

Original change: https://android-review.googlesource.com/c/platform/build/soong/+/1672428

Change-Id: I51e37217758d8d3d949cdc4ce610cf8fc130b2d1

android/bazel_handler.go

@@ -67,7 +67,7 @@ type BazelContext interface {
 
 	// TODO(cparsons): Other cquery-related methods should be added here.
 	// Returns the results of GetOutputFiles and GetCcObjectFiles in a single query (in that order).
-	GetCcInfo(label string, archType ArchType) (cquery.CcInfo, bool)
+	GetCcInfo(label string, archType ArchType) (cquery.CcInfo, bool, error)
 
 	// ** End cquery methods
 
@@ -132,9 +132,9 @@ func (m MockBazelContext) GetOutputFiles(label string, archType ArchType) ([]str
 	return result, ok
 }
 
-func (m MockBazelContext) GetCcInfo(label string, archType ArchType) (cquery.CcInfo, bool) {
+func (m MockBazelContext) GetCcInfo(label string, archType ArchType) (cquery.CcInfo, bool, error) {
 	result, ok := m.LabelToCcInfo[label]
-	return result, ok
+	return result, ok, nil
 }
 
 func (m MockBazelContext) InvokeBazel() error {
@@ -163,21 +163,22 @@ func (bazelCtx *bazelContext) GetOutputFiles(label string, archType ArchType) ([
 	return ret, ok
 }
 
-func (bazelCtx *bazelContext) GetCcInfo(label string, archType ArchType) (cquery.CcInfo, bool) {
+func (bazelCtx *bazelContext) GetCcInfo(label string, archType ArchType) (cquery.CcInfo, bool, error) {
 	result, ok := bazelCtx.cquery(label, cquery.GetCcInfo, archType)
 	if !ok {
-		return cquery.CcInfo{}, ok
+		return cquery.CcInfo{}, ok, nil
 	}
 
 	bazelOutput := strings.TrimSpace(result)
-	return cquery.GetCcInfo.ParseResult(bazelOutput), ok
+	ret, err := cquery.GetCcInfo.ParseResult(bazelOutput)
+	return ret, ok, err
 }
 
 func (n noopBazelContext) GetOutputFiles(label string, archType ArchType) ([]string, bool) {
 	panic("unimplemented")
 }
 
-func (n noopBazelContext) GetCcInfo(label string, archType ArchType) (cquery.CcInfo, bool) {
+func (n noopBazelContext) GetCcInfo(label string, archType ArchType) (cquery.CcInfo, bool, error) {
 	panic("unimplemented")
 }
 

bazel/cquery/request_type.go

@@ -1,6 +1,7 @@
 package cquery
 
 import (
+	"fmt"
 	"strings"
 )
 
@@ -39,7 +40,7 @@ func (g getOutputFilesRequestType) StarlarkFunctionBody() string {
 // The given rawString must correspond to the string output which was created by evaluating the
 // Starlark given in StarlarkFunctionBody.
 func (g getOutputFilesRequestType) ParseResult(rawString string) []string {
-	return strings.Split(rawString, ", ")
+	return splitOrEmpty(rawString, ", ")
 }
 
 type getCcInfoType struct{}
@@ -85,11 +86,14 @@ return "|".join([", ".join(r) for r in returns])`
 // 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 getCcInfoType) ParseResult(rawString string) CcInfo {
+func (g getCcInfoType) ParseResult(rawString string) (CcInfo, error) {
 	var outputFiles []string
 	var ccObjects []string
 
 	splitString := strings.Split(rawString, "|")
+	if expectedLen := 3; len(splitString) != expectedLen {
+		return CcInfo{}, fmt.Errorf("Expected %d items, got %q", expectedLen, splitString)
+	}
 	outputFilesString := splitString[0]
 	ccStaticLibrariesString := splitString[1]
 	ccObjectsString := splitString[2]
@@ -100,7 +104,7 @@ func (g getCcInfoType) ParseResult(rawString string) CcInfo {
 		OutputFiles:          outputFiles,
 		CcObjectFiles:        ccObjects,
 		CcStaticLibraryFiles: ccStaticLibraries,
-	}
+	}, nil
 }
 
 // splitOrEmpty is a modification of strings.Split() that returns an empty list

bazel/cquery/request_type_test.go

@@ -0,0 +1,89 @@
+package cquery
+
+import (
+	"fmt"
+	"reflect"
+	"testing"
+)
+
+func TestGetOutputFilesParseResults(t *testing.T) {
+	testCases := []struct {
+		description    string
+		input          string
+		expectedOutput []string
+	}{
+		{
+			description:    "no result",
+			input:          "",
+			expectedOutput: []string{},
+		},
+		{
+			description:    "one result",
+			input:          "test",
+			expectedOutput: []string{"test"},
+		},
+		{
+			description:    "splits on comma with space",
+			input:          "foo, bar",
+			expectedOutput: []string{"foo", "bar"},
+		},
+	}
+	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)
+		}
+	}
+}
+
+func TestGetCcInfoParseResults(t *testing.T) {
+	testCases := []struct {
+		description          string
+		input                string
+		expectedOutput       CcInfo
+		expectedErrorMessage string
+	}{
+		{
+			description: "no result",
+			input:       "||",
+			expectedOutput: CcInfo{
+				OutputFiles:          []string{},
+				CcObjectFiles:        []string{},
+				CcStaticLibraryFiles: []string{},
+			},
+		},
+		{
+			description: "only output",
+			input:       "test||",
+			expectedOutput: CcInfo{
+				OutputFiles:          []string{"test"},
+				CcObjectFiles:        []string{},
+				CcStaticLibraryFiles: []string{},
+			},
+		},
+		{
+			description: "all items set",
+			input:       "out1, out2|static_lib1, static_lib2|object1, object2",
+			expectedOutput: CcInfo{
+				OutputFiles:          []string{"out1", "out2"},
+				CcObjectFiles:        []string{"object1", "object2"},
+				CcStaticLibraryFiles: []string{"static_lib1", "static_lib2"},
+			},
+		},
+		{
+			description:          "too few result splits",
+			input:                "|",
+			expectedOutput:       CcInfo{},
+			expectedErrorMessage: fmt.Sprintf("Expected %d items, got %q", 3, []string{"", ""}),
+		},
+	}
+	for _, tc := range testCases {
+		actualOutput, err := GetCcInfo.ParseResult(tc.input)
+		if (err == nil && tc.expectedErrorMessage != "") ||
+			(err != nil && err.Error() != tc.expectedErrorMessage) {
+			t.Errorf("%q: expected Error %s, got %s", tc.description, tc.expectedErrorMessage, err)
+		} else if err == nil && !reflect.DeepEqual(tc.expectedOutput, actualOutput) {
+			t.Errorf("%q: expected %#v != actual %#v", tc.description, tc.expectedOutput, actualOutput)
+		}
+	}
+}

cc/library.go

@@ -483,12 +483,16 @@ type staticLibraryBazelHandler struct {
 
 func (handler *staticLibraryBazelHandler) generateBazelBuildActions(ctx android.ModuleContext, label string) bool {
 	bazelCtx := ctx.Config().BazelContext
-	ccInfo, ok := bazelCtx.GetCcInfo(label, ctx.Arch().ArchType)
+	ccInfo, ok, err := bazelCtx.GetCcInfo(label, ctx.Arch().ArchType)
-	outputPaths := ccInfo.OutputFiles
+	if err != nil {
-	objPaths := ccInfo.CcObjectFiles
+		ctx.ModuleErrorf("Error getting Bazel CcInfo: %s", err)
+		return false
+	}
 	if !ok {
 		return ok
 	}
+	outputPaths := ccInfo.OutputFiles
+	objPaths := ccInfo.CcObjectFiles
 	if len(outputPaths) > 1 {
 		// TODO(cparsons): This is actually expected behavior for static libraries with no srcs.
 		// We should support this.

cc/prebuilt.go

@@ -329,11 +329,14 @@ type prebuiltStaticLibraryBazelHandler struct {
 
 func (h *prebuiltStaticLibraryBazelHandler) generateBazelBuildActions(ctx android.ModuleContext, label string) bool {
 	bazelCtx := ctx.Config().BazelContext
-	ccInfo, ok := bazelCtx.GetCcInfo(label, ctx.Arch().ArchType)
+	ccInfo, ok, err := bazelCtx.GetCcInfo(label, ctx.Arch().ArchType)
-	staticLibs := ccInfo.CcStaticLibraryFiles
+	if err != nil {
+		ctx.ModuleErrorf("Error getting Bazel CcInfo: %s", err)
+	}
 	if !ok {
 		return false
 	}
+	staticLibs := ccInfo.CcStaticLibraryFiles
 	if len(staticLibs) > 1 {
 		ctx.ModuleErrorf("expected 1 static library from bazel target %q, got %s", label, staticLibs)
 		return false