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

Tracking issue for eRFC 2318, Custom test frameworks #50297

Open
10 tasks
Centril opened this issue Apr 28, 2018 · 99 comments
Open
10 tasks

Tracking issue for eRFC 2318, Custom test frameworks #50297

Centril opened this issue Apr 28, 2018 · 99 comments
Labels
A-libtest Area: `#[test]` / the `test` library B-RFC-approved Blocker: Approved by a merged RFC but not yet implemented. C-tracking-issue Category: An issue tracking the progress of sth. like the implementation of an RFC F-custom_test_frameworks `#![feature(custom_test_frameworks)]` S-tracking-design-concerns Status: There are blocking design concerns. T-cargo Relevant to the cargo team, which will review and decide on the PR/issue. T-dev-tools Relevant to the dev-tools subteam, which will review and decide on the PR/issue. T-lang Relevant to the language team, which will review and decide on the PR/issue. WG-embedded Working group: Embedded systems

Comments

@Centril
Copy link
Contributor

Centril commented Apr 28, 2018

This is tracking the #[bench] and #[test_case] attributes and the custom_test_frameworks lang item as well as the libtest crate. There is currently no plan/RFC for stabilizing them.

Old issue description.

This is a tracking issue for the eRFC "Custom test frameworks" (rust-lang/rfcs#2318).

Documentation:

Steps:

Unresolved questions:

Notes:

Implementation history:

@Centril Centril added B-RFC-approved Blocker: Approved by a merged RFC but not yet implemented. T-dev-tools Relevant to the dev-tools subteam, which will review and decide on the PR/issue. T-cargo Relevant to the cargo team, which will review and decide on the PR/issue. A-libtest Area: `#[test]` / the `test` library C-tracking-issue Category: An issue tracking the progress of sth. like the implementation of an RFC labels Apr 28, 2018
@CAD97
Copy link
Contributor

CAD97 commented May 14, 2018

On stdout/err capture (after doing some research around the proof-of-concept #50457):

The current state is sub-optimal, to say the least. Both stdout and stderr exist as thread-local handles that only the stdlib has access to, which print family of macros use, as does panic.

Relevant libstd snippets

thread_local! {
static LOCAL_STDOUT: RefCell<Option<Box<Write + Send>>> = {
RefCell::new(None)
}
}

thread_local! {
pub static LOCAL_STDERR: RefCell<Option<Box<Write + Send>>> = {
RefCell::new(None)
}
}

rust/src/libstd/io/stdio.rs

Lines 614 to 711 in 935a2f1

/// Resets the thread-local stderr handle to the specified writer
///
/// This will replace the current thread's stderr handle, returning the old
/// handle. All future calls to `panic!` and friends will emit their output to
/// this specified handle.
///
/// Note that this does not need to be called for all new threads; the default
/// output handle is to the process's stderr stream.
#[unstable(feature = "set_stdio",
reason = "this function may disappear completely or be replaced \
with a more general mechanism",
issue = "0")]
#[doc(hidden)]
pub fn set_panic(sink: Option<Box<Write + Send>>) -> Option<Box<Write + Send>> {
use panicking::LOCAL_STDERR;
use mem;
LOCAL_STDERR.with(move |slot| {
mem::replace(&mut *slot.borrow_mut(), sink)
}).and_then(|mut s| {
let _ = s.flush();
Some(s)
})
}
/// Resets the thread-local stdout handle to the specified writer
///
/// This will replace the current thread's stdout handle, returning the old
/// handle. All future calls to `print!` and friends will emit their output to
/// this specified handle.
///
/// Note that this does not need to be called for all new threads; the default
/// output handle is to the process's stdout stream.
#[unstable(feature = "set_stdio",
reason = "this function may disappear completely or be replaced \
with a more general mechanism",
issue = "0")]
#[doc(hidden)]
pub fn set_print(sink: Option<Box<Write + Send>>) -> Option<Box<Write + Send>> {
use mem;
LOCAL_STDOUT.with(move |slot| {
mem::replace(&mut *slot.borrow_mut(), sink)
}).and_then(|mut s| {
let _ = s.flush();
Some(s)
})
}
/// Write `args` to output stream `local_s` if possible, `global_s`
/// otherwise. `label` identifies the stream in a panic message.
///
/// This function is used to print error messages, so it takes extra
/// care to avoid causing a panic when `local_stream` is unusable.
/// For instance, if the TLS key for the local stream is
/// already destroyed, or if the local stream is locked by another
/// thread, it will just fall back to the global stream.
///
/// However, if the actual I/O causes an error, this function does panic.
fn print_to<T>(
args: fmt::Arguments,
local_s: &'static LocalKey<RefCell<Option<Box<Write+Send>>>>,
global_s: fn() -> T,
label: &str,
)
where
T: Write,
{
let result = local_s.try_with(|s| {
if let Ok(mut borrowed) = s.try_borrow_mut() {
if let Some(w) = borrowed.as_mut() {
return w.write_fmt(args);
}
}
global_s().write_fmt(args)
}).unwrap_or_else(|_| {
global_s().write_fmt(args)
});
if let Err(e) = result {
panic!("failed printing to {}: {}", label, e);
}
}
#[unstable(feature = "print_internals",
reason = "implementation detail which may disappear or be replaced at any time",
issue = "0")]
#[doc(hidden)]
pub fn _print(args: fmt::Arguments) {
print_to(args, &LOCAL_STDOUT, stdout, "stdout");
}
#[unstable(feature = "print_internals",
reason = "implementation detail which may disappear or be replaced at any time",
issue = "0")]
#[doc(hidden)]
pub fn _eprint(args: fmt::Arguments) {
use panicking::LOCAL_STDERR;
print_to(args, &LOCAL_STDERR, stderr, "stderr");
}


