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

Do not invoke the test runner when calling cargo bench and harness=false #6156

Closed
lu-zero opened this issue Oct 8, 2018 · 10 comments
Closed
Labels
A-cargo-targets Area: selection and definition of targets (lib, bins, examples, tests, benches) C-feature-request Category: proposal for a feature. Before PR, ping rust-lang/cargo if this is not `Feature accepted`

Comments

@lu-zero
Copy link
Contributor

lu-zero commented Oct 8, 2018

Benchers such as criterion have custom options that are not available to the test runner.

@alexcrichton
Copy link
Member

I'm not sure I understand the context for this issue, can you elaboate? All Cargo does is execute a binary, so are you saying Cargo shouldn't execute a binary?

@lu-zero
Copy link
Contributor Author

lu-zero commented Oct 8, 2018

Assume you do

# cargo bench -- -s mybaseline

if there are tests and benchmarks in the crate cargo will run the test runner and the bench runner

some example (current rav1e):

cargo bench -- -s newbase
    Finished release [optimized] target(s) in 0.05s
     Running target/release/deps/rav1e-d400d42c32908d29
error: Unrecognized option: 's'
error: bench failed
cargo bench -- --help
    Finished release [optimized] target(s) in 0.05s
     Running target/release/deps/rav1e-d400d42c32908d29
Usage: --help [OPTIONS] [FILTER]

Options:
        --ignored       Run ignored tests
        --test          Run tests and not benchmarks
        --bench         Run benchmarks instead of tests
<snip>

     Running target/release/deps/rav1e-34abbf14cafc2f7c
Usage: --help [OPTIONS] [FILTER]

Options:
        --ignored       Run ignored tests

<snip>

     Running target/release/deps/bench-34ea4bfc7d854e31
Criterion Benchmark

USAGE:
    bench-34ea4bfc7d854e31 [FLAGS] [OPTIONS] [FILTER]

<snip>

@alexcrichton
Copy link
Member

@lu-zero can you provide a standalone example? harness = false works locally for me.

@lu-zero
Copy link
Contributor Author

lu-zero commented Oct 8, 2018

Sure, here the minimal testcase

@alexcrichton
Copy link
Member

@lu-zero I think you're seeing the benchmark for src/lib.rs, you'll want to say bench = false in the [lib] section.

@lu-zero
Copy link
Contributor Author

lu-zero commented Oct 9, 2018

I had to sprinkle it around all the [[bin]] sections as well, but did the trick :)

Now, could we make this the default behavior ?

@alexcrichton alexcrichton added the C-feature-request Category: proposal for a feature. Before PR, ping rust-lang/cargo if this is not `Feature accepted` label Oct 9, 2018
@dwijnand
Copy link
Member

What's the default behaviour change? That harness=false under [[bench]] makes cargo bench not run lib and bins benchmarks? If I understood correctly, that doesn't sound right. So maybe I didn't understand correctly. 🙂

@lu-zero
Copy link
Contributor Author

lu-zero commented Nov 12, 2018

It is right.

If you are using criterion, you would not expect to have another runner being used by default.

@ehuss ehuss added the A-cargo-targets Area: selection and definition of targets (lib, bins, examples, tests, benches) label Apr 6, 2020
@epage
Copy link
Contributor

epage commented Oct 24, 2023

It is reasonable to have a mixture of different harnesses, built-in and otherwise. The main issue here is more that there isn't a stable bench harness.

I feel like solving this issue is not aligned with longer term goals (stable bench harness, separating the concerns of harnesses and runners, etc). I'm going to close this. If there is a reason we should re-evaluate this, let us know!

Note that #6945 would help lower the boilerplate involved in this issue until things have improved otherwise.

@epage epage closed this as not planned Won't fix, can't repro, duplicate, stale Oct 24, 2023
@lu-zero
Copy link
Contributor Author

lu-zero commented Oct 24, 2023

in order to support mix&match we'd need something more complex to pass arguments to the different runners, as long that's available the workaround default behavior suggested stops being needed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-cargo-targets Area: selection and definition of targets (lib, bins, examples, tests, benches) C-feature-request Category: proposal for a feature. Before PR, ping rust-lang/cargo if this is not `Feature accepted`
Projects
None yet
Development

No branches or pull requests

5 participants