rust-lang / rust

Empowering everyone to build reliable and efficient software.
https://www.rust-lang.org
Other
98.13k stars 12.69k forks source link

Doctests don't work in bin targets, non-public items #50784

Open kornelski opened 6 years ago

kornelski commented 6 years ago

It's surprising that doctests sometimes don't run at all, and there's no warning about this.

cargo new foo --bin
/// ```rust
/// assert!(false);
/// ```
fn main() {
    println!("Hello, world!");
}
cargo test --all

I'd expect the above to fail, but it looks like the doc-comment code is never ran. Even if I explicitly enable doctests for the binary:

[[bin]]
name = "foo"
path = "src/main.rs"
doctest = true

they still aren't run.

(moved from https://github.com/rust-lang/cargo/issues/5477)

GuillaumeGomez commented 6 years ago

It sounds weird to have documentation examples in a bin target, don't you think? There is no documentation to generate so why would there be documentation's code to run?

kornelski commented 6 years ago

This bug is based on feedback from a user.

Cargo makes new users create bin targets by default, so the first time a new Rust user tries doc tests, it's probably in a bin.

There's a mismatch between what rustdoc does (tests publicly-visible documentation) and expectations (that tests in doccomments, in general, are run).

GuillaumeGomez commented 6 years ago

Maybe we give bad information ahead? I wonder if users know the difference between comments and doc comments... For now I don't see this as a rustdoc bug but as a misunderstanding that should be resolved in tutorials/books.

cc @rust-lang/docs

vi commented 6 years ago

Maybe doctests should just run unconditionally?

The code for a non-public bin-crate item may end up in a public, lib-crate item later, so what's wrong having documentation and tests a bit earlier?

QuietMisdreavus commented 6 years ago

The problem with running doctests on bin targets lies with how doctests work. The way rustdoc creates doctests is by building the code sample as a bin target of its own, and linking it against your crate. If "your crate" is really an executable, there's nothing to link against.

If you run cargo test --doc --verbose, you can see that Cargo needs to build your crate ahead of time, and pass the rlib to rustdoc:

[misdreavus@tonberry asdf]$ cargo test --doc --verbose
   Compiling asdf v0.1.0 (file:///home/misdreavus/git/asdf)
     Running `rustc --crate-name asdf src/lib.rs --crate-type lib --emit=dep-info,link -C debuginfo=2 -C metadata=31e1a62d36de9a99 -C extra-filename=-31e1a62d36de9a99 --out-dir /home/misdreavus/git/asdf/target/debug/deps -C incremental=/home/misdreavus/git/asdf/target/debug/incremental -L dependency=/home/misdreavus/git/asdf/target/debug/deps`
    Finished dev [unoptimized + debuginfo] target(s) in 0.79 secs
   Doc-tests asdf
     Running `rustdoc --test /home/misdreavus/git/asdf/src/lib.rs --crate-name asdf -L dependency=/home/misdreavus/git/asdf/target/debug/deps -L dependency=/home/misdreavus/git/asdf/target/debug/deps --extern asdf=/home/misdreavus/git/asdf/target/debug/deps/libasdf-31e1a62d36de9a99.rlib`

running 1 test
test src/lib.rs - SomeStruct (line 3) ... ok

test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out

Semantically, i find nothing wrong with allowing doctests in bin crates. However, to pull this off, we would need to tell Cargo to compile bin crates as libs, then hand that rlib to rustdoc for testing. There's nothing stopping us from doing that right now:

[misdreavus@tonberry asdf]$ rustc --crate-type lib --crate-name asdf src/main.rs
[misdreavus@tonberry asdf]$ rustdoc --test --crate-name asdf src/main.rs --extern asdf=libasdf.rlib

running 1 test
test src/main.rs - SomeStruct (line 3) ... ok

test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out

cc @rust-lang/cargo

kornelski commented 6 years ago

Wow, that's a nice surprise that it works. I was afraid that duplicate main would be a problem, but I guess rlib namespaces it!

QuietMisdreavus commented 6 years ago

Yeah, the main in your bin goes inside the lib crate, so it's just a regular function at that point. I did have to tag it with #[allow(dead_code)] to silence a warning, but otherwise it's all good.

PhilipDaniels commented 4 years ago

I just hit this today. I came here after finding https://github.com/rust-lang/cargo/issues/2009 in a Google search, which mentions that it is 'addressed in the documentation'. I suspect it may have been at one point but it doesn't appear to be now. I checked

https://doc.rust-lang.org/book/ch11-01-writing-tests.html

and from there the link

https://doc.rust-lang.org/book/ch14-02-publishing-to-crates-io.html#documentation-comments-as-tests

(Of course it's possible its mentioned somewhere and I missed it. A very careful reading of the surrounding text implies it, because it's all talking about libs, but nowhere I see is it mentioned explicitly.)

joshtriplett commented 4 years ago

I ran into this today, and found it confusing. I would love to be able to write doctests in the modules of my binary crate.

What would it take to make that work out-of-the-box?

ehuss commented 4 years ago

I can think of a few hurdles that someone would need to address:

/// ```
/// do_something();
/// ```
fn do_something() {}

fn main() {}

Here, do_something(); would fail because it is not in scope. You could try to scope it with the name of the binary (mything::do_something();), but then you have two new problems: what if you have a lib target of the same name? And, deal with the fact that you normally don't export any public items in a binary.

One (rough) idea to address this would be to fundamentally change how rustdoc --test works. Instead of creating a synthetic test that links to a library, embed the tests directly in the module. So it would rewrite the above example to:

fn do_something() {}

#[test]
fn do_something_doctest() {
    do_something();
}

So all local (private) symbols would be in scope.

That's a pretty radical difference, so I'm not sure if it would fly (would probably be confusing if there are two styles of doctests).

Also, I think it's worth examining if this is a good idea from the start. Documenting binaries still seems like a strange concept to me. Why can't you just move all the code to a library?

joshtriplett commented 4 years ago

On Sat, Mar 28, 2020 at 02:23:19PM -0700, Eric Huss wrote:

I can think of a few hurdles that someone would need to address:

  • How to resolve symbols in a binary? Doctests behave as-if they are a separate crate linking in the library. But a binary is normally not used a library, so it has no name to refer to. For example:
/// ```
/// do_something();
/// ```
fn do_something() {}

fn main() {}

Here, do_something(); would fail because it is not in scope. You could try to scope it with the name of the binary (mything::do_something();), but then you have two new problems: what if you have a lib target of the same name? And, deal with the fact that you normally don't export any public items in a binary.

  • How to deal with lack of public exports? Building a binary as a library will result in a lot of unused warnings. And since rustdoc imports the crate, usually nothing will be publicly exported, so you won't be able to access anything.

One (rough) idea to address this would be to fundamentally change how rustdoc --test works. Instead of creating a synthetic test that links to a library, embed the tests directly in the module. So it would rewrite the above example to:

fn do_something() {}

#[test]
fn do_something_doctest() {
    do_something();
}

So all local (private) symbols would be in scope.

I would love to see that as the normal approach to doctests, personally. It would also unify the different kinds of tests. I do agree that that would be a more invasive change, though.

Also, I think it's worth examining if this is a good idea from the start. Documenting binaries still seems like a strange concept to me. Why can't you just move all the code to a library?

Why should the concept of documentation and testing be limited to libraries, and not supported in binary crates? I use documentation in binary crates to document the code for later understanding. I'd like to use tests to verify individual modules of functionality; that doesn't necessarily mean I want to split that functionality into a separate library crate.

dkasak commented 4 years ago

Just hit this today. I also frequently document internal modules of binary crates and intuitively expected that doctests would still work. Effectively, I use internal modules as non-public libraries. The reason I do not make them into separate library crates is that they are often not useful in a general context, but I do sometimes end up turning them into libraries. This kind of flexibility seems like a win.

ayroblu commented 4 years ago

Hey, so I just learnt about this while learning about the language. It's disappointing, and I'm not sure this has been prioritised so it doesn't sound like it's going to be addressed in the near future.

Anyways, I guess the only way then is to do the plebeian way and write a test for it

GuillaumeGomez commented 4 years ago

You can split your crate into two parts: the binary part (containing only the main function and the args parsing eventually) which calls all other functions and the "lib" part. Then you can document all functions inside the lib part. I did that for a few crates of mine and I think it's more than enough and is a better match for how doc should be handled. What do you think?

ayroblu commented 4 years ago

@GuillaumeGomez Do you have a good example or minimum POC with like cargo build? Sorry still learning here

GuillaumeGomez commented 4 years ago

Sure, here you go: https://github.com/gtk-rs/gir/blob/master/Cargo.toml

slightlytwisted commented 4 years ago

It sounds weird to have documentation examples in a bin target, don't you think? There is no documentation to generate so why would there be documentation's code to run?

I think mostly for consistency. If someone is used to writing docs with rustdoc and doctests on their library functions, it makes sense that they would want apply the same documentation style to their bin functions. On a small project, having to separate out helper functions to a separate library is an extra hoop to jump through, especially if it is just for the sake of getting tests to run.

I think underneath this is the assumption that people don't need to generate documentation for binaries. But in a team environment, its a good idea to document as much as you can, including examples. Having to switch documentation and test styles between libraries and binaries seems like an arbitrary constraint.

dkasak commented 4 years ago

But in a team environment, its a good idea to document as much as you can, including examples.

Yes, I even document extensively for myself! And you can never be sure whether someone will join you on a project in the future.

steveklabnik commented 4 years ago

I think underneath this is the assumption that people don't need to generate documentation for binaries.

It's not quite this, exactly. Rustdoc is a tool for documenting a Rust API. A binary does not have a Rust API. So rustdoc is not really meant for documenting binaries.

Where the friction really comes is when you want to produce internal documentation. This is a concern for libraries too, and that's why --document-private-items exists. But the way that rustdoc goes about generating this information ends up being tough for binaries, because it's oriented around lbiraries, by nature.

kornelski commented 4 years ago

Doccomments unfortunately couple two things, but I think in this case the concern of running (doc)tests could be separate from concern of documenting things. Some tests like compile_fail just have to live in a doccomment, regardless whether one wants to generate documentation or not.

To look at it from a different perspective: tests are run by cargo test --all, not cargo doc. The fact that rustdoc is involved in testing is an implementation detail. If you ignore the mechanics of how the tests are extracted, in the grand scheme it's just an issue of cargo test --all not running all tests.

dogweather commented 4 years ago

@joshtriplett Take a look at how Elixir does doc tests: in the form of an interactive REPL session:

defmodule MyModule do
  @doc """
  Given a number, returns true if the number is even and else otherwise.

  ## Example
    iex> MyModule.even?(2)
    true
    iex> MyModule.even?(3)
    false
  """
  def even?(number) do
    rem(number, 2) == 0
  end
end

This would be a big change, and implies a REPL that everyone is familiar with. But it's a very low-friction way to write tests. Maybe as a custom testing framework when that hits stable...

samwightt commented 4 years ago

Just hit this roadblock today with a smaller bin project that I was working on and it was very annoying. Doc tests should be standard across all project types, not just library.

huntc commented 4 years ago

I'm seeing this issue cropping up when I do have a library, albeit a static library...:

% cargo test --doc --verbose      
warning: doc tests are not supported for crate type(s) `staticlib` in package `mylib`
error: no library targets found in package `mylib`

Should I raise this as a separate issue? Thanks.

rvolgers commented 3 years ago

Just ran into this today.

To the people explaining why this is tricky to implement in a consistent way: I hear you, and I understand.

To the people who (seem to?) be arguing that this is good and normal and expected: you are seriously wrong.

Things that normally work and then in another context quietly don't do anything are really bad papercuts.

While I understand there needs to be some guiding principle behind rustdoc, the idea that it is for documenting Rust APIs and therefore it doesn't make sense for binaries is not a helpful one. By that logic we could ban doc comments entirely in bin crates, at least that would be a consistent position.

TLDR: doc tests not working may be a tricky bug, but it sure isn't a feature

GuillaumeGomez commented 3 years ago

TLDR: doc tests not working may be a tricky bug, but it sure isn't a feature

No one said it was a feature, we just said it was out of rustdoc scope. Having doc tests in a binary doesn't sound crazy to me, it's simply something that we need to agree upon and then to update rustdoc source code so that it can work there too (which might be quite tricky...).

ganzhi commented 3 years ago

Just hit it today. Understand it was out of rustdoc scope till now. How can we make it part of rustdoc scope? :)

slanden commented 3 years ago

It sounds weird to have documentation examples in a bin target, don't you think? There is no documentation to generate so why would there be documentation's code to run?

Just my two cents, I think having tests to validate documented examples in a bin target is just as important. After all, contributors of the code need to know how it works. New contributors or old returning after months away.

Somebody might've changed the code but forgot to update the docs. Having dedicated tests of course is better and helps, but they're different; I like having both.

jyn514 commented 3 years ago

This is not actually a rustdoc bug: you can run rustdoc --test on a binary just fine:

$ cat src/main.rs
/// ```
/// assert!(true);
/// ```
fn main() {}
$ rustdoc --test src/main.rs 

running 1 test
test src/main.rs - main (line 1) ... ok

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

The problem is Cargo doesn't run rustdoc --test when you run cargo test --bin or cargo test --all-targets.

@ehuss could you move this to rust-lang/cargo?

ehuss commented 3 years ago

I don't think this is a cargo issue, as I think there are some fundamental issues with how doctests link with the surrounding code, as described above at https://github.com/rust-lang/rust/issues/50784#issuecomment-605521414. It would be feasible to run doctests on binaries as you mentioned, but not very useful since you wouldn't be able to access any of the code within the binary.

My opinion is that it would be good for someone to write a proposal for a different way for doctests to work. I think embedding them as regular unit tests is a compelling option, as it would solve the issue of working with binaries, working with private items (#60820), and improving performance (#75341). It won't be easy and it will likely need to be opt-in (with possibly a per-test opt-out), but I think it is worthwhile.

quixoticaxis commented 2 years ago

Are there any updates on what the current consensus about running the doctests for binaries and private items is?

EvanCarroll commented 2 years ago

This is a real gotcha. It's not clear to me why doc tests should not run on a binary, when the same code (in src/main.rs) generates code docs with cargo doc

kupiakos commented 2 years ago

Got bit by this today: I have a private util module deep in a crate. However, I'd still like to provide testable examples to the in-crate users of this module. There's no way to do this, since rustdoc was initially designed with the idea of "document this public Rust API" and not "document this Rust code". rustdoc needs a paradigm shift here to make doctests consistently useful. Documenting monolithic internals is useful too :)

I begin every one of my doctests with # use crate_name::path::to::item::ItemUnderTest. I understand why this is currently required, but requiring it of every doc test is a papercut, and is the key reason why it's impossible to doctest in a private module. Perhaps we can resolve paths inside doctests to be relative to the item under test? Or implicitly expose the item under test? Or somehow grant an exception for access to private items when accessing the item under test in a doctest? These should all be backwards-compatible solutions, or at least edition-compatible.

Kromey commented 2 years ago

This bit me today, I'm working on a large (well, will be) project, and for both myself and potential future contributors I've been using rustdoc with --document-private-items to create internal documentation. I threw a simple doctest in one, but because it's inside a private module it of course failed to compile.

Not knowing anything about how they work, would it be possible to have rustdoc generate unit tests from doctests, rather than separate bin crates? I realize this would probably be a breaking change to existing (and functional) doctests, not to mention may violate the original intent of documenting public APIs, so perhaps in the same way we can add attributes such as ignore or compile_fail, could a unit_test attribute be added to doctests we want to use this behavior?

My current workaround is to add the ignore tag to my code blocks, so that at least I can still provide example code in the documentation, but of course this largely defeats the purpose of having doctests in the first place, which is to help ensure that code and documentation remain in sync.

bryanlarsen commented 2 years ago

As a workaround it's fairly straightforward to add a lib.rs and a [lib] entry to your Cargo.toml to make your crate both a binary and a library.

GuillaumeGomez commented 2 years ago

Already suggested here: https://github.com/rust-lang/rust/issues/50784#issuecomment-653895749. :wink:

AntoniosBarotsis commented 2 years ago

I also stumbled upon this.

If you happen to have both a main and a lib, be sure to specify them in your cargo.toml like so:

[[bin]]
name = "main"
path = "src/main.rs"

[lib]
name = "lib"
path = "src/lib.rs"

Got this from here.

ghost commented 1 year ago

Could rustdoc tests be turned into regular tests? That way a use super::*; could be added and then it would actually be possible to test crate private things and in binaries as well.

dcormier commented 1 year ago

Could rustdoc tests be turned into regular tests?

They cannot. You can write doctests that are expected to not compile (with the compile_fail attribute). You cannot do the same with regular tests.

vi commented 1 year ago

Just add compile_fail regular tests? (one such test per file, triggered by some special comment)

jyn514 commented 1 year ago

https://github.com/dtolnay/trybuild is a library for testing compile errors. I think it's unlikely it will become part of libtest itself.

DragonDev1906 commented 1 year ago

Not knowing anything about how they work, would it be possible to have rustdoc generate unit tests from doctests, rather than separate bin crates? I realize this would probably be a breaking change to existing (and functional) doctests, not to mention may violate the original intent of documenting public APIs, so perhaps in the same way we can add attributes such as ignore or compile_fail, could a unit_test attribute be added to doctests we want to use this behavior?

I'd like to point out this comment by @Kromey:

While it would be impossible to use the compile_fail attribute for private functions (use ignore as a fallback or keep compile_fail even if it always fails compilation due to using private methods), this would at least allow us to use doctests for private functions or in private modules that correctly compile (i.e. everything usually done without attributes or should_panic. At the same time this would not be a breaking change, since these examples currently do not compile anyways.

For those tests we could automatically use super::* as with unit tests and it would make sense to do so because they should behave like a unit test.

Alternatively, this could be done automatically (without a unit_test attribute) for all doctests in private modules, where it is already impossible to run the examples if they use private functions. This might be a breaking change, since existing doctests for private modules/functions that do not use these functions would be run in a different context.

If you ask me, having the ability to execute doctests in private modules iff they are supposed to compile is a big benefit for internal documentation of private modules in a library, even if it has the limitation that the example must compile. Especially when this can be done without a breaking change or in a way where adding the attribute can be automated later. It is really annoying that we have to add ignore to all examples in private modules just because we can not import the function the example is about for the doctest.

TheButlah commented 1 year ago

This bit me today. I am writing a backend service, which has a JSON api. I wanted to document an example of valid JSON, for someone reading the code who is unfamiliar with rust.

Conceptually this should only need to be a bin crate - its an application. But I still have a need to document its API. It has an API because its exposing HTTP endpoints, and that is a form of API.

It would be good if I didn't have to resort to making a trivial main.rs that calls a lib.rs with a main() function. That is not a good developer experience.

chrisp60 commented 1 year ago

This caught me today. It seems like very unintuitive behavior. Since I am still very new to the language, the docs are mostly for my own benefit and sanity. I know there is an unstable feature to scrape examples, but I really wish I could just write my examples outside of markdown blocks while still having them be within the same file of what they are documenting.

Ofenhed commented 1 year ago

I will humbly admit I'm not well versed in rustc or cargo internals, so I may be way off. I would still like to make a few comments based on what I would want and which behaviors I would expect as a developer when running cargo test:

  1. Public facing documentation targets in libraries should really work as they do today. Giving them access to super:: would be detrimental to the quality of documentation.
  2. Private facing documentation targets (such as private functions in libraries or any function in a binary) could be converted to anonymous submodules where they are documented. This would give us access to super::.
    1. For documentation tests marked as compile_fail, the use statement should be conditional. Maybe behind a feature flag, maybe conditionally generated.
    2. Fully private documentation targets could be converted to tests in the same module, as the declared use statements are valid for all functions which can call this function. This is obviously not true for tests marked with compile_fail.
    3. Tests for documentation targets which are not fully private should never just be converted to tests in the same module. I think that declaring usage makes the documentation clearer.
    4. The anonymous submodules could be confusing in compiler error text (file paths), so we would need to process that output. Can we do this?
  3. I guess that for this to work we would also need to inject code into main.rs/lib.rs to publish the anonymous test modules to the function which runs the tests?
    1. An alternative could be to parse the main function for mod statements (or something smarter) to create a new library source file we can use for the tests.
    2. It would be great if we could disable unused warnings completely. It would be perfect if we could do it for all modules except our anonymous test modules.
khionu commented 1 year ago

@daniel-pfeiffer People aren't always going to have the variety of perspectives at the early stages of a given exploration. We should be empathetic to that people come from all walks of life and may need some help to understand other perspectives. Your response here was very much not how we should help each other understand things, and that's putting aside that you're replying to something from almost 5 years ago. In addition to the empathy, please watch for context.

Lastly, a lot of your response was loaded with emotional context that didn't do anything for the conversation. All feelings are valid, but how you act on them matters. This wasn't the place to vent about your feelings with how contributions are done.

JeffThomas commented 1 year ago

Yeah this caught me today too. I'm writing something that's not intended to be used as-is (such as a library) but is meant to be forked and expanded on for the users special case. Working examples in the documentation would be very valuable in demonstrating this use, I think. More-so than making potential users dig into tests. I understand that this is a tricky issue but since people are still commenting on it some 4 years after it was first reported, maybe it can be moved up on in priority?

Having functioning examples in the documentation is such a powerful and useful feature it's a shame that it's limited in such a way.

GuillaumeGomez commented 1 year ago

As suggested in the first comments, it could already work by providing a small fix in cargo, which would be the simplest way to achieve this feature. You can try your luck there directly.

modulok commented 1 year ago

I was bit by this today too. I'm in favor of doctests just running, binary or not. It can be very useful when teaching Rust to be able to put a doctest in a binary. Given the number of people just here who have hit this - and bothered to comment - and the number of years this has bitten folks, it'd probably more of an expected behavior if they just always run, even in binaries.

vi commented 1 year ago

If it is tricky to run those doctests, maybe it can be easier to at least detect them and warn.

/// ```
///   2+2
/// ```
fn main() {
    println!("Hello, world!");
}
$ cargo test
warning: doctests do not work in [[bin]] targets or in private items
 --> src/main.rs:1:4
modulok commented 1 year ago

Agreed. That would be helpful until an always-run-tests feature can (if possible and desired) be properly designed and implemented. I doubt implementing such a warning could break anyone's existing code.