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

Dev-dependencies are not linked when testing subpackages #860

Closed
lifthrasiir opened this issue Nov 13, 2014 · 9 comments
Closed

Dev-dependencies are not linked when testing subpackages #860

lifthrasiir opened this issue Nov 13, 2014 · 9 comments
Labels

Comments

@lifthrasiir
Copy link
Contributor

$ cat Cargo.toml
[package]
name = "p"
version = "0.0.1"
authors = []

[dependencies.q]
path = "src/q"

$ cat src/lib.rs
extern crate q;

$ cat src/q/Cargo.toml
[package]
name = "q"
version = "0.0.1"
authors = []

[dev-dependencies.r]
path = "../r"

$ cat src/q/src/lib.rs
#[cfg(test)] extern crate r;
#[cfg(test)] #[test] fn foo() { assert_eq!(r::f(), 42); }

$ cat src/r/Cargo.toml
[package]
name = "r"
version = "0.0.1"
authors = []

[lib]
name = "r"
plugin = true

$ cat src/r/src/lib.rs
pub fn f() -> uint { 42 }

$ cargo test -p q
   Compiling q v0.0.1 (file:///path/to/p)
/path/to/p/src/q/src/lib.rs:1:14: 1:29 error: can't find crate for `r`
/path/to/p/src/q/src/lib.rs:1 #[cfg(test)] extern crate r;
[snip]

$ cd src/q && cargo test
     Running target/q-[hash]

running 1 test
test foo ... ok
[snip]
@alexcrichton
Copy link
Member

This is something that we'll actually need to deny with a first-class error I believe. The lockfile doesn't list any dev-dependencies of upstream packages, so we won't actually know which copies to build if they're requested.

@luser
Copy link
Contributor

luser commented Nov 4, 2015

When you say "deny" do you mean cargo test -p q should error in this situation?

Would this be fixable if we added an explicit test-dependencies section to the manifest?

@alexcrichton
Copy link
Member

Heh well now being almost a year later, my opinions have changed on this issue a bit! Back then I did indeed intend that cargo test -p q would return an error, but I now today would like this to "Just Work" for most cases (but perhaps not all).

Unfortunately adding test-dependencies wouldn't solve the problem (that's what dev-dependencies are), it basically means that dev-deps need to be included as part of the normal resolution graph no matter what.

@SimonSapin
Copy link
Contributor

