[autocorrelator] Filter out try builds with matching change num
Per go/ci-cq-auto-correlator, ignore try builds which originate from
the same change as -change-num.
In practice, this means that if a tryjob fails on fxrev.dev/123456, and
previously failed on the same change before with the same failure
message, that failure should not increase the confidence of a false
negative.
Bug: 67694
Change-Id: I118f15ef6feb8ff4a0e533037ff55327337c9d02
Reviewed-on: https://fuchsia-review.googlesource.com/c/infra/infra/+/471665
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 6f39f20..705632b 100644
--- a/cmd/autocorrelator/check_try.go
+++ b/cmd/autocorrelator/check_try.go
@@ -27,7 +27,7 @@
func cmdCheckTry(authOpts auth.Options) *subcommands.Command {
return &subcommands.Command{
- UsageLine: "check-try -builder <project/bucket/builder> -build-status <build-status-code> -search-range <search-range> -summary-markdown-path <path/to/summary/markdown> -json-output <json-output>",
+ UsageLine: "check-try -builder <project/bucket/builder> -change-num <change-num> -build-status <build-status-code> -search-range <search-range> -summary-markdown-path <path/to/summary/markdown> -json-output <json-output>",
ShortDesc: "Compare text similarity between a summary markdown and recent try builds' summary markdowns.",
LongDesc: "Compare text similarity between the provided -summary-markdown and the summary markdown of -search-range recent builds of a try -builder.",
CommandRun: func() subcommands.CommandRun {
@@ -43,6 +43,7 @@
// TODO(atyfto): Fix the buildbucket library to return a BuilderID instead
// of a flag.
builder buildbucket.BuilderIDFlag
+ changeNum int64
buildStatus int
searchRange int
summaryMarkdownPath string
@@ -56,6 +57,7 @@
// 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.")
@@ -69,6 +71,9 @@
if &c.builder == nil {
return errors.New("-builder is required")
}
+ if c.changeNum == 0 {
+ return errors.New("-change-num is required")
+ }
if c.summaryMarkdownPath == "" {
return errors.New("-summary-markdown-path is required")
}
@@ -89,9 +94,13 @@
// 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, 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) []*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
+ }
// 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.
@@ -138,6 +147,7 @@
},
Fields: &field_mask.FieldMask{Paths: []string{
"builds.*.id",
+ "builds.*.input.gerrit_changes",
"builds.*.status",
"builds.*.summary_markdown",
}},
@@ -156,7 +166,7 @@
if err != nil {
return fmt.Errorf("could not read summary markdown input: %v", err)
}
- fs := checkTry(builds, 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})
// 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 2a1ed0d..9a003e2 100644
--- a/cmd/autocorrelator/check_try_test.go
+++ b/cmd/autocorrelator/check_try_test.go
@@ -18,6 +18,7 @@
var tests = []struct {
builds []*buildbucketpb.Build
+ changeNum int64
status buildbucketpb.Status
summaryMarkdown string
expected []*findings.SummarySimilarity
@@ -28,11 +29,38 @@
Id: int64(999999),
Status: buildbucketpb.Status_SUCCESS,
SummaryMarkdown: "",
+ Input: &buildbucketpb.Build_Input{
+ GerritChanges: []*buildbucketpb.GerritChange{
+ {
+ Change: int64(111111),
+ },
+ },
+ },
},
{
Id: int64(999998),
Status: buildbucketpb.Status_FAILURE,
SummaryMarkdown: "ERROR",
+ Input: &buildbucketpb.Build_Input{
+ GerritChanges: []*buildbucketpb.GerritChange{
+ {
+ Change: int64(222222),
+ },
+ },
+ },
+ },
+ // This matches the input change number, and should be ignored.
+ {
+ Id: int64(999998),
+ Status: buildbucketpb.Status_FAILURE,
+ SummaryMarkdown: "ERROR",
+ Input: &buildbucketpb.Build_Input{
+ GerritChanges: []*buildbucketpb.GerritChange{
+ {
+ Change: int64(123456),
+ },
+ },
+ },
},
// This does not match the input status of FAILURE, and should
// be ignored.
@@ -40,9 +68,17 @@
Id: int64(999997),
Status: buildbucketpb.Status_INFRA_FAILURE,
SummaryMarkdown: "ERROR: checkout failed",
+ Input: &buildbucketpb.Build_Input{
+ GerritChanges: []*buildbucketpb.GerritChange{
+ {
+ Change: int64(333333),
+ },
+ },
+ },
},
},
status: buildbucketpb.Status_FAILURE,
+ changeNum: int64(123456),
summaryMarkdown: "ERROR",
expected: []*findings.SummarySimilarity{
{
@@ -61,7 +97,7 @@
for _, test := range tests {
comparator := mockComparator{}
- ss := checkTry(test.builds, test.status, test.summaryMarkdown, comparator)
+ ss := checkTry(test.builds, test.changeNum, test.status, test.summaryMarkdown, comparator)
if diff := cmp.Diff(test.expected, ss); diff != "" {
t.Fatalf("different (-want +got):\n%s", diff)
}
