Merge pull request #17 from pksunkara/master

Added options to make PR compare against base branch and then output result

README.md

@@ -368,6 +368,23 @@ Please see the 'Commit comment' section for more details.
 If it is set to `true`, this action automatically pushes the generated commit to GitHub Pages branch.
 Otherwise, you need to push it by your own. Please read 'Commit comment' section above for more details.
 
+#### `comment-always` (Optional)
+
+- Type: Boolean
+- Default: `false`
+
+If it is set to `true`, this action will leave a commit comment comparing the current benchmark with previous.
+`github-token` is necessary as well. Please note that a personal access token is not necessary to
+send a commit comment. `secrets.GITHUB_TOKEN` is sufficient.
+
+#### `save-data-file` (Optional)
+
+- Type: Boolean
+- Default: `true`
+
+If it is set to `true`, this action will not save the current benchmark to the external data file.
+You can use this option to set up your action to compare the benchmarks between PR and base branch.
+
 #### `alert-threshold` (Optional)
 
 - Type: String

action.yml

@@ -35,6 +35,14 @@ inputs:
     description: 'Skip pulling GitHub Pages branch before generating an auto commit'
     required: false
     default: false
+  comment-always:
+    description: 'Leave a comment with benchmark result comparison. To enable this feature, github-token input must be given as well'
+    required: false
+    default: false
+  save-data-file:
+    description: 'Save the benchmark data to external file'
+    required: false
+    default: true
   comment-on-alert:
     description: 'Leave an alert comment when current benchmark result is worse than previous. Threshold is specified with alert-comment-threshold input. To enable this feature, github-token input must be given as well'
     required: false

src/config.ts

