Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Comment on PR and auto update comment #223

Merged
merged 35 commits into from
Mar 28, 2024
Merged
Show file tree
Hide file tree
Changes from 28 commits
Commits
Show all changes
35 commits
Select commit Hold shift + click to select a range
8335d42
feat: auto update PR comment
ktrz Jan 31, 2024
4b06822
feat: auto update PR comment
ktrz Jan 31, 2024
91dda8a
feat: auto update PR comment
ktrz Jan 31, 2024
d01fbb8
feat: auto update PR comment
ktrz Jan 31, 2024
d647ff1
feat: auto update PR comment
ktrz Jan 31, 2024
0ee988e
feat: auto update PR comment
ktrz Jan 31, 2024
d830ad7
feat: auto update PR comment
ktrz Feb 2, 2024
b92acc3
feat: auto update PR comment
ktrz Feb 2, 2024
2e0dafc
enable go benchmark
ktrz Feb 2, 2024
a18867a
add debug comments
ktrz Feb 2, 2024
050be97
fix updating comment request
ktrz Feb 2, 2024
962b44f
add GH token to Go bench
ktrz Feb 2, 2024
a43b1aa
enable benchmarkjs ci run
ktrz Feb 2, 2024
d0dea1e
enable remaining ci steps
ktrz Feb 2, 2024
87dacbc
Merge branch 'master' into feat/98-autoupdate-pr-comment
ktrz Mar 27, 2024
1f4122e
fix tests
ktrz Mar 27, 2024
65bdfe1
debug
ktrz Mar 27, 2024
f8f4cd3
add try catch
ktrz Mar 27, 2024
d231b43
fix build err
ktrz Mar 27, 2024
75d81ca
debug
ktrz Mar 27, 2024
ee3d6fb
debug
ktrz Mar 27, 2024
24e5892
debug
ktrz Mar 27, 2024
41f775e
debug
ktrz Mar 27, 2024
e5c39e5
cleanup
ktrz Mar 27, 2024
dc8231a
check if permissions needed
ktrz Mar 27, 2024
49be1e5
cleanup
ktrz Mar 27, 2024
378418e
comment on alert
ktrz Mar 27, 2024
e1ab906
break bench to test
ktrz Mar 27, 2024
c2148aa
break bench to test
ktrz Mar 27, 2024
213cd6a
fix bench
ktrz Mar 27, 2024
0e53188
break other bench
ktrz Mar 27, 2024
b7f1fa1
break other bench
ktrz Mar 27, 2024
362fe82
break even more
ktrz Mar 27, 2024
a87e362
fix benches
ktrz Mar 27, 2024
fddd1b8
add README
ktrz Mar 28, 2024
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/workflows/ci-results-repo.yml
Original file line number Diff line number Diff line change
Expand Up @@ -378,7 +378,7 @@ jobs:
cp ./dist/other-repo/dev/bench/data.js before_data.js

- name: Store benchmark result
uses: benchmark-action/github-action-benchmark@v1
uses: ./
with:
name: Criterion.rs Benchmark
tool: 'cargo'
Expand Down
22 changes: 21 additions & 1 deletion .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,8 @@ jobs:
skip-fetch-gh-pages: true
fail-on-alert: true
summary-always: true
github-token: ${{ secrets.GITHUB_TOKEN }}
comment-on-alert: true
- run: node ./dist/scripts/ci_validate_modification.js before_data.js 'Benchmark.Net Benchmark'

benchmarkjs:
Expand Down Expand Up @@ -69,6 +71,8 @@ jobs:
skip-fetch-gh-pages: true
fail-on-alert: true
summary-always: true
github-token: ${{ secrets.GITHUB_TOKEN }}
comment-on-alert: true
- run: node ./dist/scripts/ci_validate_modification.js before_data.js 'Benchmark.js Benchmark'

catch2-framework:
Expand Down Expand Up @@ -104,6 +108,8 @@ jobs:
skip-fetch-gh-pages: true
fail-on-alert: true
summary-always: true
github-token: ${{ secrets.GITHUB_TOKEN }}
comment-on-alert: true
- run: node ./dist/scripts/ci_validate_modification.js before_data.js 'Catch2 Benchmark'

cpp-framework:
Expand Down Expand Up @@ -141,6 +147,8 @@ jobs:
skip-fetch-gh-pages: true
fail-on-alert: true
summary-always: true
github-token: ${{ secrets.GITHUB_TOKEN }}
comment-on-alert: true
- run: node ./dist/scripts/ci_validate_modification.js before_data.js 'C++ Benchmark'

go:
Expand Down Expand Up @@ -174,6 +182,8 @@ jobs:
skip-fetch-gh-pages: true
fail-on-alert: true
summary-always: true
github-token: ${{ secrets.GITHUB_TOKEN }}
comment-on-alert: true
- run: node ./dist/scripts/ci_validate_modification.js before_data.js 'Go Benchmark'

java-jmh:
Expand Down Expand Up @@ -211,6 +221,8 @@ jobs:
skip-fetch-gh-pages: true
fail-on-alert: true
summary-always: true
github-token: ${{ secrets.GITHUB_TOKEN }}
comment-on-alert: true
- run: node ./dist/scripts/ci_validate_modification.js before_data.js 'JMH Benchmark'

