Skip to content

Commit

Permalink
Auto merge of #118548 - Enselic:bench-padding, r=thomcc,ChrisDenton
Browse files Browse the repository at this point in the history
libtest: Fix padding of benchmarks run as tests

### Summary

The first commit adds regression tests for libtest padding.

The second commit fixes padding for benches run as tests and updates the blessed output of the regression tests to make it clear what effect the fix has on padding.

Closes #104092 which is **E-help-wanted** and **regression-from-stable-to-stable**

### More details

Before this fix we applied padding _before_ manually doing what `convert_benchmarks_to_tests()` does which affects padding calculations. Instead use `convert_benchmarks_to_tests()` first if applicable and then apply padding afterwards so it becomes correct.

Benches should only be padded when run as benches to make it easy to compare the benchmark numbers. Not when run as tests.

r? `@ghost` until CI passes.
  • Loading branch information
bors committed Jan 12, 2024
2 parents 6029085 + 12e6bcf commit 5431404
Show file tree
Hide file tree
Showing 5 changed files with 58 additions and 14 deletions.
22 changes: 8 additions & 14 deletions library/test/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -298,24 +298,18 @@ where

let mut filtered = FilteredTests { tests: Vec::new(), benches: Vec::new(), next_id: 0 };

for test in filter_tests(opts, tests) {
let mut filtered_tests = filter_tests(opts, tests);
if !opts.bench_benchmarks {
filtered_tests = convert_benchmarks_to_tests(filtered_tests);
}

for test in filtered_tests {
let mut desc = test.desc;
desc.name = desc.name.with_padding(test.testfn.padding());

match test.testfn {
DynBenchFn(benchfn) => {
if opts.bench_benchmarks {
filtered.add_bench(desc, DynBenchFn(benchfn));
} else {
filtered.add_test(desc, DynBenchAsTestFn(benchfn));
}
}
StaticBenchFn(benchfn) => {
if opts.bench_benchmarks {
filtered.add_bench(desc, StaticBenchFn(benchfn));
} else {
filtered.add_test(desc, StaticBenchAsTestFn(benchfn));
}
DynBenchFn(_) | StaticBenchFn(_) => {
filtered.add_bench(desc, test.testfn);
}
testfn => {
filtered.add_test(desc, testfn);
Expand Down
14 changes: 14 additions & 0 deletions tests/run-make/libtest-padding/Makefile
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
# ignore-cross-compile because we run the compiled code
# needs-unwind because #[bench] and -Cpanic=abort requires -Zpanic-abort-tests
include ../tools.mk

NORMALIZE=sed 's%[0-9,]\{1,\} ns/iter (+/- [0-9,]\{1,\})%?? ns/iter (+/- ??)%' | sed 's%finished in [0-9\.]\{1,\}%finished in ??%'

all:
$(RUSTC) --test tests.rs

$(call RUN,tests) --test-threads=1 | $(NORMALIZE) > "$(TMPDIR)"/test.stdout
$(RUSTC_TEST_OP) "$(TMPDIR)"/test.stdout test.stdout

$(call RUN,tests) --test-threads=1 --bench | $(NORMALIZE) > "$(TMPDIR)"/bench.stdout
$(RUSTC_TEST_OP) "$(TMPDIR)"/bench.stdout bench.stdout
9 changes: 9 additions & 0 deletions tests/run-make/libtest-padding/bench.stdout
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@

running 4 tests
test short_test_name ... ignored
test this_is_a_really_long_test_name ... ignored
test short_bench_name ... bench: ?? ns/iter (+/- ??)
test this_is_a_really_long_bench_name ... bench: ?? ns/iter (+/- ??)

test result: ok. 0 passed; 0 failed; 2 ignored; 2 measured; 0 filtered out; finished in ??s

9 changes: 9 additions & 0 deletions tests/run-make/libtest-padding/test.stdout
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@

running 4 tests
test short_bench_name ... ok
test short_test_name ... ok
test this_is_a_really_long_bench_name ... ok
test this_is_a_really_long_test_name ... ok

test result: ok. 4 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in ??s

18 changes: 18 additions & 0 deletions tests/run-make/libtest-padding/tests.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
#![feature(test)]
extern crate test;

#[test]
fn short_test_name() {}

#[test]
fn this_is_a_really_long_test_name() {}

#[bench]
fn short_bench_name(b: &mut test::Bencher) {
b.iter(|| 1);
}

#[bench]
fn this_is_a_really_long_bench_name(b: &mut test::Bencher) {
b.iter(|| 1);
}

0 comments on commit 5431404

Please sign in to comment.