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

Set benchmark runs to error if the subprocess errors #5434

Merged
merged 4 commits into from Aug 18, 2023

Conversation

trexfeathers
Copy link
Contributor

@trexfeathers trexfeathers commented Aug 18, 2023

🚀 Pull Request

Requires

Will need git rebase upstream/FEATURE_benchmarks once those are merged.

Description

Fixes an oversight introduced when we switched away from Nox (#5215). Called process errors in bm_runner.py should cause an error upstream. Without this: we have had some benchmarks erroring without us knowing about it.

Also included handling to allow the overnight run to report whatever results it has, before erroring.

Demonstrations

I have temporarily created a different default branch on my fork: trexfeathers:demo_20230817_main, which can simulate what would happen on main.


Consult Iris pull request check list

@trexfeathers trexfeathers added the Type: Feature Branch Highlight this for a feature branch label Aug 18, 2023
@trexfeathers trexfeathers changed the title Bm pr6 check Set benchmark runs to error if the subprocess errors Aug 18, 2023
@codecov
Copy link

codecov bot commented Aug 18, 2023

Codecov Report

❗ No coverage uploaded for pull request base (FEATURE_benchmarks@016fc37). Click here to learn what that means.
Patch has no changes to coverable lines.

Additional details and impacted files
@@                  Coverage Diff                  @@
##             FEATURE_benchmarks    #5434   +/-   ##
=====================================================
  Coverage                      ?   89.35%           
=====================================================
  Files                         ?       89           
  Lines                         ?    22417           
  Branches                      ?     5382           
=====================================================
  Hits                          ?    20030           
  Misses                        ?     1639           
  Partials                      ?      748           

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

@trexfeathers trexfeathers marked this pull request as ready for review August 18, 2023 13:42
Copy link
Contributor

@ESadek-MO ESadek-MO left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Happy with this once rebased and tests run, thanks @trexfeathers

@ESadek-MO ESadek-MO merged commit 25451fd into SciTools:FEATURE_benchmarks Aug 18, 2023
16 of 17 checks passed
ESadek-MO pushed a commit that referenced this pull request Aug 21, 2023
* Improve benchmark runner printing (#5429)

* More sensible print and run functions.

* Avoid permanent modifications in _subprocess_runner.

* Post on demand benchmark results as comment (big refactor) (#5430)

* On demand benchmarking.

* Correct gh query.

* Correct assignee spacing.

* What's new entry.

* Better comparison commits for PR benchmarking (#5431)

* Don't check out head_ref - benchmark the GH simulated merge commit instead.

* What's New entry.

* Warn via issue if overnight benchmarks fail (#5432)

* Include a warning step for overnight benchmarking.

* Fix for failure warning script.

* Better formatting of warning issue title.

* What's new entry.

* Minor benchmark improvements (#5433)

* Use shlex.split() for bm_runner commands.

* Minor documentation clarifications.

* Set benchmark runs to error if the subprocess errors (#5434)

* Set benchmark runs to error if the subprocess errors.

* Still compare results even from a broken run.

* Still upload reports if overnight run fails.

* What's New entry.

* Hard-code conda channel into asv_delegated_conda.py (#5435)

* What's new entry.

* What's New entry.

* Hard-code conda channel into asv_delegated_conda.py .

* Fix some rebase confusion in the What's New.

* Inflate benchmark data to ensure laziness (#5436)

* Inflate benchmark data to ensure laziness.

* What's New entry.

* Benchmark feature branch what's new entry (#5438)

* What's new entry.

* Correct user name @ESadek-MO.

* Missing colon.
@trexfeathers trexfeathers deleted the bm_pr6_check branch May 3, 2024 15:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Feature Branch Highlight this for a feature branch
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants