Update logging to be info level, fix un-staling of issues (#83)

* The bot would un-stale issues because of how it checked staleness
* Made logging info by default so its easier to troubleshoot customer issues/runs
* Updated operation counts
This commit is contained in:
Ross Brodbeck 2020-05-26 09:16:38 -04:00 committed by GitHub
parent 96b682d29f
commit b6f9559915
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 142 additions and 106 deletions

View File

@ -426,6 +426,7 @@ test('exempt issue labels will not be marked stale (multi issue label)', async (
expect(processor.staleIssues.length).toEqual(0); expect(processor.staleIssues.length).toEqual(0);
expect(processor.closedIssues.length).toEqual(0); expect(processor.closedIssues.length).toEqual(0);
expect(processor.removedLabelIssues.length).toEqual(0);
}); });
test('exempt pr labels will not be marked stale', async () => { test('exempt pr labels will not be marked stale', async () => {
@ -474,6 +475,7 @@ test('stale issues should not be closed if days is set to -1', async () => {
await processor.processIssues(1); await processor.processIssues(1);
expect(processor.closedIssues.length).toEqual(0); expect(processor.closedIssues.length).toEqual(0);
expect(processor.removedLabelIssues.length).toEqual(0);
}); });
test('stale label should be removed if a comment was added to a stale issue', async () => { test('stale label should be removed if a comment was added to a stale issue', async () => {
@ -562,6 +564,7 @@ test('stale issues should not be closed until after the closed number of days',
await processor.processIssues(1); await processor.processIssues(1);
expect(processor.closedIssues.length).toEqual(0); expect(processor.closedIssues.length).toEqual(0);
expect(processor.removedLabelIssues.length).toEqual(0);
expect(processor.staleIssues.length).toEqual(1); expect(processor.staleIssues.length).toEqual(1);
}); });
@ -593,5 +596,37 @@ test('stale issues should be closed if the closed nubmer of days (additive) is a
await processor.processIssues(1); await processor.processIssues(1);
expect(processor.closedIssues.length).toEqual(1); expect(processor.closedIssues.length).toEqual(1);
expect(processor.removedLabelIssues.length).toEqual(0);
expect(processor.staleIssues.length).toEqual(0); expect(processor.staleIssues.length).toEqual(0);
}); });
test('stale issues should not be closed until after the closed number of days (long)', async () => {
let lastUpdate = new Date();
lastUpdate.setDate(lastUpdate.getDate() - 10);
const TestIssueList: Issue[] = [
generateIssue(
1,
'An issue that should be marked stale but not closed',
lastUpdate.toString(),
false
)
];
const opts = DefaultProcessorOptions;
opts.daysBeforeStale = 5; // stale after 5 days
opts.daysBeforeClose = 20; // closes after 25 days
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.closedIssues.length).toEqual(0);
expect(processor.removedLabelIssues.length).toEqual(0);
expect(processor.staleIssues.length).toEqual(1);
});

87
dist/index.js vendored
View File

