From fe8853d2e606fc8dfc4565cc0136e936993b7fb3 Mon Sep 17 00:00:00 2001
From: Liz Kammer <eakammer@google.com>
Date: Wed, 16 Dec 2020 09:34:33 -0800
Subject: [PATCH] Add test & documentation for PropertiesToApply

Document PropertiesToApply expectations about props, and suggest it
should generally be generated via CreateProperties.

Test: go soong tests
Test: m nothing
Change-Id: I7cc2590db96865382ad6e0da333d4a4e2c697f45
---
 android/soongconfig/modules.go      | 17 +++++--
 android/soongconfig/modules_test.go | 71 +++++++++++++++++++++++++++++
 2 files changed, 83 insertions(+), 5 deletions(-)

diff --git a/android/soongconfig/modules.go b/android/soongconfig/modules.go
index 142a81387..5a6917ed0 100644
--- a/android/soongconfig/modules.go
+++ b/android/soongconfig/modules.go
@@ -158,11 +158,7 @@ func processModuleTypeDef(v *SoongConfigDefinition, def *parser.Module) (errs []
 			return []error{fmt.Errorf("bool_variable name must not be blank")}
 		}
 
-		mt.Variables = append(mt.Variables, &boolVariable{
-			baseVariable: baseVariable{
-				variable: name,
-			},
-		})
+		mt.Variables = append(mt.Variables, newBoolVariable(name))
 	}
 
 	for _, name := range props.Value_variables {
@@ -420,6 +416,9 @@ func typeForPropertyFromPropertyStruct(ps interface{}, property string) reflect.
 
 // PropertiesToApply returns the applicable properties from a ModuleType that should be applied
 // based on SoongConfig values.
+// Expects that props contains a struct field with name soong_config_variables. The fields within
+// soong_config_variables are expected to be in the same order as moduleType.Variables. In general,
+// props should be generated via CreateProperties.
 func PropertiesToApply(moduleType *ModuleType, props reflect.Value, config SoongConfig) ([]interface{}, error) {
 	var ret []interface{}
 	props = props.Elem().FieldByName(soongConfigProperty)
@@ -505,6 +504,14 @@ type boolVariable struct {
 	baseVariable
 }
 
+func newBoolVariable(name string) *boolVariable {
+	return &boolVariable{
+		baseVariable{
+			variable: name,
+		},
+	}
+}
+
 func (b boolVariable) variableValuesType() reflect.Type {
 	return emptyInterfaceType
 }
diff --git a/android/soongconfig/modules_test.go b/android/soongconfig/modules_test.go
index 419001670..fb0e1899d 100644
--- a/android/soongconfig/modules_test.go
+++ b/android/soongconfig/modules_test.go
@@ -17,6 +17,8 @@ package soongconfig
 import (
 	"reflect"
 	"testing"
+
+	"github.com/google/blueprint/proptools"
 )
 
 func Test_CanonicalizeToProperty(t *testing.T) {
@@ -247,3 +249,72 @@ func Test_createAffectablePropertiesType(t *testing.T) {
 		})
 	}
 }
+
+type properties struct {
+	A *string
+	B bool
+}
+type soongConfigVariables struct {
+	Bool_var       properties
+	Other_bool_var properties
+}
+
+type soongConfigProps struct {
+	Soong_config_variables soongConfigVariables
+}
+
+func Test_PropertiesToApply(t *testing.T) {
+
+	mt := &ModuleType{
+		BaseModuleType:  "foo",
+		ConfigNamespace: "bar",
+		Variables: []soongConfigVariable{
+			newBoolVariable("bool_var"),
+			newBoolVariable("other_bool_var"),
+		},
+		affectableProperties: []string{
+			"a",
+			"b",
+		},
+	}
+	props := soongConfigProps{
+		Soong_config_variables: soongConfigVariables{
+			Bool_var: properties{
+				A: proptools.StringPtr("a"),
+				B: true,
+			},
+			Other_bool_var: properties{
+				A: proptools.StringPtr("other"),
+				B: false,
+			},
+		},
+	}
+
+	testCases := []struct {
+		config    SoongConfig
+		wantProps []interface{}
+	}{
+		{
+			config: Config(map[string]string{}),
+		},
+		{
+			config:    Config(map[string]string{"bool_var": "y"}),
+			wantProps: []interface{}{props.Soong_config_variables.Bool_var},
+		},
+		{
+			config:    Config(map[string]string{"other_bool_var": "y"}),
+			wantProps: []interface{}{props.Soong_config_variables.Other_bool_var},
+		},
+	}
+
+	for _, tc := range testCases {
+		gotProps, err := PropertiesToApply(mt, reflect.ValueOf(&props), tc.config)
+		if err != nil {
+			t.Errorf("Unexpected error in PropertiesToApply: %s", err)
+		}
+
+		if !reflect.DeepEqual(gotProps, tc.wantProps) {
+			t.Errorf("Expected %s, got %s", tc.wantProps, gotProps)
+		}
+	}
+}