From ddc7648635b53aba5da59101d0acdf81a9f39af2 Mon Sep 17 00:00:00 2001 From: Geoffrey Testelin Date: Fri, 15 Jan 2021 12:51:24 +0100 Subject: [PATCH] fix(remove-label): do not encode the label to make sure to remove it (#220) * test: add two more tests relating the label syntax issues both are failing * chore: add more logs and fix the tests on error meaning that I did not find a reproduction... * chore: minor improvements with the types and logs * fix(remove-label): do not encode the label to make sure to remove it could lead to an issue since based on the comment it was here on purpose --- __tests__/main.test.ts | 95 +++++++++++++++++++++++++++++++++++-- src/IssueProcessor.ts | 16 +++++-- src/functions/is-labeled.ts | 4 +- 3 files changed, 107 insertions(+), 8 deletions(-) diff --git a/__tests__/main.test.ts b/__tests__/main.test.ts index 3c62ef99..495f2a9e 100644 --- a/__tests__/main.test.ts +++ b/__tests__/main.test.ts @@ -1,11 +1,8 @@ -import * as core from '@actions/core'; import * as github from '@actions/github'; -import {Octokit} from '@octokit/rest'; import { IssueProcessor, Issue, - Label, IssueProcessorOptions } from '../src/IssueProcessor'; @@ -189,6 +186,66 @@ test('processing a stale issue will close it', async () => { expect(processor.closedIssues.length).toEqual(1); }); +test('processing a stale issue containing a space in the label will close it', async () => { + const TestIssueList: Issue[] = [ + generateIssue( + 1, + 'A stale issue that should be closed', + '2020-01-01T17:00:00Z', + false, + ['state: stale'] + ) + ]; + + const opts: IssueProcessorOptions = { + ...DefaultProcessorOptions, + staleIssueLabel: 'state: stale' + }; + + const processor = new IssueProcessor( + opts, + async p => (p == 1 ? TestIssueList : []), + async (num, dt) => [], + async (issue, label) => new Date().toDateString() + ); + + // process our fake issue list + await processor.processIssues(1); + + expect(processor.staleIssues.length).toEqual(0); + expect(processor.closedIssues.length).toEqual(1); +}); + +test('processing a stale issue containing a slash in the label will close it', async () => { + const TestIssueList: Issue[] = [ + generateIssue( + 1, + 'A stale issue that should be closed', + '2020-01-01T17:00:00Z', + false, + ['lifecycle/stale'] + ) + ]; + + const opts: IssueProcessorOptions = { + ...DefaultProcessorOptions, + staleIssueLabel: 'lifecycle/stale' + }; + + const processor = new IssueProcessor( + opts, + async p => (p == 1 ? TestIssueList : []), + async (num, dt) => [], + async (issue, label) => new Date().toDateString() + ); + + // process our fake issue list + await processor.processIssues(1); + + expect(processor.staleIssues.length).toEqual(0); + expect(processor.closedIssues.length).toEqual(1); +}); + test('processing a stale PR will close it', async () => { const TestIssueList: Issue[] = [ generateIssue( @@ -650,6 +707,38 @@ test('stale label should not be removed if a comment was added by the bot (and t expect(processor.removedLabelIssues.length).toEqual(0); }); +test('stale label containing a space should be removed if a comment was added to a stale issue', async () => { + const TestIssueList: Issue[] = [ + generateIssue( + 1, + 'An issue that should un-stale', + '2020-01-01T17:00:00Z', + false, + ['stat: stale'] + ) + ]; + + const opts: IssueProcessorOptions = { + ...DefaultProcessorOptions, + removeStaleWhenUpdated: true, + staleIssueLabel: 'stat: stale' + }; + + const processor = new IssueProcessor( + opts, + async p => (p == 1 ? TestIssueList : []), + async (num, dt) => [{user: {login: 'notme', type: 'User'}}], // return a fake comment to indicate there was an update + async (issue, label) => new Date().toDateString() + ); + + // process our fake issue list + await processor.processIssues(1); + + expect(processor.closedIssues.length).toEqual(0); + expect(processor.staleIssues.length).toEqual(0); + expect(processor.removedLabelIssues.length).toEqual(1); +}); + test('stale issues should not be closed until after the closed number of days', async () => { let lastUpdate = new Date(); lastUpdate.setDate(lastUpdate.getDate() - 5); diff --git a/src/IssueProcessor.ts b/src/IssueProcessor.ts index 7572a5e4..329543a9 100644 --- a/src/IssueProcessor.ts +++ b/src/IssueProcessor.ts @@ -1,5 +1,6 @@ import * as core from '@actions/core'; import {context, getOctokit} from '@actions/github'; +import {GitHub} from '@actions/github/lib/utils'; import {GetResponseTypeFromEndpointMethod} from '@octokit/types'; import {isLabeled} from './functions/is-labeled'; import {labelsToList} from './functions/labels-to-list'; @@ -68,7 +69,7 @@ export interface IssueProcessorOptions { * Handle processing of issues for staleness/closure. */ export class IssueProcessor { - readonly client: any; // need to make this the correct type + readonly client: InstanceType; readonly options: IssueProcessorOptions; private operationsLeft = 0; @@ -176,7 +177,13 @@ export class IssueProcessor { } // does this issue have a stale label? - let isStale = isLabeled(issue, staleLabel); + let isStale: boolean = isLabeled(issue, staleLabel); + + if (isStale) { + core.info(`This issue has a stale label`); + } else { + core.info(`This issue hasn't a stale label`); + } // should this issue be marked stale? const shouldBeStale = !IssueProcessor.updatedSince( @@ -506,12 +513,13 @@ export class IssueProcessor { // Remove a label from an issue private async removeLabel(issue: Issue, label: string): Promise { - core.info(`Removing label from issue #${issue.number}`); + core.info(`Removing label "${label}" from issue #${issue.number}`); this.removedLabelIssues.push(issue); this.operationsLeft -= 1; + // @todo remove the debug only to be able to test the code below if (this.options.debugOnly) { return; } @@ -521,7 +529,7 @@ export class IssueProcessor { owner: context.repo.owner, repo: context.repo.repo, issue_number: issue.number, - name: encodeURIComponent(label) // A label can have a "?" in the name + name: label }); } catch (error) { core.error(`Error removing a label: ${error.message}`); diff --git a/src/functions/is-labeled.ts b/src/functions/is-labeled.ts index 97ee5e37..5ad332a7 100644 --- a/src/functions/is-labeled.ts +++ b/src/functions/is-labeled.ts @@ -1,6 +1,8 @@ import deburr from 'lodash.deburr'; import {Issue, Label} from '../IssueProcessor'; +type CleanLabel = string; + /** * @description * Check if the label is listed as a label of the issue @@ -19,6 +21,6 @@ export function isLabeled( }); } -function cleanLabel(label: Readonly): string { +function cleanLabel(label: Readonly): CleanLabel { return deburr(label.toLowerCase()); }