Disable benchmarks on PRs

[kratman]

Description

Stop the benchmarks from running on PRs. The reasons for this are the frequent failures and the long runtime. Benchmarks are still run daily and for main/develop

Related: #3662

Type of change

• Optimization (back-end change that speeds up the code)

Key checklist:

• No style issues: $ pre-commit run (or $ nox -s pre-commit) (see CONTRIBUTING.md for how to set this up to run automatically when committing locally, in just two lines of code)
• All tests pass: $ python run-tests.py --all (or $ nox -s tests)
• The documentation builds: $ python run-tests.py --doctest (or $ nox -s doctests)

You can run integration tests, unit tests, and doctests together at once, using $ python run-tests.py --quick (or $ nox -s quick).

Further checks:

• Code is commented, particularly in hard-to-understand areas
• Tests added that prove fix is effective or that feature works

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 99.60%. Comparing base (dfdc376) to head (5d8af5f).
Report is 1 commits behind head on develop.

❗ Current head 5d8af5f differs from pull request most recent head da17ded. Consider uploading reports for the commit da17ded to get more accurate results

Additional details and impacted files

@@           Coverage Diff            @@
##           develop    #3876   +/-   ##
========================================
  Coverage    99.60%   99.60%           
========================================
  Files          259      259           
  Lines        21270    21270           
========================================
  Hits         21186    21186           
  Misses          84       84           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[agriyakhetarpal]

I would say that completely stopping benchmark runs is not a good idea in this case. We could fairly easily write a check that skips running benchmarks on PRs by default, but does run them if [run bench] exists in the commit message – it won't be too hard to maintain either. A lot of repositories do it and we can document this somewhere in the Testing section of the Contributing guide.

This should be the required one-liner condition (off the top of my head):

if: ${{ contains(github.event.head_commit.message, '[run bench]') }}

[kratman]

I would say that completely stopping benchmark runs is not a good idea in this case.

They are not stopped completely. They run every day at 3am and run after a PR is merged into main and develop. This only stops them in PRs.

We also have workflows for manual benchmarks. This just speeds it up and stops a frequent failure. I don't think we need to complicated it further by adding extra checks

[agriyakhetarpal]

Yes, they aren't stopped completely, but they are disabled on PRs this way. My opinion is that there should be a way to run the benchmarks on a PR if one needs it, through this condition – and making it explicitly stated how to enable them if needed while reasonably keeping defaults to mark them as neutral/skipped (which speeds up the PR checks too as you mentioned)

The manual benchmarks workflow will have to be triggered manually from a branch through the Actions tab, which isn't convenient if you want to experiment with something – this is where commit messages can help

[agriyakhetarpal]

Hi @BradyPlanden, please let us know if we can get started on adding a self-hosted runner for the benchmarks (that will now run nightly and for changes from the the develop branch's context)