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

"jobs" CLI flag possibly not taking effect #1591

Closed
jimmycuadra opened this issue May 7, 2015 · 9 comments
Closed

"jobs" CLI flag possibly not taking effect #1591

jimmycuadra opened this issue May 7, 2015 · 9 comments
Labels
A-documenting-cargo-itself Area: Cargo's documentation

Comments

@jimmycuadra
Copy link
Contributor

Hello! I'm trying to run the test suite for one of my Rust programs with only a single thread, because each test changes state in an external database and needs to clean up after itself before the next test starts. (This clean up is implemented via the Drop trait on a struct being created in each test.)

However, each of my tests fails intermittently, even when running cargo test -j1. I put print statements in the constructor and Drop::drop to see if each struct was being dropped before the next one was being created. What I discovered was that test runs frequently printed multiple constructor calls before their corresponding drops. Perhaps I'm misunderstanding what's actually happening, but this seems to suggest that test cases are actually running in parallel even though I specified that only one job should be used.

Here is a small program that demonstrates the issue:

struct Foo;

impl Foo {
    fn new() -> Foo {
        println!("new");

        Foo
    }
}

impl Drop for Foo {
    fn drop(&mut self) {
        println!("drop");
    }
}

#[test]
fn one() {
    Foo::new();
}

#[test]
fn two() {
    Foo::new();
}

#[test]
fn three() {
    Foo::new();
}

#[test]
fn four() {
    Foo::new();
}

Example output:

$ cargo test --jobs 1 -- --nocapture
     Running target/debug/thread_test-8ec58673041f054d

running 4 tests
new
new
drop
drop
new
drop
new
drop
test two ... ok
test three ... ok
test four ... ok
test one ... ok

test result: ok. 4 passed; 0 failed; 0 ignored; 0 measured

As you can see, a second call to "new" is being made before the previous one has called "drop."

Am I using the --jobs flag incorrectly? Am I misunderstanding what it actually does? How can I get my test cases to run sequentially? Thanks!

@alexcrichton
Copy link
Member

Thanks for the report! Currently the --jobs flag is only for Cargo's own internal parallelization of your build, actually running the binaries afterwards is independent. Here you'll want to set the RUST_TEST_THREADS environment variable to 1 to indicate that tests should run serially.

@jimmycuadra
Copy link
Contributor Author

Thanks for the reply! When you say "currently" do you mean the intent is for that flag to eventually do what I expected it to do? If so, I might look into a PR for that. If not, there is probably a documentation change I could make to make it clearer that the flag doesn't affect the test run itself.

@alexcrichton
Copy link
Member

Ah no sorry I didn't mean that we intended it to do so. Perhaps the --help page for the test and/or bench command could have a few words? I wouldn't want to add too much text about this though.

@jimmycuadra
Copy link
Contributor Author

After talking to both @wycats and @alexcrichton about whether or not Cargo should behave as I originally expected it would, I'd like to nominate this issue for reopen and more discussion. I think one of the following things should happen:

  1. It should do what I thought it would do.
  2. cargo test -h should indicate that --jobs only affects the building of the test binary, and a new flag, perhaps --test-threads should control what I originally wanted, so Cargo users don't need to know about RUST_TEST_THREADS.

@thijsc
Copy link

thijsc commented May 19, 2015

I just spent an hour beging confused about this too. 👍 on the change @jimmycuadra proposes.

@alexcrichton
Copy link
Member

Hm ok, I'll reopen for discussion, but I'd like to explain a few more things that I was thinking as well:

  1. It should do what I thought it would do.

I agree, but I also want Cargo to support multiple testing frameworks one day, and I want Cargo to continue to do what I expect it to do at that point. If Cargo only passes down parallelism via an environment variable then we're requiring that all testing frameworks respect this environment variable, which may not always be desired. I do not personally expect cargo do be anything beyond a wrapper around rustc, and setting environment variables by default and/or passing arguments by default would be very surprising to me as it makes Cargo less agnostic to what it's driving.

  1. cargo test -h should indicate that --jobs only affects the building of the test binary, and a new flag

I agree!

@alexcrichton alexcrichton reopened this May 19, 2015
@alexcrichton alexcrichton added the A-documenting-cargo-itself Area: Cargo's documentation label May 19, 2015
@jimmycuadra
Copy link
Contributor Author

One possible compromise would be making the docs change so that it's clear --jobs doesn't affect the thread count for the test run, and opening an issue on Rust itself to get a --test-threads flag added to replace the environment variable.

I'm still not sure how best to balance the desire for Cargo to be agnostic to the test framework with an intuitive user experience for people just starting with Rust/Cargo.

@alexcrichton
Copy link
Member

My preferred route for now is to use this issue to track adding documentation to --jobs on cargo test that it doesn't affect the parallelism of tests, and open a bug for --test-threads against Rust itself.

bors added a commit that referenced this issue May 27, 2015
Addresses #1591.

I believe this issue only affects `cargo bench` and `cargo test`. If it's applicable to any of the other commands, let me know and I'll amend this.
@steveklabnik
Copy link
Member

#1639 was merged, so I'm closing this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-documenting-cargo-itself Area: Cargo's documentation
Projects
None yet
Development

No branches or pull requests

4 participants