The existing stdout/err capture for the test harness works by setting the thread-local, which, if set, the print family of macros will use, otherwise falling back to io::stdout(). This setup is problematic because it's fairly trivial to escape this (#12309) and 99.9% of logging frameworks will escape accidentally (#40298).

The reason for this is that any solution that's generic over io::Write is going to use io::stdout() as the sink to hit stdout, and that doesn't go through the tread-local plumbing, so hits the true stdout directly.

The way I see it, there are three ways to resolve this quirk:

  • Create a new stdlib io::Write sink that uses the thread-local plumbing that the print macros do today, and implement the print macros as writing to that sink. That's the approach I proved was possible with Reusable handle to thread-local replaceable stdout/err #50457.
  • Create a new io::Write sink that uses (e)print! behind the scenes. This is possible entirely in an external crate:
(E)PrintWriter - feel free to use these, I am the author and release them under MIT/Unlicense
/// IO sink that uses the `print` macro, for test stdout capture.
pub struct PrintWriter;
impl std::io::Write for PrintWriter {
    fn write(&mut self, buf: &[u8]) -> std::io::Result<usize> {
        print!("{}", String::from_utf8_lossy(buf));
        Ok(buf.len())
    }
    fn flush(&mut self) -> std::io::Result<()> { std::io::stdout().flush() }
}

/// IO sink that uses the `eprint` macro, for test stdout capture.
pub struct EPrintWriter;
impl std::io::Write for EPrintWriter {
    fn write(&mut self, buf: &[u8]) -> std::io::Result<usize> {
        eprint!("{}", String::from_utf8_lossy(buf));
        Ok(buf.len())
    }
    fn flush(&mut self) -> std::io::Result<()> { std::io::stdout().flush() }
}

Unfortunately, this suffers from a large number of drawbacks. Because we only have access to the print macro, we have to interpret whatever sequence of bytes the sink gets as a UTF-8 string, even though that's potentially not true, etc. etc. not zero-cost when it's not, and so on. It's a decent solution for hooking up logging in tests, but is very much not a general solution that could be used outside tests.

That takes us to the third option, which is almost certainly the best, but also the most complicated:

  • Actually change stdout/err

It sounds simple, but it's a lot simpler said than done with the current test harness setup. io::Stderr boils down to a handle to a write stream available to the process, and is a process-local resource -- there's only one stdout per process. The test harness currently runs tests parallel on different threads within the same process, thus the current thread-local replacement.

The other issue is that the test harness doesn't just want to capture stdout of the tests, it wants to capture stdout of the tests separately for each test, so it can print just the stdout of the failing test(s) and not have the other tests' output interspersed with it.

The only solution I see at this time that maintains current behavior of separate captures per test run is to run them all in their own process, and capture stdout that way. That way each process can pipe its stdout/err to the test runner, which can then synchronize them and decide which ones to output to the true stdout/err if desired.

A sketch of the plumbing required to make an out-of-tree libtest that works with this behavior:

  • #[test] fn are all collected into a list
  • A fn main is generated that takes an argument of one of the paths, and runs that test
  • (The existing fn main is hidden)
  • As normal, successful execution is success, and a panic is a failure exit code
  • Our runner calls the generated binary with each of the test paths that match any filtering done as separate processes, captures their stdout/err and reports them on failure

