Skip to content

Allow building helper binaries for tests. #4846

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

Closed
wants to merge 1 commit into from

Conversation

fa7ca7
Copy link
Contributor

@fa7ca7 fa7ca7 commented Dec 21, 2017

Binary should be placed in tests/bin directory.
Both cargo build --bins and cargo build --tests build the binary helpers.
The helper can be executed directly from a test (by using
std::process::Command, for example).

Impl for #4356.
Comments and suggestions are welcome

Binary should be placed in `tests/bin` directory.
Both `cargo build --bins` and `cargo build --tests` build the binary helpers.
The helpers can be executed directly from a test (by using
std::process::Command, for example).
@rust-highfive
Copy link

r? @alexcrichton

(rust_highfive has picked a reviewer for you, use r? to override)

@alexcrichton
Copy link
Member

Thanks for the PR! This is a relatively major feature addition, however, so we'll want to take care with this. I think now that Cargo has unstable features we should primarily add it behind an unstable feature first. Other than that I'm not sure this is the best solution for this because how do you, for example build just a binary? (aka cargo build --bin foo). Can these targets be configured via Cargo.toml? If so, how? (etc...)

@fa7ca7
Copy link
Contributor Author

fa7ca7 commented Dec 22, 2017

Good questions! I don't have answers yet, but I see next steps for implementation of this feature now:

  1. Hide feature behind the unstable flag.
  2. Add --test-bin and --test-bins flags.
  3. Handle test binaries separately from regular ones. I suppose adding a new TargetKind::TestBin should arrange everything. Am I on the right way?

Do you think we need a new target for test binaries in Cargo.toml? If so what configuration options should we provide there?

@lukaslueg
Copy link
Contributor

FWIW: Test binaries appear to me as walking and talking like ducks, so let's realize they are ducks packages. There is an endless chain of questions just as the ones above; here is another one: Where do I put my test-binaries to test the binaries that run while the tests are run for the binary that I test? All those questions are already answered by Cargo itself, namely by thinking about them in terms of crates. Packaging test-binaries in path-specific crate(s) and dev-dependency on them, allowing path-specific dev-dependencies in published crates and allowing cargo to put those binaries into known places makes much more sense to me.

@alexcrichton
Copy link
Member

@dethoter unfortunately I'm not sure this is a small enough feature to add in behind an unstable flag, this seems something that's sort of on the order of an RFC in terms of size of feature and size of implementation. Would you be interested in doing so?

@fa7ca7
Copy link
Contributor Author

fa7ca7 commented Jan 2, 2018

Thank you all for your feedback.

@lukaslueg sounds reasonable, but usually, test binaries are quite simple and don't require all settings like binaries do. So for me, much more convenient just to put all the test binaries in tests/bin instead of trying to arrange a mess from paths and Cargo.toml files.

@alexcrichton I think I'm not ready for proposing an RFC now, so just closing this PR for now

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

Successfully merging this pull request may close these issues.

4 participants