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

bazel/bench: add tests to benchmarks #24817

Merged
merged 3 commits into from
Jan 15, 2025
Merged

Conversation

rockwotj
Copy link
Contributor

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

  • none - not a bug fix
  • none - this is a backport
  • none - issue does not exist in previous branches
  • none - papercut/not impactful enough to backport
  • v24.3.x
  • v24.2.x
  • v24.1.x

Release Notes

  • none

@rockwotj rockwotj requested review from a team and travisdowns and removed request for a team January 15, 2025 14:56
Copy link
Member

@StephanDollberg StephanDollberg left a comment

Choose a reason for hiding this comment

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

Makes sense.

bazel/test.bzl Outdated Show resolved Hide resolved
Copy link
Contributor

@IoannisRP IoannisRP left a comment

Choose a reason for hiding this comment

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

logic looks solid 👍

@rockwotj
Copy link
Contributor Author

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.
@rockwotj
Copy link
Contributor Author

Force push: fixed propagating environment variables and data into benchmark tests also bumped timeouts for a couple of tests that are slow in fastbuild.

@rockwotj
Copy link
Contributor Author

force push: fix linter

@rockwotj rockwotj enabled auto-merge January 15, 2025 17:47
@vbotbuildovich
Copy link
Collaborator

CI test results

test results on build#60792
test_id test_kind job_url test_status passed
idempotency_tests_rpunit.idempotency_tests_rpunit unit https://buildkite.com/redpanda/redpanda/builds/60792#01946b01-d71d-4e9c-92ca-495db6e61d4d FLAKY 1/2

@rockwotj rockwotj merged commit 0650a4f into redpanda-data:dev Jan 15, 2025
16 checks passed
@rockwotj rockwotj deleted the bench_test branch January 15, 2025 21:01
Comment on lines +391 to +396
"exec $@ --iterations=1 --runs=1 --duration=0 --no-stdout --overprovisioned",
],
)
native.sh_test(
name = name + "_test",
timeout = timeout,
Copy link
Member

Choose a reason for hiding this comment

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

great idea

Copy link
Contributor Author

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 😛

@nvartolomei
Copy link
Contributor

@rockwotj is it intended that running something like bazel test //src/v/datalake/tests:record_multiplexer_rpbench_test --test_output=streamed --cache_test_results=no doesn't show stdout/bench results but only the stderr?

@rockwotj
Copy link
Contributor Author

I think my intention was to not care about the benchmark numbers for *_test. If you care about the benchmark numbers then just run the benchmark directly bazel run //src/v/datalake/tests:record_multiplexer_rpbench

@rockwotj
Copy link
Contributor Author

Happy to take a PR to revert the --no-stdout bit tho

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants