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

streamline bin injection into integration test #917

Closed
Gankra opened this issue Jul 27, 2023 · 4 comments
Closed

streamline bin injection into integration test #917

Gankra opened this issue Jul 27, 2023 · 4 comments

Comments

@Gankra
Copy link

Gankra commented Jul 27, 2023

A frequent ask for cargo-dist is automated testing of shippable builds. One of the most obvious ways to do that would be to inject the shippable from e.g. your Github Release into your Cargo binary integration tests (/tests/), which already are injecting binaries for you with CARGO_BIN_EXE_my-app (and NEXTEST_BIN_EXE_my-app).

It seems like the machinery is basically there with nextest's features for reusing builds, but all the docs seem focused on reusing builds of the tests and not builds of the apps, so it's not clear to me what needs to be done to make it work.

Ideally I could have a workflow like:

  • fetch+unpack my-app's binaries for the current platform
  • cargo nextest run --archive-file=... -- my-app:/path/to/my-app.exe my-other-app:/path/to/my-other-app.exe

(paths could also be injected with env-vars or json files, I don't terribly care how it works as long as it's easy/reliable to do on all platforms. ideally i wouldn't have to edit the archive-file to inject the paths, because the paths may be difficult to predict before runtime.)

Also it would of course be ideal if this also worked with all the great cross-compilation-prebuild stuff you have.

@sunshowers
Copy link
Member

sunshowers commented Jul 27, 2023

Some notes for my future self:

  1. I don't think this necessarily needs to be tied to archive/reuse work, because you could in principle also provide a directory to redirect NEXTEST_BIN_EXE_<my-app> to.
  2. There is some orchestration that needs to be done here. For example, I suspect cargo-dist makes separate archives for each binary. It's a bit hard to build all the orchestration into nextest itself because it doesn't control both ends of the process.
  3. Note that CARGO_BIN_EXE_ is not set at run time, only at build time. NEXTEST_BIN_EXE_ is set at runtime. This means that tests will have to use NEXTEST_BIN_EXE_ at runtime.
  4. The integration tests will have to support nextest. This can't really be backported to cargo test in an easy fashion.
  5. Does this bring up use cases for exposing the NEXTEST_BIN_EXE_ variables to more kinds of tests via a config option? Need to think about this. One challenge is that in the regular flow, binaries might not actually be built if there's no corresponding integration test or benchmark. Maybe defer this to the future.
  6. cargo-dist could set its own CARGO_DIST_EXE_ environment variables. The advantage of building this into nextest is that any tests containing NEXTEST_BIN_EXE_ would work transparently with cargo-dist. The disadvantage is that projects would have to switch to nextest.

@sunshowers
Copy link
Member

sunshowers commented Jul 27, 2023

One use case that we need to think about is integration tests that use multiple binaries:

  • Imagine a multi-crate workspace with two crates, binary-a and binary-b.
  • You want to write an integration test that runs both binary-a and binary-b.
  • You pretty much have to make it a third crate that's test-only, and run cargo build from within the tests for that crate.

Not supporting this would diminish the value of nextest and cargo-dist, I think.

An alternative might be that nextest lets you say: if you're building a particular crate's tests, also build this other binary -- then set NEXTEST_BIN_EXE_ for those tests. However, nextest doesn't currently do this, and supporting it can be rather delicate (eg what features to build the binary with? What if you want to use different set of flags/debug vs release/etc)?

Another option is to support this in setup scripts (#683). That requires even more design work.

@Gankra
Copy link
Author

Gankra commented Jul 27, 2023

After some discussion it became clear that point 6 was especially salient. Users would have needed to modify their code to conditionally check for NEXTEST_BIN_EXE_* anyway, so we can just do the same thing with our own var:

let my_app = std::env::var("OVERRIDE_CARGO_BIN_EXE_my-app")
  .unwrap_or_else(|_| env!("CARGO_BIN_EXE_my-app").to_owned());

With this you can cargo test like normal, but then have a preprocessing step that sets OVERRIDE_CARGO_BIN_EXE_my-app before running cargo test to inject prebuilt bins!

@Gankra
Copy link
Author

Gankra commented Jul 27, 2023

I'm going to close this as resolved for now, but I'll reopen if I see more room for tighter integration.

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

No branches or pull requests

2 participants