julia-benchmark:
Expand Down Expand Up @@ -250,6 +262,8 @@ jobs:
skip-fetch-gh-pages: true
fail-on-alert: true
summary-always: true
github-token: ${{ secrets.GITHUB_TOKEN }}
comment-on-alert: true
- run: node ./dist/scripts/ci_validate_modification.js before_data.js 'Julia benchmark'

pytest-benchmark:
Expand Down Expand Up @@ -286,6 +300,8 @@ jobs:
skip-fetch-gh-pages: true
fail-on-alert: true
summary-always: true
github-token: ${{ secrets.GITHUB_TOKEN }}
comment-on-alert: true
- run: node ./dist/scripts/ci_validate_modification.js before_data.js 'Python Benchmark with pytest-benchmark'

rust:
Expand Down Expand Up @@ -316,6 +332,8 @@ jobs:
output-file-path: examples/rust/output.txt
fail-on-alert: true
summary-always: true
github-token: ${{ secrets.GITHUB_TOKEN }}
comment-on-alert: true
- run: node ./dist/scripts/ci_validate_modification.js before_data.js 'Rust Benchmark'

rust-criterion-rs-framework:
Expand All @@ -339,13 +357,15 @@ jobs:
cp ./dev/bench/data.js before_data.js
git checkout -
- name: Store benchmark result
uses: benchmark-action/github-action-benchmark@v1
uses: ./
with:
name: Criterion.rs Benchmark
tool: 'cargo'
output-file-path: examples/criterion-rs/output.txt
fail-on-alert: true
summary-always: true
github-token: ${{ secrets.GITHUB_TOKEN }}
comment-on-alert: true
- run: node ./dist/scripts/ci_validate_modification.js before_data.js 'Criterion.rs Benchmark'

only-alert-with-cache:
Expand Down
2 changes: 1 addition & 1 deletion examples/benchmarkjs/bench.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ const { fib } = require('./index');

