Question: Does this action support alerts on pull requests?

[ColinEberhardt]

I've successfully configured this action and everything is running very nicely on the main branch. I'm keen to also run this action on pull requests and receive alerts when there are benchmark regressions.

I updated my action config to run on PRs, however, it fails as follows:

0s
Run rhysd/github-action-benchmark@v1
  with:
    tool: benchmarkjs
    output-file-path: benchmark/output.txt
    comment-on-alert: true
    alert-comment-cc-users: @colineberhardt
    auto-push: true
    name: Benchmark
    gh-pages-branch: gh-pages
    benchmark-data-dir-path: dev/bench
    skip-fetch-gh-pages: false
    comment-always: false
    save-data-file: true
    alert-threshold: 200%
    fail-on-alert: false
Error: 'auto-push' is enabled but 'github-token' is not set. Please give API token to push GitHub pages branch to remote

/see ColinEberhardt/assemblyscript-regex#21

Any ideas?

[ktrz]

hi @ColinEberhardt
This is weird and should not happen. Unfortunately, logs are not available anymore

Anyway, I would suggest to not set auto-push option for benchmarks on a PR as it will push the results to gh-pages for every successful benchmark run on every PR which will pollute the data. I suggest having a separate workflow that runs for PRs and doesn't have the auto-push option set

[embano1]

jFYI, running on pull_request won't work as the action would run under the forked context w/ a different token which only has read permissions on the target repo (base). Thus comment-on-alert would also throw an error as the token cannot write. We are facing the same issue.

[gaby]

@embano1 Did you find a fix for using comment-on-alert during PR's ?

[embano1]

@embano1 Did you find a fix for using comment-on-alert during PR's ?

Nope, AFAIK since the security model around PRs hasn't change (which is good), Github Actions are still limited in those cases and there's no secure workaround available.

[ktrz]

I merged and released this new feature yesterday. Maybe instead of trying to add comments on PR, you could use a GitHub summary.

Official blog post here

[gaby]

@ktrz Thanks for the new feature. It seems the actions.yml was not updated with the new field. We are getting an unexpected field error

[miparnisari]

@ColinEberhardt your original issue is because you forgot to set this: github-token: ${{ secrets.GITHUB_TOKEN }}

I was able to get this action to publish a comment with the benchmarks on every PR: miparnisari/benchtest@05de567#commitcomment-130473327

How it works: on every push to main I generate benchmarks and save them. And on every PR I generate benchmarks and compare them against the results on main.

What I don't know: how do I get it to publish a comment on the PR and not on every commit in the PR?

[embano1]

@miparnisari

@ColinEberhardt your original issue is because you forgot to set this: github-token: ${{ secrets.GITHUB_TOKEN }}

Just to be clear, this does not work on PRs against forks.

[miparnisari]

Oh. I didn't try that scenario 😅

[gaby]

@ktrz @embano1 Seems the summary also doesn't work in PR's. Annotations do work, but they are not formatted correctly.

Is there really no way of using this Action in a Pull Request? I'm not even using a fork, it's a branch in the repo.

This is my config:

on:
  push:
    branches:
      - master
      - main
    paths-ignore:
      - "**/*.md"
  pull_request:
    paths-ignore:
      - "**/*.md"

permissions:
  # deployments permission to deploy GitHub pages website
  deployments: write
  # contents permission to update benchmark contents in gh-pages branch
  contents: write

name: Benchmark
jobs:
  Compare:
    runs-on: ubuntu-latest
    steps:
      - name: Fetch Repository
        uses: actions/checkout@v4
        with:
          fetch-depth: 0 # to be able to retrieve the last commit in main

      - name: Install Go
        uses: actions/setup-go@v5
        with:
          # NOTE: Keep this in sync with the version from go.mod
          go-version: "1.21.x"

      - name: Run Benchmark
        run: set -o pipefail; go test ./... -benchmem -run=^$ -bench . | tee output.txt

      - name: Get GitHub Runner System Information
        uses: kenchan0130/[email protected]
        id: system-info

      - name: Get Main branch SHA
        id: get-main-branch-sha
        run: |
          SHA=$(git rev-parse origin/main)
          echo "sha=$SHA" >> $GITHUB_OUTPUT

      - name: Get Benchmark Results from main branch
        id: cache
        uses: actions/cache/restore@v4
        with:
          path: ./cache
          key: ${{ steps.get-main-branch-sha.outputs.sha }}-${{ runner.os }}-${{ steps.system-info.outputs.cpu-model }}-benchmark

      # This will only run if we have Benchmark Results from main branch
      - name: Compare PR Benchmark Results with main branch
        uses: benchmark-action/[email protected]
        if: steps.cache.outputs.cache-hit == 'true'
        with:
          tool: 'go'
          output-file-path: output.txt
          external-data-json-path: ./cache/benchmark-data.json
          # Do not save the data (This allows comparing benchmarks)
          save-data-file: false
          fail-on-alert: true
          comment-on-alert: false
          github-token: ${{ secrets.GITHUB_TOKEN }}
          summary-always: true
          alert-threshold: "150%"