FWIW my use case (#2620) is testing multiple crates in the same git repository. They are all reachable from the root Cargo.toml file through path dependencies. In that sense they’re not "upstream" creates, just parts of a large program split up into multiple compilations units to keep build times manageable.

mbrubeck added a commit to mbrubeck/cargo that referenced this issue Apr 26, 2016
Fixes rust-lang#860.  Transitive dev dependencies are now always resolved and included
in the lock file, but are built only when testing a crate that depends
directly on them.
alexcrichton added a commit to alexcrichton/rust that referenced this issue May 12, 2016
This commit adds support to rustbuild to run crate unit tests (those defined by
`#[test]`) as well as documentation tests. All tests are powered by `cargo test`
under the hood.

Each step requires the `libtest` library is built for that corresponding stage.
Ideally the `test` crate would be a dev-dependency, but for now it's just easier
to ensure that we sequence everything in the right order.

Currently no filtering is implemented, so there's not actually a method of
testing *only* libstd or *only* libcore, but rather entire swaths of crates are
tested all at once.

A few points of note here are:

* The `coretest` and `collectionstest` crates are just listed as `[[test]]`
  entires for `cargo test` to naturally pick up. This mean that `cargo test -p
  core` actually runs all the tests for libcore.
* Libraries that aren't tested all mention `test = false` in their `Cargo.toml`
* Crates aren't currently allowed to have dev-dependencies due to
  rust-lang/cargo#860, but we can likely alleviate this restriction once
  workspaces are implemented.

cc rust-lang#31590
eddyb added a commit to eddyb/rust that referenced this issue May 12, 2016
… r=brson

rustbuild: Add support for crate tests + doctests

This commit adds support to rustbuild to run crate unit tests (those defined by
`#[test]`) as well as documentation tests. All tests are powered by `cargo test`
under the hood.

Each step requires the `libtest` library is built for that corresponding stage.
Ideally the `test` crate would be a dev-dependency, but for now it's just easier
to ensure that we sequence everything in the right order.

Currently no filtering is implemented, so there's not actually a method of
testing *only* libstd or *only* libcore, but rather entire swaths of crates are
tested all at once.

A few points of note here are:

* The `coretest` and `collectionstest` crates are just listed as `[[test]]`
  entires for `cargo test` to naturally pick up. This mean that `cargo test -p
  core` actually runs all the tests for libcore.
* Libraries that aren't tested all mention `test = false` in their `Cargo.toml`
* Crates aren't currently allowed to have dev-dependencies due to
  rust-lang/cargo#860, but we can likely alleviate this restriction once
  workspaces are implemented.

cc rust-lang#31590
eddyb added a commit to eddyb/rust that referenced this issue May 12, 2016
… r=brson

rustbuild: Add support for crate tests + doctests

This commit adds support to rustbuild to run crate unit tests (those defined by
`#[test]`) as well as documentation tests. All tests are powered by `cargo test`
under the hood.

Each step requires the `libtest` library is built for that corresponding stage.
Ideally the `test` crate would be a dev-dependency, but for now it's just easier
to ensure that we sequence everything in the right order.

Currently no filtering is implemented, so there's not actually a method of
testing *only* libstd or *only* libcore, but rather entire swaths of crates are
tested all at once.

A few points of note here are:

* The `coretest` and `collectionstest` crates are just listed as `[[test]]`
  entires for `cargo test` to naturally pick up. This mean that `cargo test -p
  core` actually runs all the tests for libcore.
* Libraries that aren't tested all mention `test = false` in their `Cargo.toml`
* Crates aren't currently allowed to have dev-dependencies due to
  rust-lang/cargo#860, but we can likely alleviate this restriction once
  workspaces are implemented.

cc rust-lang#31590
bors added a commit to rust-lang/rust that referenced this issue May 12, 2016
rustbuild: Add support for crate tests + doctests

This commit adds support to rustbuild to run crate unit tests (those defined by
`#[test]`) as well as documentation tests. All tests are powered by `cargo test`
under the hood.

Each step requires the `libtest` library is built for that corresponding stage.
Ideally the `test` crate would be a dev-dependency, but for now it's just easier
to ensure that we sequence everything in the right order.

Currently no filtering is implemented, so there's not actually a method of
testing *only* libstd or *only* libcore, but rather entire swaths of crates are
tested all at once.

A few points of note here are:

* The `coretest` and `collectionstest` crates are just listed as `[[test]]`
  entires for `cargo test` to naturally pick up. This mean that `cargo test -p
  core` actually runs all the tests for libcore.
* Libraries that aren't tested all mention `test = false` in their `Cargo.toml`
* Crates aren't currently allowed to have dev-dependencies due to
  rust-lang/cargo#860, but we can likely alleviate this restriction once
  workspaces are implemented.

cc #31590
Emilgardis added a commit to Emilgardis/amethyst that referenced this issue Jul 20, 2016
Due to a problem with cargo test (see rust-lang/cargo#860),
dev-dependencies are not imported when using -p.
@luser
Copy link
Contributor

luser commented Aug 31, 2016

Given the existence of Cargo workspaces, could we make this work for that case? That is, if you have a workspace that has a crate member "foo", which has a [dev-dependencies] section, could cargo test -p foo from the root be made to work?

@alexcrichton
Copy link
Member

@luser correct!

This was yet another motivation for workspaces, and I believe all the infrastructure is there for this to "just work".

@mbrubeck
Copy link
Contributor

Now that workspaces are here, Cargo.lock contains the necessary information but cargo test -p foo doesn't yet use it. I submitted #3125 to fix this.

mbrubeck added a commit to mbrubeck/cargo that referenced this issue Sep 27, 2016
When running `cargo test -p foo` where `foo` is a crate in the current
workspace, build and link `foo`'s dev-dependencies. Fixes rust-lang#860.
bors added a commit that referenced this issue Sep 27, 2016
Build transitive dev-dependencies when needed

When running `cargo test -p foo` where `foo` is a crate in the current workspace, build and link `foo`'s dev-dependencies. Fixes #860.
@kvark
Copy link

kvark commented May 10, 2017

@mbrubeck
I don't understand why this issue is closed. Cargo workspaces are good, but this issue is not about them, and there are cases (gfx-rs/gfx#1258) where workspaces are not available, and we'd still like to be able to use cargo test -p something. As far as I'm concern, this is still not fixed.

@mbrubeck
Copy link
Contributor

My patch to fix this for the non-workspace case was rejected in #2621, with the conclusion that this is officially not supported for crates outside of a workspace, mainly because it can't be done while preserving lockfile compatibility with older Cargo versions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants