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

Action Will Not Create Summary Tables if Performance Alerts Occur #254

Open
morgangibbs87 opened this issue Jun 26, 2024 · 0 comments · May be fixed by #285
Open

Action Will Not Create Summary Tables if Performance Alerts Occur #254

morgangibbs87 opened this issue Jun 26, 2024 · 0 comments · May be fixed by #285

Comments

@morgangibbs87
Copy link

morgangibbs87 commented Jun 26, 2024

We are trying to have the Summary table always appear - even after an alert-threshold is exceeded.

We use this action to perform benchmarks on several os and device types in a matrix. This splits our benchmarks into separate jobs for each ${{ matrix.os }}-${{ matrix.device }}.

The Summary does not appear for devices where the alert-threshold is exceeded, even when fail-on-alert: false is set and summary-always: true is enabled.

This is true whether comment-on-alert is set or not, and even when we set a very high fail-threshold to ensure it is not using the alert-threshold value by default and make sure that the job does not fail.

After testing several different ways, it looks like the action is not set up to create a summary table at all if a performance alert occurs.

- name: Analyze benchmark results
    id: analyze-bm
    uses: risc0/[email protected]
    with:
      name: "${{ matrix.os }}-${{ matrix.device }}"
      tool: 'customBiggerIsBetter'
      output-file-path: target/hotbench/fib/benchmark.json
      external-data-json-path: ./cache/external.json
      github-token: ${{ secrets.GITHUB_TOKEN }}
      alert-threshold: '120%'
      fail-threshold: '10000%'
      comment-on-alert: true
      fail-on-alert: false
      summary-always: true
414owen added a commit to 414owen/github-action-benchmark that referenced this issue Dec 22, 2024
Alerts are currently generated by throwing an exception. This means that
all code that comes after that exception is skipped, including generating
the Action summary.

We can easily fix this, by generating the summary first.

Longer term, we might want to reconsider the code style here.
An exception should model something exceptional happening in our code.
Seeing bad benchmark numbers is not exceptional. It's what this tool is
supposed to handle.

Closes benchmark-action#254
@414owen 414owen linked a pull request Dec 22, 2024 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant