testplan: Parse builds from a file instead of CLI.
- Calls to get-testable with too many builds were causing OS
errors because the arguments were too long. Add an option to
parse builds from a file instead of on the command line.
BUG=b:283907263
TEST=CQ
TEST=Manually ran `testplan get-testable`
Change-Id: I01dea1f23de32e105d283c0d9a6dcf72c53bfff7
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/dev-util/+/4568625
Reviewed-by: Chris Gerber <gerb@google.com>
Commit-Queue: Chris Gerber <gerb@google.com>
Auto-Submit: Andrew Lamb <andrewlamb@chromium.org>
Tested-by: Andrew Lamb <andrewlamb@chromium.org>
diff --git a/src/chromiumos/test/plan/cmd/testplan.go b/src/chromiumos/test/plan/cmd/testplan.go
index 6087d85..6a58744 100644
--- a/src/chromiumos/test/plan/cmd/testplan.go
+++ b/src/chromiumos/test/plan/cmd/testplan.go
@@ -436,6 +436,7 @@
subcommands.CommandRunBase
planPaths []string
builds []*bbpb.Build
+ buildsPath string
buildMetadataListPath string
dutAttributeListPath string
configBundleListPath string
@@ -443,12 +444,13 @@
}
var cmdGetTestable = &subcommands.Command{
- UsageLine: `get-testable -plan plan1.star [-plan plan2.star] -build BUILD1 [-build BUILD2] -dutattributes PATH -buildmetadata PATH -configbundlelist PATH -builderconfigs PATH`,
+ UsageLine: `get-testable -plan plan1.star [-plan plan2.star] -builds PATH -dutattributes PATH -buildmetadata PATH -configbundlelist PATH -builderconfigs PATH`,
ShortDesc: "get a list of builds that could possibly be tested by plans",
LongDesc: `Get a list of builds that could possibly be tested by plans.
-First compute a set of CoverageRules from plans, then compute which builds in
-GenerateTestPlanRequest could possibly be tested based off the CoverageRules.
+First compute a set of CoverageRules from plans, then compute which builds could
+possibly be tested based off the CoverageRules.
+
This doesn't take the status, output test artifacts, etc. of builds into
account, just whether their build target, variant, and profile could be included
in one of the CoverageRules.
@@ -471,11 +473,20 @@
r.Flags.Var(
luciflag.MessageSliceFlag(&r.builds),
"build",
- "Buildbucket build protos to analyze, as JSON proto. Each proto must"+
- "include the `builder.builder` field and the `build_target.name`"+
- "input property, all other fields will be ignored",
+ "Deprecated, use `-builds` instead. Buildbucket build protos to "+
+ "analyze, as JSON proto. Each proto must include the "+
+ "`builder.builder` field and the `build_target.name` input "+
+ "property, all other fields will be ignored",
)
r.Flags.StringVar(
+ &r.buildsPath,
+ "builds",
+ "",
+ "Path to a file containing Buildbucket build protos to analyze, with"+
+ "one JSON proto per-line. Each proto must include the "+
+ "`builder.builder` field and the `build_target.name` input "+
+ "property, all other fields will be ignored")
+ r.Flags.StringVar(
&r.dutAttributeListPath,
"dutattributes",
"",
@@ -521,6 +532,20 @@
return errors.New("at least one -plan is required")
}
+ if len(r.builds) > 0 && len(r.buildsPath) > 0 {
+ return errors.New("only one of -build and -builds can be set")
+ }
+
+ // If buildsPath was used, parse the builds into r.builds.
+ if len(r.buildsPath) > 0 {
+ parsedBuilds, err := protoio.ReadJsonl(r.buildsPath, func() *bbpb.Build { return &bbpb.Build{} })
+ if err != nil {
+ return err
+ }
+
+ r.builds = parsedBuilds
+ }
+
if len(r.builds) == 0 {
return errors.New("at least one -build is required")
}
diff --git a/src/chromiumos/test/plan/internal/protoio/protoio.go b/src/chromiumos/test/plan/internal/protoio/protoio.go
index a628375..674932e 100644
--- a/src/chromiumos/test/plan/internal/protoio/protoio.go
+++ b/src/chromiumos/test/plan/internal/protoio/protoio.go
@@ -7,6 +7,7 @@
package protoio
import (
+ "bufio"
"io/ioutil"
"os"
"path/filepath"
@@ -57,6 +58,31 @@
return unmarshalOpts.Unmarshal(b, protov1.MessageV2(m))
}
+// ReadJsonl parses the newline-delimited jsonprotos in inPath. ctor must return
+// a new empty proto to parse each line into.
+func ReadJsonl[M proto.Message](inPath string, ctor func() M) ([]M, error) {
+ f, err := os.Open(inPath)
+ if err != nil {
+ return nil, err
+ }
+ defer f.Close()
+
+ messages := make([]M, 0)
+ scanner := bufio.NewScanner(f)
+ unmarshalOpts := protojson.UnmarshalOptions{DiscardUnknown: true}
+
+ for scanner.Scan() {
+ m := ctor()
+ if err := unmarshalOpts.Unmarshal(scanner.Bytes(), m); err != nil {
+ return nil, err
+ }
+
+ messages = append(messages, m)
+ }
+
+ return messages, nil
+}
+
// WriteJsonl writes a newline-delimited json file containing messages to outPath.
func WriteJsonl[M proto.Message](messages []M, outPath string) error {
outFile, err := os.Create(outPath)
diff --git a/src/chromiumos/test/plan/internal/protoio/protoio_test.go b/src/chromiumos/test/plan/internal/protoio/protoio_test.go
index eb46d17..92c673a 100644
--- a/src/chromiumos/test/plan/internal/protoio/protoio_test.go
+++ b/src/chromiumos/test/plan/internal/protoio/protoio_test.go
@@ -13,6 +13,7 @@
protov1 "github.com/golang/protobuf/proto"
"github.com/google/go-cmp/cmp"
test_api_v1 "go.chromium.org/chromiumos/config/go/test/api/v1"
+ bbpb "go.chromium.org/luci/buildbucket/proto"
"google.golang.org/protobuf/testing/protocmp"
)
@@ -81,6 +82,32 @@
}
}
+func TestReadJsonl(t *testing.T) {
+ inPath := filepath.Join(t.TempDir(), "in.jsonl")
+ buildsContent := `{"builder": {"builder": "builderA"}}
+{"builder": {"builder": "builderB"}}`
+ if err := os.WriteFile(inPath, []byte(buildsContent), os.ModePerm); err != nil {
+ t.Fatal(err)
+ }
+
+ parsedBuilds, err := protoio.ReadJsonl(inPath, func() *bbpb.Build { return &bbpb.Build{} })
+ if err != nil {
+ t.Fatal(err)
+ }
+
+ expectedBuilds := []*bbpb.Build{
+ {
+ Builder: &bbpb.BuilderID{Builder: "builderA"},
+ },
+ {
+ Builder: &bbpb.BuilderID{Builder: "builderB"},
+ },
+ }
+ if diff := cmp.Diff(expectedBuilds, parsedBuilds, protocmp.Transform()); diff != "" {
+ t.Errorf("returned unexpected diff in read message (-want +got):\n%s", diff)
+ }
+}
+
func TestFilepathAsJsonpb(t *testing.T) {
tests := []struct {
name, input, expected string
