testplan: Add Starlark functions to create VMTestPlan protos.
- Follows similar patterns to Starlark fns. to create HWTestPlan
protos.
- Backwards compatibility with CTPV1 will be supported in next CL,
in this CL testplan returns an error when VMTestPlan protos are
created.
BUG=b:218319842
TEST=CQ
Change-Id: Ic80657219697fc6cc29f939a8b6a7bd9925ca5fd
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/dev-util/+/3726853
Tested-by: Andrew Lamb <andrewlamb@chromium.org>
Commit-Queue: Andrew Lamb <andrewlamb@chromium.org>
Reviewed-by: Navil Perez <navil@google.com>
diff --git a/src/chromiumos/test/plan/cmd/example_plan.star b/src/chromiumos/test/plan/cmd/example_plan.star
index 878f669..7ee5086 100644
--- a/src/chromiumos/test/plan/cmd/example_plan.star
+++ b/src/chromiumos/test/plan/cmd/example_plan.star
@@ -5,9 +5,9 @@
load("@proto//chromiumos/test/api/test_suite.proto", test_suite_pb = "chromiumos.test.api")
build_metadata = testplan.get_build_metadata()
-flat_configs = testplan.get_flat_config_list()
+config_bundle_list = testplan.get_config_bundle_list()
print("Got {} BuildMetadatas".format(len(build_metadata.values)))
-print("Got {} FlatConfigs".format(len(flat_configs.values)))
+print("Got {} ConfigBundles".format(len(config_bundle_list.values)))
def add_test_plans():
for bm in build_metadata.values:
diff --git a/src/chromiumos/test/plan/cmd/run_example.sh b/src/chromiumos/test/plan/cmd/run_example.sh
index eedb575..ccd4b7c 100755
--- a/src/chromiumos/test/plan/cmd/run_example.sh
+++ b/src/chromiumos/test/plan/cmd/run_example.sh
@@ -43,36 +43,7 @@
cd "${script_dir}"
-config_internal_dir="$(realpath -e ../../../../../../../config-internal)"
-config_dir="$(realpath -e ../../../../../../../config)"
-
-dut_attributes="${config_dir}/generated/dut_attributes.jsonproto"
-build_metadata="${config_internal_dir}/build/generated/build_metadata.jsonproto"
-flat_config_list="${config_internal_dir}/hw_design/generated/flattened.binaryproto"
-
-if [[ ! -f ${dut_attributes} ]]; then
- echo "Expected to find DutAttributesList at ${dut_attributes}"
- exit 1
-else
- echo "Using DutAttributeList at ${dut_attributes}"
-fi
-
-if [[ ! -f ${build_metadata} ]]; then
- echo "Expected to find BuildMetadataList at ${build_metadata}"
- exit 1
-else
- echo "Using BuildMetadataList at ${build_metadata}"
-fi
-
-
-if [[ ! -f ${flat_config_list} ]]; then
- echo "Expected to find FlatConfigList at ${flat_config_list}"
- exit 1
-else
- echo "Using FlatConfigList at ${flat_config_list}"
-fi
-
-
+crosSrcRoot="$(realpath -e ../../../../../../../..)"
outDir=$(mktemp -d)
out=${outDir}/coverage_rules.jsonproto
textSummaryOut=${outDir}/coverage_rules_summary.txt
@@ -83,9 +54,7 @@
go run testplan.go generate \
-plan "${script_dir}/example_plan.star" \
- -dutattributes "${dut_attributes}" \
- -buildmetadata "${build_metadata}" \
- -flatconfiglist "${flat_config_list}" \
+ -crossrcroot "${crosSrcRoot}" \
-out "${out}" \
-textsummaryout "${textSummaryOut}" \
"$@"
diff --git a/src/chromiumos/test/plan/cmd/testplan.go b/src/chromiumos/test/plan/cmd/testplan.go
index 95cf2d6..208bc94 100644
--- a/src/chromiumos/test/plan/cmd/testplan.go
+++ b/src/chromiumos/test/plan/cmd/testplan.go
@@ -424,13 +424,17 @@
glog.Infof("Read %d ConfigBundles from %s", len(configBundleList.Values), r.configBundleListPath)
- hwTestPlans, err := testplan.Generate(
+ hwTestPlans, vmTestPlans, err := testplan.Generate(
ctx, r.planPaths, buildMetadataList, dutAttributeList, configBundleList,
)
if err != nil {
return err
}
+ if len(vmTestPlans) > 0 {
+ return fmt.Errorf("VMTestPlans not yet supported")
+ }
+
if r.ctpV1 {
glog.Infof(
"Outputting GenerateTestPlanRequest to %s instead of HWTestPlan, for backwards compatibility with CTPV1",
diff --git a/src/chromiumos/test/plan/internal/generate.go b/src/chromiumos/test/plan/internal/generate.go
index fb82e66..f8b72a3 100644
--- a/src/chromiumos/test/plan/internal/generate.go
+++ b/src/chromiumos/test/plan/internal/generate.go
@@ -19,7 +19,7 @@
)
// Generate evals the Starlark files in planFilenames to produce a list of
-// HWTestPlans.
+// HWTestPlans and VMTestPlans.
//
// planFilenames must be non-empty. buildMetadataList, dutAttributeList, and
// configBundleList must be non-nil.
@@ -29,36 +29,38 @@
buildMetadataList *buildpb.SystemImage_BuildMetadataList,
dutAttributeList *testpb.DutAttributeList,
configBundleList *payload.ConfigBundleList,
-) ([]*test_api_v1.HWTestPlan, error) {
+) ([]*test_api_v1.HWTestPlan, []*test_api_v1.VMTestPlan, error) {
if len(planFilenames) == 0 {
- return nil, errors.New("planFilenames must be non-empty")
+ return nil, nil, errors.New("planFilenames must be non-empty")
}
if buildMetadataList == nil {
- return nil, errors.New("buildMetadataList must be non-nil")
+ return nil, nil, errors.New("buildMetadataList must be non-nil")
}
if dutAttributeList == nil {
- return nil, errors.New("dutAttributeList must be non-nil")
+ return nil, nil, errors.New("dutAttributeList must be non-nil")
}
if configBundleList == nil {
- return nil, errors.New("configBundleList must be non-nil")
+ return nil, nil, errors.New("configBundleList must be non-nil")
}
- var allTestPlans []*test_api_v1.HWTestPlan
+ var allHwTestPlans []*test_api_v1.HWTestPlan
+ var allVmTestPlans []*test_api_v1.VMTestPlan
for _, planFilename := range planFilenames {
- testPlans, err := starlark.ExecTestPlan(ctx, planFilename, buildMetadataList, configBundleList)
+ hwTestPlans, vmTestPlans, err := starlark.ExecTestPlan(ctx, planFilename, buildMetadataList, configBundleList)
if err != nil {
- return nil, err
+ return nil, nil, err
}
- if len(testPlans) == 0 {
+ if len(hwTestPlans) == 0 && len(vmTestPlans) == 0 {
glog.Warningf("starlark file %q returned no TestPlans", planFilename)
}
- allTestPlans = append(allTestPlans, testPlans...)
+ allHwTestPlans = append(allHwTestPlans, hwTestPlans...)
+ allVmTestPlans = append(allVmTestPlans, vmTestPlans...)
}
- return allTestPlans, nil
+ return allHwTestPlans, allVmTestPlans, nil
}
diff --git a/src/chromiumos/test/plan/internal/generate_test.go b/src/chromiumos/test/plan/internal/generate_test.go
index 4b4c6d0..0a563b1 100644
--- a/src/chromiumos/test/plan/internal/generate_test.go
+++ b/src/chromiumos/test/plan/internal/generate_test.go
@@ -149,29 +149,62 @@
testplan.add_hw_test_plan(
plan_pb.HWTestPlan(id=plan_pb.HWTestPlan.TestPlanId(value='plan1'))
)
+testplan.add_vm_test_plan(
+ plan_pb.VMTestPlan(id=plan_pb.VMTestPlan.TestPlanId(value='plan2'))
+)
+ `
+
+ noPlansStarlarkSource := `
+load("@proto//chromiumos/test/api/v1/plan.proto", plan_pb = "chromiumos.test.api.v1")
+
+def pointless_fn():
+ if 1 == 2:
+ testplan.add_hw_test_plan(
+ plan_pb.HWTestPlan(id=plan_pb.HWTestPlan.TestPlanId(value='plan1'))
+ )
+
+pointless_fn()
`
planFilename := writeTempStarlarkFile(
t, starlarkSource,
)
- testPlans, err := testplan.Generate(
- ctx, []string{planFilename}, buildMetadataList, dutAttributeList, configBundleList,
+ noPlanFilename := writeTempStarlarkFile(
+ t, noPlansStarlarkSource,
+ )
+
+ hwTestPlans, vmTestPlans, err := testplan.Generate(
+ ctx, []string{planFilename, noPlanFilename}, buildMetadataList, dutAttributeList, configBundleList,
)
if err != nil {
t.Fatal(err)
}
- expectedTestPlans := []*test_api_v1.HWTestPlan{
+ expectedHwTestPlans := []*test_api_v1.HWTestPlan{
{Id: &test_api_v1.HWTestPlan_TestPlanId{Value: "plan1"}},
}
- if len(expectedTestPlans) != len(testPlans) {
- t.Errorf("expected %d test plans, got %d", len(expectedTestPlans), len(testPlans))
+ expectedVmTestPlans := []*test_api_v1.VMTestPlan{
+ {Id: &test_api_v1.VMTestPlan_TestPlanId{Value: "plan2"}},
}
- for i, expected := range expectedTestPlans {
- if diff := cmp.Diff(expected, testPlans[i], protocmp.Transform()); diff != "" {
+ if len(expectedHwTestPlans) != len(hwTestPlans) {
+ t.Errorf("expected %d test plans, got %d", len(expectedHwTestPlans), len(hwTestPlans))
+ }
+
+ for i, expected := range expectedHwTestPlans {
+ if diff := cmp.Diff(expected, hwTestPlans[i], protocmp.Transform()); diff != "" {
+ t.Errorf("returned unexpected diff in test plan %d (-want +got):\n%s", i, diff)
+ }
+ }
+
+ if len(expectedVmTestPlans) != len(vmTestPlans) {
+ t.Errorf("expected %d test plans, got %d", len(expectedVmTestPlans), len(vmTestPlans))
+ }
+
+ for i, expected := range expectedVmTestPlans {
+ if diff := cmp.Diff(expected, vmTestPlans[i], protocmp.Transform()); diff != "" {
t.Errorf("returned unexpected diff in test plan %d (-want +got):\n%s", i, diff)
}
}
@@ -180,6 +213,9 @@
func TestGenerateErrors(t *testing.T) {
ctx := context.Background()
+ badStarlarkFile := "testplan.invalidcall()"
+ badPlanFilename := writeTempStarlarkFile(t, badStarlarkFile)
+
tests := []struct {
name string
planFilenames []string
@@ -215,11 +251,18 @@
dutAttributeList: dutAttributeList,
configBundleList: nil,
},
+ {
+ name: "bad Starlark file",
+ planFilenames: []string{badPlanFilename},
+ buildMetadataList: buildMetadataList,
+ dutAttributeList: dutAttributeList,
+ configBundleList: configBundleList,
+ },
}
for _, test := range tests {
t.Run(test.name, func(t *testing.T) {
- if _, err := testplan.Generate(
+ if _, _, err := testplan.Generate(
ctx, test.planFilenames, test.buildMetadataList, test.dutAttributeList, test.configBundleList,
); err == nil {
t.Error("Expected error from Generate")
diff --git a/src/chromiumos/test/plan/internal/starlark/exec.go b/src/chromiumos/test/plan/internal/starlark/exec.go
index 47099d7..5d2efb1 100644
--- a/src/chromiumos/test/plan/internal/starlark/exec.go
+++ b/src/chromiumos/test/plan/internal/starlark/exec.go
@@ -50,6 +50,43 @@
)
}
+// starlarkValueToProto converts value to proto Message m. An error is returned
+// if value is not a starlarkproto.Message, with a protoreflect.FullName that is
+// exactly the same as the protoreflect.FullName of m.
+func starlarkValueToProto(value starlark.Value, m protov1.Message) error {
+ mName := protov1.MessageReflect(m).Descriptor().FullName()
+
+ // Assert value is a starlarkproto.Message.
+ starlarkMessage, ok := value.(*starlarkproto.Message)
+ if !ok {
+ return fmt.Errorf("arg must be a %s, got %q", mName, value)
+ }
+
+ // It is not possible to use type assertions to convert the
+ // starlarkproto.Message to a protov1.Message, so marshal it to bytes and
+ // then unmarshal as a protov1.Message.
+ //
+ // First check that the full name of the message passed in exactly
+ // matches the full name of m, to avoid confusing errors
+ // from unmarshalling.
+ starlarkProto := starlarkMessage.ToProto()
+
+ if mName != starlarkProto.ProtoReflect().Descriptor().FullName() {
+ return fmt.Errorf("arg must be a %s, got %q", mName, starlarkProto)
+ }
+
+ bytes, err := proto.Marshal(starlarkMessage.ToProto())
+ if err != nil {
+ return err
+ }
+
+ if err := protov1.Unmarshal(bytes, m); err != nil {
+ return err
+ }
+
+ return nil
+}
+
// buildAddHWTestPlanBuiltin returns a Builtin that takes a single HWTestPlan
// and adds it to result.
func buildAddHWTestPlanBuiltin(result *[]*test_api_v1.HWTestPlan) *starlark.Builtin {
@@ -61,37 +98,9 @@
return nil, err
}
- // Assert the value passed in is a starlarkproto.Message.
- starlarkMessage, ok := starlarkValue.(*starlarkproto.Message)
- if !ok {
- return nil, fmt.Errorf("arg to %s must be a HWTestPlan, got %q", fn.Name(), starlarkValue)
- }
-
- // It is not possible to use type assertions to convert the
- // starlarkproto.Message to a HWTestPlan, so marshal it to bytes and
- // then unmarshal as a HWTestPlan.
- //
- // First check that the full name of the message passed in exactly
- // matches the full name of HWTestPlan, to avoid confusing errors
- // from unmarshalling.
- starlarkProto := starlarkMessage.ToProto()
hwTestPlan := &test_api_v1.HWTestPlan{}
-
- if protov1.MessageReflect(hwTestPlan).Descriptor().FullName() != starlarkProto.ProtoReflect().Descriptor().FullName() {
- return nil, fmt.Errorf(
- "arg to %s must be a HWTestPlan, got %q",
- fn.Name(),
- starlarkProto,
- )
- }
-
- bytes, err := proto.Marshal(starlarkMessage.ToProto())
- if err != nil {
- return nil, err
- }
-
- if err := protov1.Unmarshal(bytes, hwTestPlan); err != nil {
- return nil, err
+ if err := starlarkValueToProto(starlarkValue, hwTestPlan); err != nil {
+ return nil, fmt.Errorf("%s: %w", fn.Name(), err)
}
*result = append(*result, hwTestPlan)
@@ -100,23 +109,42 @@
)
}
+// buildAddVMTestPlanBuiltin returns a Builtin that takes a single HWTestPlan
+// and adds it to result.
+func buildAddVMTestPlanBuiltin(result *[]*test_api_v1.VMTestPlan) *starlark.Builtin {
+ return starlark.NewBuiltin(
+ "add_vm_test_plan",
+ func(thread *starlark.Thread, fn *starlark.Builtin, args starlark.Tuple, kwargs []starlark.Tuple) (starlark.Value, error) {
+ var starlarkValue starlark.Value
+ if err := starlark.UnpackArgs(fn.Name(), args, kwargs, "vm_test_plan", &starlarkValue); err != nil {
+ return nil, err
+ }
+
+ vmTestPlan := &test_api_v1.VMTestPlan{}
+ if err := starlarkValueToProto(starlarkValue, vmTestPlan); err != nil {
+ return nil, fmt.Errorf("%s: %w", fn.Name(), err)
+ }
+
+ *result = append(*result, vmTestPlan)
+ return starlark.None, nil
+ },
+ )
+}
+
// ExecTestPlan executes the Starlark file planFilename.
// Builtins are provided to planFilename to access buildMetadataList and
-// configBundleList, and add HWTestPlans to the output.
+// configBundleList, and add [VM,HW]TestPlans to the output.
//
// A loader is provided to load proto constructors.
-//
-// TODO(b/182898188): Provide more extensive documentation of Starlark execution
-// environment.
func ExecTestPlan(
ctx context.Context,
planFilename string,
buildMetadataList *buildpb.SystemImage_BuildMetadataList,
configBundleList *payload.ConfigBundleList,
-) ([]*test_api_v1.HWTestPlan, error) {
+) ([]*test_api_v1.HWTestPlan, []*test_api_v1.VMTestPlan, error) {
protoLoader, err := buildProtoLoader()
if err != nil {
- return nil, err
+ return nil, nil, err
}
getBuildMetadataBuiltin := protoAccessorBuiltin(
@@ -127,8 +155,11 @@
protoLoader, "get_config_bundle_list", configBundleList,
)
- var test_plans []*test_api_v1.HWTestPlan
- addHWTestPlanBuiltin := buildAddHWTestPlanBuiltin(&test_plans)
+ var hw_test_plans []*test_api_v1.HWTestPlan
+ addHWTestPlanBuiltin := buildAddHWTestPlanBuiltin(&hw_test_plans)
+
+ var vm_test_plans []*test_api_v1.VMTestPlan
+ addVMTestPlanBuiltin := buildAddVMTestPlanBuiltin(&vm_test_plans)
testplanModule := &starlarkstruct.Module{
Name: "testplan",
@@ -136,6 +167,7 @@
getBuildMetadataBuiltin.Name(): getBuildMetadataBuiltin,
getFlatConfigListBuiltin.Name(): getFlatConfigListBuiltin,
addHWTestPlanBuiltin.Name(): addHWTestPlanBuiltin,
+ addVMTestPlanBuiltin.Name(): addVMTestPlanBuiltin,
},
}
@@ -169,12 +201,12 @@
}
if err := intr.Init(ctx); err != nil {
- return nil, err
+ return nil, nil, err
}
if _, err := intr.ExecModule(ctx, interpreter.MainPkg, planBasename); err != nil {
- return nil, fmt.Errorf("failed executing Starlark file %q: %w", planFilename, err)
+ return nil, nil, fmt.Errorf("failed executing Starlark file %q: %w", planFilename, err)
}
- return test_plans, nil
+ return hw_test_plans, vm_test_plans, nil
}
diff --git a/src/chromiumos/test/plan/internal/starlark/exec_test.go b/src/chromiumos/test/plan/internal/starlark/exec_test.go
index b7c89fd..28a5c18 100644
--- a/src/chromiumos/test/plan/internal/starlark/exec_test.go
+++ b/src/chromiumos/test/plan/internal/starlark/exec_test.go
@@ -98,19 +98,26 @@
config_bundles = testplan.get_config_bundle_list()
print('Got {} BuildMetadatas'.format(len(build_metadata.values)))
print('Got {} ConfigBundles'.format(len(config_bundles.values)))
-coverage_rule = coverage_rule_pb.CoverageRule(name='ruleA')
+coverage_rule_a = coverage_rule_pb.CoverageRule(name='ruleA')
+coverage_rule_b = coverage_rule_pb.CoverageRule(name='ruleB')
testplan.add_hw_test_plan(
plan_pb.HWTestPlan(
id=plan_pb.HWTestPlan.TestPlanId(value='plan1'),
- coverage_rules=[coverage_rule],
+ coverage_rules=[coverage_rule_a],
),
)
+testplan.add_vm_test_plan(
+ plan_pb.VMTestPlan(
+ id=plan_pb.VMTestPlan.TestPlanId(value='vm_plan2'),
+ coverage_rules=[coverage_rule_b],
+ )
+)
`
planFilename := writeTempStarlarkFile(
t, starlarkSource,
)
- testPlans, err := starlark.ExecTestPlan(
+ hwTestPlans, vmTestPlans, err := starlark.ExecTestPlan(
ctx,
planFilename,
buildMetadataList,
@@ -121,7 +128,7 @@
t.Fatalf("ExecTestPlan failed: %s", err)
}
- expectedTestPlans := []*test_api_v1.HWTestPlan{
+ expectedHwTestPlans := []*test_api_v1.HWTestPlan{
{
Id: &test_api_v1.HWTestPlan_TestPlanId{Value: "plan1"},
CoverageRules: []*test_api.CoverageRule{
@@ -132,12 +139,33 @@
},
}
- if len(expectedTestPlans) != len(testPlans) {
- t.Errorf("expected %d test plans, got %d", len(expectedTestPlans), len(testPlans))
+ expectedVmTestPlans := []*test_api_v1.VMTestPlan{
+ {
+ Id: &test_api_v1.VMTestPlan_TestPlanId{Value: "vm_plan2"},
+ CoverageRules: []*test_api.CoverageRule{
+ {
+ Name: "ruleB",
+ },
+ },
+ },
}
- for i, expected := range expectedTestPlans {
- if diff := cmp.Diff(expected, testPlans[i], protocmp.Transform()); diff != "" {
+ if len(expectedHwTestPlans) != len(hwTestPlans) {
+ t.Errorf("expected %d test plans, got %d", len(expectedHwTestPlans), len(hwTestPlans))
+ }
+
+ for i, expected := range expectedHwTestPlans {
+ if diff := cmp.Diff(expected, hwTestPlans[i], protocmp.Transform()); diff != "" {
+ t.Errorf("returned unexpected diff in test plan %d (-want +got):\n%s", i, diff)
+ }
+ }
+
+ if len(expectedVmTestPlans) != len(vmTestPlans) {
+ t.Errorf("expected %d test plans, got %d", len(expectedVmTestPlans), len(vmTestPlans))
+ }
+
+ for i, expected := range expectedVmTestPlans {
+ if diff := cmp.Diff(expected, vmTestPlans[i], protocmp.Transform()); diff != "" {
t.Errorf("returned unexpected diff in test plan %d (-want +got):\n%s", i, diff)
}
}
@@ -162,14 +190,24 @@
err: "get_config_bundle_list: unexpected keyword argument \"somearg\"",
},
{
- name: "invalid named args ctor",
+ name: "invalid named args ctor HW",
starlarkSource: "testplan.add_hw_test_plan(somearg='abc')",
err: "add_hw_test_plan: unexpected keyword argument \"somearg\"",
},
{
- name: "invalid type ctor",
+ name: "invalid named args ctor VM",
+ starlarkSource: "testplan.add_vm_test_plan(somearg='abc')",
+ err: "add_vm_test_plan: unexpected keyword argument \"somearg\"",
+ },
+ {
+ name: "invalid type ctor HW",
starlarkSource: "testplan.add_hw_test_plan(hw_test_plan='abc')",
- err: "arg to add_hw_test_plan must be a HWTestPlan, got \"\\\"abc\\\"\"",
+ err: "add_hw_test_plan: arg must be a chromiumos.test.api.v1.HWTestPlan, got \"\\\"abc\\\"\"",
+ },
+ {
+ name: "invalid type ctor VM",
+ starlarkSource: "testplan.add_vm_test_plan(vm_test_plan='abc')",
+ err: "add_vm_test_plan: arg must be a chromiumos.test.api.v1.VMTestPlan, got \"\\\"abc\\\"\"",
},
{
name: "invalid proto ctor",
@@ -177,7 +215,7 @@
load("@proto//chromiumos/test/api/v1/plan.proto", plan_pb = "chromiumos.test.api.v1")
testplan.add_hw_test_plan(hw_test_plan=plan_pb.HWTestPlan.TestPlanId(value='abc'))
`,
- err: "arg to add_hw_test_plan must be a HWTestPlan, got \"value:\\\"abc\\\"",
+ err: "add_hw_test_plan: arg must be a chromiumos.test.api.v1.HWTestPlan, got \"value:\\\"abc\\\"\"",
},
}
@@ -187,7 +225,7 @@
t, tc.starlarkSource,
)
- _, err := starlark.ExecTestPlan(
+ _, _, err := starlark.ExecTestPlan(
ctx, planFilename, buildMetadataList, configBundleList,
)