From 951a2fecf1ba837c2092a44d22feee0343e0f454 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20Mochol=C3=AD?= Date: Thu, 10 Nov 2022 16:08:36 +0100 Subject: [PATCH] v2 (#2) * Refactor the checkgroup loop. Now runs straight away. Also manages the timeout itself * Add comment that suggests pinging maintainers when it times out. --- .eslintrc.yml | 14 ++++ action.yml | 9 ++- dist/auto-cc/auto-cc-bot.js | 3 - dist/auto-cc/utils.js | 1 - dist/check-group/core/index.js | 91 +++++++++++++--------- dist/index.js | 6 +- src/action.ts | 4 +- src/auto-cc/auto-cc-bot.ts | 3 - src/auto-cc/utils.ts | 1 - src/check-group/core/index.ts | 77 +++++++++++------- src/check-group/utils/generate_progress.ts | 5 +- src/index.ts | 14 ++-- 12 files changed, 138 insertions(+), 90 deletions(-) create mode 100644 .eslintrc.yml diff --git a/.eslintrc.yml b/.eslintrc.yml new file mode 100644 index 000000000..6c95aabd2 --- /dev/null +++ b/.eslintrc.yml @@ -0,0 +1,14 @@ +env: + browser: true + es2021: true +extends: + - eslint:recommended + - plugin:@typescript-eslint/recommended +overrides: [] +parser: '@typescript-eslint/parser' +parserOptions: + ecmaVersion: latest + sourceType: module +plugins: + - '@typescript-eslint' +rules: {} diff --git a/action.yml b/action.yml index 974c0e097..7315ca7ef 100644 --- a/action.yml +++ b/action.yml @@ -4,9 +4,16 @@ inputs: job: required: true default: auto-cc + maintainers: + required: true + owner: + required: true + timeout: + required: false + default: 60 # minutes interval: required: false - default: 120 + default: 120 # seconds runs: using: 'node16' main: 'dist/action.js' diff --git a/dist/auto-cc/auto-cc-bot.js b/dist/auto-cc/auto-cc-bot.js index c96bb0d4d..c8d3550ea 100644 --- a/dist/auto-cc/auto-cc-bot.js +++ b/dist/auto-cc/auto-cc-bot.js @@ -63,10 +63,8 @@ function myBot(app) { labels = context.payload[name]['labels'].map(function (e) { return e['name']; }); context.log({ labels: labels }); cc = new Set(); - // eslint-disable-next-line github/array-foreach labels.forEach(function (l) { if (l in subscriptions) { - // eslint-disable-next-line github/array-foreach subscriptions[l].forEach(function (u) { return cc.add(u); }); } }); @@ -89,7 +87,6 @@ function myBot(app) { } if (!(prevCC.size !== cc.size)) return [3 /*break*/, 6]; newCCString_1 = 'cc'; - // eslint-disable-next-line github/array-foreach cc.forEach(function (u) { newCCString_1 += " @".concat(u); }); diff --git a/dist/auto-cc/utils.js b/dist/auto-cc/utils.js index edf2c83f9..a280a42ad 100644 --- a/dist/auto-cc/utils.js +++ b/dist/auto-cc/utils.js @@ -168,7 +168,6 @@ function parseSubscriptions(rawSubsText) { if (subsRows == null) { return subscriptions; } - // eslint-disable-next-line github/array-foreach subsRows.forEach(function (row) { var labelMatch = row.match(/^\* +([^@]+)/); if (labelMatch) { diff --git a/dist/check-group/core/index.js b/dist/check-group/core/index.js index 46d132fc6..c6e1122a0 100644 --- a/dist/check-group/core/index.js +++ b/dist/check-group/core/index.js @@ -74,6 +74,8 @@ var utils_3 = require("../utils"); */ var CheckGroup = /** @class */ (function () { function CheckGroup(pullRequestNumber, config, context, sha) { + this.intervalTimer = setTimeout(function () { return ''; }, 0); + this.timeoutTimer = setTimeout(function () { return ''; }, 0); this.pullRequestNumber = pullRequestNumber; this.config = config; this.context = context; @@ -81,7 +83,8 @@ var CheckGroup = /** @class */ (function () { } CheckGroup.prototype.run = function () { return __awaiter(this, void 0, void 0, function () { - var filenames, subprojs, expectedChecks, interval, tries, conclusion, loop; + var filenames, subprojs, expectedChecks, interval, timeout; + var _this = this; return __generator(this, function (_a) { switch (_a.label) { case 0: return [4 /*yield*/, this.files()]; @@ -96,46 +99,62 @@ var CheckGroup = /** @class */ (function () { } interval = parseInt(core.getInput('interval')); core.info("Check interval: ".concat(interval)); - tries = 0; - conclusion = undefined; - loop = setInterval(function (that) { - return __awaiter(this, void 0, void 0, function () { - var postedChecks, summary, details, error_1; - return __generator(this, function (_a) { - switch (_a.label) { - case 0: - _a.trys.push([0, 2, , 3]); - tries += 1; - core.startGroup("Check ".concat(tries)); - return [4 /*yield*/, getPostedChecks(that.context, that.sha)]; - case 1: - postedChecks = _a.sent(); - core.debug("postedChecks: ".concat(JSON.stringify(postedChecks))); - conclusion = (0, utils_3.satisfyExpectedChecks)(subprojs, postedChecks); - summary = (0, utils_1.generateProgressSummary)(subprojs, postedChecks); - details = (0, utils_1.generateProgressDetails)(subprojs, postedChecks); - core.info("".concat(that.config.customServiceName, " conclusion: '").concat(conclusion, "':\n").concat(summary, "\n").concat(details)); - core.endGroup(); - if (conclusion === "all_passing") { - core.info("Required checks were successful!"); - clearInterval(loop); - } - return [3 /*break*/, 3]; - case 2: - error_1 = _a.sent(); - core.setFailed(error_1); - clearInterval(loop); - return [3 /*break*/, 3]; - case 3: return [2 /*return*/]; - } - }); - }); - }, interval * 1000, this); + this.runCheck(subprojs, 1, interval * 1000); + timeout = parseInt(core.getInput('timeout')); + core.info("Timeout: ".concat(timeout)); + // set a timeout that will stop the job to avoid polling the GitHub API infinitely + this.timeoutTimer = setTimeout(function () { + clearTimeout(_this.intervalTimer); + core.setFailed("The timeout of ".concat(timeout, " minutes has triggered but not all required jobs were passing.") + + " This job will need to be re-run to merge your PR." + + " If you do not have write access to the repository you can ask ".concat(core.getInput('maintainers'), " to re-run it for you.") + + " If you have any other questions, you can reach out to ".concat(core.getInput('owner'), " for help.")); + }, timeout * 60 * 1000); return [2 /*return*/]; } }); }); }; + CheckGroup.prototype.runCheck = function (subprojs, tries, interval) { + return __awaiter(this, void 0, void 0, function () { + var postedChecks, conclusion, summary, details, error_1; + var _this = this; + return __generator(this, function (_a) { + switch (_a.label) { + case 0: + _a.trys.push([0, 2, , 3]); + // print in a group to reduce verbosity + core.startGroup("Check ".concat(tries)); + return [4 /*yield*/, getPostedChecks(this.context, this.sha)]; + case 1: + postedChecks = _a.sent(); + core.debug("postedChecks: ".concat(JSON.stringify(postedChecks))); + conclusion = (0, utils_3.satisfyExpectedChecks)(subprojs, postedChecks); + summary = (0, utils_1.generateProgressSummary)(subprojs, postedChecks); + details = (0, utils_1.generateProgressDetails)(subprojs, postedChecks); + core.info("".concat(this.config.customServiceName, " conclusion: '").concat(conclusion, "':\n").concat(summary, "\n").concat(details)); + core.endGroup(); + if (conclusion === "all_passing") { + core.info("All required checks were successful!"); + clearTimeout(this.intervalTimer); + clearTimeout(this.timeoutTimer); + } + else { + this.intervalTimer = setTimeout(function () { return _this.runCheck(subprojs, tries + 1, interval); }, interval); + } + return [3 /*break*/, 3]; + case 2: + error_1 = _a.sent(); + // bubble up the error to the job + core.setFailed(error_1); + clearTimeout(this.intervalTimer); + clearTimeout(this.timeoutTimer); + return [3 /*break*/, 3]; + case 3: return [2 /*return*/]; + } + }); + }); + }; /** * Gets a list of files that are modified in * a pull request. diff --git a/dist/index.js b/dist/index.js index b9c3471a1..7915a8f3e 100644 --- a/dist/index.js +++ b/dist/index.js @@ -30,11 +30,11 @@ var auto_cc_bot_1 = __importDefault(require("./auto-cc/auto-cc-bot")); var index_1 = __importDefault(require("./check-group/index")); var core = __importStar(require("@actions/core")); function botSteps(app) { - var job = core.getInput('job'); - if (job === 'auto-cc') { + var job = core.getInput("job"); + if (job === "auto-cc") { (0, auto_cc_bot_1.default)(app); } - else if (job === 'check-group') { + else if (job === "check-group") { (0, index_1.default)(app); } else { diff --git a/src/action.ts b/src/action.ts index 52d13175d..bfc43ad3d 100644 --- a/src/action.ts +++ b/src/action.ts @@ -1,7 +1,7 @@ // Require your Probot app's entrypoint, usually this is just index.js -import botSteps from './index'; +import botSteps from "./index"; // Require the adapter -import probot from '@probot/adapter-github-actions'; +import probot from "@probot/adapter-github-actions"; // Adapt the Probot app for Actions // This also acts as the main entrypoint for the Action diff --git a/src/auto-cc/auto-cc-bot.ts b/src/auto-cc/auto-cc-bot.ts index 1e10e2ccd..43b0cf5fc 100644 --- a/src/auto-cc/auto-cc-bot.ts +++ b/src/auto-cc/auto-cc-bot.ts @@ -25,10 +25,8 @@ function myBot(app: Probot): void { const labels = context.payload[name]['labels'].map(e => e['name']); context.log({labels}); const cc = new Set(); - // eslint-disable-next-line github/array-foreach labels.forEach(l => { if (l in subscriptions) { - // eslint-disable-next-line github/array-foreach subscriptions[l].forEach(u => cc.add(u)); } }); @@ -52,7 +50,6 @@ function myBot(app: Probot): void { // Invariant: prevCC is a subset of cc if (prevCC.size !== cc.size) { let newCCString = 'cc'; - // eslint-disable-next-line github/array-foreach cc.forEach(u => { newCCString += ` @${u}`; }); diff --git a/src/auto-cc/utils.ts b/src/auto-cc/utils.ts index e3af6b1b8..3590e0622 100644 --- a/src/auto-cc/utils.ts +++ b/src/auto-cc/utils.ts @@ -80,7 +80,6 @@ export function parseSubscriptions(rawSubsText): object { if (subsRows == null) { return subscriptions; } - // eslint-disable-next-line github/array-foreach subsRows.forEach((row: string) => { const labelMatch = row.match(/^\* +([^@]+)/); if (labelMatch) { diff --git a/src/check-group/core/index.ts b/src/check-group/core/index.ts index 4ef066029..afd80d4c5 100644 --- a/src/check-group/core/index.ts +++ b/src/check-group/core/index.ts @@ -23,6 +23,9 @@ export class CheckGroup { context: Context; sha: string; + intervalTimer: ReturnType = setTimeout(() => '', 0); + timeoutTimer: ReturnType = setTimeout(() => '', 0); + constructor( pullRequestNumber: number, config: CheckGroupConfig, @@ -49,39 +52,55 @@ export class CheckGroup { const interval = parseInt(core.getInput('interval')) core.info(`Check interval: ${interval}`); - - let tries = 0; - let conclusion = undefined; - // IMPORTANT: a timeout should be set in the action workflow - const loop = setInterval( - async function(that) { - try { - tries += 1; - core.startGroup(`Check ${tries}`); + this.runCheck(subprojs, 1, interval * 1000) - const postedChecks = await getPostedChecks(that.context, that.sha); - core.debug(`postedChecks: ${JSON.stringify(postedChecks)}`); - - conclusion = satisfyExpectedChecks(subprojs, postedChecks); - const summary = generateProgressSummary(subprojs, postedChecks) - const details = generateProgressDetails(subprojs, postedChecks) - core.info( - `${that.config.customServiceName} conclusion: '${conclusion}':\n${summary}\n${details}` - ) - core.endGroup(); - - if (conclusion === "all_passing") { - core.info("Required checks were successful!") - clearInterval(loop) - } - } catch (error) { - core.setFailed(error); - clearInterval(loop) - } - }, interval * 1000, this + const timeout = parseInt(core.getInput('timeout')) + core.info(`Timeout: ${timeout}`); + // set a timeout that will stop the job to avoid polling the GitHub API infinitely + this.timeoutTimer = setTimeout( + () => { + clearTimeout(this.intervalTimer) + core.setFailed( + `The timeout of ${timeout} minutes has triggered but not all required jobs were passing.` + + ` This job will need to be re-run to merge your PR.` + + ` If you do not have write access to the repository you can ask ${core.getInput('maintainers')} to re-run it for you.` + + ` If you have any other questions, you can reach out to ${core.getInput('owner')} for help.` + ) + }, timeout * 60 * 1000 ) } + async runCheck(subprojs, tries: number, interval: number) { + try { + // print in a group to reduce verbosity + core.startGroup(`Check ${tries}`); + const postedChecks = await getPostedChecks(this.context, this.sha); + core.debug(`postedChecks: ${JSON.stringify(postedChecks)}`); + + const conclusion = satisfyExpectedChecks(subprojs, postedChecks); + const summary = generateProgressSummary(subprojs, postedChecks) + const details = generateProgressDetails(subprojs, postedChecks) + core.info( + `${this.config.customServiceName} conclusion: '${conclusion}':\n${summary}\n${details}` + ) + core.endGroup(); + + if (conclusion === "all_passing") { + core.info("All required checks were successful!") + clearTimeout(this.intervalTimer) + clearTimeout(this.timeoutTimer) + } else { + this.intervalTimer = setTimeout(() => this.runCheck(subprojs, tries + 1, interval), interval); + } + + } catch (error) { + // bubble up the error to the job + core.setFailed(error); + clearTimeout(this.intervalTimer) + clearTimeout(this.timeoutTimer) + } + } + /** * Gets a list of files that are modified in * a pull request. diff --git a/src/check-group/utils/generate_progress.ts b/src/check-group/utils/generate_progress.ts index 50aef280d..9e0ad54bf 100644 --- a/src/check-group/utils/generate_progress.ts +++ b/src/check-group/utils/generate_progress.ts @@ -1,7 +1,4 @@ -/* eslint-disable @typescript-eslint/no-unused-vars */ -import { CheckGroupConfig, ProgressReport, SubProjConfig } from "../types"; -/* eslint-enable @typescript-eslint/no-unused-vars */ -import { defaultCheckId } from "../config"; +import { ProgressReport, SubProjConfig } from "../types"; export const generateProgressReport = ( subprojects: SubProjConfig[], diff --git a/src/index.ts b/src/index.ts index 8ad25cd95..07d22b8e0 100644 --- a/src/index.ts +++ b/src/index.ts @@ -1,13 +1,13 @@ -import autoCcBot from './auto-cc/auto-cc-bot'; -import checkGroupApp from './check-group/index'; -import {Probot} from 'probot'; -import * as core from '@actions/core'; +import autoCcBot from "./auto-cc/auto-cc-bot"; +import checkGroupApp from "./check-group/index"; +import { Probot } from "probot"; +import * as core from "@actions/core"; export default function botSteps(app: Probot): void { - const job = core.getInput('job'); - if (job === 'auto-cc') { + const job = core.getInput("job"); + if (job === "auto-cc") { autoCcBot(app); - } else if (job === 'check-group') { + } else if (job === "check-group") { checkGroupApp(app); } else { throw new Error(`Job ${job} should be one of ['auto-cc', 'check-group']`);