Action fixes (#30)

* Added all options to the action definition This corrects the action metadata for GitHub, upgrades dependencies and does some small fixes. * Better debugging for failed comments * Updated dependencies * Added some more debugging, improved formatting * Added required import * Fixed output
boa-dev · Jan 29, 2022 · 3548f9d · 3548f9d
1 parent 6937eca
commit 3548f9d
Show file tree

Hide file tree

Showing 8 changed files with 434 additions and 70 deletions.
diff --git a/.github/workflows/checks.yml b/.github/workflows/checks.yml
@@ -0,0 +1,19 @@
+name: Checks
+
+on:
+  pull_request:
+    branches:
+      - main
+  push:
+    branches:
+      - main
+
+jobs:
+  checkPrettier:
+    name: Check Prettier
+    runs-on: ubuntu-latest
+    steps:
+      - name: Checkout
+        uses: actions/checkout@v2.4.0
+      - name: Check code formatting
+        run: npx prettier --check .
diff --git a/.prettierignore b/.prettierignore
@@ -0,0 +1 @@
+dist/*
diff --git a/README.md b/README.md
@@ -1,11 +1,12 @@
-<h3 align="center">criterion-compare</h3>
-<p align="center">Compare the performance of a PR against master</p>
+# criterion-compare
+
+Compare the performance of a PR against the base branch.
 
 ---
 
 > ⚠️ Performance benchmarks provided by this action may fluctuate as load on GitHub Actions does. Run benchmarks locally before making any decisions based on the results.
 
-A GitHub action that will compare the benchmark output between a PR and master, using the project's [criterion.rs](https://github.com/bheisler/criterion.rs/) benchmarks.
+A GitHub action that will compare the benchmark output between a PR and the base branch, using the project's [criterion.rs](https://github.com/bheisler/criterion.rs/) benchmarks.
 
 ## Example
 
@@ -24,16 +25,18 @@ jobs:
     runs-on: ubuntu-latest
     steps:
       - uses: actions/checkout@master
-      - uses: jasonwilliams/criterion-compare-action@move_to_actions
+      - uses: jasonwilliams/criterion-compare-action@3.0.0
         with:
           cwd: "subDirectory (optional)"
           # Optional. Compare only this benchmark target
-          benchName: "example-bench-target" 
-          # Needed. The name of the branch to compare with.  This default uses the branch which is being pulled against
+          benchName: "example-bench-target"
+          # Needed. The name of the branch to compare with. This default uses the branch which is being pulled against
           branchName: ${{ github.base_ref }}
           token: ${{ secrets.GITHUB_TOKEN }}
 ```
 
 ## Troubleshooting
+
 ### `Unrecognized option: 'save-baseline'`
+
 If you encounter this error, you can check [this Criterion FAQ](https://bheisler.github.io/criterion.rs/book/faq.html#cargo-bench-gives-unrecognized-option-errors-for-valid-command-line-options), to find a workaround.
diff --git a/action.yml b/action.yml
@@ -2,13 +2,21 @@ name: "Criterion Compare PRs"
 description: "Run Criterion against PRs"
 inputs:
   token:
-    description: "Github Token required to upload to the PR"
+    description: "Github Token required to comment in the PR"
+    required: true
+    default: ""
+  branchName:
+    description: "The name of the branch to compare with. This default uses the branch which is being pulled against"
     required: true
     default: ""
   cwd:
     description: "Sets the directory to run the benchmarks, relative to the project directory"
     required: false
     default: ""
+  benchName:
+    description: "Compare only this benchmark"
+    required: false
+    default: ""
 runs:
-  using: "node12"
+  using: "node16"
   main: "dist/index.js"
diff --git a/dist/index.js b/dist/index.js
diff --git a/main.js b/main.js
@@ -1,23 +1,29 @@
+const { inspect } = require("util");
 const exec = require("@actions/exec");
 const core = require("@actions/core");
 const github = require("@actions/github");
 
 const context = github.context;
 
 async function main() {
-  const myToken = core.getInput("token", { required: true });
-  const options = {};
-  let myOutput;
-  let myError;
-  let cwd;
+  const inputs = {
+    token: core.getInput("token", { required: true }),
+    branchName: core.getInput("branchName", { required: true }),
+    cwd: core.getInput("cwd"),
+    benchName: core.getInput("benchName"),
+  };
+  core.debug(`Inputs: ${inspect(inputs)}`);
 
-  if ((cwd = core.getInput("cwd"))) {
-    options.cwd = cwd;
+  const options = {};
+  let myOutput = "";
+  let myError = "";
+  if (inputs.cwd) {
+    options.cwd = inputs.cwd;
   }
 
-  benchCmd = ["bench"];
-  if ((cargoBenchName = core.getInput("benchName"))) {
-    benchCmd = benchCmd.concat(["--bench", cargoBenchName]);
+  let benchCmd = ["bench"];
+  if (inputs.benchName) {
+    benchCmd = benchCmd.concat(["--bench", inputs.benchName]);
   }
 
   core.debug("### Install Critcmp ###");
@@ -30,7 +36,10 @@ async function main() {
     options
   );
   core.debug("Changes benchmarked");
-  await exec.exec("git", ["checkout", core.getInput("branchName") || github.base_ref]);
+  await exec.exec("git", [
+    "checkout",
+    core.getInput("branchName") || github.base_ref,
+  ]);
   core.debug("Checked out to base branch");
   await exec.exec(
     "cargo",
@@ -49,29 +58,36 @@ async function main() {
   };
 
   await exec.exec("critcmp", ["base", "changes"], options);
+
+  core.setOutput("stdout", myOutput);
+  core.setOutput("stderr", myError);
+
   const resultsAsMarkdown = convertToMarkdown(myOutput);
 
   // An authenticated instance of `@octokit/rest`
-  const octokit = github.getOctokit(myToken);
+  const octokit = github.getOctokit(inputs.token);
 
   const contextObj = { ...context.issue };
 
   try {
-    await octokit.issues.createComment({
+    const { data: comment } = await octokit.rest.issues.createComment({
       owner: contextObj.owner,
       repo: contextObj.repo,
       issue_number: contextObj.number,
       body: resultsAsMarkdown,
     });
-  } catch (e) {
+    core.info(
+      `Created comment id '${comment.id}' on issue '${contextObj.number}'.`
+    );
+    core.setOutput("comment-id", comment.id);
+  } catch (err) {
+    core.warning(`Failed to comment: ${err}`);
+
     // If we can't post to the comment, display results here.
     // forkedRepos only have READ ONLY access on GITHUB_TOKEN
     // https://github.saobby.my.eu.orgmunity/t5/GitHub-Actions/quot-Resource-not-accessible-by-integration-quot-for-adding-a/td-p/33925
     const resultsAsObject = convertToTableObject(myOutput);
     console.table(resultsAsObject);
-
-    core.debug(e);
-    core.debug("Failed to comment");
   }
 
   core.debug("Succesfully run!");
@@ -143,19 +159,38 @@ function convertToMarkdown(results) {
           changesFactor = Number(changesFactor);
           baseFactor = Number(baseFactor);
 
-          let changesDurSplit = changesDuration.split('±');
+          let changesDurSplit = changesDuration.split("±");
           let changesUnits = changesDurSplit[1].slice(-2);
-          let changesDurSecs = convertDurToSeconds(changesDurSplit[0], changesUnits);
-          let changesErrorSecs = convertDurToSeconds(changesDurSplit[1].slice(0, -2), changesUnits);
-
-          let baseDurSplit = baseDuration.split('±');
+          let changesDurSecs = convertDurToSeconds(
+            changesDurSplit[0],
+            changesUnits
+          );
+          let changesErrorSecs = convertDurToSeconds(
+            changesDurSplit[1].slice(0, -2),
+            changesUnits
+          );
+
+          let baseDurSplit = baseDuration.split("±");
           let baseUnits = baseDurSplit[1].slice(-2);
           let baseDurSecs = convertDurToSeconds(baseDurSplit[0], baseUnits);
-          let baseErrorSecs = convertDurToSeconds(baseDurSplit[1].slice(0, -2), baseUnits);
+          let baseErrorSecs = convertDurToSeconds(
+            baseDurSplit[1].slice(0, -2),
+            baseUnits
+          );
 
           difference = -(1 - changesDurSecs / baseDurSecs) * 100;
-          difference = (changesDurSecs <= baseDurSecs ? "" : "+") + difference.toFixed(2) + "%";
-          if (isSignificant(changesDurSecs, changesErrorSecs, baseDurSecs, baseErrorSecs)) {
+          difference =
+            (changesDurSecs <= baseDurSecs ? "" : "+") +
+            difference.toFixed(2) +
+            "%";
+          if (
+            isSignificant(
+              changesDurSecs,
+              changesErrorSecs,
+              baseDurSecs,
+              baseErrorSecs
+            )
+          ) {
             if (changesDurSecs < baseDurSecs) {
               changesDuration = `**${changesDuration}**`;
             } else if (changesDurSecs > baseDurSecs) {
@@ -183,6 +218,7 @@ function convertToMarkdown(results) {
   return `## Benchmark for ${shortSha}
   <details>
     <summary>Click to view benchmark</summary>
+
 | Test | Base         | PR               | % |
 |------|--------------|------------------|---|
 ${benchResults}
@@ -219,8 +255,7 @@ function convertToTableObject(results) {
 
         let difference = -(1 - changesFactor / baseFactor) * 100;
         difference =
-          (changesFactor <= baseFactor ? "" : "+") +
-          difference.toPrecision(2);
+          (changesFactor <= baseFactor ? "" : "+") + difference.toPrecision(2);
         if (changesFactor < baseFactor) {
           changesDuration = `**${changesDuration}**`;
         } else if (changesFactor > baseFactor) {