Replace stale alert comments, even when there are no new alerts

Author: 414owen

src/comment/leaveCommitComment.ts

@@ -1,6 +1,6 @@
 import * as github from '@actions/github';
 import * as core from '@actions/core';
-import { wrapBodyWithBenchmarkTags } from './benchmarkCommentTags';
+import { benchmarkStartTag, wrapBodyWithBenchmarkTags } from './benchmarkCommentTags';
 
 export async function leaveCommitComment(
     repoOwner: string,
@@ -23,3 +23,45 @@ export async function leaveCommitComment(
     core.debug('leaveCommitComment end');
     return response;
 }
+
+// Commit comments are attached to the commit where the alert was detected. When the alert is later
+// resolved (no alert on the current commit), replace the existing alert comment with a new message
+// instead of leaving the stale alert comment. Returns null when no matching comment was found.
+export async function updateCommitCommentIfExists(
+    repoOwner: string,
+    repoName: string,
+    commitId: string,
+    body: string,
+    commentId: string,
+    token: string,
+) {
+    core.debug('updateCommitCommentIfExists start');
+    const client = github.getOctokit(token);
+
+    const existingCommentsResponse = await client.rest.repos.listCommentsForCommit({
+        owner: repoOwner,
+        repo: repoName,
+        // eslint-disable-next-line @typescript-eslint/naming-convention
+        commit_sha: commitId,
+    });
+
+    const existingComment = existingCommentsResponse.data.find((comment) =>
+        comment.body.startsWith(benchmarkStartTag(commentId)),
+    );
+
+    if (!existingComment) {
+        core.debug(`No existing alert comment was found on commit ${commitId}. Skipping comment update`);
+        return null;
+    }
+
+    const updateResponse = await client.rest.repos.updateCommitComment({
+        owner: repoOwner,
+        repo: repoName,
+        // eslint-disable-next-line @typescript-eslint/naming-convention
+        comment_id: existingComment.id,
+        body: wrapBodyWithBenchmarkTags(commentId, body),
+    });
+    console.log(`Comment was updated via ${updateResponse.url}. Response:`, updateResponse.status, updateResponse.data);
+    core.debug('updateCommitCommentIfExists end');
+    return updateResponse;
+}

src/comment/leavePRComment.ts

@@ -10,6 +10,7 @@ export async function leavePRComment(
     body: string,
     commentId: string,
     token: string,
+    updateOnly = false,
 ) {
     try {
         core.debug('leavePRComment start');
@@ -26,6 +27,10 @@ export async function leavePRComment(
         );
 
         if (!existingCommentId) {
+            if (updateOnly) {
+                core.debug('No existing alert comment was found. Skipping comment creation');
+                return null;
+            }
             core.debug('creating new pr comment');
             const createReviewResponse = await client.rest.pulls.createReview({
                 owner: repoOwner,

src/write.ts

@@ -8,7 +8,7 @@ import { Benchmark, BenchmarkResult } from './extract';
 import { Config, ToolType } from './config';
 import { DEFAULT_INDEX_HTML } from './default_index_html';
 import { leavePRComment } from './comment/leavePRComment';
-import { leaveCommitComment } from './comment/leaveCommitComment';
+import { leaveCommitComment, updateCommitCommentIfExists } from './comment/leaveCommitComment';
 import { addBenchmarkEntry } from './addBenchmarkEntry';
 
 export type BenchmarkSuites = { [name: string]: Benchmark[] };
@@ -348,6 +348,28 @@ function buildAlertComment(
     return lines.join('\n');
 }
 
+function buildNoAlertsComment(benchName: string, curSuite: Benchmark, prevSuite: Benchmark, cc: string[]): string {
+    // Do not show benchmark name if it is the default value 'Benchmark'.
+    const benchmarkText = benchName === 'Benchmark' ? '' : ` for **'${benchName}'**`;
+    const lines = [
+        '# Performance Report',
+        '',
+        `No performance alerts${benchmarkText}.`,
+        '',
+        `Previous commit: ${prevSuite.commit.id}`,
+        `Current commit: ${curSuite.commit.id}`,
+    ];
+
+    // Footer
+    lines.push('', commentFooter());
+
+    if (cc.length > 0) {
+        lines.push('', `CC: ${cc.join(' ')}`);
+    }
+
+    return lines.join('\n');
+}
+
 async function leaveComment(commitId: string, body: string, commentId: string, token: string) {
     core.debug('Sending comment:\n' + body);
 
@@ -378,6 +400,37 @@ async function handleComment(benchName: string, curSuite: Benchmark, prevSuite:
     await leaveComment(curSuite.commit.id, body, `${benchName} Summary`, githubToken);
 }
 
+async function replaceAlertCommentWithNoAlerts(body: string, commentId: string, token: string, prevCommitId: string) {
+    core.debug('Replacing an existing alert comment with a no-alerts message:\n' + body);
+
+    const repoMetadata = getCurrentRepoMetadata();
+    const pr = github.context.payload.pull_request;
+
+    if (pr?.number) {
+        // PR review comments are updated in place, so only update an existing alert comment
+        return await leavePRComment(
+            repoMetadata.owner.login,
+            repoMetadata.name,
+            pr.number,
+            body,
+            commentId,
+            token,
+            true,
+        );
+    }
+
+    // Commit comments are attached to the commit where the alert was detected. When the alert is
+    // resolved, update the alert comment left on the previous commit.
+    return await updateCommitCommentIfExists(
+        repoMetadata.owner.login,
+        repoMetadata.name,
+        prevCommitId,
+        body,
+        commentId,
+        token,
+    );
+}
+
 async function handleAlert(benchName: string, curSuite: Benchmark, prevSuite: Benchmark, config: Config) {
     const { alertThreshold, githubToken, commentOnAlert, failOnAlert, alertCommentCcUsers, failThreshold } = config;
 
@@ -389,7 +442,19 @@ async function handleAlert(benchName: string, curSuite: Benchmark, prevSuite: Be
     const [losses, gains] = findAlerts(curSuite, prevSuite, alertThreshold);
     const alerts = [...losses, ...gains];
     if (alerts.length === 0) {
-        core.debug('No performance alert found happily');
+        if (commentOnAlert) {
+            if (!githubToken) {
+                throw new Error("'comment-on-alert' input is set but 'github-token' input is not set");
+            }
+            // When a previous alert comment exists (e.g. an alert was left on an earlier commit of
+            // this PR), replace it with a message saying that no alerts are detected anymore instead
+            // of leaving the stale alert comment. Do not create a new comment when none exists.
+            core.debug('No performance alert was found. Replacing an existing alert comment with a no-alerts message');
+            const body = buildNoAlertsComment(benchName, curSuite, prevSuite, alertCommentCcUsers);
+            await replaceAlertCommentWithNoAlerts(body, `${benchName} Alert`, githubToken, prevSuite.commit.id);
+        } else {
+            core.debug('No performance alert found happily');
+        }
         return;
     }
 
@@ -409,8 +474,10 @@ async function handleAlert(benchName: string, curSuite: Benchmark, prevSuite: Be
             throw new Error("'comment-on-alert' input is set but 'github-token' input is not set");
         }
         const res = await leaveComment(curSuite.commit.id, body, `${benchName} Alert`, githubToken);
-        const url = res.data.html_url;
-        message = body + `\nComment was generated at ${url}`;
+        if (res) {
+            const url = res.data.html_url;
+            message = body + `\nComment was generated at ${url}`;
+        }
     }
 
     if (failOnAlert) {

test/fakedOctokit.ts

@@ -1,9 +1,44 @@
 type OctokitOpts = { owner: string; repo: string; commit_sha: string; body: string };
+type UpdateCommitCommentOpts = { owner: string; repo: string; comment_id: number; body: string };
+type ReviewOpts = {
+    owner: string;
+    repo: string;
+    pull_number: number;
+    event?: string;
+    body: string;
+    review_id?: number;
+};
+type ReviewCall = { method: 'createReview' | 'updateReview'; opts: ReviewOpts };
+type CommentLike = { id: number; body: string };
+
 class FakedOctokitRepos {
     spyOpts: OctokitOpts[];
+    updatedCommitComments: UpdateCommitCommentOpts[];
+    commitComments: CommentLike[];
+
     constructor() {
         this.spyOpts = [];
+        this.updatedCommitComments = [];
+        this.commitComments = [];
+    }
+
+    setCommitComments(comments: CommentLike[]) {
+        this.commitComments = comments;
+    }
+
+    listCommentsForCommit() {
+        return Promise.resolve({ data: this.commitComments });
+    }
+
+    updateCommitComment(opt: UpdateCommitCommentOpts) {
+        this.updatedCommitComments.push(opt);
+        return Promise.resolve({
+            url: 'https://dummy-comment-url',
+            status: 200,
+            data: {},
+        });
     }
+
     createCommitComment(opt: OctokitOpts) {
         this.spyOpts.push(opt);
         return Promise.resolve({
@@ -13,19 +48,69 @@ class FakedOctokitRepos {
             },
         });
     }
+
     lastCall(): OctokitOpts {
         return this.spyOpts[this.spyOpts.length - 1];
     }
+
     clear() {
         this.spyOpts = [];
+        this.updatedCommitComments = [];
+        this.commitComments = [];
     }
 }
 
 export const fakedRepos = new FakedOctokitRepos();
 
+class FakedOctokitPulls {
+    reviews: CommentLike[];
+    reviewCalls: ReviewCall[];
+
+    constructor() {
+        this.reviews = [];
+        this.reviewCalls = [];
+    }
+
+    setReviews(reviews: CommentLike[]) {
+        this.reviews = reviews;
+    }
+
+    listReviews() {
+        return Promise.resolve({ data: this.reviews });
+    }
+
+    createReview(opt: ReviewOpts) {
+        this.reviewCalls.push({ method: 'createReview', opts: opt });
+        return Promise.resolve({
+            status: 200,
+            data: {
+                html_url: 'https://dummy-comment-url',
+            },
+        });
+    }
+
+    updateReview(opt: ReviewOpts) {
+        this.reviewCalls.push({ method: 'updateReview', opts: opt });
+        return Promise.resolve({
+            status: 200,
+            data: {
+                html_url: 'https://dummy-comment-url',
+            },
+        });
+    }
+
+    clear() {
+        this.reviews = [];
+        this.reviewCalls = [];
+    }
+}
+
+export const fakedPulls = new FakedOctokitPulls();
+
 export class FakedOctokit {
     rest = {
         repos: fakedRepos,
+        pulls: fakedPulls,
     };
     opt: { token: string };
     constructor(token: string) {