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

Add an option to skip building binaries when there are integration tests #12980

Open
xxchan opened this issue Nov 15, 2023 · 7 comments
Open
Labels
A-cargo-targets Area: selection and definition of targets (lib, bins, examples, tests, benches) C-feature-request Category: proposal for a feature. Before PR, ping rust-lang/cargo if this is not `Feature accepted` Command-test S-needs-design Status: Needs someone to work further on the design for the feature or fix. NOT YET accepted.

Comments

@xxchan
Copy link
Contributor

xxchan commented Nov 15, 2023

Problem

https://doc.rust-lang.org/cargo/reference/cargo-targets.html#integration-tests

Binary targets are automatically built if there is an integration test. This allows an integration test to execute the binary

#7958

There is not currently a way to skip building the binaries.

We want to skip it because we don't use this feature, and our project is very slow to compile.

Here we wasted a lot of time to build the unnecessary bins.

image

Some other users may have other reasons e.g., #7958 (comment).

Proposed Solution

I feel this is kind of similar to disable auto-discovery https://doc.rust-lang.org/cargo/reference/cargo-targets.html#target-auto-discovery

[package]
# ...
autobins = false
autoexamples = false
autotests = false
autobenches = false

So maybe we can add an option like build-bins-for-integration-tests (Can't come up with a good name, sorry)

Notes

No response

@xxchan xxchan added C-feature-request Category: proposal for a feature. Before PR, ping rust-lang/cargo if this is not `Feature accepted` S-triage Status: This issue is waiting on initial triage. labels Nov 15, 2023
@weihanglo
Copy link
Member

Thanks for the report.

At this moment, some workarounds I can think of:

  • Use #[cfg(…)] to conditionally build your binaries, e.g. #[cfg(not(integration_tests))].
  • Make your binaries a separate workspace member.

@weihanglo weihanglo added Command-test S-needs-design Status: Needs someone to work further on the design for the feature or fix. NOT YET accepted. A-cargo-targets Area: selection and definition of targets (lib, bins, examples, tests, benches) and removed S-triage Status: This issue is waiting on initial triage. labels Nov 15, 2023
@epage
Copy link
Contributor

epage commented Nov 15, 2023

Another workaround is to create a cli feature and put required-features on your bins.

@epage
Copy link
Contributor

epage commented Nov 15, 2023

@xxchan to help us better understand the problem, you specifically are having,

  • Why these are "integration" tests, rather than "unit" tests?
  • Why the binaries are a part of this package, rather than in another package?

@xxchan
Copy link
Contributor Author

xxchan commented Nov 15, 2023

Good question. I guess you are asking why we have both bins and integration tests and run into this problem.

In our example, sqlsmith is basically a test binary with CLI args (a fuzzer). But we added more stuff and it just ends up with this..

Why these are "integration" tests, rather than "unit" tests?

I think there were no specific reasons when it was introduced, unit tests should also work.

However, recently we are preferring integration tests and adding test=false, because unit tests will lead to a crate being compiled twice.

Why the binaries are a part of this package, rather than in another package?

No specific reasons also. I think this way is natural to reuse common lib to build different binaries. It would be more troublesome to add a new package. Our workspace is quite large, so I guess developers tend not to add more packages when we can put related stuff in a package. It just works.

@lpotthast
Copy link

I would like to bring this up again.

My use-case consists of

  1. cross-compiling my binary for a different architecture (using cross)
  2. building a docker-image with that binary
  3. starting that image using testcontainers, providing a fresh playground
  4. testing against that binary running in the created container

Let's say this is my only integration test. (I would clearly see that as an integration test, as it is testing the binary as a whole).
Let's say that I just want to call cargo test and nothing fancier.
Let's say that I do not want to further complicate the project structure, as in

  • moving tests to a new project
  • creating a workspace that wasn't necessary before
  • ...

Running cargo test currently results in

  1. cargo compiling my crate for my host architecture (although we know that the resulting binary is not needed for the test!)
  2. my test cross-compiling for a different architecture
  3. ...

Where point 1 is useless, and could be really slow/expensive, even more so in (potentially slow) cloud CI/CD tools.

Automatically compiling a binary for integration tests is of course generally the correct thing to do, as devs will most-likely be able to simply launch it and test against it.
One could argue that my approach here is wasteful. I would have to trigger/require the cross-compilation output in each integration test. But as most of it is cached, that is not much of a problem. Having a way to instruct cargo to cross-compile for one test would most-likely be very hard and is therefore not of consideration.

I would like to propose an optional annotation, available on integration-tests, instructing cargo that that specific test does not need a compiled binary, as in:

#[test]
#[no_compile(<bin-name>)]
fn test() {
    // ...
}

With a logic in cargo as of: "if all integration-test specify that the binary named X is not needed , do not compile the binary before executing tests"

@epage
Copy link
Contributor

epage commented Jul 15, 2024

As @lpotthast brings up, another way of working around this is to use a workspace. While their case is a little different, it sounds like for @xxchan that they are mixing [[bin]]s and [lib]s in the same package and aren't trying to test the [[bin]]s. If the [[bin]] was in a separate package, this wouldn't be a problem. This issue is another manifestation of the divide over when to use multi-crate packages vs crate-per-package workspaces.

For @lpotthast it sounds like things are a little different, that their "integration" tests are being built and run on the host but inside of them, they then perform a cross-compilation and execution of the bin.

In that case, another workaround is to instead build the binary and tests for the cross-compiled architecture and run all of them in the container.

I would like to propose an optional annotation, available on integration-tests, instructing cargo that that specific test does not need a compiled binary, as in:

This gets messy and is unlikely to happen. One way to implement this is for the compiler to emit a message that a test requires the integration test (or not). If we were to be most optimal, we'd then need to re-implement test harness filtering to know whether it is needed for this run and then decide whether to build the binary. This defers a major decision of the build graph to late in the process and creates a level of compiler, test harness, and cargo integration we likely don't want. Another way to implement this is for cargo to build the test and then run it in discovery mode, see anything needs the binary, and then build it. This defers the build graph decision even later and still increases the API surfave area of the test harness.

@ArtemGr
Copy link

ArtemGr commented Jul 16, 2024

Skipping the final link could be a workaround also for #3501, particularly with rust-analyzer.cargo.buildScripts.overrideCommand.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-cargo-targets Area: selection and definition of targets (lib, bins, examples, tests, benches) C-feature-request Category: proposal for a feature. Before PR, ping rust-lang/cargo if this is not `Feature accepted` Command-test S-needs-design Status: Needs someone to work further on the design for the feature or fix. NOT YET accepted.
Projects
Status: No status
Development

No branches or pull requests

5 participants