Do not drop M-failed-staging-checks prematurely

src/GitHubUtil.js

@@ -86,6 +86,24 @@ function getLabels(prNum) {
     });
 }
 
+function getIssueEvents(prNum) {
+    let params = commonParams();
+    params.issue_number = prNum;
+    return new Promise( (resolve, reject) => {
+        GitHub.authenticate(GitHubAuthentication);
+        GitHub.issues.getEvents(params, async (err, res) => {
+            if (err) {
+                reject(new ErrorContext(err, getIssueEvents.name, params));
+                return;
+            }
+            res = await pager(res);
+            const result = res.data.length;
+            logApiResult(getIssueEvents.name, params, result);
+            resolve(res.data);
+        });
+    });
+}
+
 // Gets PR metadata from GitHub
 // If requested and needed, retries until GitHub calculates PR mergeable flag.
 // Those retries, if any, are limited to a few minutes.
@@ -445,6 +463,7 @@ function getUserEmails() {
 module.exports = {
     getOpenPrs: getOpenPrs,
     getLabels: getLabels,
+    getIssueEvents: getIssueEvents,
     getPR: getPR,
     getReviews: getReviews,
     getStatuses: getStatuses,

src/MergeContext.js

@@ -484,13 +484,24 @@ class PullRequest {
         // GitHub statuses of the staged commit
         this._stagedStatuses = null;
 
+        // GitHub statuses of an abandoned staged commit, if that commit is
+        // equivalent to a staged commit that would have been created right now.
+        // _stagedStatuses and _abandonedStagedStatuses are mutually exclusive
+        this._abandonedStagedStatuses = null;
+
         // GitHub statuses of the PR branch head commit
         this._prStatuses = null;
 
         this._updated = false; // _update() has been called
 
         // truthy value contains a reason for disabling _pushLabelsToGitHub()
         this._labelPushBan = false;
+
+        // Whether the PR has a relevant staged commit that failed its checks.
+        // The relevant commit can be a fresh staged commit, a merged commit (for
+        // still-open PRs) or an abandoned staged commit, if it is equivalent
+        // to a staged commit that would have been created right now.
+        this._stagingFailed = false;
     }
 
     // this PR will need to be reprocessed in this many milliseconds
@@ -637,8 +648,10 @@ class PullRequest {
     }
 
     // returns filled StatusChecks object
-    async _getStagingStatuses() {
-        const combinedStagingStatuses = await GH.getStatuses(this._stagedSha());
+    // recalculates this._stagingFailed value
+    async _getStagingStatuses(stagedSha) {
+        assert(stagedSha);
+        const combinedStagingStatuses = await GH.getStatuses(stagedSha);
         const genuineStatuses = combinedStagingStatuses.statuses.filter(st => !st.description.endsWith(Config.copiedDescriptionSuffix()));
         assert(genuineStatuses.length <= Config.stagingChecks());
         let statusChecks = new StatusChecks(Config.stagingChecks(), "Staging");
@@ -724,8 +737,10 @@ class PullRequest {
         // already checked in _checkForHumanLabels()
         assert(!this._labels.has(Config.failedStagingOtherLabel()));
 
-        if (this._labels.has(Config.failedStagingChecksLabel()))
-            throw this._exObviousFailure("staged commit tests failed");
+        // Do not vainly recreate staged commit which will definitely fail again,
+        // since the PR+base code is yet unchanged and the existing errors still persist
+        if (this._abandonedStagedStatuses && this._abandonedStagedStatuses.failed())
+            throw this._exLabeledFailure("staged commit tests failed", Config.failedStagingChecksLabel());
 
         if (this._wipPr())
             throw this._exObviousFailure("work-in-progress");
@@ -954,12 +969,50 @@ class PullRequest {
         return false;
     }
 
+    async _getMergeCommit() {
+        const mergeSha = await GH.getReference("pull/" + this._prNumber() + "/merge");
+        return await GH.getCommit(mergeSha);
+    }
+
+    // obtains abandoned staging checks
+    async _getAbandonedStagingStatuses() {
+        assert(!this._stagedSha());
+
+        const allEvents = await GH.getIssueEvents(this._prNumber());
+        // staging events are PR events where the bot user created a commit referencing this PR
+        let stagingEvents = allEvents.filter(ev => ev.event === "referenced" && ev.actor.login === Config.githubUserLogin());
+        if (!stagingEvents.length)
+            return;
+
+        // just in case: events should be already in chronological order already
+        stagingEvents = stagingEvents.sort((ev1, ev2) => Date.parse(ev1.created_at) - Date.parse(ev2.created_at));
+        const lastStagingEvent = stagingEvents[stagingEvents.length - 1];
+        const mergeCommit = await this._getMergeCommit();
+        const mergeCommitCreatedAt = Date.parse(mergeCommit.author.date); // for merge commits author.date and committer.date should be the same
+        const lastStagingEventCreatedAt = Date.parse(lastStagingEvent.created_at);
+        assert(mergeCommitCreatedAt);
+        assert(lastStagingEventCreatedAt);
+        // the staging event for merge commit X is always created after X so we do not
+        // need to deal with the lastStagingEventCreatedAt == mergeCommitCreatedAt case
+        assert(lastStagingEventCreatedAt != mergeCommitCreatedAt);
+        if (lastStagingEventCreatedAt < mergeCommitCreatedAt)
+            return; // merge commit created after the last staging event
+
+        // Merge commit created before the last staging event.
+        // Thus, we can get statuses of that event commit SHA.
+        this._abandonedStagedStatuses = await this._getStagingStatuses(lastStagingEvent.commit_id);
+        // TODO: If something made this (previously failed) commit succeed, then
+        // we should use it further, if possible.
+    }
+
     async _loadPrState() {
         if (!this._stagedSha()) {
-            if (await this._mergedSomeTimeAgo())
+            await this._getAbandonedStagingStatuses();
+            if (await this._mergedSomeTimeAgo()) {
                 this._enterMerged();
-            else
-                await this._enterBrewing();
+                return;
+            }
+            await this._enterBrewing();
             return;
         }
 
@@ -979,11 +1032,10 @@ class PullRequest {
 
         assert(this._stagedPosition.ahead());
 
-        const stagedStatuses = await this._getStagingStatuses();
-        // Do not vainly recreate staged commit which will definitely fail again,
-        // since the PR+base code is yet unchanged and the existing errors still persist
+        const stagedStatuses = await this._getStagingStatuses(this._stagedSha());
+        // if staging failed, enter the "brewing (with failed staging tests)" state
         if (stagedStatuses.failed()) {
-            this._labels.add(Config.failedStagingChecksLabel());
+            this._abandonedStagedStatuses = stagedStatuses;
             await this._enterBrewing();
             return;
         }
@@ -1003,7 +1055,8 @@ class PullRequest {
         if (stagedStatuses)
             this._stagedStatuses = stagedStatuses;
         else
-            this._stagedStatuses = await this._getStagingStatuses();
+            this._stagedStatuses = await this._getStagingStatuses(this._stagedSha());
+        this._abandonedStagedStatuses = null;
         this._prStatuses = await this._getPrStatuses();
     }
 
@@ -1184,8 +1237,7 @@ class PullRequest {
 
     async _createStaged() {
         const baseSha = await GH.getReference(this._prBaseBranchPath());
-        const mergeSha = await GH.getReference("pull/" + this._prNumber() + "/merge");
-        const mergeCommit = await GH.getCommit(mergeSha);
+        const mergeCommit = await this._getMergeCommit();
         if (!Config.githubUserName())
             await this._acquireUserProperties();
         let now = new Date();
@@ -1270,6 +1322,19 @@ class PullRequest {
         await this._finalize();
     }
 
+    // Whether the PR has a relevant staged commit that failed its checks.
+    // The relevant commit can be a fresh staged commit, a merged commit (for
+    // still-open PRs) or an abandoned staged commit (if it is equivalent
+    // to a staged commit that would have been created right now)
+    stagingFailed() {
+        assert(!(this._abandonedStagedStatuses && this._stagedStatuses));
+        if (this._abandonedStagedStatuses)
+            return this._abandonedStagedStatuses.failed();
+        else if (this._stagedStatuses)
+            return this._stagedStatuses.failed();
+        return false;
+    }
+
     // Updates and, if possible, advances PR towards (and including) merging.
     // If reprocessing is needed in X milliseconds, returns X.
     // Otherwise, returns null.
@@ -1290,11 +1355,17 @@ class PullRequest {
 
             let result = new ProcessResult();
 
+            if (!this._labels)
+                this._labels = new Labels([], this._prNumber());
+
             if (this._prState && this._prState.staged() && suspended)
                 result.setPrStaged(true);
             else
                 this._removePositiveStagingLabels();
 
+            if (this.stagingFailed())
+                this._labels.add(Config.failedStagingChecksLabel()); // may be set already in the exception
+
             if (knownProblem) {
                 result.setDelayMsIfAny(this._delayMs());
                 return result;
@@ -1303,8 +1374,6 @@ class PullRequest {
             // report this unknown but probably PR-specific problem on GitHub
             // XXX: We may keep redoing this PR every run() step forever, without any GitHub events.
             // TODO: Process Config.failedOtherLabel() PRs last and ignore their failures.
-            if (!this._labels)
-                this._labels = new Labels([], this._prNumber());
 
             if (this._stagedSha()) { // the PR is staged now or was staged some time ago
                 // avoid livelocking