Note that this is unfortunately stateful: we need to fold over all #[test] fn before we can emit the proper main. I'm not knowledgeable enough around compiler internals to be much help implementing the machinery right now (though I'd love to learn my way around and help out!), but give me an API to build against and I'll definitely help with any test frameworks built against the plumbing the eRFC has us set to experiment with.

@CAD97
Copy link
Contributor

CAD97 commented May 14, 2018

TL;DR:

To have sane capture behavior for stdout and stderr for print-family macros, panic, and io::{stdout, stderr}, running each test in its own (child) process is probably required. I'd love to help out wherever I can, but lack confidence.

@retep998
Copy link
Member

The only solution I see at this time that maintains current behavior of separate captures per test run is to run them all in their own process, and capture stdout that way.

This would be significantly slower on certain platforms where process creation is slow, notably Windows. Especially when the tests are all very quick this will cause severe regressions in total time.

@CAD97
Copy link
Contributor

CAD97 commented May 15, 2018

Another possibility for process separation of tests is to spawn a process for each unit of parallelism, but each process is reused for multiple tests.

Of course, we could also just promote the thread-local replaceable stdout/err to io::Stdout/Err, but that would be suboptimal itself.

The issue is that stdout is, as far as I can tell, a process-local resource. To capture io::Stdout we either need our own stdout replacement machinery on top or to separate processes.

@SoniEx2
Copy link
Contributor

SoniEx2 commented Jun 13, 2018

if I want to argue about this do I argue about it here?

so like I made hexchat-plugin crate. the problem is you can't test hexchat side without hexchat. so I'd need some sort of #[hexchat_test].

but I still want the users to be able to use plain old vanilla rust #[test]s. I shouldn't have to reimplement/replace the whole testing system just to be able to produce some hexchat-based unit tests (in addition to normal tests).

