From 0a2ee95c8e7e99791cf77f8085c924e79b4811d8 Mon Sep 17 00:00:00 2001 From: TESTELIN Geoffrey Date: Wed, 2 Jun 2021 21:49:08 +0200 Subject: [PATCH 1/6] fix(operations): fail fast the current batch to respect the operations limit Instead of processing an entire batch of 100 issues before checking the operations left, simply do it before processing an issue so that we respect as expected the limitation of the operations per run Fixes #466 --- dist/index.js | 6 +++++- src/classes/issues-processor.ts | 5 +++++ 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/dist/index.js b/dist/index.js index c0de1d825..f8e3376b9 100644 --- a/dist/index.js +++ b/dist/index.js @@ -297,6 +297,10 @@ class IssuesProcessor { this._logger.info(`${logger_service_1.LoggerService.yellow('Processing the batch of issues')} ${logger_service_1.LoggerService.cyan(`#${page}`)} ${logger_service_1.LoggerService.yellow('containing')} ${logger_service_1.LoggerService.cyan(issues.length)} ${logger_service_1.LoggerService.yellow(`issue${issues.length > 1 ? 's' : ''}...`)}`); } for (const issue of issues.values()) { + // Stop the processing if no more operations remains + if (!this._operations.hasRemainingOperations()) { + break; + } const issueLogger = new issue_logger_1.IssueLogger(issue); (_b = this._statistics) === null || _b === void 0 ? void 0 : _b.incrementProcessedItemsCount(issue); issueLogger.info(`Found this $$type last updated at: ${logger_service_1.LoggerService.cyan(issue.updated_at)}`); @@ -8866,4 +8870,4 @@ module.exports = require("zlib");; /******/ // Load entry module and return exports /******/ return __nccwpck_require__(3109); /******/ })() -; \ No newline at end of file +; diff --git a/src/classes/issues-processor.ts b/src/classes/issues-processor.ts index 0706ec7d6..f8066dd53 100644 --- a/src/classes/issues-processor.ts +++ b/src/classes/issues-processor.ts @@ -121,6 +121,11 @@ export class IssuesProcessor { } for (const issue of issues.values()) { + // Stop the processing if no more operations remains + if (!this.operations.hasRemainingOperations()) { + break; + } + const issueLogger: IssueLogger = new IssueLogger(issue); this._statistics?.incrementProcessedItemsCount(issue); From 217e42fda787602e0902fd456e436df337df14a2 Mon Sep 17 00:00:00 2001 From: TESTELIN Geoffrey Date: Wed, 2 Jun 2021 21:49:54 +0200 Subject: [PATCH 2/6] test(debug): disable the dry-run for the test by default we will be able to test the operations per run and have more complete logs that could help us debug the workflow --- __tests__/constants/default-processor-options.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/__tests__/constants/default-processor-options.ts b/__tests__/constants/default-processor-options.ts index a0550f4b6..38067653e 100644 --- a/__tests__/constants/default-processor-options.ts +++ b/__tests__/constants/default-processor-options.ts @@ -25,7 +25,7 @@ export const DefaultProcessorOptions: IIssuesProcessorOptions = Object.freeze({ anyOfIssueLabels: '', anyOfPrLabels: '', operationsPerRun: 100, - debugOnly: true, + debugOnly: false, removeStaleWhenUpdated: false, removeIssueStaleWhenUpdated: undefined, removePrStaleWhenUpdated: undefined, From 07bfcdcab5404694085a98328308e902f73227ec Mon Sep 17 00:00:00 2001 From: TESTELIN Geoffrey Date: Wed, 2 Jun 2021 21:50:33 +0200 Subject: [PATCH 3/6] chore(logs): also display the stats when the operations per run stopped the workflow --- src/classes/issues-processor.ts | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/classes/issues-processor.ts b/src/classes/issues-processor.ts index f8066dd53..7cf07ea56 100644 --- a/src/classes/issues-processor.ts +++ b/src/classes/issues-processor.ts @@ -417,6 +417,9 @@ export class IssuesProcessor { 'option which is currently set to' )} ${LoggerService.cyan(this.options.operationsPerRun)}` ); + this._statistics + ?.setOperationsCount(this.operations.getConsumedOperationsCount()) + .logStats(); return 0; } From ec69b310041f7409a9d465040a967b3a3df04303 Mon Sep 17 00:00:00 2001 From: TESTELIN Geoffrey Date: Wed, 2 Jun 2021 21:51:17 +0200 Subject: [PATCH 4/6] chore(stats): fix a bad stats related to the consumed operations --- src/classes/issues-processor.ts | 18 +++++++++--------- src/classes/statistics.ts | 4 ++-- 2 files changed, 11 insertions(+), 11 deletions(-) diff --git a/src/classes/issues-processor.ts b/src/classes/issues-processor.ts index 7cf07ea56..3c189adcd 100644 --- a/src/classes/issues-processor.ts +++ b/src/classes/issues-processor.ts @@ -60,8 +60,8 @@ export class IssuesProcessor { } private readonly _logger: Logger = new Logger(); - private readonly _operations: StaleOperations; private readonly _statistics: Statistics | undefined; + readonly operations: StaleOperations; readonly client: InstanceType; readonly options: IIssuesProcessorOptions; readonly staleIssues: Issue[] = []; @@ -72,7 +72,7 @@ export class IssuesProcessor { constructor(options: IIssuesProcessorOptions) { this.options = options; this.client = getOctokit(this.options.repoToken); - this._operations = new StaleOperations(this.options); + this.operations = new StaleOperations(this.options); this._logger.info( LoggerService.yellow(`Starting the stale action process...`) @@ -104,10 +104,10 @@ export class IssuesProcessor { LoggerService.green(`No more issues found to process. Exiting...`) ); this._statistics - ?.setRemainingOperations(this._operations.getRemainingOperationsCount()) + ?.setOperationsCount(this.operations.getConsumedOperationsCount()) .logStats(); - return this._operations.getRemainingOperationsCount(); + return this.operations.getRemainingOperationsCount(); } else { this._logger.info( `${LoggerService.yellow( @@ -404,7 +404,7 @@ export class IssuesProcessor { IssuesProcessor._endIssueProcessing(issue); } - if (!this._operations.hasRemainingOperations()) { + if (!this.operations.hasRemainingOperations()) { this._logger.warning( LoggerService.yellowBright(`No more operations left! Exiting...`) ); @@ -441,7 +441,7 @@ export class IssuesProcessor { ): Promise { // Find any comments since date on the given issue try { - this._operations.consumeOperation(); + this.operations.consumeOperation(); this._statistics?.incrementFetchedItemsCommentsCount(); const comments = await this.client.issues.listComments({ owner: context.repo.owner, @@ -461,7 +461,7 @@ export class IssuesProcessor { let actor; try { - this._operations.consumeOperation(); + this.operations.consumeOperation(); actor = await this.client.users.getAuthenticated(); } catch (error) { return context.actor; @@ -477,7 +477,7 @@ export class IssuesProcessor { type OctoKitIssueList = GetResponseTypeFromEndpointMethod; try { - this._operations.consumeOperation(); + this.operations.consumeOperation(); const issueResult: OctoKitIssueList = await this.client.issues.listForRepo( { owner: context.repo.owner, @@ -960,7 +960,7 @@ export class IssuesProcessor { } private _consumeIssueOperation(issue: Readonly): void { - this._operations.consumeOperation(); + this.operations.consumeOperation(); issue.operations.consumeOperation(); } diff --git a/src/classes/statistics.ts b/src/classes/statistics.ts index 851e48935..11f665c7f 100644 --- a/src/classes/statistics.ts +++ b/src/classes/statistics.ts @@ -65,8 +65,8 @@ export class Statistics { return this._incrementUndoStaleIssuesCount(increment); } - setRemainingOperations(remainingOperations: Readonly): Statistics { - this._operationsCount = remainingOperations; + setOperationsCount(operationsCount: Readonly): Statistics { + this._operationsCount = operationsCount; return this; } From 987620421c994eb6dc8ee624f43de813ccd2f13c Mon Sep 17 00:00:00 2001 From: TESTELIN Geoffrey Date: Wed, 2 Jun 2021 21:57:02 +0200 Subject: [PATCH 5/6] test(operations-per-run): add coverage --- __tests__/operations-per-run.spec.ts | 230 +++++++++++++++++++++++++++ 1 file changed, 230 insertions(+) create mode 100644 __tests__/operations-per-run.spec.ts diff --git a/__tests__/operations-per-run.spec.ts b/__tests__/operations-per-run.spec.ts new file mode 100644 index 000000000..46035cb5d --- /dev/null +++ b/__tests__/operations-per-run.spec.ts @@ -0,0 +1,230 @@ +import {Issue} from '../src/classes/issue'; +import {IIssuesProcessorOptions} from '../src/interfaces/issues-processor-options'; +import {IsoDateString} from '../src/types/iso-date-string'; +import {IssuesProcessorMock} from './classes/issues-processor-mock'; +import {DefaultProcessorOptions} from './constants/default-processor-options'; +import {generateIssue} from './functions/generate-issue'; + +describe('operations per run option', (): void => { + let sut: SUT; + + beforeEach((): void => { + sut = new SUT(); + }); + + describe('when one issue should be stale within 10 days and updated 20 days ago', (): void => { + beforeEach((): void => { + sut.staleIn(10).newIssue().updated(20); + }); + + describe('when the operations per run option is set to 1', (): void => { + beforeEach((): void => { + sut.operationsPerRun(1); + }); + + it('should consume 1 operation (stale label)', async () => { + expect.assertions(2); + + await sut.test(); + + expect(sut.processor.staleIssues).toHaveLength(1); + expect( + sut.processor.operations.getConsumedOperationsCount() + ).toStrictEqual(1); + }); + }); + }); + + describe('when one issue should be stale within 10 days and updated 20 days ago and a comment should be added when stale', (): void => { + beforeEach((): void => { + sut.staleIn(10).commentOnStale().newIssue().updated(20); + }); + + describe('when the operations per run option is set to 2', (): void => { + beforeEach((): void => { + sut.operationsPerRun(2); + }); + + it('should consume 2 operations (stale label, comment)', async () => { + expect.assertions(2); + + await sut.test(); + + expect(sut.processor.staleIssues).toHaveLength(1); + expect( + sut.processor.operations.getConsumedOperationsCount() + ).toStrictEqual(2); + }); + }); + + // Special case were we continue the issue processing even if the operations per run is reached + describe('when the operations per run option is set to 1', (): void => { + beforeEach((): void => { + sut.operationsPerRun(1); + }); + + it('should consume 2 operations (stale label, comment)', async () => { + expect.assertions(2); + + await sut.test(); + + expect(sut.processor.staleIssues).toHaveLength(1); + expect( + sut.processor.operations.getConsumedOperationsCount() + ).toStrictEqual(2); + }); + }); + }); + + describe('when two issues should be stale within 10 days and updated 20 days ago and a comment should be added when stale', (): void => { + beforeEach((): void => { + sut.staleIn(10).commentOnStale(); + sut.newIssue().updated(20); + sut.newIssue().updated(20); + }); + + describe('when the operations per run option is set to 3', (): void => { + beforeEach((): void => { + sut.operationsPerRun(3); + }); + + it('should consume 4 operations (stale label, comment)', async () => { + expect.assertions(2); + + await sut.test(); + + expect(sut.processor.staleIssues).toHaveLength(2); + expect( + sut.processor.operations.getConsumedOperationsCount() + ).toStrictEqual(4); + }); + }); + + describe('when the operations per run option is set to 2', (): void => { + beforeEach((): void => { + sut.operationsPerRun(2); + }); + + it('should consume 2 operations (stale label, comment) and stop', async () => { + expect.assertions(2); + + await sut.test(); + + expect(sut.processor.staleIssues).toHaveLength(1); + expect( + sut.processor.operations.getConsumedOperationsCount() + ).toStrictEqual(2); + }); + }); + + // Special case were we continue the issue processing even if the operations per run is reached + describe('when the operations per run option is set to 1', (): void => { + beforeEach((): void => { + sut.operationsPerRun(1); + }); + + it('should consume 2 operations (stale label, comment) and stop', async () => { + expect.assertions(2); + + await sut.test(); + + expect(sut.processor.staleIssues).toHaveLength(1); + expect( + sut.processor.operations.getConsumedOperationsCount() + ).toStrictEqual(2); + }); + }); + }); +}); + +class SUT { + processor!: IssuesProcessorMock; + private _opts: IIssuesProcessorOptions = { + ...DefaultProcessorOptions, + staleIssueMessage: '' + }; + private _testIssueList: Issue[] = []; + private _sutIssues: SUTIssue[] = []; + + newIssue(): SUTIssue { + const sutIssue: SUTIssue = new SUTIssue(); + this._sutIssues.push(sutIssue); + + return sutIssue; + } + + staleIn(days: number): SUT { + this._updateOptions({ + daysBeforeIssueStale: days + }); + + return this; + } + + commentOnStale(): SUT { + this._updateOptions({ + staleIssueMessage: 'Dummy stale issue message' + }); + + return this; + } + + operationsPerRun(count: number): SUT { + this._updateOptions({ + operationsPerRun: count + }); + + return this; + } + + async test(): Promise { + return this._setTestIssueList()._setProcessor(); + } + + private _updateOptions(opts: Partial): SUT { + this._opts = {...this._opts, ...opts}; + + return this; + } + + private _setTestIssueList(): SUT { + this._testIssueList = this._sutIssues.map( + (sutIssue: SUTIssue): Issue => { + return generateIssue( + this._opts, + 1, + 'My first issue', + sutIssue.updatedAt, + sutIssue.updatedAt, + false + ); + } + ); + + return this; + } + + private async _setProcessor(): Promise { + this.processor = new IssuesProcessorMock( + this._opts, + async () => 'abot', + async p => (p === 1 ? this._testIssueList : []), + async () => [], + async () => new Date().toDateString() + ); + + return this.processor.processIssues(1); + } +} + +class SUTIssue { + updatedAt: IsoDateString = '2020-01-01T17:00:00Z'; + + updated(daysAgo: number): SUTIssue { + const today = new Date(); + today.setDate(today.getDate() - daysAgo); + this.updatedAt = today.toISOString(); + + return this; + } +} From 2f93a2a49f4c195ae653bb1d88195f1ada50c8f7 Mon Sep 17 00:00:00 2001 From: TESTELIN Geoffrey Date: Wed, 2 Jun 2021 22:07:10 +0200 Subject: [PATCH 6/6] chore: update index --- dist/index.js | 25 +++++++++++++------------ 1 file changed, 13 insertions(+), 12 deletions(-) diff --git a/dist/index.js b/dist/index.js index f8e3376b9..40257c1ce 100644 --- a/dist/index.js +++ b/dist/index.js @@ -255,7 +255,7 @@ class IssuesProcessor { this.removedLabelIssues = []; this.options = options; this.client = github_1.getOctokit(this.options.repoToken); - this._operations = new stale_operations_1.StaleOperations(this.options); + this.operations = new stale_operations_1.StaleOperations(this.options); this._logger.info(logger_service_1.LoggerService.yellow(`Starting the stale action process...`)); if (this.options.debugOnly) { this._logger.warning(logger_service_1.LoggerService.yellowBright(`Executing in debug mode!`)); @@ -283,22 +283,22 @@ class IssuesProcessor { : option_1.Option.StaleIssueMessage; } processIssues(page = 1) { - var _a, _b; + var _a, _b, _c; return __awaiter(this, void 0, void 0, function* () { // get the next batch of issues const issues = yield this.getIssues(page); const actor = yield this.getActor(); if (issues.length <= 0) { this._logger.info(logger_service_1.LoggerService.green(`No more issues found to process. Exiting...`)); - (_a = this._statistics) === null || _a === void 0 ? void 0 : _a.setRemainingOperations(this._operations.getRemainingOperationsCount()).logStats(); - return this._operations.getRemainingOperationsCount(); + (_a = this._statistics) === null || _a === void 0 ? void 0 : _a.setOperationsCount(this.operations.getConsumedOperationsCount()).logStats(); + return this.operations.getRemainingOperationsCount(); } else { this._logger.info(`${logger_service_1.LoggerService.yellow('Processing the batch of issues')} ${logger_service_1.LoggerService.cyan(`#${page}`)} ${logger_service_1.LoggerService.yellow('containing')} ${logger_service_1.LoggerService.cyan(issues.length)} ${logger_service_1.LoggerService.yellow(`issue${issues.length > 1 ? 's' : ''}...`)}`); } for (const issue of issues.values()) { // Stop the processing if no more operations remains - if (!this._operations.hasRemainingOperations()) { + if (!this.operations.hasRemainingOperations()) { break; } const issueLogger = new issue_logger_1.IssueLogger(issue); @@ -451,9 +451,10 @@ class IssuesProcessor { } IssuesProcessor._endIssueProcessing(issue); } - if (!this._operations.hasRemainingOperations()) { + if (!this.operations.hasRemainingOperations()) { this._logger.warning(logger_service_1.LoggerService.yellowBright(`No more operations left! Exiting...`)); this._logger.warning(`${logger_service_1.LoggerService.yellowBright('If you think that not enough issues were processed you could try to increase the quantity related to the')} ${this._logger.createOptionLink(option_1.Option.OperationsPerRun)} ${logger_service_1.LoggerService.yellowBright('option which is currently set to')} ${logger_service_1.LoggerService.cyan(this.options.operationsPerRun)}`); + (_c = this._statistics) === null || _c === void 0 ? void 0 : _c.setOperationsCount(this.operations.getConsumedOperationsCount()).logStats(); return 0; } this._logger.info(`${logger_service_1.LoggerService.green('Batch')} ${logger_service_1.LoggerService.cyan(`#${page}`)} ${logger_service_1.LoggerService.green('processed.')}`); @@ -467,7 +468,7 @@ class IssuesProcessor { return __awaiter(this, void 0, void 0, function* () { // Find any comments since date on the given issue try { - this._operations.consumeOperation(); + this.operations.consumeOperation(); (_a = this._statistics) === null || _a === void 0 ? void 0 : _a.incrementFetchedItemsCommentsCount(); const comments = yield this.client.issues.listComments({ owner: github_1.context.repo.owner, @@ -488,7 +489,7 @@ class IssuesProcessor { return __awaiter(this, void 0, void 0, function* () { let actor; try { - this._operations.consumeOperation(); + this.operations.consumeOperation(); actor = yield this.client.users.getAuthenticated(); } catch (error) { @@ -504,7 +505,7 @@ class IssuesProcessor { // generate type for response const endpoint = this.client.issues.listForRepo; try { - this._operations.consumeOperation(); + this.operations.consumeOperation(); const issueResult = yield this.client.issues.listForRepo({ owner: github_1.context.repo.owner, repo: github_1.context.repo.repo, @@ -863,7 +864,7 @@ class IssuesProcessor { }); } _consumeIssueOperation(issue) { - this._operations.consumeOperation(); + this.operations.consumeOperation(); issue.operations.consumeOperation(); } _getDaysBeforeStaleUsedOptionName(issue) { @@ -1263,8 +1264,8 @@ class Statistics { } return this._incrementUndoStaleIssuesCount(increment); } - setRemainingOperations(remainingOperations) { - this._operationsCount = remainingOperations; + setOperationsCount(operationsCount) { + this._operationsCount = operationsCount; return this; } incrementClosedItemsCount(issue, increment = 1) {