@@ -13,6 +13,8 @@ export interface Config {
     githubToken: string | undefined;
     autoPush: boolean;
     skipFetchGhPages: boolean;
+    commentAlways: boolean;
+    saveDataFile: boolean;
     commentOnAlert: boolean;
     alertThreshold: number;
     failOnAlert: boolean;
@@ -210,6 +212,8 @@ export async function configFromJobInput(): Promise<Config> {
     const githubToken: string | undefined = core.getInput('github-token') || undefined;
     const autoPush = getBoolInput('auto-push');
     const skipFetchGhPages = getBoolInput('skip-fetch-gh-pages');
+    const commentAlways = getBoolInput('comment-always');
+    const saveDataFile = getBoolInput('save-data-file');
     const commentOnAlert = getBoolInput('comment-on-alert');
     const alertThreshold = getPercentageInput('alert-threshold');
     const failOnAlert = getBoolInput('fail-on-alert');
@@ -226,6 +230,9 @@ export async function configFromJobInput(): Promise<Config> {
     if (autoPush) {
         validateGitHubToken('auto-push', githubToken, 'to push GitHub pages branch to remote');
     }
+    if (commentAlways) {
+        validateGitHubToken('comment-always', githubToken, 'to send commit comment');
+    }
     if (commentOnAlert) {
         validateGitHubToken('comment-on-alert', githubToken, 'to send commit comment on alert');
     }
@@ -246,6 +253,8 @@ export async function configFromJobInput(): Promise<Config> {
         githubToken,
         autoPush,
         skipFetchGhPages,
+        commentAlways,
+        saveDataFile,
         commentOnAlert,
         alertThreshold,
         failOnAlert,

src/extract.ts

@@ -178,7 +178,7 @@ function extractCargoResult(output: string): BenchmarkResult[] {
     const ret = [];
     // Example:
     //   test bench_fib_20 ... bench:      37,174 ns/iter (+/- 7,527)
-    const reExtract = /^test (\w+)\s+\.\.\. bench:\s+([0-9,]+) ns\/iter \((\+\/- [0-9,]+)\)$/;
+    const reExtract = /^test (\w+)\s+\.\.\. bench:\s+([0-9,]+) ns\/iter \(\+\/- ([0-9,]+)\)$/;
     const reComma = /,/g;
 
     for (const line of lines) {
@@ -189,12 +189,12 @@ function extractCargoResult(output: string): BenchmarkResult[] {
 
         const name = m[1];
         const value = parseInt(m[2].replace(reComma, ''), 10);
-        const range = m[3];
+        const range = m[3].replace(reComma, '');
 
         ret.push({
             name,
             value,
-            range,
+            range: `± ${range}`,
             unit: 'ns/iter',
         });
     }
@@ -363,7 +363,7 @@ function extractCatch2Result(output: string): BenchmarkResult[] {
             );
         }
 
-        const range = '+/- ' + stdDev[1].trim();
+        const range = '± ' + stdDev[1].trim();
 
         // Skip empty line
         const [emptyLine, emptyLineNum] = nextLine();

src/write.ts

@@ -117,7 +117,65 @@ function getCurrentRepo() {
 }
 
 function floatStr(n: number) {
-    return Number.isInteger(n) ? n.toFixed(1) : n.toString();
+    if (Number.isInteger(n)) {
+        return n.toFixed(0);
+    }
+
+    if (n > 1) {
+        return n.toFixed(2);
+    }
+
+    return n.toString();
+}
+
+function strVal(b: BenchmarkResult): string {
+    let s = `\`${b.value}\` ${b.unit}`;
+    if (b.range) {
+        s += ` (\`${b.range}\`)`;
+    }
+    return s;
+}
+
+function commentFooter(): string {
+    const repo = getCurrentRepo();
+    // eslint-disable-next-line @typescript-eslint/camelcase
+    const repoUrl = repo.html_url ?? '';
+    const actionUrl = repoUrl + '/actions?query=workflow%3A' + encodeURIComponent(github.context.workflow);
+
+    return `This comment was automatically generated by [workflow](${actionUrl}) using [github-action-benchmark](https://github.com/marketplace/actions/continuous-benchmark).`;
+}
+
+function buildComment(benchName: string, curSuite: Benchmark, prevSuite: Benchmark): string {
+    const lines = [
+        `# ${benchName}`,
+        '',
+        '<details>',
+        '',
+        `| Benchmark suite | Current: ${curSuite.commit.id} | Previous: ${prevSuite.commit.id} | Ratio |`,
+        '|-|-|-|-|',
+    ];
+
+    for (const current of curSuite.benches) {
+        let line;
+        const prev = prevSuite.benches.find(i => i.name === current.name);
+
+        if (prev) {
+            const ratio = biggerIsBetter(curSuite.tool)
+                ? prev.value / current.value // e.g. current=100, prev=200
+                : current.value / prev.value;
+
+            line = `| \`${current.name}\` | ${strVal(current)} | ${strVal(prev)} | \`${floatStr(ratio)}\` |`;
+        } else {
+            line = `| \`${current.name}\` | ${strVal(current)} | | |`;
+        }
+
+        lines.push(line);
+    }
+
+    // Footer
+    lines.push('', '</details>', '', commentFooter());
+
+    return lines.join('\n');
 }
 
 function buildAlertComment(
@@ -128,14 +186,6 @@ function buildAlertComment(
     threshold: number,
     cc: string[],
 ): string {
-    function strVal(b: BenchmarkResult): string {
-        let s = `\`${b.value}\` ${b.unit}`;
-        if (b.range) {
-            s += ` (\`${b.range}\`)`;
-        }
-        return s;
-    }
-
     // Do not show benchmark name if it is the default value 'Benchmark'.
     const benchmarkText = benchName === 'Benchmark' ? '' : ` **'${benchName}'**`;
     const title = threshold === 0 ? '# Performance Report' : '# :warning: **Performance Alert** :warning:';
@@ -156,17 +206,8 @@ function buildAlertComment(
         lines.push(line);
     }
 
-    const repo = getCurrentRepo();
-    // eslint-disable-next-line @typescript-eslint/camelcase
-    const repoUrl = repo.html_url ?? '';
-    const actionUrl = repoUrl + '/actions?query=workflow%3A' + encodeURIComponent(github.context.workflow);
-    core.debug(`Action URL: ${actionUrl}`);
-
     // Footer
-    lines.push(
-        '',
-        `This comment was automatically generated by [workflow](${actionUrl}) using [github-action-benchmark](https://github.com/marketplace/actions/continuous-benchmark).`,
-    );
+    lines.push('', commentFooter());
 
     if (cc.length > 0) {
         lines.push('', `CC: ${cc.join(' ')}`);
@@ -176,7 +217,7 @@ function buildAlertComment(
 }
 
 async function leaveComment(commitId: string, body: string, token: string) {
-    core.debug('Sending alert comment:\n' + body);
+    core.debug('Sending comment:\n' + body);
 
     const repo = getCurrentRepo();
     // eslint-disable-next-line @typescript-eslint/camelcase
@@ -191,11 +232,30 @@ async function leaveComment(commitId: string, body: string, token: string) {
     });
 
     const commitUrl = `${repoUrl}/commit/${commitId}`;
-    console.log(`Alert comment was sent to ${commitUrl}. Response:`, res.status, res.data);
+    console.log(`Comment was sent to ${commitUrl}. Response:`, res.status, res.data);
 
     return res;
 }
 
+async function handleComment(benchName: string, curSuite: Benchmark, prevSuite: Benchmark, config: Config) {
+    const { commentAlways, githubToken } = config;
+
+    if (!commentAlways) {
+        core.debug('Comment check was skipped because comment-always is disabled');
+        return;
+    }
+
+    if (!githubToken) {
+        throw new Error("'comment-always' input is set but 'github-token' input is not set");
+    }
+
+    core.debug('Commenting about benchmark comparison');
+
+    const body = buildComment(benchName, curSuite, prevSuite);
+
+    await leaveComment(curSuite.commit.id, body, githubToken);
+}
+
 async function handleAlert(benchName: string, curSuite: Benchmark, prevSuite: Benchmark, config: Config) {
     const { alertThreshold, githubToken, commentOnAlert, failOnAlert, alertCommentCcUsers, failThreshold } = config;
 
@@ -324,6 +384,7 @@ async function writeBenchmarkToGitHubPagesWithRetry(
 
     const data = await loadDataJs(dataPath);
     const prevBench = addBenchmarkToDataJson(name, bench, data, maxItemsInChart);
+
     await storeDataJs(dataPath, data);
 
     await git.cmd('add', dataPath);
@@ -401,10 +462,15 @@ async function writeBenchmarkToExternalJson(
     jsonFilePath: string,
     config: Config,
 ): Promise<Benchmark | null> {
-    const { name, maxItemsInChart } = config;
+    const { name, maxItemsInChart, saveDataFile } = config;
     const data = await loadDataJson(jsonFilePath);
     const prevBench = addBenchmarkToDataJson(name, bench, data, maxItemsInChart);
 
+    if (!saveDataFile) {
+        core.debug('Skipping storing benchmarks in external data file');
+        return prevBench;
+    }
+
     try {
         const jsonDirPath = path.dirname(jsonFilePath);
         await io.mkdirP(jsonDirPath);
@@ -427,6 +493,7 @@ export async function writeBenchmark(bench: Benchmark, config: Config) {
     if (prevBench === null) {
         core.debug('Alert check was skipped because previous benchmark result was not found');
     } else {
+        await handleComment(name, bench, prevBench, config);
         await handleAlert(name, bench, prevBench, config);
     }
 }

test/data/write/data-dir/original_data.js

@@ -16,7 +16,7 @@ window.BENCHMARK_DATA = {
                 },
                 "date": 1574927127603,
                 "tool": "cargo",
-                "benches": [{ "name": "bench_fib_10", "range": "+/- 20", "unit": "ns/iter", "value": 100 }]
+                "benches": [{ "name": "bench_fib_10", "range": "± 20", "unit": "ns/iter", "value": 100 }]
             }
         ]
     }

test/extract.ts

@@ -42,13 +42,13 @@ describe('extractResult()', function() {
             expected: [
                 {
                     name: 'bench_fib_10',
-                    range: '+/- 24',
+                    range: '± 24',
                     unit: 'ns/iter',
                     value: 135,
                 },
                 {
                     name: 'bench_fib_20',
-                    range: '+/- 755',
+                    range: '± 755',
                     unit: 'ns/iter',
                     value: 18149,
                 },
@@ -59,28 +59,28 @@ describe('extractResult()', function() {
             expected: [
                 {
                     name: 'Fibonacci 10',
-                    range: '+/- 19',
+                    range: '± 19',
                     unit: 'ns',
                     value: 344,
                     extra: '100 samples\n208 iterations',
                 },
                 {
                     name: 'Fibonacci 20',
-                    range: '+/- 3.256',
+                    range: '± 3.256',
                     unit: 'us',
                     value: 41.731,
                     extra: '100 samples\n2 iterations',
                 },
                 {
                     name: 'Fibonacci~ 5!',
-                    range: '+/- 4',
+                    range: '± 4',
                     unit: 'ns',
                     value: 36,
                     extra: '100 samples\n1961 iterations',
                 },
                 {
                     name: 'Fibonacci-15_bench',
-                    range: '+/- 362',
+                    range: '± 362',
                     unit: 'us',
                     value: 3.789,
                     extra: '100 samples\n20 iterations',
@@ -207,14 +207,14 @@ describe('extractResult()', function() {
                 {
                     extra: '100 samples\n76353 iterations',
                     name: 'Fibonacci 10',
-                    range: '+/- 0',
+                    range: '± 0',
                     unit: 'ns',
                     value: 0,
                 },
                 {
                     extra: '100 samples\n75814 iterations',
                     name: 'Fibonacci 20',
-                    range: '+/- 0',
+                    range: '± 0',
                     unit: 'ns',
                     value: 1,
                 },