(disclaimer: I don't fully understand the RFC, but it seems to require either replacing everything, or not having tests)

@jonhoo
Copy link
Contributor

jonhoo commented Jun 13, 2018

@SoniEx2 this RFC is about how we'd like to change the underlying mechanism that is used to arrange testing in Rust so that authors can write new test frameworks (e.g., quickcheck) that have access to the same mechanisms as existing built-in tests ( able to access private functions, can find all tagged functions, etc.). The current proposal is relatively wholesale (you change the test framework for a whole crate), but this is something that may be improved upon. We decided against more sophisticated designs in the initial implementation phase (this is just an "experimental" RFC) so that we don't bite off more than we can chew, but I think down the line we'd want more fine-grained test framework selection too.

As for your specific example, you don't actually need custom frameworks for that feature :) It seems like you want to only run a test if hexchat isn't running? To do that, you could either mark the test with #[allow_fail], or you could have the test just return if it fails to connect. If you wanted a more fine-grained approach, you could write a procedural macro that re-writes any function annotated with #[hexchat_test] into a #[test] that returns if hexchat isn't running.

@SoniEx2
Copy link
Contributor

SoniEx2 commented Jun 13, 2018

no you don't understand, they have to run inside hexchat. loaded with dlopen (or equivalent). after plugin registration.

@djrenren
Copy link
Contributor

djrenren commented Aug 7, 2018

Hey all, I've come up with a simplified proposal and I have an implementation now (though it needs a little cleaning up before a PR).

https://blog.jrenner.net/rust/testing/2018/08/06/custom-test-framework-prop.html

Lemme know what you think!

@CAD97
Copy link
Contributor

CAD97 commented Aug 21, 2018

I've thought some more about stdin/stderr capture. Here's a draft of a system that should be able to "perfectly" capture stdin/stdout, though not without O(1) (once per test suite) overhead:

  • The main process is started by cargo test. It does no actual testing; its job is to coordinate child processes to do the actual tests.
  • At startup of the main process, it spawns a child process for each unit of parallelism in the test runner. This is a one-time cost, as these child processes are reused for multiple tests.
  • With each child process spawned, a communication channel is created between the processes distinct of stdin/out/err. This is used to instruct the child process which test to run next, and for the child process to report back the success/failure of the test.
  • We avoid talking to the child process on stdin because the test could listen to stdin, and we want to avoid accidentally polluting that. (Also, we need a back-channel that's not stdout to communicate test results anyway.)
  • The stdout/stderr of the child processes are piped into communication channels into the main process, which handles reporting those if the test failed.
  • Tests are run off the main thread in child processes, so that the child process doesn't die to a panic and that it can (or the main thread can instruct it to) terminate runaway tests early.
  • After the main process has handed out all tests to the child processes, it shuts them down as it receives the final reports, and then outputs the final report to the user before terminating.

@jonhoo
Copy link
Contributor

jonhoo commented Aug 21, 2018

@djrenren finally had a chance to read your proposal. I'm excited by the progress on this, though your write-up did raise some questions for me:

  • Early on, you say that "Annotated items must be a nameable const, static, or fn.". Further down, when you discuss the use-case of a framework that supports test suites, you show a #[test_suite] annotation on a mod with #[suite_member] on the nested tests. It'd be good to show how that expansion works. Does a proc macro turn #[test_suite] into a const with an element for each #[suite_member]? How does it find all of them? Are the #[suite_member]s all turned into #[test_case]? How do you avoid double-executing those test cases (once as a "child" and once due to the #[test-case] annotation)?
  • For #![test_runner] you say "The type of the function must be Fn(&[&mut Foo]) -> impl Termination for some Foo which is the test type", yet further down you write a Fn(&[&dyn Foo]). I assume the latter is the correct version?
  • Is TestDescAndFn sufficient for our purposes here? I don't think that can (currently) wrap a struct or const in a meaningful way?
  • I find the "test runner" and "test format" examples a little backwards. To me, "formatting" is about output, and "running" is about the structure of the tests themselves (and how they're run). We can bikeshed names later though.
  • The "editor wants to integrate with testing" example makes me sad. It seems really sad for the crate-under-test to have to be modified to work with my editor. One of the earlier RFCs had a section on separating the test harness from the test executor, with the former controlling output and the latter how tests are written and executed. And crucially, the former should be possible to swap without changing the source code of the c-u-t. That doesn't solve the case of an editor wanting to control test execution as well, but I think ideally we'd be able to fold that into the harness somehow too.

@CAD97
Copy link
Contributor

CAD97 commented Aug 21, 2018

I think in most cases the CUT won't need to change itself to integrate with the IDE test runner, so long as the IDE understands the test runner you're working with. As I understand how JetBrains' IntelliJ IDEA integrates with both JUnit and Rust's libtest is that it communicates using the standard API and stdin/out. JUnit's integration is obviously tighter, as IDEA was a Java IDE first, but the integration with libtest tests in Rust is already quite good even without any specific integration required.

Obviously the IDE needs to understand how the test runner works or the test runner needs to understand how the IDE works, but under most cases of using a popular test runner, the CUT shouldn't need to change to be tested with in-IDE functionality.

@SoniEx2

This comment has been minimized.

@djrenren
Copy link
Contributor

@jonhoo

Early on, you say that...

I don't want to get too bikesheddy on how that particular macro would work but basically we'd just produce a single const that had the #[test_case] annotation. The children would be aggregated inside it.

I assume the latter is the correct version?

Fn(&[&Foo]) -> () is a subtype Fn(&[&mut Foo]) -> impl Termination. It's not required that Foo be a Trait but if it is, then you need dyn, so those two lines aren't in contradiction.

Is TestDescAndFn sufficient for our purposes here?

I agree TestDescAndFn is not sufficient, though it could successfully be constructed from all sorts of things with the use of proc macros. I think for libtest we should just make a trait that essentially mirrors TestDescAndFn and accept that.

One of the earlier RFCs had a section on separating the test harness from the test executor, with the former controlling output and the latter how tests are written and executed.

This is still entirely possible under the current design, it just requires the test runner to be designed to be pluggable.

@djrenren
Copy link
Contributor

@SoniEx2 It is for crate authors! 😄

For most crate authors (unless you're authoring test machinery), you'll just import any macros you want for declaring tests or benchmarks, and then you'll pick a test runner. For most cases the built-in one should be fine though.

@SoniEx2

This comment has been minimized.

@SoniEx2

This comment has been minimized.

@SoniEx2

This comment has been minimized.

@djrenren
Copy link
Contributor

For those worried about stdout capture, I've just demonstrated how this proposal can allow for @CAD97's style of multiprocess stdout capturing:
https://github.com/djrenren/rust-test-frameworks/blob/master/multiprocess_runner/src/lib.rs

This code compiles and runs under my PR: #53410

The output of cargo test is available in the same directory and is called output.txt

@CAD97
Copy link
Contributor

CAD97 commented Aug 22, 2018

And just for clarity, the example does spawn a process for each test, which is suboptimal on OSes where process creation is slow, e.g. Windows. A real runner would implement a scheme like I described earlier to avoid spawning more processes than necessary. (And actually parallelize the running of tests.)

@japaric
Copy link
Member

japaric commented Sep 1, 2018

Whoops, I'm (very) late to the party.

I have read the blog post but not looked at the rustc implementation and I'm concerned with the presence of impl Termination and String (see Testable.name) in the proposal.

How will this proposal work with the version of #![no_std] binaries that's being stabilized for the edition release? Those #![no_std] binaries use #![no_main] to avoid the unstable start (i.e. fn main() interface) and termination (i.e. Termination trait) lang items. Without start if rustc --test injects a main function into the crate it will end up being discarded as these binaries use a custom entry point. Without termination the Termination trait doesn't exist (it is defined in std after all).

Also it would be best if using a custom test runner didn't require an allocator as that requires quite a bit of boilerplate (e.g. #[alloc_error_handler]) and uses up precious Flash memory.

Of course I'm not going to block any PR since this is in eRFC state and the idea is to experiment;
i just want to make sure that the embedded use case is not forgotten and that what we end up stabilizing does support it.

cc @Manishearth I thought cargo fuzz used #![no_main] and that it had similar needs as the embedded use case, but perhaps the fuzzed programs do link to std so they are not that similar?.

@Ezrashaw
Copy link
Contributor

Ezrashaw commented Mar 18, 2023

@joshtriplett @Manishearth I'm interested in restarting this proposal, do you have any advice?

@Manishearth
Copy link
Member

Manishearth commented Mar 18, 2023

Not much more than go through the RFC and the current code (and any PRs linked to this issue) and see what's missing. I don't remember the status quo unfortunately. I think there's nothing.

If you would like to repropose my original proposal that I mentioned above, I think it can be found in the RFC PR's commit history.

@gilescope
Copy link
Contributor

gilescope commented Mar 19, 2023 via email

@asomers
Copy link
Contributor

asomers commented Mar 19, 2023

I think the biggest change in rust testing has been the arrival of nextest on the scene.

cargo-nextest really doesn't address this issue. It's just a different runner that can execute the same test. But it doesn't allow you to alter the design of the tests themselves, for example by creating test functions at runtime, or deciding at runtime that a particular test function should be skipped.

@Ezrashaw
Copy link
Contributor

Yes, although I think @gilescope is talking about changes to the Rust testing ecosystem generally.

@amab8901
Copy link
Contributor

if this issue is closed, then maybe this (https://doc.rust-lang.org/core/prelude/v1/macro.test_case.html) should no longer be labeled as "nightly" and "experimental"?

anforowicz added a commit to anforowicz/qr_code that referenced this issue Apr 20, 2023
This commit makes `use std:convert::TryInto` conditional via
`#[cfg(debug_assertions)]`.  This avoids the following build error:

    ```
    $ cargo bench
    ...
    error: unused import: `std::convert::TryInto`
     --> src/cast.rs:1:5
      |
    1 | use std::convert::TryInto;
      |     ^^^^^^^^^^^^^^^^^^^^^
    ```

This commit also opts into the `test` feature (only if the "bench"
feature is requested, because benchmarking is only available in the
nightly version of the compiler) and declares the `extern crate test`
dependency on the compiler-provided `test` crate. This avoids the
following build errors:

    ```
    $ cargo bench
    ...
    error[E0433]: failed to resolve: use of undeclared crate or module `test`
       --> src/optimize.rs:710:33
        |
    710 | fn bench_optimize(bencher: &mut test::Bencher) {
        |                                 ^^^^
                   use of undeclared crate or module `test`
    ```

    ```
    $ cargo bench
    ...
    error[E0658]: use of unstable library feature 'test'
       --> src/optimize.rs:710:33
        |
    710 | fn bench_optimize(bencher: &mut test::Bencher) {
        |                                 ^^^^^^^^^^^^^
        |
        = note: see issue #50297
                <rust-lang/rust#50297>
                for more information
        = help: add `#![feature(test)]` to the crate attributes to enable
    ```

This commit also makes small tweaks to how `README.md` is included.
It seems that this doesn't actually test the examples in the `README.md`
file (this commit adds a TODO for this observation), but it does avoid
the following build error:

    ```
    $ cargo bench --features=bench
    ...
    error: unknown `doc` attribute `include`
      --> src/lib.rs:14:36
       |
    14 | #![cfg_attr(feature = "bench", doc(include = "../README.md"))]
       |                                ----^^^^^^^^^^^^^^^^^^^^^^^^- help:
       |                                use `doc = include_str!` instead:
       |                           `#![doc = include_str!("../README.md")]`
       |
       = warning: this was previously accepted by the compiler but is being
         phased out; it will become a hard error in a future release!
       = note: for more information, see issue #82730
         <rust-lang/rust#82730>
    ```

Finally, this commit declares the "bench" feature in `Cargo.toml`.
After these changes the following command line succeeds:

    ```
    $ cargo bench --features=bench
    ...
    test bits::bench_find_min_version    ... bench:           1 ns/iter (+/- 0)
    test bits::bench_push_splitted_bytes ... bench:       3,862 ns/iter (+/- 58)
    test optimize::bench_optimize        ... bench:          19 ns/iter (+/- 0)
    ```
anforowicz added a commit to anforowicz/qr_code that referenced this issue Apr 27, 2023
This commit makes `use std:convert::TryInto` conditional via
`#[cfg(debug_assertions)]`.  This avoids the following build error:

    ```
    $ cargo bench
    ...
    error: unused import: `std::convert::TryInto`
     --> src/cast.rs:1:5
      |
    1 | use std::convert::TryInto;
      |     ^^^^^^^^^^^^^^^^^^^^^
    ```

This commit also opts into the `test` feature (only if the "bench"
feature is requested, because benchmarking is only available in the
nightly version of the compiler) and declares the `extern crate test`
dependency on the compiler-provided `test` crate. This avoids the
following build errors:

    ```
    $ cargo bench
    ...
    error[E0433]: failed to resolve: use of undeclared crate or module `test`
       --> src/optimize.rs:710:33
        |
    710 | fn bench_optimize(bencher: &mut test::Bencher) {
        |                                 ^^^^
                   use of undeclared crate or module `test`
    ```

    ```
    $ cargo bench
    ...
    error[E0658]: use of unstable library feature 'test'
       --> src/optimize.rs:710:33
        |
    710 | fn bench_optimize(bencher: &mut test::Bencher) {
        |                                 ^^^^^^^^^^^^^
        |
        = note: see issue #50297
                <rust-lang/rust#50297>
                for more information
        = help: add `#![feature(test)]` to the crate attributes to enable
    ```

This commit also makes small tweaks to how `README.md` is included.
It seems that this doesn't actually test the examples in the `README.md`
file (this commit adds a TODO for this observation), but it does avoid
the following build error:

    ```
    $ cargo bench --features=bench
    ...
    error: unknown `doc` attribute `include`
      --> src/lib.rs:14:36
       |
    14 | #![cfg_attr(feature = "bench", doc(include = "../README.md"))]
       |                                ----^^^^^^^^^^^^^^^^^^^^^^^^- help:
       |                                use `doc = include_str!` instead:
       |                           `#![doc = include_str!("../README.md")]`
       |
       = warning: this was previously accepted by the compiler but is being
         phased out; it will become a hard error in a future release!
       = note: for more information, see issue #82730
         <rust-lang/rust#82730>
    ```

Finally, this commit declares the "bench" feature in `Cargo.toml`.
After these changes the following command line succeeds:

    ```
    $ cargo bench --features=bench
    ...
    test bits::bench_find_min_version    ... bench:           1 ns/iter (+/- 0)
    test bits::bench_push_splitted_bytes ... bench:       3,862 ns/iter (+/- 58)
    test optimize::bench_optimize        ... bench:          19 ns/iter (+/- 0)
    ```
anforowicz added a commit to anforowicz/qr_code that referenced this issue Apr 27, 2023
This commit makes `use std:convert::TryInto` conditional via
`#[cfg(debug_assertions)]`.  This avoids the following build error:

    ```
    $ cargo bench
    ...
    error: unused import: `std::convert::TryInto`
     --> src/cast.rs:1:5
      |
    1 | use std::convert::TryInto;
      |     ^^^^^^^^^^^^^^^^^^^^^
    ```

This commit also opts into the `test` feature (only if the "bench"
feature is requested, because benchmarking is only available in the
nightly version of the compiler) and declares the `extern crate test`
dependency on the compiler-provided `test` crate. This avoids the
following build errors:

    ```
    $ cargo bench
    ...
    error[E0433]: failed to resolve: use of undeclared crate or module `test`
       --> src/optimize.rs:710:33
        |
    710 | fn bench_optimize(bencher: &mut test::Bencher) {
        |                                 ^^^^
                   use of undeclared crate or module `test`
    ```

    ```
    $ cargo bench
    ...
    error[E0658]: use of unstable library feature 'test'
       --> src/optimize.rs:710:33
        |
    710 | fn bench_optimize(bencher: &mut test::Bencher) {
        |                                 ^^^^^^^^^^^^^
        |
        = note: see issue #50297
                <rust-lang/rust#50297>
                for more information
        = help: add `#![feature(test)]` to the crate attributes to enable
    ```

This commit also makes small tweaks to how `README.md` is included.
It seems that this doesn't actually test the examples in the `README.md`
file (this commit adds a TODO for this observation), but it does avoid
the following build error:

    ```
    $ cargo bench --features=bench
    ...
    error: unknown `doc` attribute `include`
      --> src/lib.rs:14:36
       |
    14 | #![cfg_attr(feature = "bench", doc(include = "../README.md"))]
       |                                ----^^^^^^^^^^^^^^^^^^^^^^^^- help:
       |                                use `doc = include_str!` instead:
       |                           `#![doc = include_str!("../README.md")]`
       |
       = warning: this was previously accepted by the compiler but is being
         phased out; it will become a hard error in a future release!
       = note: for more information, see issue #82730
         <rust-lang/rust#82730>
    ```

Finally, this commit declares the "bench" feature in `Cargo.toml`.
After these changes the following command line succeeds:

    ```
    $ cargo bench --features=bench
    ...
    test bits::bench_find_min_version    ... bench:           1 ns/iter (+/- 0)
    test bits::bench_push_splitted_bytes ... bench:       3,862 ns/iter (+/- 58)
    test optimize::bench_optimize        ... bench:          19 ns/iter (+/- 0)
    ```
anforowicz added a commit to anforowicz/qr_code that referenced this issue Apr 27, 2023
This commit makes `use std:convert::TryInto` conditional via
`#[cfg(debug_assertions)]`.  This avoids the following build error:

    ```
    $ cargo bench
    ...
    error: unused import: `std::convert::TryInto`
     --> src/cast.rs:1:5
      |
    1 | use std::convert::TryInto;
      |     ^^^^^^^^^^^^^^^^^^^^^
    ```

This commit also opts into the `test` feature (only if the "bench"
feature is requested, because benchmarking is only available in the
nightly version of the compiler) and declares the `extern crate test`
dependency on the compiler-provided `test` crate. This avoids the
following build errors:

    ```
    $ cargo bench
    ...
    error[E0433]: failed to resolve: use of undeclared crate or module `test`
       --> src/optimize.rs:710:33
        |
    710 | fn bench_optimize(bencher: &mut test::Bencher) {
        |                                 ^^^^
                   use of undeclared crate or module `test`
    ```

    ```
    $ cargo bench
    ...
    error[E0658]: use of unstable library feature 'test'
       --> src/optimize.rs:710:33
        |
    710 | fn bench_optimize(bencher: &mut test::Bencher) {
        |                                 ^^^^^^^^^^^^^
        |
        = note: see issue #50297
                <rust-lang/rust#50297>
                for more information
        = help: add `#![feature(test)]` to the crate attributes to enable
    ```

This commit also makes small tweaks to how `README.md` is included.
It seems that this doesn't actually test the examples in the `README.md`
file (this commit adds a TODO for this observation), but it does avoid
the following build error:

    ```
    $ cargo bench --features=bench
    ...
    error: unknown `doc` attribute `include`
      --> src/lib.rs:14:36
       |
    14 | #![cfg_attr(feature = "bench", doc(include = "../README.md"))]
       |                                ----^^^^^^^^^^^^^^^^^^^^^^^^- help:
       |                                use `doc = include_str!` instead:
       |                           `#![doc = include_str!("../README.md")]`
       |
       = warning: this was previously accepted by the compiler but is being
         phased out; it will become a hard error in a future release!
       = note: for more information, see issue #82730
         <rust-lang/rust#82730>
    ```

Finally, this commit declares the "bench" feature in `Cargo.toml`.
After these changes the following command line succeeds:

    ```
    $ cargo bench --features=bench
    ...
    test bits::bench_find_min_version    ... bench:           1 ns/iter (+/- 0)
    test bits::bench_push_splitted_bytes ... bench:       3,862 ns/iter (+/- 58)
    test optimize::bench_optimize        ... bench:          19 ns/iter (+/- 0)
    ```
anforowicz added a commit to anforowicz/qr_code that referenced this issue Apr 28, 2023
Benchmarking is only supported by Rust nightly and therefore before
this commit it was disabled by default and hidden behind `bench`
feature.  OTOH, declaring such feature in `Cargo.toml` would have
broken `cargo +stable test --all-features`.  Therefore the commit
switches from using a `bench` *feature* to using a `bench` config.

This commit also makes `use std:convert::TryInto` conditional via
`#[cfg(debug_assertions)]`.  This avoids the following build error:

    ```
    $ RUSTFLAGS='--cfg=bench' cargo +nightly bench --all-features
    ...
    error: unused import: `std::convert::TryInto`
     --> src/cast.rs:1:5
      |
    1 | use std::convert::TryInto;
      |     ^^^^^^^^^^^^^^^^^^^^^
    ```

Additionally, this commit also opts into the `test` feature (only if the
"bench" configurtaion is requested, because benchmarking is only available in
the nightly version of the compiler) and declares the `extern crate
test` dependency on the compiler-provided `test` crate. This avoids the
following build errors:

    ```
    $ RUSTFLAGS='--cfg=bench' cargo +nightly bench --all-features
    ...
    error[E0433]: failed to resolve: use of undeclared crate or module `test`
       --> src/optimize.rs:710:33
        |
    710 | fn bench_optimize(bencher: &mut test::Bencher) {
        |                                 ^^^^
                   use of undeclared crate or module `test`
    ```

    ```
    $ RUSTFLAGS='--cfg=bench' cargo +nightly bench --all-features
    ...
    error[E0658]: use of unstable library feature 'test'
       --> src/optimize.rs:710:33
        |
    710 | fn bench_optimize(bencher: &mut test::Bencher) {
        |                                 ^^^^^^^^^^^^^
        |
        = note: see issue #50297
                <rust-lang/rust#50297>
                for more information
        = help: add `#![feature(test)]` to the crate attributes to enable
    ```

After these changes the following command line succeeds:

    ```
    $ RUSTFLAGS='--cfg=bench' cargo +nightly bench --all-features
    ...
    test bits::bench_find_min_version    ... bench:           1 ns/iter (+/- 0)
    test bits::bench_push_splitted_bytes ... bench:       3,862 ns/iter (+/- 58)
    test optimize::bench_optimize        ... bench:          19 ns/iter (+/- 0)
    ```
@mzabaluev
Copy link
Contributor

if this issue is closed, then maybe this (https://doc.rust-lang.org/core/prelude/v1/macro.test_case.html) should no longer be labeled as "nightly" and "experimental"?

Some crates already exploit the test_case attribute in stable Rust, so the effects need to be documented at least.

@ctz
Copy link
Contributor

ctz commented Jun 2, 2024

I was looking at using datatest -- it seems to address my needs to produce test cases from json. However, it requires custom_test_frameworks which is nightly-only.

And this being the tracking RFC for that feature, what does it mean for it to be closed? Is this effectively nightly-only, forever? Will it be removed? Is it destined for the same purgatory as #[bench]?

If this is permanently abandoned, it would be good if this issue, and the unstable book recorded that in a clear way.

@jdonszelmann
Copy link
Contributor

I just wanted to link this tracking issue here, as it's somewhat related:
#125119

@HernandoR
Copy link

I followed unstable book of test::Bencher to here, after browsing dozens of issues, may i conclude that crate test is unstable, hence we should open to discussion here? also, what is unstable and needs to be discussed and decided to stabilize test lib?

@weihanglo
Copy link
Member

See rust-lang/testing-devex-team#2

@RalfJung
Copy link
Member

Reopening, since this issue is still referenced as tracking issue by compiler messages (e.g. here).

@RalfJung RalfJung reopened this Dec 14, 2024
bors added a commit to rust-lang-ci/rust that referenced this issue Dec 20, 2024
de-stabilize bench attribute

This has been soft-unstable since forever (rust-lang#64066), and shown in future-compat reports since Rust 1.77 (rust-lang#116274).

The feature covering `bench` itself is tracked in rust-lang#50297, which has been closed despite still having active feature gates referencing it.

Cc `@rust-lang/libs-api`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-libtest Area: `#[test]` / the `test` library B-RFC-approved Blocker: Approved by a merged RFC but not yet implemented. C-tracking-issue Category: An issue tracking the progress of sth. like the implementation of an RFC F-custom_test_frameworks `#![feature(custom_test_frameworks)]` S-tracking-design-concerns Status: There are blocking design concerns. T-cargo Relevant to the cargo team, which will review and decide on the PR/issue. T-dev-tools Relevant to the dev-tools subteam, which will review and decide on the PR/issue. T-lang Relevant to the language team, which will review and decide on the PR/issue. WG-embedded Working group: Embedded systems
Projects
None yet
Development

No branches or pull requests