diff --git a/README.md b/README.md index 97635af..aa70067 100644 --- a/README.md +++ b/README.md @@ -36,10 +36,10 @@ are satisfied: text displayed by GitHub for these so called "mergeable" PRs varies depending on whether the changes are approved and whether the PR branch is out of date with its target branch. -* All the _required_ checks have succeeded on the PR branch: - * If _all_ checks have succeeded, then GitHub says "All checks have - passed" next to a green check mark: - ![](./docs/images/all_passed.png) +* All the _required_ checks have succeeded on the PR branch. + * _all_ checks, except for the special-purpose "automated merge status" + test (see below) have succeeded: + ![](./docs/images/automated_status_pending.png) * If any _optional_ checks have failed, then GitHub will show "Some checks were not successful" message: ![](./docs/images/required_passed.png) @@ -90,6 +90,12 @@ algorithm: If the bot is killed while executing the above algorithm, it will resume from the beginning of the unfinished step. +The above algorithm can be divided into three merging phases: + +1. pre-staging: step 1 +2. staging: step 2 and 3 +3. merged: step 4 and 5 + While the bot is merging a pull request, it does not touch other PRs. If the bot receives a GitHub event while merging a pull request, then the bot re-examines all PRs _after_ it is done merging the current PR. This @@ -112,6 +118,73 @@ If a PR processing step fails for PR-specific reasons (e.g., a CI test failure), then the bot moves on to the next pull request, labeling the failed PR if/as needed (see below for PR labels). +## Automated merge status + +The bot adds this required status automatically to PR and staging commit. +During PR lifecycle the status is "pending" or "failure", thus preventing +the manual merge button on the GitHub PR page from becoming green. Anubis +uses this mechanism to forbid manual merges, which result in wrong commit +messages and missed staging checks. The bot satisfies this check just before +merging (for the staging commit) and just after merging (for PR). + +Depending on the current PR merging phase, an encountered problem or event +(such as approval), the automated status is assigned a status (failure, pending, or +success) and is supplied with a description, consisting of the problem-specific +PR label and message, detailing it. + +Two tables below outlines possible states and messages for both commits. + +PR HEAD commit: + +*state* | *label* | *message* | *phase* | +--- | --- | --- | --- +failure | - | GitHub will not be able to merge | pre-staging +failure | - | failed PR tests | pre-staging +failure | M-failed-staging-other | an unexpected error during staging some time ago | pre-staging +failure | M-failed-staging-checks | staged commit tests failed some time ago (3) | pre-staging +failure | M-failed-description | invalid commit message | pre-staging +pending | - | waiting on WIP | pre-staging +pending | - | waiting for approval | pre-staging +pending | - | waiting for objections | pre-staging +pending | - | waiting for PR tests | pre-staging +pending | - | waiting for another staged PR | pre-staging +pending | - | staged at SHA | staging +success | M-merged | - | merged + +Staging commit: + +*state* | *label* | *message* | *phase* | +--- | --- | --- | --- +failure | - | failed PR tests | staging +failure | M-failed-staging-other | an unexpected error during staging | staging +failure | M-failed-staging-checks | staged commit tests have failed | staging +failure | M-failed-description | invalid commit message | staging +failure | - | staged commit tests are stale (1) | staging +pending | - | waiting on WIP | staging +pending | - | waiting for approval | staging +pending | - | waiting for objections | staging +pending | - | waiting for PR tests | staging +pending | - | waiting for the dry-run mode to end | staging +pending | - | waiting for staging-only mode to end | staging +pending | - | waiting for staging tests (...) (2)| staging +success | - | will be merged as SHA | staging +success | M-merged | - | merged + +(1): Staged commit tests become stale when either the PR state changes (e.g., +the author commits new code) or the staging_branch is unexpectedly modified. +The status lets the developer know why Anubis ignored (failed, unfinished, or +successful) staging tests. + +(2): The (...) in the table above denotes specific information about the number +of tests succeeded/pending/failed so far, e.g.: +![](./docs/images/automated_status_details.png) + +(3): This state means that the bot will ignore this PR (even if the unterlying +problem has been fixed) until the M-ffailed-staging-checks is manually removed. +Please also see the description of M-failed-staging-checks below. + +In some cases (usually errors), the message is prefixed with a label, e.g.: +![](./docs/images/automated_status_problem.png) ## Pull request labels @@ -140,14 +213,6 @@ request state: mode (see `config::staged_run` and `config::guarded_run`). The bot removes this label when either the PR was successfully merged or its staging results are no longer fresh/applicable. -* `M-abandoned-staging-checks`: Anubis discovered a stale _staging commit_. - This usually happens when either the PR state changes (e.g., the author - commits new code) or the staging_branch is unexpectedly modified. After - labeling the PR, the bot ignores any failed or unfinished tests associated - with that stale commit. This label was added so that the developer observing - the PR page on GitHub knows why Anubis ignored (failed, unfinished, or - successful) staging tests. The bot removes this (usually short-lived) label - after creating a fresh staging commit for the PR. * `M-failed-staging-checks`: Essentially duplicates GitHub "red x" mark for the _staging commit_. The bot does not attempt to merge this PR again until a human decides that this problem is resolved and removes @@ -296,8 +361,9 @@ All configuration fields are required. *sufficient_approvals* | The minimal number of core developers required for a PR to be merged fast (i.e., without waiting for `config::voting_delay_max`) | 2 *voting_delay_min*| The minimum merging age of a PR. Younger PRs are not merged, regardless of the number of votes. The PR age string should comply with [timestring](https://github.com/mike182uk/timestring) parser. | "2d" *voting_delay_max* | The maximum merging age of a PR that has fewer than `config::sufficient_approvals` votes. The PR age string should comply with [timestring](https://github.com/mike182uk/timestring) parser. | "10d" -*staging_checks*| The expected number of CI tests executed against the staging branch. | 2 +*staging_checks*| The expected number of checks executed against the staging branch. This includes CI checks and bot-generated checks. | 2 *approval_url*| The URL associated with an approval status test description. | "" +*automated_merge_url*| The URL associated with an automated merge status test description. | "" *logger_params* | A JSON-formatted parameter list for the [Bunyan](https://github.com/trentm/node-bunyan) logging library [constructor](https://github.com/trentm/node-bunyan#constructor-api). | 
{
    "name": "anubis",
    "streams": [ ... ]
}
TODO: Merge all three "mutually exclusive" boolean `*_run` options into one `run_mode` option accepting on of four mode names, including "production". Document individual string values in a separate table (here). diff --git a/docs/images/automated_status_details.png b/docs/images/automated_status_details.png new file mode 100644 index 0000000..3b29004 Binary files /dev/null and b/docs/images/automated_status_details.png differ diff --git a/docs/images/automated_status_pending.png b/docs/images/automated_status_pending.png new file mode 100644 index 0000000..b55f749 Binary files /dev/null and b/docs/images/automated_status_pending.png differ diff --git a/docs/images/automated_status_problem.png b/docs/images/automated_status_problem.png new file mode 100644 index 0000000..150e13d Binary files /dev/null and b/docs/images/automated_status_problem.png differ diff --git a/docs/images/required_passed.png b/docs/images/required_passed.png index 78f83df..edc7226 100644 Binary files a/docs/images/required_passed.png and b/docs/images/required_passed.png differ diff --git a/src/Config.js b/src/Config.js index 1e333cf..5e6b3b4 100644 --- a/src/Config.js +++ b/src/Config.js @@ -32,6 +32,7 @@ class ConfigOptions { this._stagingChecks = conf.staging_checks; this._loggerParams = conf.logger_params; this._approvalUrl = conf.approval_url; + this._automatedMergeUrl = conf.automated_merge_url; // unused this._githubUserNoreplyEmail = null; @@ -81,6 +82,12 @@ class ConfigOptions { votingDelayMax() { return this._votingDelayMax; } votingDelayMin() { return this._votingDelayMin; } stagingChecks() { return this._stagingChecks; } + // the number of staged commit checks supplied by CIs + stagingChecksCI() { return this._stagingChecks - this.derivativeRequiredChecks() - this.copiedRequiredChecks(); } + // the number of 'internal' checks + derivativeRequiredChecks() { return this.manageApprovalStatus() + this.manageAutomatedMergeStatus(); } + // the number of checks copied from PR commit to staged commit + copiedRequiredChecks() { return 1; } loggerParams() { return this._loggerParams; } // an unexpected error occurred outside the "staged" phase @@ -99,8 +106,6 @@ class ConfigOptions { failedDescriptionLabel() { return "M-failed-description"; } // allows target branch update in 'guarded_run' mode clearedForMergeLabel() { return "M-cleared-for-merge"; } - // whether the PR was abandoned due to a stale staged commit - abandonedStagingChecksLabel() { return "M-abandoned-staging-checks"; } // an URL of the description of the approval test status approvalUrl() { return this._approvalUrl; } @@ -112,6 +117,15 @@ class ConfigOptions { approvalContext() { return "PR approval"; } copiedDescriptionSuffix() { return " (copied from PR by Anubis)"; } + + // an URL of the description of the automated merge test status + automatedMergeStatusUrl() { return this._automatedMergeUrl; } + + // the 'context name' of the automated merge test status + automatedMergeStatusContext() { return "Automated merge"; } + + // whether the bot will create the automated merge test statuses for PR and staged commit + manageAutomatedMergeStatus() { return this.automatedMergeStatusUrl().length > 0; } } const configFile = process.argv.length > 2 ? process.argv[2] : './config.json'; diff --git a/src/MergeContext.js b/src/MergeContext.js index a1c3084..1aff244 100644 --- a/src/MergeContext.js +++ b/src/MergeContext.js @@ -46,6 +46,7 @@ class ProcessResult // Exception Abandon Push-Labels Result-of-process() // _exLostControl() yes no approval delay (if any) // _exObviousFailure() yes yes approval delay (if any) +// _exWaiting() yes yes approval delay (if any) // _exLabeledFailure() yes yes approval delay (if any) // _exSuspend() no yes approval delay (if any) // any-unlisted-above yes yes exception + M-failed-other @@ -59,20 +60,37 @@ class ProcessResult // Other exceptions get Config.failedOtherLabel() added automatically. // Babel does not support extending Error; this class requires node v8+. class PrProblem extends Error { - constructor(message, ...params) { + constructor(aStatus, message, ...params) { super(message, ...params); this.name = this.constructor.name; + // the automated merge status associated with this problem, + // "failure", "pending" or "success" + this._status = aStatus; Error.captureStackTrace(this, this.constructor); this._keepStaged = false; // catcher should preserve the staged commit + this._label = null; // the label associated with this problem + // whether there is some indication on GitHub that this problem happened + // some time ago and is still unresolved + this.alreadyExists = false; } keepStagedRequested() { return this._keepStaged; } + status() { return this._status; } + + setLabel(aLabel) { this._label = aLabel; } + requestToKeepStaged() { assert(!this.keepStagedRequested()); this._keepStaged = true; } + + toString() { + return this._label ? + this._label + ' (' + this.message + ')' : + this.message; + } } // Contains properties used for approval test status creation @@ -142,25 +160,93 @@ class StatusCheck success() { return this.state === 'success'; } pending() { return this.state === 'pending'; } + + equals(check) { + return this.context.trim() === check.context.trim() && + this.state === check.state && + this.targetUrl === check.targetUrl && + this.description === check.description; + } } -// aggregates status checks for a PR or commit -class StatusChecks +// aggregates a list of 'internal' (i.e., bot-reported) status checks for a PR or staged commit +class DerivedStatusChecks { - // TODO: we should distinguish CI-reported (genuine) statuses from - // bot-reported (derivative) statuses, such as approvals. - // For example, failed() should count only genuine statuses, whereas - // final() should count all statuses. + constructor(context) { + this._context = context; + // a list of internal StatusChecks existing on GitHub + this._checks = []; + } + // whether this check is present on GitHub + has(check) { return this._checks.some(el => el.equals(check)); } + + // adds a check that is present on GitHub + import(check) { + if (!Config.manageAutomatedMergeStatus()) + return false; + if (!DerivedStatusChecks.Has(check.context)) + return false; + this._checks = this._checks.filter(el => el.context !== check.context); + this._checks.push(check); + return true; + } + + // creates the 'automated merge' status check + static CreateAutomated(state, description) { + assert(state); + assert(description); + const raw = { + state: state, + target_url: Config.automatedMergeStatusUrl(), + description: description, + context: Config.automatedMergeStatusContext() + }; + return new StatusCheck(raw); + } + + // creates the 'approval' status check from Approval + static CreateApproval(approval) { + assert(approval); + const raw = { + state: approval.state, + target_url: Config.approvalUrl(), + description: approval.description, + context: Config.approvalContext() + }; + return new StatusCheck(raw); + } + + // whether the context belongs to an internal status check + static Has(context) { + return (Config.manageApprovalStatus() && context === Config.approvalContext()) || + (Config.manageAutomatedMergeStatus() && context === Config.automatedMergeStatusContext()); + } + + toString() { + let statuses = ""; + for (let s of this._checks) { + if (statuses) + statuses += ", "; + statuses += s.context + ": " + s.state + "(" + s.description + ")"; + } + return "context: " + this._context + " checks: " + statuses; + } +} + +// aggregates 'external' (i.e., reported by a CI) status checks for a PR or staged commit +// does not include bot-reported 'internal' checks (approvals, etc.) +class StatusChecks +{ // expectedStatusCount: - // for staged commits: the bot-configured number of required checks (Config.stagingChecks()), - // for PR commits: GitHub configured number of required checks (requested from GitHub) + // for staged commits: the bot-configured number of required(external) checks (Config.stagingChecksCI()), + // for PR commits: GitHub configured number of required(external) checks (requested from GitHub) // context: either "PR" or "Staging"; constructor(expectedStatusCount, context) { assert(expectedStatusCount !== undefined); assert(expectedStatusCount !== null); assert(context); - this.expectedStatusCount = expectedStatusCount; + this._expectedStatusCount = expectedStatusCount; this.context = context; this.requiredStatuses = []; this.optionalStatuses = []; @@ -183,40 +269,15 @@ class StatusChecks this.optionalStatuses.some(el => el.context.trim() === context.trim()); } - hasApprovalStatus(approval) { - return this.requiredStatuses.some(el => - el.context.trim() === Config.approvalContext() && - el.state === approval.state && - el.description === approval.description); - } - - setApprovalStatus(approval) { - this.requiredStatuses = this.requiredStatuses.filter(st => st.context !== Config.approvalContext()); - let raw = { - state: approval.state, - target_url: Config.approvalUrl(), - description: approval.description, - context: Config.approvalContext() - }; - this.addRequiredStatus(new StatusCheck(raw)); - } - // no more required status changes or additions are expected final() { - return (this.requiredStatuses.length >= this.expectedStatusCount) && + return (this.requiredStatuses.length >= this._expectedStatusCount) && this.requiredStatuses.every(check => !check.pending()); } // something went wrong with at least one of the required status checks failed() { - return this._failedExcept(Config.approvalContext()); - } - - // Whether at least one of the required status checks failed. - // Ignores checks with the given context, when searching. - _failedExcept(context) { - const filteredChecks = this.requiredStatuses.filter(st => context ? st.context !== context : true); - return filteredChecks.some(check => check.failed()); + return this.requiredStatuses.some(check => check.failed()); } // the results are final and all checks were a success @@ -224,8 +285,15 @@ class StatusChecks return this.final() && this.requiredStatuses.every(check => check.success()); } + progressString() { + const succeeded = this.requiredStatuses.filter(check => check.success()); + const pending = this.requiredStatuses.filter(check => check.pending()); + const failed = this.requiredStatuses.filter(check => check.failed()); + return succeeded.length + " succeeded, " + pending.length + " pending, " + failed.length + " failed out of " + this._expectedStatusCount; + } + toString() { - let combinedStatus = "context: " + this.context + " expected/required/optional: " + this.expectedStatusCount + "/" + + let combinedStatus = "context: " + this.context + " expected/required/optional: " + this._expectedStatusCount + "/" + this.requiredStatuses.length + "/" + this.optionalStatuses.length + ", combined: "; if (this.failed()) combinedStatus += "failure"; @@ -481,12 +549,18 @@ class PullRequest { // while unexpected, PR merging and closing is not prohibited when staging is this._stagingBanned = banStaging; - // GitHub statuses of the staged commit + // GitHub 'external' statuses of the staged commit this._stagedStatuses = null; - // GitHub statuses of the PR branch head commit + // GitHub 'external' statuses of the PR branch head commit this._prStatuses = null; + // GitHub 'internal' statuses of the staged commit + this._derivedStagedStatuses = null; + + // GitHub 'internal' statuses of the PR branch head commit + this._derivedPrStatuses = null; + this._updated = false; // _update() has been called // truthy value contains a reason for disabling _pushLabelsToGitHub() @@ -580,23 +654,118 @@ class PullRequest { if (!Config.manageApprovalStatus()) return; - assert(this._prStatuses); + await this._createApprovalStatus(this._prHeadSha()); + if (this._stagedSha()) + await this._createApprovalStatus(this._stagedSha()); + } + + async _createApprovalStatus(sha) { + if (this._dryRun("creating approval status")) + return; + + assert(sha); + let derivedStatuses = this._derivedStatuses(sha); + assert(derivedStatuses); - if (!this._prStatuses.hasApprovalStatus(this._approval)) { - await this._createApprovalStatus(this._prHeadSha()); - this._prStatuses.setApprovalStatus(this._approval); + const approvalCheck = DerivedStatusChecks.CreateApproval(this._approval); + if (!derivedStatuses.has(approvalCheck)) { + await GH.createStatus(sha, this._approval.state, Config.approvalUrl(), this._approval.description, Config.approvalContext()); + derivedStatuses.import(approvalCheck); } + } - if (this._stagedStatuses && !this._stagedStatuses.hasApprovalStatus(this._approval)) { - await this._createApprovalStatus(this._stagedSha()); - this._stagedStatuses.setApprovalStatus(this._approval); + _derivedStatuses(sha) { + return (sha === this._prHeadSha()) ? this._derivedPrStatuses : this._derivedStagedStatuses; + } + + _waitingForStagingTestsMsg() { + assert(this._stagedStatuses); + return "waiting for staging tests (" + this._stagedStatuses.progressString() + ")"; + } + + async _setAutomatedStepBrewing() { + assert(this._prState.brewing()); + const prCheck = DerivedStatusChecks.CreateAutomated("pending", "not staged yet"); + await this._applyAutomatedStatus(prCheck, this._prHeadSha()); + // staged commit does not exist yet + } + + async _setAutomatedStepStaged() { + assert(this._prState.staged()); + const prCheck = DerivedStatusChecks.CreateAutomated("pending", "staged at " + this._stagedShaBrief()); + await this._applyAutomatedStatus(prCheck, this._prHeadSha()); + + assert(this._stagedStatuses); + if (!this._stagedStatuses.final() && !this._stagedStatuses.failed()) { + const stagedCheck = DerivedStatusChecks.CreateAutomated("pending", this._waitingForStagingTestsMsg()); + await this._applyAutomatedStatus(stagedCheck, this._stagedSha()); } + // other cases (errors or final_success) are handled elsewhere } - async _createApprovalStatus(sha) { - if (this._dryRun("creating approval status")) + async _setAutomatedStepMerged() { + assert(this._prState.merged()); + const check = DerivedStatusChecks.CreateAutomated("success", Config.mergedLabel()); + await this._applyAutomatedStatus(check, this._prHeadSha()); + if (this._stagedSha()) + await this._applyAutomatedStatus(check, this._stagedSha()); + } + + async _setAutomatedForKnownProblem(problem) { + // for a recurrent problem the statuses must have been already created + if (problem.alreadyExists) return; - await GH.createStatus(sha, this._approval.state, Config.approvalUrl(), this._approval.description, Config.approvalContext()); + + // the PR state and statuses must be already loaded in a case + // of a known (and not recurrent) problem + assert(this._prState); + assert(this._derivedPrStatuses); + if (this._stagedSha()) + assert(this._derivedStagedStatuses); + + this._setAutomatedForProblem(problem); + } + + async _setAutomatedForUnexpectedProblem(problem) { + assert(!problem.alreadyExists); + this._setAutomatedForProblem(problem); + } + + async _setAutomatedForProblem(problem) { + const prCheck = DerivedStatusChecks.CreateAutomated(problem.status(), + (this._prState && this._prState.staged()) ? "staged at " + this._stagedShaBrief() : problem.toString()); + await this._applyAutomatedStatus(prCheck, this._prHeadSha()); + + if (this._stagedSha()) { + const stagedCheck = DerivedStatusChecks.CreateAutomated(problem.status(), problem.toString()); + await this._applyAutomatedStatus(stagedCheck, this._stagedSha()); + } + } + + async _setAutomatedForStaged(state, description) { + assert(this._stagedSha()); + const check = DerivedStatusChecks.CreateAutomated(state, description); + await this._applyAutomatedStatus(check, this._stagedSha()); + } + + async _applyAutomatedStatus(check, sha) { + if (!Config.manageAutomatedMergeStatus()) + return; + + assert(DerivedStatusChecks.Has(check.context)); + + if (this._dryRun("creating automated merge status")) + return; + + let derivedStatuses = this._derivedStatuses(sha); + // Statuses may be still empty in a case of an unexpected error. + // We should avoid updating the automated status until this general + // (and possibly PR-unrelated) problem is resolved. + if (!derivedStatuses || derivedStatuses.has(check)) + return; + + await GH.createStatus(sha, check.state, check.targetUrl, check.description, check.context); + derivedStatuses.import(check); } async _getRequiredContexts() { @@ -620,38 +789,54 @@ class PullRequest { return this._requiredContextsCache; } - // returns filled StatusChecks object async _getPrStatuses() { const requiredContexts = await this._getRequiredContexts(); const combinedPrStatus = await GH.getStatuses(this._prHeadSha()); - let statusChecks = new StatusChecks(requiredContexts.length, "PR"); + this._derivedPrStatuses = new DerivedStatusChecks("PR"); + this._prStatuses = new StatusChecks(requiredContexts.length - Config.derivativeRequiredChecks(), "PR"); // fill with required status checks for (let st of combinedPrStatus.statuses) { + const check = new StatusCheck(st); + if (DerivedStatusChecks.Has(st.context)) { + this._derivedPrStatuses.import(check, "PR"); + continue; + } if (requiredContexts.some(el => el.trim() === st.context.trim())) - statusChecks.addRequiredStatus(new StatusCheck(st)); + this._prStatuses.addRequiredStatus(check); else - statusChecks.addOptionalStatus(new StatusCheck(st)); + this._prStatuses.addOptionalStatus(check); } - this._log("pr status details: " + statusChecks); - return statusChecks; + this._log("pr internal status details: " + this._derivedPrStatuses); + this._log("pr external status details: " + this._prStatuses); } - // returns filled StatusChecks object async _getStagingStatuses() { const combinedStagingStatuses = await GH.getStatuses(this._stagedSha()); - const genuineStatuses = combinedStagingStatuses.statuses.filter(st => !st.description.endsWith(Config.copiedDescriptionSuffix())); - assert(genuineStatuses.length <= Config.stagingChecks()); - let statusChecks = new StatusChecks(Config.stagingChecks(), "Staging"); + const genuineStatuses = combinedStagingStatuses.statuses.filter(st => + !st.description.endsWith(Config.copiedDescriptionSuffix())); + assert(genuineStatuses.length <= Config.stagingChecks()); // PR approval + this._derivedStagedStatuses = new DerivedStatusChecks("Staging"); + this._stagedStatuses = new StatusChecks(Config.stagingChecksCI(), "Staging"); // all genuine checks are 'required' - for (let st of genuineStatuses) - statusChecks.addRequiredStatus(new StatusCheck(st)); + for (let st of genuineStatuses) { + const check = new StatusCheck(st); + if (DerivedStatusChecks.Has(st.context)) + this._derivedStagedStatuses.import(check, "staged"); + else + this._stagedStatuses.addRequiredStatus(check); + } const optionalStatuses = combinedStagingStatuses.statuses.filter(st => st.description.endsWith(Config.copiedDescriptionSuffix())); - for (let st of optionalStatuses) - statusChecks.addOptionalStatus(new StatusCheck(st)); + for (let st of optionalStatuses) { + const check = new StatusCheck(st); + if (DerivedStatusChecks.Has(st.context)) + this._derivedStagedStatuses.import(check, "staged"); + else + this._stagedStatuses.addOptionalStatus(check); + } - this._log("staging status details: " + statusChecks); - return statusChecks; + this._log("staging internal status details: " + this._derivedStagedStatuses); + this._log("staging external status details: " + this._stagedStatuses); } // Determines whether the existing staged commit is equivalent to a staged @@ -700,7 +885,7 @@ class PullRequest { // stop processing if it is prohibited by a human-controlled label _checkForHumanLabels() { if (this._labels.has(Config.failedStagingOtherLabel())) - throw this._exObviousFailure("an unexpected error during staging some time ago"); + throw this._exLabeledFailure("an unexpected error during staging some time ago", Config.failedStagingOtherLabel()); } // Checks whether this PR is still open and still wants to be merged. @@ -725,10 +910,10 @@ class PullRequest { assert(!this._labels.has(Config.failedStagingOtherLabel())); if (this._labels.has(Config.failedStagingChecksLabel())) - throw this._exObviousFailure("staged commit tests failed"); + throw this._exLabeledFailure("staged commit tests failed some time ago", Config.failedStagingChecksLabel()); if (this._wipPr()) - throw this._exObviousFailure("work-in-progress"); + throw this._exWaiting("waiting on WIP"); if (!this._messageValid) throw this._exLabeledFailure("invalid commit message", Config.failedDescriptionLabel()); @@ -737,19 +922,19 @@ class PullRequest { throw this._exObviousFailure("GitHub will not be able to merge"); if (!this._approval.granted()) - throw this._exObviousFailure("waiting for approval"); + throw this._exWaiting("waiting for approval"); if (this._approval.grantedTimeout()) - throw this._exObviousFailure("waiting for objections"); + throw this._exWaiting("waiting for objections"); if (this._stagingBanned) - throw this._exObviousFailure("waiting for another staged PR"); + throw this._exWaiting("waiting for another staged PR"); if (this._prStatuses.failed()) throw this._exObviousFailure("failed PR tests"); if (!this._prStatuses.final()) - throw this._exObviousFailure("waiting for PR checks"); + throw this._exWaiting("waiting for PR tests"); assert(this._prStatuses.succeeded()); } @@ -879,10 +1064,13 @@ class PullRequest { _stagedSha() { return this._stagedCommit ? this._stagedCommit.sha : null; } + _stagedShaBrief() { return this._stagedSha() ? this._stagedSha().substr(0, this._shaLimit) : null; } + staged() { return this._prState.staged(); } _debugString() { - const staged = this._stagedSha() ? "staged: " + this._stagedSha().substr(0, this._shaLimit) + ' ' : ""; + const sha = this._stagedShaBrief(); + const staged = sha ? "staged: " + sha + ' ' : ""; const detail = "head: " + this._rawPr.head.sha.substr(0, this._shaLimit) + ' ' + staged + "history: " + this._breadcrumbs.join(); @@ -970,7 +1158,7 @@ class PullRequest { async _loadPrState() { if (!this._stagedSha()) { if (await this._mergedSomeTimeAgo()) - this._enterMerged(); + await this._enterMerged(); else await this._enterBrewing(); return; @@ -980,48 +1168,59 @@ class PullRequest { if (this._stagedPosition.merged()) { this._log("already merged into base some time ago"); - this._enterMerged(); + await this._enterMerged(); return; } + // statuses are needed by _setAutomatedForStaged() below + await this._getStagingStatuses(); + if (!(await this._stagedCommitIsFresh())) { - await this._labels.addImmediately(Config.abandonedStagingChecksLabel()); + const problem = this._exObviousFailure("staged commit tests are stale"); + await this._setAutomatedForStaged(problem.status(), problem.toString()); await this._enterBrewing(); return; } 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 - if (stagedStatuses.failed()) { + if (this._stagedStatuses.failed()) { this._labels.add(Config.failedStagingChecksLabel()); + const problem = this._exLabeledFailure("staged commit tests failed some time ago", Config.failedStagingChecksLabel()); + await this._setAutomatedForStaged(problem.status(), problem.toString()); await this._enterBrewing(); return; } - await this._enterStaged(stagedStatuses); + await this._enterStaged(); } async _enterBrewing() { + this._log("_enterBrewing"); this._prState = PrState.Brewing(); + this._stagedCommit = null; + this._stagedStatuses = null; + this._derivedStagedStatuses = null; assert(this._prStatuses === null); - this._prStatuses = await this._getPrStatuses(); + await this._getPrStatuses(); + await this._setAutomatedStepBrewing(); } - async _enterStaged(stagedStatuses) { + async _enterStaged() { + this._log("_enterStaged"); this._prState = PrState.Staged(); - assert(this._stagedStatuses === null); - if (stagedStatuses) - this._stagedStatuses = stagedStatuses; - else - this._stagedStatuses = await this._getStagingStatuses(); - this._prStatuses = await this._getPrStatuses(); + if (!this._stagedStatuses) + await this._getStagingStatuses(); + await this._getPrStatuses(); + await this._setAutomatedStepStaged(); } - _enterMerged() { + async _enterMerged() { + this._log("_enterMerged"); this._prState = PrState.Merged(); + await this._setAutomatedStepMerged(); } // loads raw PR metadata from GitHub @@ -1047,13 +1246,15 @@ class PullRequest { } async _processStagingStatuses() { - assert(this._stagedStatuses); + if (!this._stagedStatuses) + this._stagedStatuses = new StatusChecks(Config.stagingChecksCI(), "Staging"); + if (this._stagedStatuses.failed()) - throw this._exLabeledFailure("staging tests failed", Config.failedStagingChecksLabel()); + throw this._exLabeledFailure("staged commit tests have failed", Config.failedStagingChecksLabel()); if (!this._stagedStatuses.final()) { this._labels.add(Config.waitingStagingChecksLabel()); - throw this._exSuspend("waiting for staging tests completion"); + throw this._exSuspend(this._waitingForStagingTestsMsg()); } assert(this._stagedStatuses.succeeded()); @@ -1076,6 +1277,12 @@ class PullRequest { this._log("_supplyStagingWithPrRequired: skip existing " + requiredContext); continue; } + + if (DerivedStatusChecks.Has(requiredContext)) { + this._log("_supplyStagingWithPrRequired: skip internal " + requiredContext); + continue; + } + const requiredPrStatus = this._prStatuses.requiredStatuses.find(el => el.context.trim() === requiredContext.trim()); assert(requiredPrStatus); assert(!requiredPrStatus.description.endsWith(Config.copiedDescriptionSuffix())); @@ -1129,24 +1336,25 @@ class PullRequest { // yes, _checkStagingPreconditions() has checked the same message // already, but our _criteria_ might have changed since that check if (!this._messageValid) - throw this._exLabeledFailure("commit message is now considered invalid", Config.failedDescriptionLabel()); + throw this._exLabeledFailure("invalid commit message", Config.failedDescriptionLabel()); - assert(!this._wipPr()); + if (this._wipPr()) + throw this._exWaiting("waiting on WIP"); // TODO: unstage only if there is competition for being staged // yes, _checkStagingPreconditions() has checked approval already, but // humans may have changed their mind since that check if (!this._approval.granted()) - throw this._exObviousFailure("lost approval"); + throw this._exWaiting("waiting for approval"); if (this._approval.grantedTimeout()) - throw this._exObviousFailure("restart waiting for objections"); + throw this._exWaiting("waiting for objections"); if (this._prStatuses.failed()) - throw this._exObviousFailure("new PR branch tests appeared/failed after staging"); + throw this._exObviousFailure("failed PR tests"); if (!this._prStatuses.final()) - throw this._exSuspend("waiting for PR branch tests that appeared after staging"); + throw this._exSuspend("waiting for PR tests"); assert(this._prStatuses.succeeded()); @@ -1159,13 +1367,15 @@ class PullRequest { this._log("merging to base..."); if (this._dryRun("merging to base")) - throw this._exSuspend("dryRun"); + throw this._exSuspend("waiting for the dry-run mode to end"); if (this._stagingOnly()) throw this._exSuspend("waiting for staging-only mode to end"); assert(!this._stagingBanned); + await this._setAutomatedForStaged('success', 'will be merged as ' + this._stagedShaBrief()); + try { await GH.updateReference(this._prBaseBranchPath(), this._stagedSha(), false); } catch (e) { @@ -1177,7 +1387,7 @@ class PullRequest { throw e; } - this._enterMerged(); + await this._enterMerged(); } async _acquireUserProperties() { @@ -1205,7 +1415,7 @@ class PullRequest { const committer = {name: Config.githubUserName(), email: Config.githubUserEmail(), date: now.toISOString()}; if (this._dryRun("create staged commit")) - throw this._exObviousFailure("dryRun"); + throw this._exWaiting("waiting for the dry-run mode to end"); this._stagedCommit = await GH.createCommit(mergeCommit.tree.sha, this._prMessage(), [baseSha], mergeCommit.author, committer); @@ -1293,11 +1503,14 @@ class PullRequest { return new ProcessResult(); } catch (e) { const knownProblem = e instanceof PrProblem; + let currentKnownProblem = null; - if (knownProblem) + if (knownProblem) { this._log("did not merge: " + e.message); - else + currentKnownProblem = e; + } else { this._logEx(e, "process() failure"); + } const suspended = knownProblem && e.keepStagedRequested(); // whether _exSuspend() occured @@ -1310,6 +1523,7 @@ class PullRequest { if (knownProblem) { result.setDelayMsIfAny(this._delayMs()); + await this._setAutomatedForKnownProblem(currentKnownProblem); return result; } @@ -1322,13 +1536,18 @@ class PullRequest { if (this._stagedSha()) { // the PR is staged now or was staged some time ago // avoid livelocking this._labels.add(Config.failedStagingOtherLabel()); + currentKnownProblem = this._exLabeledFailure("an unexpected error during staging", + Config.failedStagingOtherLabel()); } else { // Since knownProblem is false, either there was no failedStagingOtherLabel() // or the problem happened before we could check for it. In the latter case, // that label will remain set, and we will add failedOtherLabel(), reflecting the // compound nature of the problem. this._labels.add(Config.failedOtherLabel()); + currentKnownProblem = this._exLabeledFailure("an unexpected error", + Config.failedOtherLabel()); } + await this._setAutomatedForUnexpectedProblem(currentKnownProblem); throw e; } finally { await this._pushLabelsToGitHub(); @@ -1351,7 +1570,6 @@ class PullRequest { this._labels.remove(Config.failedOtherLabel()); this._labels.remove(Config.passedStagingChecksLabel()); this._labels.remove(Config.waitingStagingChecksLabel()); - this._labels.remove(Config.abandonedStagingChecksLabel()); // Config.mergedLabel() is not meant to be removed by anybody } @@ -1367,7 +1585,7 @@ class PullRequest { // only meaningful for staged PRs _exSuspend(why) { assert(arguments.length === 1); - let problem = new PrProblem(why); + let problem = new PrProblem("pending", why); problem.requestToKeepStaged(); return problem; } @@ -1378,14 +1596,21 @@ class PullRequest { assert(arguments.length === 1); this._labelPushBan = why; assert(this._labelPushBan); // paranoid: `why` is truthy - return new PrProblem(why); + return new PrProblem("failure", why); } // a problem that humans can discern via GitHub-maintained PR state // reprocessing will be required after the problem is fixed _exObviousFailure(why) { assert(arguments.length === 1); - return new PrProblem(why); + return new PrProblem("failure", why); + } + + // Indicates an event which this PR is waiting for. Differs from _exSuspend() + // that reprocessing from scratch will be required after eceiving the event. + _exWaiting(why) { + assert(arguments.length === 1); + return new PrProblem("pending", why); } // a problem that humans cannot easily detect without an Anubis-set label @@ -1394,7 +1619,10 @@ class PullRequest { assert(arguments.length === 2); assert(!this._labelPushBan); this._labels.add(label); - return new PrProblem(why); + let problem = new PrProblem("failure", why); + problem.setLabel(label); + problem.alreadyExists = this._labels.has(label); + return problem; } }