[Autorollers] Check a PR's checks only if it is older than 15 mins
Bug: skia:10477
Change-Id: I08c86da47510e462c7cd60160ff71711bd3cb72d
Reviewed-on: https://skia-review.googlesource.com/c/buildbot/+/304679
Reviewed-by: Eric Boren <borenet@google.com>
Commit-Queue: Ravi Mistry <rmistry@google.com>
diff --git a/autoroll/go/codereview/roll.go b/autoroll/go/codereview/roll.go
index ee4a220..2f7ddec 100644
--- a/autoroll/go/codereview/roll.go
+++ b/autoroll/go/codereview/roll.go
@@ -4,6 +4,7 @@
"context"
"errors"
"fmt"
+ "time"
github_api "github.com/google/go-github/v29/github"
"go.skia.org/infra/autoroll/go/recent_rolls"
@@ -16,6 +17,11 @@
"go.skia.org/infra/go/travisci"
)
+const (
+ // The duration after a PR is created that checks should be looked at.
+ GITHUB_PR_DURATION_FOR_CHECKS = time.Minute * 15
+)
+
type RollImpl interface {
state_machine.RollCLImpl
@@ -327,10 +333,21 @@
if i.IsDryRun {
i.CqFinished = false
i.CqSuccess = false
- // TODO(rmistry): Sometimes the github API does not return the correct number of checks, so this might
- // return some false positives.
- i.DryRunFinished = pullRequest.GetState() == github.CLOSED_STATE || pullRequest.GetMerged() || (len(i.TryResults) > 0 && i.AllTrybotsFinished())
- i.DryRunSuccess = pullRequest.GetMerged() || (i.DryRunFinished && i.AllTrybotsSucceeded())
+ // The roller should not be looking at github checks to determine when the dry run passes,
+ // it should be relying on the project's "commit queue" to do this instead.
+ // Flutter's merge bot does not handle dry runs yet. https://github.com/flutter/flutter/issues/61955
+ // has been filed to add this functionality.
+ //
+ // Sometimes the github API does not return the correct number of checks, try to workaround
+ // this by only looking at a PR if it is > GITHUB_PR_DURATION_FOR_CHECKS old (Flutter's merge
+ // bot waits for 1 hour).
+ if time.Since(pullRequest.GetCreatedAt()) > GITHUB_PR_DURATION_FOR_CHECKS {
+ i.DryRunFinished = pullRequest.GetState() == github.CLOSED_STATE || pullRequest.GetMerged() || (len(i.TryResults) > 0 && i.AllTrybotsFinished())
+ i.DryRunSuccess = pullRequest.GetMerged() || (i.DryRunFinished && i.AllTrybotsSucceeded())
+ } else {
+ i.DryRunFinished = false
+ i.DryRunSuccess = false
+ }
} else {
i.CqFinished = pullRequest.GetState() == github.CLOSED_STATE || pullRequest.GetMerged() || !doesWaitingForTreeLabelExist || (pullRequest.GetMergeableState() == github.MERGEABLE_STATE_DIRTY)
i.CqSuccess = pullRequest.GetMerged()
diff --git a/autoroll/go/codereview/roll_test.go b/autoroll/go/codereview/roll_test.go
index 5c622f4..57dbc43 100644
--- a/autoroll/go/codereview/roll_test.go
+++ b/autoroll/go/codereview/roll_test.go
@@ -672,7 +672,23 @@
require.NoError(t, updateIssueFromGitHubPullRequest(a, pr))
assertdeep.Equal(t, expect, a)
+ // Set Created to now and add try results. Dry run should still be running because
+ // PR is not older than 15 mins.
+ now = time.Now()
+ pr.CreatedAt = &now
+ pr.UpdatedAt = &now
+ expect.Created = now
+ expect.Modified = now
+ a.TryResults = expect.TryResults
+ require.NoError(t, updateIssueFromGitHubPullRequest(a, pr))
+ assertdeep.Equal(t, expect, a)
+
// Dry run failed.
+ before16Mins := now.Add(-16 * time.Minute)
+ pr.CreatedAt = &before16Mins
+ pr.UpdatedAt = &before16Mins
+ expect.Created = before16Mins
+ expect.Modified = before16Mins
expect.DryRunFinished = true
expect.DryRunSuccess = false
expect.Result = autoroll.ROLL_RESULT_DRY_RUN_FAILURE
