Skip to content

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 requested a review from ESadek-MO August 18, 2023 08:01
@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
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.

2 participants