test_planner: move pruning-related methods to new source file
These methods are all related to removing test suites from the eventual
test plan.
This refactor will simplify changes for crbug.com/976530
BUG=chromium:976530
TEST=unit tests (generator_test.go covers this new file)
Change-Id: I3ba45b8ce116595a34faa7080a089ac13c522b59
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/infra/test_planner/+/1673550
Reviewed-by: Jason Clinton <jclinton@chromium.org>
Commit-Queue: Sean Abraham <seanabraham@chromium.org>
Tested-by: Sean Abraham <seanabraham@chromium.org>
diff --git a/src/testplans/go.mod b/src/testplans/go.mod
index 7c0d0b9..1449a9c 100644
--- a/src/testplans/go.mod
+++ b/src/testplans/go.mod
@@ -16,7 +16,7 @@
github.com/smartystreets/assertions v0.0.0-20190401211740-f487f9de1cd3 // indirect
github.com/smartystreets/goconvey v0.0.0-20190330032615-68dc04aab96a // indirect
github.com/texttheater/golang-levenshtein v0.0.0-20180516184445-d188e65d659e // indirect
- go.chromium.org/chromiumos/infra/proto/go v0.0.0-20190523171533-39bf919eecb4
+ go.chromium.org/chromiumos/infra/proto/go v0.0.0-20190620221159-9206f8f25ade
go.chromium.org/luci v0.0.0-20190404171609-14fb4fbce8ea
golang.org/x/oauth2 v0.0.0-20190402181905-9f3314589c9a // indirect
google.golang.org/grpc v1.19.1 // indirect
diff --git a/src/testplans/go.sum b/src/testplans/go.sum
index 2790da4..cb2ccc5 100644
--- a/src/testplans/go.sum
+++ b/src/testplans/go.sum
@@ -128,6 +128,7 @@
go.chromium.org/chromiumos/infra/proto v0.0.0-20190521155230-020e6e4c3db7 h1:NfZmFRahNWbucJxPu590Uango4DhkZf+/sghHrboRAo=
go.chromium.org/chromiumos/infra/proto v0.0.0-20190522220408-28d21b420351 h1:KNLtgp0J88Hncy7GXvYUec8VVarBWQmkiG1yVJqiMcc=
go.chromium.org/chromiumos/infra/proto v0.0.0-20190523171533-39bf919eecb4 h1:MbjVVWG/KH+oEO3Vb1/ru10VbrnYHwbpyRNJsi1lFpo=
+go.chromium.org/chromiumos/infra/proto v0.0.0-20190620221159-9206f8f25ade h1:Ryr72FOi62Wa6HXtykgbNyp3yA+tXmRRSCSHOw4ALhY=
go.chromium.org/chromiumos/infra/proto/go v0.0.0-20190506153826-232eeaaa29a9 h1:87ML72wybRl6dKe4qyvTH5Y6ihvrTxT13+xTsz7F6tE=
go.chromium.org/chromiumos/infra/proto/go v0.0.0-20190506153826-232eeaaa29a9/go.mod h1:Y1AiM5bpNKKecKy3wgiBwF9q1PJ/f57z86DpkNrMehk=
go.chromium.org/chromiumos/infra/proto/go v0.0.0-20190514195506-9aeeb525f8a2 h1:kAJWnYFu1Vvk3RdZLKz9rDDBsi4m8WPDXv7r9plSEaY=
@@ -148,6 +149,8 @@
go.chromium.org/chromiumos/infra/proto/go v0.0.0-20190522220408-28d21b420351/go.mod h1:Y1AiM5bpNKKecKy3wgiBwF9q1PJ/f57z86DpkNrMehk=
go.chromium.org/chromiumos/infra/proto/go v0.0.0-20190523171533-39bf919eecb4 h1:WLlJOjc1h7tWmpiaB/fZsf9a12fcj1NstLJjbX7soM0=
go.chromium.org/chromiumos/infra/proto/go v0.0.0-20190523171533-39bf919eecb4/go.mod h1:Y1AiM5bpNKKecKy3wgiBwF9q1PJ/f57z86DpkNrMehk=
+go.chromium.org/chromiumos/infra/proto/go v0.0.0-20190620221159-9206f8f25ade h1:nEbrOlXy2f7gxNceKIdRj+epA1DsydTERwGiW5llzjM=
+go.chromium.org/chromiumos/infra/proto/go v0.0.0-20190620221159-9206f8f25ade/go.mod h1:Y1AiM5bpNKKecKy3wgiBwF9q1PJ/f57z86DpkNrMehk=
go.chromium.org/luci v0.0.0-20190404171609-14fb4fbce8ea h1:qf3Ri7sv3yJPL4E0iq4hBYn3AFSf2dNNJpuZDBXAT2E=
go.chromium.org/luci v0.0.0-20190404171609-14fb4fbce8ea/go.mod h1:MIQewVTLvOvc0UioV0JNqTNO/RspKFS0XEeoKrOxsdM=
go.opencensus.io v0.18.0/go.mod h1:vKdFvxhtzZ9onBp9VKHK8z/sRpBMnKAsufL7wlDrCOA=
diff --git a/src/testplans/internal/generator/generator.go b/src/testplans/internal/generator/generator.go
index bda4563..a111f6d 100644
--- a/src/testplans/internal/generator/generator.go
+++ b/src/testplans/internal/generator/generator.go
@@ -1,6 +1,7 @@
// Copyright 2019 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
+
package generator
import (
@@ -11,27 +12,9 @@
"go.chromium.org/chromiumos/infra/proto/go/testplans"
bbproto "go.chromium.org/luci/buildbucket/proto"
"log"
- "strings"
"testplans/internal/git"
)
-type testType int
-
-const (
- hw testType = iota
- vm
-)
-
-var (
- testTypeFilter = map[testType]func(testReqs *testplans.TestRestriction) bool{
- hw: func(testReqs *testplans.TestRestriction) bool { return testReqs.DisableHwTests },
- vm: func(testReqs *testplans.TestRestriction) bool { return testReqs.DisableVmTests },
- }
-)
-
-func (tt testType) String() string {
- return [...]string{"Hw", "Vm"}[tt]
-}
// BuildTarget is an OS build target, such as "kevin" or "eve".
type BuildTarget string
@@ -235,34 +218,6 @@
return tsc
}
-// extractSkippableTests maps BuildTargets to the test types that can be skipped for those targets,
-// based on source tree test restrictions.
-func extractSkippableTests(
- sourceTreeCfg *testplans.SourceTreeTestCfg,
- buildReports []*bbproto.Build,
- changeRevs *git.ChangeRevData,
- repoToSrcRoot map[string]string) (map[BuildTarget]map[testType]bool, error) {
-
- skippableTests := make(map[BuildTarget]map[testType]bool)
- for _, report := range buildReports {
- buildTarget := BuildTarget(getBuildTarget(report))
- skippableTests[buildTarget] = make(map[testType]bool)
-
- disableHWTesting, err := canDisableTesting(sourceTreeCfg, report, changeRevs, repoToSrcRoot, hw)
- if err != nil {
- return skippableTests, err
- }
- skippableTests[buildTarget][hw] = disableHWTesting
- disableVMTesting, err := canDisableTesting(sourceTreeCfg, report, changeRevs, repoToSrcRoot, vm)
- if err != nil {
- return skippableTests, err
- }
- skippableTests[buildTarget][vm] = disableVMTesting
- log.Printf("For build %s, got disableHWTesting = %t, disableVMTesting = %t", getBuildTarget(report), disableHWTesting, disableVMTesting)
- }
- return skippableTests, nil
-}
-
// selectBuildForRequirements finds a build that best matches the provided PerTargetTestRequirements.
// e.g. if the requirements want a build for a reef build target, this method will find a successful,
// non-early-terminated build.
@@ -323,61 +278,3 @@
log.Printf("can't test for build target(s) %v because all builders failed\n", buildTargets)
return nil, nil
}
-
-// canDisableTesting determines whether a particular testing type is unnecessary for a given
-// builder, based on source tree test restrictions.
-func canDisableTesting(
- sourceTreeCfg *testplans.SourceTreeTestCfg,
- buildResult *bbproto.Build,
- changeRevs *git.ChangeRevData,
- repoToSrcRoot map[string]string,
- tt testType) (bool, error) {
- if len(buildResult.Input.GerritChanges) == 0 {
- // This happens during postsubmit runs, for example.
- log.Printf("build doesn't contain gerrit_changes %s, so no tests will be skipped", getBuildTarget(buildResult))
- return false, nil
- }
- fileCount := 0
- for _, commit := range buildResult.Input.GerritChanges {
- chRev, err := changeRevs.GetChangeRev(commit.Host, commit.Change, int32(commit.Patchset))
- if err != nil {
- return false, err
- }
- for _, file := range chRev.Files {
- fileCount++
- srcRootMapping, found := repoToSrcRoot[chRev.Project]
- if !found {
- return false, fmt.Errorf("Found no source mapping for project %s", chRev.Project)
- }
- fileSrcPath := fmt.Sprintf("%s/%s", srcRootMapping, file)
- disableTesting, err := canDisableTestingForPath(fileSrcPath, sourceTreeCfg, tt)
- if err != nil {
- return false, err
- }
- if !disableTesting {
- log.Printf("Can't disable %s testing due to file %s", tt, fileSrcPath)
- return false, nil
- }
- }
- }
- // Either testing is disabled for all of the files or there are zero files.
- log.Printf("%s testing is not needed for %d files in the %s build", tt, fileCount, getBuildTarget(buildResult))
- return true, nil
-}
-
-// canDisableTestingForPath determines whether a particular testing type is unnecessary for
-// a given file, based on source tree test restrictions.
-func canDisableTestingForPath(sourceTreePath string, sourceTreeCfg *testplans.SourceTreeTestCfg, tt testType) (bool, error) {
- for _, sourceTreeRestriction := range sourceTreeCfg.SourceTreeTestRestriction {
- testFilter, ok := testTypeFilter[tt]
- if !ok {
- return false, fmt.Errorf("Missing test filter for %v", tt)
- }
- if testFilter(sourceTreeRestriction.TestRestriction) {
- if strings.HasPrefix(sourceTreePath, sourceTreeRestriction.SourceTree.Path) {
- return true, nil
- }
- }
- }
- return false, nil
-}
diff --git a/src/testplans/internal/generator/test_pruner.go b/src/testplans/internal/generator/test_pruner.go
new file mode 100644
index 0000000..0ec61bf
--- /dev/null
+++ b/src/testplans/internal/generator/test_pruner.go
@@ -0,0 +1,119 @@
+// Copyright 2019 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+package generator
+
+import (
+ "fmt"
+ "go.chromium.org/chromiumos/infra/proto/go/testplans"
+ bbproto "go.chromium.org/luci/buildbucket/proto"
+ "log"
+ "strings"
+ "testplans/internal/git"
+)
+
+type testType int
+
+const (
+ hw testType = iota
+ vm
+)
+
+var (
+ testTypeFilter = map[testType]func(testReqs *testplans.TestRestriction) bool{
+ hw: func(testReqs *testplans.TestRestriction) bool { return testReqs.DisableHwTests },
+ vm: func(testReqs *testplans.TestRestriction) bool { return testReqs.DisableVmTests },
+ }
+)
+
+func (tt testType) String() string {
+ return [...]string{"Hw", "Vm"}[tt]
+}
+
+// extractSkippableTests maps BuildTargets to the test types that can be skipped for those targets,
+// based on source tree test restrictions.
+func extractSkippableTests(
+ sourceTreeCfg *testplans.SourceTreeTestCfg,
+ buildReports []*bbproto.Build,
+ changeRevs *git.ChangeRevData,
+ repoToSrcRoot map[string]string) (map[BuildTarget]map[testType]bool, error) {
+
+ skippableTests := make(map[BuildTarget]map[testType]bool)
+ for _, report := range buildReports {
+ buildTarget := BuildTarget(getBuildTarget(report))
+ skippableTests[buildTarget] = make(map[testType]bool)
+
+ disableHWTesting, err := canDisableTesting(sourceTreeCfg, report, changeRevs, repoToSrcRoot, hw)
+ if err != nil {
+ return skippableTests, err
+ }
+ skippableTests[buildTarget][hw] = disableHWTesting
+ disableVMTesting, err := canDisableTesting(sourceTreeCfg, report, changeRevs, repoToSrcRoot, vm)
+ if err != nil {
+ return skippableTests, err
+ }
+ skippableTests[buildTarget][vm] = disableVMTesting
+ log.Printf("For build %s, got disableHWTesting = %t, disableVMTesting = %t", getBuildTarget(report), disableHWTesting, disableVMTesting)
+ }
+ return skippableTests, nil
+}
+
+
+// canDisableTesting determines whether a particular testing type is unnecessary for a given
+// builder, based on source tree test restrictions.
+func canDisableTesting(
+ sourceTreeCfg *testplans.SourceTreeTestCfg,
+ buildResult *bbproto.Build,
+ changeRevs *git.ChangeRevData,
+ repoToSrcRoot map[string]string,
+ tt testType) (bool, error) {
+ if len(buildResult.Input.GerritChanges) == 0 {
+ // This happens during postsubmit runs, for example.
+ log.Printf("build doesn't contain gerrit_changes %s, so no tests will be skipped", getBuildTarget(buildResult))
+ return false, nil
+ }
+ fileCount := 0
+ for _, commit := range buildResult.Input.GerritChanges {
+ chRev, err := changeRevs.GetChangeRev(commit.Host, commit.Change, int32(commit.Patchset))
+ if err != nil {
+ return false, err
+ }
+ for _, file := range chRev.Files {
+ fileCount++
+ srcRootMapping, found := repoToSrcRoot[chRev.Project]
+ if !found {
+ return false, fmt.Errorf("Found no source mapping for project %s", chRev.Project)
+ }
+ fileSrcPath := fmt.Sprintf("%s/%s", srcRootMapping, file)
+ disableTesting, err := canDisableTestingForPath(fileSrcPath, sourceTreeCfg, tt)
+ if err != nil {
+ return false, err
+ }
+ if !disableTesting {
+ log.Printf("Can't disable %s testing due to file %s", tt, fileSrcPath)
+ return false, nil
+ }
+ }
+ }
+ // Either testing is disabled for all of the files or there are zero files.
+ log.Printf("%s testing is not needed for %d files in the %s build", tt, fileCount, getBuildTarget(buildResult))
+ return true, nil
+}
+
+// canDisableTestingForPath determines whether a particular testing type is unnecessary for
+// a given file, based on source tree test restrictions.
+func canDisableTestingForPath(sourceTreePath string, sourceTreeCfg *testplans.SourceTreeTestCfg, tt testType) (bool, error) {
+ for _, sourceTreeRestriction := range sourceTreeCfg.SourceTreeTestRestriction {
+ testFilter, ok := testTypeFilter[tt]
+ if !ok {
+ return false, fmt.Errorf("Missing test filter for %v", tt)
+ }
+ if testFilter(sourceTreeRestriction.TestRestriction) {
+ if strings.HasPrefix(sourceTreePath, sourceTreeRestriction.SourceTree.Path) {
+ return true, nil
+ }
+ }
+ }
+ return false, nil
+}
