Is it safe to do only `cargo test` in my travis jobs?

Hi,

currently I'm running basically cargo build && cargo test in my travis jobs. I am thinking about removing the first part and am not sure about the implications. Is it safe to do only cargo test in my CI jobs? I guess as cargo test still does a cargo build, it would catch all errors cargo build would catch, right?

Or am I missing something?

Seems perfectly safe to me! AFAIK, cargo test is a strict superset of cargo build.

3 Likes

I'm especially concerned about binaries in my project. They get build when I run cargo test --all in my workspace project, right?

Running cargo build for the second time will complete instant. So if cargo test contains a build it would complete instant right? And if so that would mean that you would not improve your build times by removing the cargo build. As the time saved would be spend by cargo test doing the build.

Should be easy to test:
Cargo clean
Cargo test
Cargo build < if the build was done by the test than this should complete instance.

On the other hand, what's the value in removing cargo build from your test script?

If you use #[cfg(test)] it's possible to break the normal build, e.g. if you make a necessary extern crate or function definition a test-only when it shouldn't be. Also tests use things, so they hide errors about otherwise dead code.

8 Likes

It is not safe to do cargo test only. You want to keep it as cargo build && cargo test.

As @kornel wrote, it is possible for cargo test to work but break cargo build as in the following minimal example.

#[cfg(test)]
struct A;

type B = A;
6 Likes

Won’t it still do a normal build if you have any integration tests at all?

1 Like

Not sure what you mean, TBH.

cargo test compiles your crate without cfg(test) for integration tests. It's why you see duplicated warnings sometimes:

// in src\lib.rs
fn foo() {
}

and running:

> cargo test
   Compiling bug v0.1.0 (file:///C:/Users/steve/tmp/bug)
warning: function is never used: `foo`
 --> src\lib.rs:1:1
  |
1 | fn foo() {
  | ^^^^^^^^
  |
  = note: #[warn(dead_code)] on by default

warning: function is never used: `foo`
 --> src\lib.rs:1:1
  |
1 | fn foo() {
  | ^^^^^^^^
  |
  = note: #[warn(dead_code)] on by default

    Finished dev [unoptimized + debuginfo] target(s) in 1.75 secs
     Running target\debug\deps\bug-3587c70570f72fb4.exe

running 0 tests

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

   Doc-tests bug

running 0 tests

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

oh, that's even without a tests directory. So yes, IMO, cargo build && cargo test is redundant.

The example @dtolnay brought up also fails, for this reason:

> cargo test
   Compiling bug v0.1.0 (file:///C:/Users/steve/tmp/bug)
error[E0412]: cannot find type `A` in this scope
 --> src\lib.rs:4:14
  |
4 | pub type B = A;
  |              ^ did you mean `B`?

error: aborting due to previous error

If you want more information on this error, try using "rustc --explain E0412"
error: Could not compile `bug`.
warning: build failed, waiting for other jobs to finish...
warning: function is never used: `foo`
 --> src\lib.rs:6:1
  |
6 | fn foo() {
  | ^^^^^^^^
  |
  = note: #[warn(dead_code)] on by default

3 Likes

Thank you all for the discussion and support. I merged a change which removes the cargo-build job from my travis setup, so this issue is solved for me.

:heart:

1 Like

Okay but I still think this is going to miss bugs that would have been caught before. For example try pasting the code from this comment into bin/core/imag/src/main.rs. It builds fine with cargo +stable test --all --all-features but breaks with cargo +stable build --all --all-features.

I use cargo build among other things to have a simpler build that runs first, it's easier to read any errors or warnings that show up at that stage.

“Safe” depends a bit on the crate. If you do a lot of low level hacking it might be prudent to run your tests with optimizations both off and on. ndarray tests with optimizations, since we've run into both our own bugs and compiler bugs that only showed up with optimizations. So in that case, it's safer to test with cargo test && cargo test --release.

1 Like

That's very interesting and surprising, @steveklabnik! I hadn't realized this before. Maybe we should note this expectation in The Cargo Book?

1 Like

Seems good to me!

Only for library crates. If your project contains any binary crates, cargo test will not build these in non-test mode, so it can miss errors that show up in the regular build.

8 Likes

Ah ha! Iiiiiiinteresting.