Output:

The summary table is never created for PR's, regardless of user/token permissions.

Other actions like the one from golangci-lint (golangci/golangci-lint-action@v6) let you use the following:

permissions:
  # Required: allow read access to the content for analysis.
  contents: read
  # Optional: allow read access to pull request. Use with `only-new-issues` option.
  pull-requests: read
  # Optional: Allow write access to checks to allow the action to annotate code in the PR.
  checks: write

[ktrz]

@gaby if you have a PR from a branch in the same repo it should work without an issue. Summary doesn't need any permission AFAIK

Could you provide a link to a workflow run?

[gaby]

@ktrz Here gofiber/fiber#3079

Workflow is here: https://github.com/gofiber/fiber/blob/betteralign/.github/workflows/benchmark.yml

[gaby]

I had to set comment-on-alert: false because I was getting HTTP 403

[ktrz]

@gaby

I will investigate the summary that is not rendering properly after I get back from holidays at the end of this week.

Making a comment requires pull-requests: write permission. It is possible that the gofiber repo has a different default value for that which would require you to add pull-requests: write to the permissions list on top of the workflow file

[gaby]

@ktrz Adding the permission and enabling comment-on-alert now adds a properly rendered table to the PR as a comment.

Will this work for a PR created from a fork? Even if we give it the permissions?

Thanks!

gofiber/fiber#3079 (review)

[ktrz]

@gaby PRs from a fork are restricted to have max permission of read for pull request so the comment won't work unfortunately in that case.

[gaby]

@ktrz I was able to get around that using a new variable that was added by GitHub. With this field I can enable comments for our branches, and auto-disable them for forks. Thanks for your help! 💪

 comment-on-alert: ${{ github.event.pull_request.head.repo.fork == false }}

This is my full workflow for reference, combining practices seen in multiple projects:

on:
  push:
    branches:
      - main
    paths-ignore:
      - "**/*.md"
  pull_request:
    paths-ignore:
      - "**/*.md"

permissions:
  # deployments permission to deploy GitHub pages website
  deployments: write
  # contents permission to update benchmark contents in gh-pages branch
  contents: write
  # allow posting comments to pull request
  pull-requests: write

name: Benchmark
jobs:
  Compare:
    runs-on: ubuntu-latest
    steps:
      - name: Fetch Repository
        uses: actions/checkout@v4
        with:
          fetch-depth: 0 # to be able to retrieve the last commit in main

      - name: Install Go
        uses: actions/setup-go@v5
        with:
          # NOTE: Keep this in sync with the version from go.mod
          go-version: "1.21.x"

      - name: Run Benchmark
        run: set -o pipefail; go test ./... -benchmem -run=^$ -bench . | tee output.txt

      # NOTE: Benchmarks could change with different CPU types
      - name: Get GitHub Runner System Information
        uses: kenchan0130/[email protected]
        id: system-info

      - name: Get Main branch SHA
        id: get-main-branch-sha
        run: |
          SHA=$(git rev-parse origin/main)
          echo "sha=$SHA" >> $GITHUB_OUTPUT

      - name: Get Benchmark Results from main branch
        id: cache
        uses: actions/cache/restore@v4
        with:
          path: ./cache
          key: ${{ steps.get-main-branch-sha.outputs.sha }}-${{ runner.os }}-${{ steps.system-info.outputs.cpu-model }}-benchmark

      # This will only run if we have Benchmark Results from main branch
      - name: Compare PR Benchmark Results with main branch
        uses: benchmark-action/[email protected]
        if: steps.cache.outputs.cache-hit == 'true'
        with:
          tool: 'go'
          output-file-path: output.txt
          external-data-json-path: ./cache/benchmark-data.json
          # Do not save the data (This allows comparing benchmarks)
          save-data-file: false
          fail-on-alert: true
          # Comment on the PR if the branch is not a fork
          comment-on-alert: ${{ github.event.pull_request.head.repo.fork == false }}
          github-token: ${{ secrets.GITHUB_TOKEN }}
          summary-always: true
          alert-threshold: "150%"

      - name: Store Benchmark Results for main branch
        uses: benchmark-action/[email protected]
        if: ${{ github.ref_name == 'main' }}
        with:
          tool: 'go'
          output-file-path: output.txt
          external-data-json-path: ./cache/benchmark-data.json
          # Save the data to external file (cache)
          save-data-file: true
          fail-on-alert: false
          github-token: ${{ secrets.GITHUB_TOKEN }}
          summary-always: true
          alert-threshold: "150%"

      - name: Update Benchmark Results cache
        uses: actions/cache/save@v4
        if: ${{ github.ref_name == 'main' }}
        with:
          path: ./cache
          key: ${{ steps.get-main-branch-sha.outputs.sha }}-${{ runner.os }}-${{ steps.system-info.outputs.cpu-model }}-benchmark

[ktrz]

@gaby that's cool! I will try to use this within the action to give more meaningful error messages if someone tries to use it on the fork