@ -8471,20 +8471,16 @@ class IssueProcessor {
} }
processIssues(page = 1) { processIssues(page = 1) {
return __awaiter(this, void 0, void 0, function* () { return __awaiter(this, void 0, void 0, function* () {
if (this.operationsLeft <= 0) {
core.warning('Reached max number of operations to process. Exiting.');
return 0;
}
// get the next batch of issues // get the next batch of issues
const issues = yield this.getIssues(page); const issues = yield this.getIssues(page);
this.operationsLeft -= 1; this.operationsLeft -= 1;
if (issues.length <= 0) { if (issues.length <= 0) {
core.debug('No more issues found to process. Exiting.'); core.info('No more issues found to process. Exiting.');
return this.operationsLeft; return this.operationsLeft;
} }
for (const issue of issues.values()) { for (const issue of issues.values()) {
const isPr = !!issue.pull_request; const isPr = !!issue.pull_request;
core.debug(`Found issue: issue #${issue.number} - ${issue.title} last updated ${issue.updated_at} (is pr? ${isPr})`); core.info(`Found issue: issue #${issue.number} - ${issue.title} last updated ${issue.updated_at} (is pr? ${isPr})`);
// calculate string based messages for this issue // calculate string based messages for this issue
const staleMessage = isPr const staleMessage = isPr
? this.options.stalePrMessage ? this.options.stalePrMessage
@ -8495,37 +8491,41 @@ class IssueProcessor {
const exemptLabels = IssueProcessor.parseCommaSeparatedString(isPr ? this.options.exemptPrLabels : this.options.exemptIssueLabels); const exemptLabels = IssueProcessor.parseCommaSeparatedString(isPr ? this.options.exemptPrLabels : this.options.exemptIssueLabels);
const issueType = isPr ? 'pr' : 'issue'; const issueType = isPr ? 'pr' : 'issue';
if (!staleMessage) { if (!staleMessage) {
core.debug(`Skipping ${issueType} due to empty stale message`); core.info(`Skipping ${issueType} due to empty stale message`);
continue; continue;
} }
if (issue.state === 'closed') { if (issue.state === 'closed') {
core.debug(`Skipping ${issueType} because it is closed`); core.info(`Skipping ${issueType} because it is closed`);
continue; // don't process closed issues continue; // don't process closed issues
} }
if (issue.locked) { if (issue.locked) {
core.debug(`Skipping ${issueType} because it is locked`); core.info(`Skipping ${issueType} because it is locked`);
continue; // don't process locked issues continue; // don't process locked issues
} }
if (exemptLabels.some((exemptLabel) => IssueProcessor.isLabeled(issue, exemptLabel))) { if (exemptLabels.some((exemptLabel) => IssueProcessor.isLabeled(issue, exemptLabel))) {
core.debug(`Skipping ${issueType} because it has an exempt label`); core.info(`Skipping ${issueType} because it has an exempt label`);
continue; // don't process exempt issues continue; // don't process exempt issues
} }
// does this issue have a stale label? // does this issue have a stale label?
let isStale = IssueProcessor.isLabeled(issue, staleLabel); let isStale = IssueProcessor.isLabeled(issue, staleLabel);
// should this issue be marked stale?
const shouldBeStale = !IssueProcessor.updatedSince(issue.updated_at, this.options.daysBeforeStale);
// determine if this issue needs to be marked stale first // determine if this issue needs to be marked stale first
if (!isStale && if (!isStale && shouldBeStale) {
!IssueProcessor.updatedSince(issue.updated_at, this.options.daysBeforeStale)) { core.info(`Marking ${issueType} stale because it was last updated on ${issue.updated_at} and it does not have a stale label`);
core.debug(`Marking ${issueType} stale because it was last updated on ${issue.updated_at} and it does not have a stale label`);
yield this.markStale(issue, staleMessage, staleLabel); yield this.markStale(issue, staleMessage, staleLabel);
this.operationsLeft -= 2;
isStale = true; // this issue is now considered stale isStale = true; // this issue is now considered stale
} }
// process any issues marked stale (including the issue above, if it was marked) // process the issue if it was marked stale
if (isStale) { if (isStale) {
core.debug(`Found a stale ${issueType}`); core.info(`Found a stale ${issueType}`);
yield this.processStaleIssue(issue, issueType, staleLabel); yield this.processStaleIssue(issue, issueType, staleLabel);
} }
} }
if (this.operationsLeft <= 0) {
core.warning('Reached max number of operations to process. Exiting.');
return 0;
}
// do the next batch // do the next batch
return this.processIssues(page + 1); return this.processIssues(page + 1);
}); });
@ -8534,45 +8534,44 @@ class IssueProcessor {
processStaleIssue(issue, issueType, staleLabel) { processStaleIssue(issue, issueType, staleLabel) {
var _a; var _a;
return __awaiter(this, void 0, void 0, function* () { return __awaiter(this, void 0, void 0, function* () {
const markedStaleOn = (yield this.getLabelCreationDate(issue, staleLabel)) || issue.updated_at;
core.info(`Issue #${issue.number} marked stale on: ${markedStaleOn}`);
const issueHasComments = yield this.hasCommentsSince(issue, markedStaleOn);
core.info(`Issue #${issue.number} has been commented on: ${issueHasComments}`);
const issueHasUpdate = IssueProcessor.updatedSince(issue.updated_at, this.options.daysBeforeClose + ((_a = this.options.daysBeforeStale) !== null && _a !== void 0 ? _a : 0));
core.info(`Issue #${issue.number} has been updated: ${issueHasUpdate}`);
// should we un-stale this issue?
if (this.options.removeStaleWhenUpdated && issueHasComments) {
core.info(`Issue #${issue.number} is no longer stale. Removing stale label.`);
yield this.removeLabel(issue, staleLabel);
}
// now start closing logic
if (this.options.daysBeforeClose < 0) { if (this.options.daysBeforeClose < 0) {
return; // nothing to do because we aren't closing stale issues return; // nothing to do because we aren't closing stale issues
} }
const markedStaleOn = yield this.getLabelCreationDate(issue, staleLabel);
const issueHasComments = yield this.isIssueStillStale(issue, markedStaleOn || issue.updated_at);
const issueHasUpdate = IssueProcessor.updatedSince(issue.updated_at, this.options.daysBeforeClose + ((_a = this.options.daysBeforeStale) !== null && _a !== void 0 ? _a : 0));
if (markedStaleOn) {
core.debug(`Issue #${issue.number} marked stale on: ${markedStaleOn}`);
}
else {
core.debug(`Issue #${issue.number} is not marked stale, but last update of ${issue.updated_at} is older than ${this.options.daysBeforeStale} days`);
}
core.debug(`Issue #${issue.number} has been updated: ${issueHasUpdate}`);
core.debug(`Issue #${issue.number} has been commented on: ${issueHasComments}`);
if (!issueHasComments && !issueHasUpdate) { if (!issueHasComments && !issueHasUpdate) {
core.debug(`Closing ${issueType} because it was last updated on ${issue.updated_at}`); core.info(`Closing ${issueType} because it was last updated on ${issue.updated_at}`);
yield this.closeIssue(issue); yield this.closeIssue(issue);
} }
else { else {
if (this.options.removeStaleWhenUpdated) { core.info(`Stale ${issueType} is not old enough to close yet (hasComments? ${issueHasComments}, hasUpdate? ${issueHasUpdate}`);
yield this.removeLabel(issue, staleLabel);
}
core.debug(`Ignoring stale ${issueType} because it was updated recently`);
} }
}); });
} }
// checks to see if a given issue is still stale (has had activity on it) // checks to see if a given issue is still stale (has had activity on it)
isIssueStillStale(issue, sinceDate) { hasCommentsSince(issue, sinceDate) {
return __awaiter(this, void 0, void 0, function* () { return __awaiter(this, void 0, void 0, function* () {
core.debug(`Checking for comments on issue #${issue.number} since ${sinceDate} to see if it is still stale`); core.info(`Checking for comments on issue #${issue.number} since ${sinceDate}`);
if (!sinceDate) { if (!sinceDate) {
return true; // if no date was provided then the issue was marked stale a long time ago return true;
} }
this.operationsLeft -= 1; // find any comments since the date
// find any comments since the stale label
const comments = yield this.listIssueComments(issue.number, sinceDate); const comments = yield this.listIssueComments(issue.number, sinceDate);
// if there are any user comments returned, and they were not by this bot, the issue is not stale anymore const filteredComments = comments.filter(comment => comment.user.type === 'User' &&
return (comments.filter(comment => comment.user.type === 'User' && comment.user.login !== github.context.actor);
comment.user.login !== github.context.actor).length > 0); core.info(`Comments not made by ${github.context.actor} or another bot: ${filteredComments.length}`);
// if there are any user comments returned
return filteredComments.length > 0;
}); });
} }
// grab comments for an issue since a given date // grab comments for an issue since a given date
@ -8605,7 +8604,7 @@ class IssueProcessor {
// Mark an issue as stale with a comment and a label // Mark an issue as stale with a comment and a label
markStale(issue, staleMessage, staleLabel) { markStale(issue, staleMessage, staleLabel) {
return __awaiter(this, void 0, void 0, function* () { return __awaiter(this, void 0, void 0, function* () {
core.debug(`Marking issue #${issue.number} - ${issue.title} as stale`); core.info(`Marking issue #${issue.number} - ${issue.title} as stale`);
this.staleIssues.push(issue); this.staleIssues.push(issue);
this.operationsLeft -= 2; this.operationsLeft -= 2;
if (this.options.debugOnly) { if (this.options.debugOnly) {
@ -8628,7 +8627,7 @@ class IssueProcessor {
// Close an issue based on staleness // Close an issue based on staleness
closeIssue(issue) { closeIssue(issue) {
return __awaiter(this, void 0, void 0, function* () { return __awaiter(this, void 0, void 0, function* () {
core.debug(`Closing issue #${issue.number} - ${issue.title} for being stale`); core.info(`Closing issue #${issue.number} - ${issue.title} for being stale`);
this.closedIssues.push(issue); this.closedIssues.push(issue);
this.operationsLeft -= 1; this.operationsLeft -= 1;
if (this.options.debugOnly) { if (this.options.debugOnly) {
@ -8645,7 +8644,7 @@ class IssueProcessor {
// Remove a label from an issue // Remove a label from an issue
removeLabel(issue, label) { removeLabel(issue, label) {
return __awaiter(this, void 0, void 0, function* () { return __awaiter(this, void 0, void 0, function* () {
core.debug(`Removing label ${label} from issue #${issue.number} - ${issue.title}`); core.info(`Removing label ${label} from issue #${issue.number} - ${issue.title}`);
this.removedLabelIssues.push(issue); this.removedLabelIssues.push(issue);
this.operationsLeft -= 1; this.operationsLeft -= 1;
if (this.options.debugOnly) { if (this.options.debugOnly) {
@ -8663,7 +8662,7 @@ class IssueProcessor {
///see https://developer.github.com/v3/activity/events/ ///see https://developer.github.com/v3/activity/events/
getLabelCreationDate(issue, label) { getLabelCreationDate(issue, label) {
return __awaiter(this, void 0, void 0, function* () { return __awaiter(this, void 0, void 0, function* () {
core.debug(`Checking for label ${label} on issue #${issue.number}`); core.info(`Checking for label ${label} on issue #${issue.number}`);
this.operationsLeft -= 1; this.operationsLeft -= 1;
const options = this.client.issues.listEvents.endpoint.merge({ const options = this.client.issues.listEvents.endpoint.merge({
owner: github.context.repo.owner, owner: github.context.repo.owner,

View File

@ -97,24 +97,19 @@ export class IssueProcessor {
} }
async processIssues(page: number = 1): Promise<number> { async processIssues(page: number = 1): Promise<number> {
if (this.operationsLeft <= 0) {
core.warning('Reached max number of operations to process. Exiting.');
return 0;
}
// get the next batch of issues // get the next batch of issues
const issues: Issue[] = await this.getIssues(page); const issues: Issue[] = await this.getIssues(page);
this.operationsLeft -= 1; this.operationsLeft -= 1;
if (issues.length <= 0) { if (issues.length <= 0) {
core.debug('No more issues found to process. Exiting.'); core.info('No more issues found to process. Exiting.');
return this.operationsLeft; return this.operationsLeft;
} }
for (const issue of issues.values()) { for (const issue of issues.values()) {
const isPr = !!issue.pull_request; const isPr = !!issue.pull_request;
core.debug( core.info(
`Found issue: issue #${issue.number} - ${issue.title} last updated ${issue.updated_at} (is pr? ${isPr})` `Found issue: issue #${issue.number} - ${issue.title} last updated ${issue.updated_at} (is pr? ${isPr})`
); );
@ -131,17 +126,17 @@ export class IssueProcessor {
const issueType: string = isPr ? 'pr' : 'issue'; const issueType: string = isPr ? 'pr' : 'issue';
if (!staleMessage) { if (!staleMessage) {
core.debug(`Skipping ${issueType} due to empty stale message`); core.info(`Skipping ${issueType} due to empty stale message`);
continue; continue;
} }
if (issue.state === 'closed') { if (issue.state === 'closed') {
core.debug(`Skipping ${issueType} because it is closed`); core.info(`Skipping ${issueType} because it is closed`);
continue; // don't process closed issues continue; // don't process closed issues
} }
if (issue.locked) { if (issue.locked) {
core.debug(`Skipping ${issueType} because it is locked`); core.info(`Skipping ${issueType} because it is locked`);
continue; // don't process locked issues continue; // don't process locked issues
} }
@ -150,36 +145,40 @@ export class IssueProcessor {
IssueProcessor.isLabeled(issue, exemptLabel) IssueProcessor.isLabeled(issue, exemptLabel)
) )
) { ) {
core.debug(`Skipping ${issueType} because it has an exempt label`); core.info(`Skipping ${issueType} because it has an exempt label`);
continue; // don't process exempt issues continue; // don't process exempt issues
} }
// does this issue have a stale label? // does this issue have a stale label?
let isStale = IssueProcessor.isLabeled(issue, staleLabel); let isStale = IssueProcessor.isLabeled(issue, staleLabel);
// determine if this issue needs to be marked stale first // should this issue be marked stale?
if ( const shouldBeStale = !IssueProcessor.updatedSince(
!isStale &&
!IssueProcessor.updatedSince(
issue.updated_at, issue.updated_at,
this.options.daysBeforeStale this.options.daysBeforeStale
) );
) {
core.debug( // determine if this issue needs to be marked stale first
if (!isStale && shouldBeStale) {
core.info(
`Marking ${issueType} stale because it was last updated on ${issue.updated_at} and it does not have a stale label` `Marking ${issueType} stale because it was last updated on ${issue.updated_at} and it does not have a stale label`
); );
await this.markStale(issue, staleMessage, staleLabel); await this.markStale(issue, staleMessage, staleLabel);
this.operationsLeft -= 2;
isStale = true; // this issue is now considered stale isStale = true; // this issue is now considered stale
} }
// process any issues marked stale (including the issue above, if it was marked) // process the issue if it was marked stale
if (isStale) { if (isStale) {
core.debug(`Found a stale ${issueType}`); core.info(`Found a stale ${issueType}`);
await this.processStaleIssue(issue, issueType, staleLabel); await this.processStaleIssue(issue, issueType, staleLabel);
} }
} }
if (this.operationsLeft <= 0) {
core.warning('Reached max number of operations to process. Exiting.');
return 0;
}
// do the next batch // do the next batch
return this.processIssues(page + 1); return this.processIssues(page + 1);
} }
@ -190,74 +189,77 @@ export class IssueProcessor {
issueType: string, issueType: string,
staleLabel: string staleLabel: string
) { ) {
if (this.options.daysBeforeClose < 0) { const markedStaleOn: string =
return; // nothing to do because we aren't closing stale issues (await this.getLabelCreationDate(issue, staleLabel)) || issue.updated_at;
} core.info(`Issue #${issue.number} marked stale on: ${markedStaleOn}`);
const markedStaleOn: string | undefined = await this.getLabelCreationDate( const issueHasComments: boolean = await this.hasCommentsSince(
issue, issue,
staleLabel markedStaleOn
); );
const issueHasComments: boolean = await this.isIssueStillStale( core.info(
issue, `Issue #${issue.number} has been commented on: ${issueHasComments}`
markedStaleOn || issue.updated_at
); );
const issueHasUpdate: boolean = IssueProcessor.updatedSince( const issueHasUpdate: boolean = IssueProcessor.updatedSince(
issue.updated_at, issue.updated_at,
this.options.daysBeforeClose + (this.options.daysBeforeStale ?? 0) this.options.daysBeforeClose + (this.options.daysBeforeStale ?? 0)
); );
core.info(`Issue #${issue.number} has been updated: ${issueHasUpdate}`);
if (markedStaleOn) { // should we un-stale this issue?
core.debug(`Issue #${issue.number} marked stale on: ${markedStaleOn}`); if (this.options.removeStaleWhenUpdated && issueHasComments) {
} else { core.info(
core.debug( `Issue #${issue.number} is no longer stale. Removing stale label.`
`Issue #${issue.number} is not marked stale, but last update of ${issue.updated_at} is older than ${this.options.daysBeforeStale} days`
); );
await this.removeLabel(issue, staleLabel);
}
// now start closing logic
if (this.options.daysBeforeClose < 0) {
return; // nothing to do because we aren't closing stale issues
} }
core.debug(`Issue #${issue.number} has been updated: ${issueHasUpdate}`);
core.debug(
`Issue #${issue.number} has been commented on: ${issueHasComments}`
);
if (!issueHasComments && !issueHasUpdate) { if (!issueHasComments && !issueHasUpdate) {
core.debug( core.info(
`Closing ${issueType} because it was last updated on ${issue.updated_at}` `Closing ${issueType} because it was last updated on ${issue.updated_at}`
); );
await this.closeIssue(issue); await this.closeIssue(issue);
} else { } else {
if (this.options.removeStaleWhenUpdated) { core.info(
await this.removeLabel(issue, staleLabel); `Stale ${issueType} is not old enough to close yet (hasComments? ${issueHasComments}, hasUpdate? ${issueHasUpdate}`
} );
core.debug(`Ignoring stale ${issueType} because it was updated recently`);
} }
} }
// checks to see if a given issue is still stale (has had activity on it) // checks to see if a given issue is still stale (has had activity on it)
private async isIssueStillStale( private async hasCommentsSince(
issue: Issue, issue: Issue,
sinceDate: string sinceDate: string
): Promise<boolean> { ): Promise<boolean> {
core.debug( core.info(
`Checking for comments on issue #${issue.number} since ${sinceDate} to see if it is still stale` `Checking for comments on issue #${issue.number} since ${sinceDate}`
); );
if (!sinceDate) { if (!sinceDate) {
return true; // if no date was provided then the issue was marked stale a long time ago return true;
} }
this.operationsLeft -= 1; // find any comments since the date
// find any comments since the stale label
const comments = await this.listIssueComments(issue.number, sinceDate); const comments = await this.listIssueComments(issue.number, sinceDate);
// if there are any user comments returned, and they were not by this bot, the issue is not stale anymore const filteredComments = comments.filter(
return (
comments.filter(
comment => comment =>
comment.user.type === 'User' && comment.user.type === 'User' &&
comment.user.login !== github.context.actor comment.user.login !== github.context.actor
).length > 0
); );
core.info(
`Comments not made by ${github.context.actor} or another bot: ${filteredComments.length}`
);
// if there are any user comments returned
return filteredComments.length > 0;
} }
// grab comments for an issue since a given date // grab comments for an issue since a given date
@ -296,7 +298,7 @@ export class IssueProcessor {
staleMessage: string, staleMessage: string,
staleLabel: string staleLabel: string
): Promise<void> { ): Promise<void> {
core.debug(`Marking issue #${issue.number} - ${issue.title} as stale`); core.info(`Marking issue #${issue.number} - ${issue.title} as stale`);
this.staleIssues.push(issue); this.staleIssues.push(issue);
@ -323,7 +325,7 @@ export class IssueProcessor {
// Close an issue based on staleness // Close an issue based on staleness
private async closeIssue(issue: Issue): Promise<void> { private async closeIssue(issue: Issue): Promise<void> {
core.debug( core.info(
`Closing issue #${issue.number} - ${issue.title} for being stale` `Closing issue #${issue.number} - ${issue.title} for being stale`
); );
@ -345,7 +347,7 @@ export class IssueProcessor {
// Remove a label from an issue // Remove a label from an issue
private async removeLabel(issue: Issue, label: string): Promise<void> { private async removeLabel(issue: Issue, label: string): Promise<void> {
core.debug( core.info(
`Removing label ${label} from issue #${issue.number} - ${issue.title}` `Removing label ${label} from issue #${issue.number} - ${issue.title}`
); );
@ -371,7 +373,7 @@ export class IssueProcessor {
issue: Issue, issue: Issue,
label: string label: string
): Promise<string | undefined> { ): Promise<string | undefined> {
core.debug(`Checking for label ${label} on issue #${issue.number}`); core.info(`Checking for label ${label} on issue #${issue.number}`);
this.operationsLeft -= 1; this.operationsLeft -= 1;