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

Cargo doesn't run doctest for staticlib #2442

Closed
rasendubi opened this issue Mar 5, 2016 · 8 comments
Closed

Cargo doesn't run doctest for staticlib #2442

rasendubi opened this issue Mar 5, 2016 · 8 comments
Labels
E-easy Experience: Easy

Comments

@rasendubi
Copy link

Steps to reproduce:

  • cargo new mylib
  • open src/lib.rs and add:
/// ```
/// assert_eq!(1, 3);
/// ```
pub fn func() {
}
  • add following to Cargo.toml:
[lib]
crate-type = ["staticlib"]
  • cargo test

Build passes. Doctests aren't run.

rasendubi added a commit to rasendubi/bkernel that referenced this issue Mar 5, 2016
See: rust-lang/cargo#2442

Seems that cargo requires `crate-type = ["lib"]` to run tests.
@alexcrichton
Copy link
Member

Ah yeah this is probably just something that needs to be filtered when we run doctests, I think we already handle other pieces specially?

@alexcrichton alexcrichton added the E-easy Experience: Easy label Mar 6, 2016
@jespino
Copy link
Contributor

jespino commented Mar 7, 2016

I think the current behavior is the correct behavior, this is filtered here:

kinds.contains(&LibKind::Rlib) || kinds.contains(&LibKind::Lib)

and reasoned here

// Note that we can *only* doctest rlib outputs here. A

If cargo run doctests will fail in any test that needs to be linked to the module.

@alexcrichton
Copy link
Member

Hm weird, so shouldn't we already handle this in that case?

@jespino
Copy link
Contributor

jespino commented Mar 7, 2016

We can add a warning when you run tests and doctested function return false.

@alexcrichton
Copy link
Member

In theory given that logic we shouldn't even attempt to test staticlibs b/c only rlibs are tested. We may just be forgetting a conditional somewhere to check that method return value.

@jespino
Copy link
Contributor

jespino commented Mar 7, 2016

Not really, the only problem with staticlibs is on "doctests" not on code tests. The problem is related with linking, then, to run doctests with staticlibs you have to compile it in another linkable format (lib or rlib). An approach (a bit weird in my opinion) when you dont have lib or rlib in your crate-type, compile it anyway and run doctests using it.

I think the solution is to warn the user that doctests aren't run when you don't have one of the doctesteable crate types.

@rasendubi
Copy link
Author

From the user point of view, I advocate producing a warning. That's enough to resolve the issue as nobody will be surprised by the behavior.

I don't like silently producing lib or rlib as I found that may change compilation result. (My library once failed to build as rlib due to linkage error to _Unwind_Resume. staticlib worked fine.)

@alexcrichton
Copy link
Member

Wait whoa, I'm sorry! I misread this bug and thought it was saying that we were trying to run doctests but they were failing (legitimately).

This is intentional behavior in that case because as pointed out by @jespino it's fundamentally not possible to run documentation tests for staticlibs (like executables). We tend to not print warnings that aren't really actionable (there's no great way to solve this), so this is currently intended, so closing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
E-easy Experience: Easy
Projects
None yet
Development

No branches or pull requests

4 participants
@alexcrichton @jespino @rasendubi and others