[autocorrelator] Add flags to ignore skipped build and/or tests
When checking for build failures, ignore try builds which skipped
building.
When checking for test failures, ignore try builds which skipped
testing.
Bug: 67694
Change-Id: Ic4c46ded825b2c4795478089c54538509f9a4e44
Reviewed-on: https://fuchsia-review.googlesource.com/c/infra/infra/+/472079
Fuchsia-Auto-Submit: Anthony Fandrianto <atyfto@google.com>
Commit-Queue: Auto-Submit <auto-submit@fuchsia-infra.iam.gserviceaccount.com>
Reviewed-by: Anirudh Mathukumilli <rudymathu@google.com>
diff --git a/cmd/autocorrelator/check_try.go b/cmd/autocorrelator/check_try.go
index 705632b..7459a18 100644
--- a/cmd/autocorrelator/check_try.go
+++ b/cmd/autocorrelator/check_try.go
@@ -18,6 +18,7 @@
"go.chromium.org/luci/auth"
buildbucketpb "go.chromium.org/luci/buildbucket/proto"
"go.chromium.org/luci/grpc/prpc"
+ "go.chromium.org/luci/luciexe/exe"
"google.golang.org/genproto/protobuf/field_mask"
"go.fuchsia.dev/infra/buildbucket"
@@ -47,20 +48,20 @@
buildStatus int
searchRange int
summaryMarkdownPath string
+ ignoreSkippedBuild bool
+ ignoreSkippedTests bool
jsonOutput string
}
func (c *checkTryRun) Init(defaultAuthOpts auth.Options) {
c.commonFlags.Init(defaultAuthOpts)
- // TODO(atyfto): Add a ChangeId input and filter out tryjobs which match
- // the ChangeId. Failures caused by the same change should be ignored,
- // otherwise we can falsely imply systemic failure when a single CL is
- // responsible for many failures.
c.Flags.Var(&c.builder, "builder", "Fully-qualified Buildbucket try builder name to inspect.")
c.Flags.Int64Var(&c.changeNum, "change-num", 0, "Change number to filter out.")
c.Flags.IntVar(&c.buildStatus, "build-status", 20, "Build status to filter on. Defaults to FAILURE.")
c.Flags.IntVar(&c.searchRange, "search-range", 10, "Inspect up to this many recent builds.")
c.Flags.StringVar(&c.summaryMarkdownPath, "summary-markdown-path", "", "Path to summary markdown input file.")
+ c.Flags.BoolVar(&c.ignoreSkippedBuild, "ignore-skipped-build", false, "Whether to ignore builds with unaffected build graphs.")
+ c.Flags.BoolVar(&c.ignoreSkippedTests, "ignore-skipped-tests", false, "Whether to ignore builds which skipped testing.")
c.Flags.StringVar(&c.jsonOutput, "json-output", "-", "Path to output finding to.")
}
@@ -94,13 +95,35 @@
// The `comparator` argument is a text-based comparator that returns a
// similarity score between the input summary markdown and a build's summary
// markdown.
-func checkTry(builds []*buildbucketpb.Build, changeNum int64, status buildbucketpb.Status, summaryMarkdown string, comparator compare.TextComparator) []*findings.SummarySimilarity {
+func checkTry(builds []*buildbucketpb.Build, changeNum int64, status buildbucketpb.Status, summaryMarkdown string, comparator compare.TextComparator, ignoreSkippedBuild, ignoreSkippedTests bool) []*findings.SummarySimilarity {
var fs []*findings.SummarySimilarity
for _, build := range builds {
// If we find a build with an identical change number, skip it.
if changeNum == build.Input.GerritChanges[0].Change {
continue
}
+ // Conditionally ignore builds with unaffected build graph, e.g. when
+ // checking for build failures.
+ if ignoreSkippedBuild {
+ var skipped bool
+ exe.ParseProperties(build.Output.Properties, map[string]interface{}{
+ "skipped_because_unaffected": &skipped,
+ })
+ if skipped {
+ continue
+ }
+ }
+ // Conditionally ignore builds which skipped testing, e.g. when checking
+ // for test failures.
+ if ignoreSkippedTests {
+ var noWork bool
+ exe.ParseProperties(build.Output.Properties, map[string]interface{}{
+ "affected_tests_no_work": &noWork,
+ })
+ if noWork {
+ continue
+ }
+ }
// If we encounter a green build, add the finding: the existence of a
// green build should reduce the confidence we have in "try" being
// systemically busted.
@@ -148,6 +171,7 @@
Fields: &field_mask.FieldMask{Paths: []string{
"builds.*.id",
"builds.*.input.gerrit_changes",
+ "builds.*.output.properties",
"builds.*.status",
"builds.*.summary_markdown",
}},
@@ -166,7 +190,7 @@
if err != nil {
return fmt.Errorf("could not read summary markdown input: %v", err)
}
- fs := checkTry(builds, c.changeNum, buildbucketpb.Status(c.buildStatus), string(summaryMarkdown), compare.LevenshteinComparator{Opts: levenshtein.DefaultOptions})
+ fs := checkTry(builds, c.changeNum, buildbucketpb.Status(c.buildStatus), string(summaryMarkdown), compare.LevenshteinComparator{Opts: levenshtein.DefaultOptions}, c.ignoreSkippedBuild, c.ignoreSkippedTests)
// Emit summary similarity findings to -json-output.
out := os.Stdout
diff --git a/cmd/autocorrelator/check_try_test.go b/cmd/autocorrelator/check_try_test.go
index 9a003e2..95c1364 100644
--- a/cmd/autocorrelator/check_try_test.go
+++ b/cmd/autocorrelator/check_try_test.go
@@ -7,6 +7,7 @@
import (
"testing"
+ structpb "github.com/golang/protobuf/ptypes/struct"
buildbucketpb "go.chromium.org/luci/buildbucket/proto"
"go.fuchsia.dev/infra/cmd/autocorrelator/findings"
@@ -17,11 +18,13 @@
t.Parallel()
var tests = []struct {
- builds []*buildbucketpb.Build
- changeNum int64
- status buildbucketpb.Status
- summaryMarkdown string
- expected []*findings.SummarySimilarity
+ builds []*buildbucketpb.Build
+ changeNum int64
+ status buildbucketpb.Status
+ summaryMarkdown string
+ ignoreSkippedBuild bool
+ ignoreSkippedTests bool
+ expected []*findings.SummarySimilarity
}{
{
builds: []*buildbucketpb.Build{
@@ -36,6 +39,11 @@
},
},
},
+ Output: &buildbucketpb.Build_Output{
+ Properties: &structpb.Struct{
+ Fields: map[string]*structpb.Value{},
+ },
+ },
},
{
Id: int64(999998),
@@ -48,10 +56,63 @@
},
},
},
+ Output: &buildbucketpb.Build_Output{
+ Properties: &structpb.Struct{
+ Fields: map[string]*structpb.Value{},
+ },
+ },
+ },
+ // This has an unaffected build graph, and should be ignored.
+ {
+ Id: int64(999997),
+ Status: buildbucketpb.Status_SUCCESS,
+ SummaryMarkdown: "",
+ Input: &buildbucketpb.Build_Input{
+ GerritChanges: []*buildbucketpb.GerritChange{
+ {
+ Change: int64(222222),
+ },
+ },
+ },
+ Output: &buildbucketpb.Build_Output{
+ Properties: &structpb.Struct{
+ Fields: map[string]*structpb.Value{
+ "skipped_because_unaffected": {
+ Kind: &structpb.Value_BoolValue{
+ BoolValue: true,
+ },
+ },
+ },
+ },
+ },
+ },
+ // This has no affected tests, and should be ignored.
+ {
+ Id: int64(999996),
+ Status: buildbucketpb.Status_SUCCESS,
+ SummaryMarkdown: "",
+ Input: &buildbucketpb.Build_Input{
+ GerritChanges: []*buildbucketpb.GerritChange{
+ {
+ Change: int64(222222),
+ },
+ },
+ },
+ Output: &buildbucketpb.Build_Output{
+ Properties: &structpb.Struct{
+ Fields: map[string]*structpb.Value{
+ "affected_tests_no_work": {
+ Kind: &structpb.Value_BoolValue{
+ BoolValue: true,
+ },
+ },
+ },
+ },
+ },
},
// This matches the input change number, and should be ignored.
{
- Id: int64(999998),
+ Id: int64(999995),
Status: buildbucketpb.Status_FAILURE,
SummaryMarkdown: "ERROR",
Input: &buildbucketpb.Build_Input{
@@ -61,11 +122,16 @@
},
},
},
+ Output: &buildbucketpb.Build_Output{
+ Properties: &structpb.Struct{
+ Fields: map[string]*structpb.Value{},
+ },
+ },
},
// This does not match the input status of FAILURE, and should
// be ignored.
{
- Id: int64(999997),
+ Id: int64(999994),
Status: buildbucketpb.Status_INFRA_FAILURE,
SummaryMarkdown: "ERROR: checkout failed",
Input: &buildbucketpb.Build_Input{
@@ -75,11 +141,18 @@
},
},
},
+ Output: &buildbucketpb.Build_Output{
+ Properties: &structpb.Struct{
+ Fields: map[string]*structpb.Value{},
+ },
+ },
},
},
- status: buildbucketpb.Status_FAILURE,
- changeNum: int64(123456),
- summaryMarkdown: "ERROR",
+ status: buildbucketpb.Status_FAILURE,
+ changeNum: int64(123456),
+ summaryMarkdown: "ERROR",
+ ignoreSkippedBuild: true,
+ ignoreSkippedTests: true,
expected: []*findings.SummarySimilarity{
{
Score: 0.0,
@@ -97,7 +170,7 @@
for _, test := range tests {
comparator := mockComparator{}
- ss := checkTry(test.builds, test.changeNum, test.status, test.summaryMarkdown, comparator)
+ ss := checkTry(test.builds, test.changeNum, test.status, test.summaryMarkdown, comparator, test.ignoreSkippedBuild, test.ignoreSkippedTests)
if diff := cmp.Diff(test.expected, ss); diff != "" {
t.Fatalf("different (-want +got):\n%s", diff)
}
