-
Notifications
You must be signed in to change notification settings - Fork 602
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
bazel/bench: add tests to benchmarks #24817
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Makes sense.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
logic looks solid 👍
Looks like some of the benchmarks are broken in bazel - I'll start debugging those. |
Just to smoke test they work. In both debug and release so hopefully we catch stuff in the sanitizer when we get that working.
Force push: fixed propagating environment variables and data into benchmark tests also bumped timeouts for a couple of tests that are slow in fastbuild. |
force push: fix linter |
CI test resultstest results on build#60792
|
"exec $@ --iterations=1 --runs=1 --duration=0 --no-stdout --overprovisioned", | ||
], | ||
) | ||
native.sh_test( | ||
name = name + "_test", | ||
timeout = timeout, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
great idea
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks! Borrowed it from envoyproxy - so keith probably 😛
@rockwotj is it intended that running something like |
I think my intention was to not care about the benchmark numbers for |
Happy to take a PR to revert the |
Just to smoke test they work. In both debug and release so hopefully we
catch stuff in the sanitizer when we get that working.
Backports Required
Release Notes