suite
.add('fib(10)', () => {
fib(10);
fib(15);
})
.add('fib(20)', () => {
fib(20);
Expand Down
13 changes: 13 additions & 0 deletions src/comment/benchmarkCommentTags.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
const BENCHMARK_COMMENT_TAG = 'github-benchmark-action-comment';

export function benchmarkStartTag(commentId: string) {
return `<!-- ${BENCHMARK_COMMENT_TAG}(start): ${commentId} -->`;
}

export function benchmarkEndTag(commentId: string) {
return `<!-- ${BENCHMARK_COMMENT_TAG}(end): ${commentId} -->`;
}

export function wrapBodyWithBenchmarkTags(commentId: string, body: string) {
return `${benchmarkStartTag(commentId)}\n${body}\n${benchmarkEndTag(commentId)}`;
}
28 changes: 28 additions & 0 deletions src/comment/findExistingPRReviewId.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
import * as github from '@actions/github';
import { benchmarkStartTag } from './benchmarkCommentTags';
import * as core from '@actions/core';

export async function findExistingPRReviewId(
repoOwner: string,
repoName: string,
pullRequestNumber: number,
benchName: string,
token: string,
) {
core.debug('findExistingPRReviewId start');
const client = github.getOctokit(token);

const existingReviewsResponse = await client.rest.pulls.listReviews({
owner: repoOwner,
repo: repoName,
// eslint-disable-next-line @typescript-eslint/naming-convention
pull_number: pullRequestNumber,
});

const existingReview = existingReviewsResponse.data.find((review) =>
review.body.startsWith(benchmarkStartTag(benchName)),
);

core.debug('findExistingPRReviewId start');
return existingReview?.id;
}
25 changes: 25 additions & 0 deletions src/comment/leaveCommitComment.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
import * as github from '@actions/github';
import * as core from '@actions/core';
import { wrapBodyWithBenchmarkTags } from './benchmarkCommentTags';

export async function leaveCommitComment(
repoOwner: string,
repoName: string,
commitId: string,
body: string,
commentId: string,
token: string,
) {
core.debug('leaveCommitComment start');
const client = github.getOctokit(token);
const response = await client.rest.repos.createCommitComment({
owner: repoOwner,
repo: repoName,
// eslint-disable-next-line @typescript-eslint/naming-convention
commit_sha: commitId,
body: wrapBodyWithBenchmarkTags(commentId, body),
});
console.log(`Comment was sent to ${response.url}. Response:`, response.status, response.data);
core.debug('leaveCommitComment end');
return response;
}
72 changes: 72 additions & 0 deletions src/comment/leavePRComment.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,72 @@
import * as github from '@actions/github';
import { wrapBodyWithBenchmarkTags } from './benchmarkCommentTags';
import { findExistingPRReviewId } from './findExistingPRReviewId';
import * as core from '@actions/core';

export async function leavePRComment(
repoOwner: string,
repoName: string,
pullRequestNumber: number,
body: string,
commentId: string,
token: string,
) {
try {
core.debug('leavePRComment start');
const client = github.getOctokit(token);

const bodyWithTags = wrapBodyWithBenchmarkTags(commentId, body);

const existingCommentId = await findExistingPRReviewId(
repoOwner,
repoName,
pullRequestNumber,
commentId,
token,
);

if (!existingCommentId) {
core.debug('creating new pr comment');
const createReviewResponse = await client.rest.pulls.createReview({
owner: repoOwner,
repo: repoName,
// eslint-disable-next-line @typescript-eslint/naming-convention
pull_number: pullRequestNumber,
event: 'COMMENT',
body: bodyWithTags,
});

console.log(
`Comment was created via ${createReviewResponse.url}. Response:`,
createReviewResponse.status,
createReviewResponse.data,
);

core.debug('leavePRComment end');
return createReviewResponse;
}

core.debug('updating existing pr comment');
const updateReviewResponse = await client.rest.pulls.updateReview({
owner: repoOwner,
repo: repoName,
// eslint-disable-next-line @typescript-eslint/naming-convention
pull_number: pullRequestNumber,
// eslint-disable-next-line @typescript-eslint/naming-convention
review_id: existingCommentId,
body: bodyWithTags,
});

console.log(
`Comment was updated via ${updateReviewResponse.url}. Response:`,
updateReviewResponse.status,
updateReviewResponse.data,
);
core.debug('leavePRComment end');

return updateReviewResponse;
} catch (err) {
console.log('error', err);
throw err;
}
}
25 changes: 9 additions & 16 deletions src/write.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@
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';

export type BenchmarkSuites = { [name: string]: Benchmark[] };
export interface DataJson {
Expand Down Expand Up @@ -236,24 +238,15 @@
return lines.join('\n');
}

async function leaveComment(commitId: string, body: string, token: string) {
async function leaveComment(commitId: string, body: string, commentId: string, token: string) {
core.debug('Sending comment:\n' + body);

const repoMetadata = getCurrentRepoMetadata();
const repoUrl = repoMetadata.html_url ?? '';
const client = github.getOctokit(token);
const res = await client.rest.repos.createCommitComment({
owner: repoMetadata.owner.login,
repo: repoMetadata.name,
// eslint-disable-next-line @typescript-eslint/naming-convention
commit_sha: commitId,
body,
});

const commitUrl = `${repoUrl}/commit/${commitId}`;
console.log(`Comment was sent to ${commitUrl}. Response:`, res.status, res.data);
const pr = github.context.payload.pull_request;

return res;
return await (pr?.number
? leavePRComment(repoMetadata.owner.login, repoMetadata.name, pr.number, body, commentId, token)
: leaveCommitComment(repoMetadata.owner.login, repoMetadata.name, commitId, body, commentId, token));
}

async function handleComment(benchName: string, curSuite: Benchmark, prevSuite: Benchmark, config: Config) {
Expand All @@ -272,7 +265,7 @@

const body = buildComment(benchName, curSuite, prevSuite);

await leaveComment(curSuite.commit.id, body, githubToken);
await leaveComment(curSuite.commit.id, body, `${benchName} Summary`, githubToken);
}

async function handleAlert(benchName: string, curSuite: Benchmark, prevSuite: Benchmark, config: Config) {
Expand All @@ -298,7 +291,7 @@
if (!githubToken) {
throw new Error("'comment-on-alert' input is set but 'github-token' input is not set");
}
const res = await leaveComment(curSuite.commit.id, body, githubToken);
const res = await leaveComment(curSuite.commit.id, body, `${benchName} Alert`, githubToken);
url = res.data.html_url;
message = body + `\nComment was generated at ${url}`;
}
Expand Down Expand Up @@ -443,7 +436,7 @@
console.log(
`Automatically pushed the generated commit to ${ghPagesBranch} branch since 'auto-push' is set to true`,
);
} catch (err: any) {

Check warning on line 439 in src/write.ts

View workflow job for this annotation

GitHub Actions / Run linting and formatting check

Unexpected any. Specify a different type
if (!isRemoteRejectedError(err)) {
throw err;
}
Expand Down
6 changes: 5 additions & 1 deletion test/write.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import { Benchmark } from '../src/extract';
import { DataJson, writeBenchmark } from '../src/write';
import { expect } from '@jest/globals';
import { FakedOctokit, fakedRepos } from './fakedOctokit';
import { wrapBodyWithBenchmarkTags } from '../src/comment/benchmarkCommentTags';

const ok: (x: any, msg?: string) => asserts x = (x, msg) => {
try {
Expand Down Expand Up @@ -771,7 +772,10 @@ describe.each(['https://github.com', 'https://github.enterprise.corp'])('writeBe
// Last line is appended only for failure message
const messageLines = caughtError.message.split('\n');
ok(messageLines.length > 0);
const expectedMessage = messageLines.slice(0, -1).join('\n');
const expectedMessage = wrapBodyWithBenchmarkTags(
'Test benchmark Alert',
messageLines.slice(0, -1).join('\n'),
);
ok(
fakedRepos.spyOpts.length > 0,
`len: ${fakedRepos.spyOpts.length}, caught: ${caughtError.message}`,
Expand Down
Loading