cargo package could run tests in the packaged tree

[sourcefrog]

Problem

Today, cargo package has an option (by default on) to run a build in the packaged tree, to increase confidence that the package will build when downloaded.

Some users would like to check that the tests also pass in the package crate.

Distro packaging of Rust tools

In particular, Debian and probably other distributions that repackage Rust software would like to:

1. start with a crate tarball, not a git checkout
2. run the tests to increase confidence that their build is correct

However in cargo-mutants we found that the tests didn't pass in a tree unpacked from a cargo crate, because some testdata files were excluded. (See https://sources.debian.org/patches/rust-cargo-mutants/23.10.0-1/disable-tests-missing-testdata.patch/, and cc @jelmer.)

Archiving packages

Arguably there's also value in crates.io archiving a copy of the tree that's sufficiently complete that someone could resume development from that crate if the VCS repository was lost, and this will be much more feasible if the tests are known to pass. For widely cloned git repos this might seem unlikely, but there might be some crates where the released source is open but the VCS is not.

Proposed Solution

If this is acceptable in principle I could send a PR that adds cargo package --test (and cargo publish --test) that will run the tests in a copy of the packaged tree. The option would be off by default.

Counter-arguments

Package size

One conceivable argument against this is that including all the tests and testdata to make the tests pass will make the package larger, which will increase bandwidth consumption for both crates.io and users. Probably a large majority of clients that download packages do not run or even build their tests, as they only want to build the library or binary.

Perhaps some crates have very large testdata directories that today are excluded, but adding this option might motivate their authors to include them in the package.

I think that might happen at the margin but I don't think it's a conclusive reason not to do it:

• There's already a cap on package size, and this would not change it.
• cargo and crates.io as far as I know doesn't make any attempt to strip packages down to only what is needed to build the binary/library: many crates will include docs and other files that aren't strictly needed for the build.
• Probably many packages do already include most or all of their tests and testdata; this would just help packages that use the option make sure that the shipped files are actually usable to achieve passing tests.
• I would guess that for many crates the integration tests and testdata are not going to greatly increase the size of the package.

Not strictly necessary

Another conceivable and reasonable argument against adding this feature is that it's not that hard to unpack the tarball and run the tests yourself, so it's not worth adding the complexity of another feature to cargo.

Against this I think there are some reasons to have it built in:

• Running the build seems like a good strong precedent.
• Finding the tarball, extracting it, and building it is not quite trivial to script on Windows CI, at least for me.
• Some package maintainers might want to be confident that all uploaded crate packages will pass their included tests: having it built in to the tool helps achieve this consistently in the same way that cargo checking the build is useful.
• It saves every package maintainer working out how to script it.
• Perhaps it helps advance a norm that many packages should include their tests in crate packages.

[epage]

Arguably there's also value in crates.io archiving a copy of the tree that's sufficiently complete that someone could resume development from that crate if the VCS repository was lost, and this will be much more feasible if the tests are known to pass. For widely cloned git repos this might seem unlikely, but there might be some crates where the released source is open but the VCS is not.

Nothing forces a crate to include tests. In fact, there are valid reasons (circular dependencies) for people to setup their package for their dev dependencies to be stripped, making the package untestable.

There is likely other content that would be relevant for restarting a project that would be missing and I don't think a .crate file should be setup for that intent. This would push us towards included way more in .crate files than is reasonable.

run the tests to increase confidence that their build is correct

Where do we draw the line on validating a .crate? We currently do a check on one feature combination. This is asking for tests. Do we also need to check other things that are normally expected to run in CI?

If this is acceptable in principle I could send a PR that adds cargo package --test (and cargo publish --test) that will run the tests in a copy of the packaged tree. The option would be off by default.

When considering CLI flags, I ask the questions

• How frequently will people need to run this?
• Is this something people will want enabled on every run?
• Does the operation have side effects where in leaving off a flag can't fully be recovered?

If we do this, I wonder if this would instead be better served with by a publish.test = <bool> config that controls whether the verify step runs tests.

Package size

In #13491, someone suggests we split .crate files much like distributions have -dev and -dbg packages.

[ehuss]

The cargo team discussed this today and overall we felt like this would need more design work and exploration. There are two orthogonal things to consider, should it be possible to run tests from a packaged crate, and separately how to run those tests. Another consideration is that "running tests" for some projects can be a relatively complex thing. One option is to have a config option for opting-in to running tests after packaging. It's not really clear how that opt-in should look exactly.

I would encourage experimenting with a plugin to move forward. We have been trying to consider how high- or low-level commands like cargo package should be. For example, there are higher-level extensions like cargo release which provide more functionality.

[sourcefrog]

That all makes sense, thanks. I'm content for you to close this unless you especially want to keep it.

[epage]

For now, I think its worth keeping open. When discussing #14941, the question came up if the verify step is a packaging sanity check or a baseline quality check. As a packaging check, we likely don't need to run tests. As a quality check, opting into tests and maybe ratcheting up on that could be useful. I plan to write up more on that discussion and do some investigation when I'm back at my computer.

[epage]

Like with #14941, a big question is what the role of the verify step should be

• Sanity check packaging (e.g. were all needed files included)
• Last-ditch baseline quality check

(see #14941 for more details on)

So the first question: is running tests on a .crate file essential?

• If so, then a verify-test mode would be needed for the sanity check as test data could be missing
  • cargo add sabotages this effort by not adding version to dev-dependencies to avoid the risk of cycles
  • People in the community are pushing for crates to remove test data to reduce .crate size, e.g. Released versions contain test data BurntSushi/bstr#200
• Otherwise, this issue is just about a last-ditch baseline quality check.

For a last-ditch baseline quality check, how much is verification is sufficient?

• Adding tests would make sure nothing was broken (if tests can be run, see above)
• Crates can be platform-dependent, --cfg dependent, --feature dependent, --profile dependent,
• Other verification could be desired like clippy or miri

So the second question is how much verification is "enough"?

• If just tests, then we could make it an enumeration of check, build, `test, built-in cargo operations
• Anything more would likely need to be a full hook which I at least don't have an appetite for.

Now the third question is whether cargo package and/or cargo publish are plumbing (meant to be used by another tool) or porcelain (end-user focused)? cargo publish is missing a lot of features for standard release processes (tagging, updating of versions in other locations, etc). However, for new users who aren't yet ready for a full process, maybe its sufficient?

If configuring verification is focused on plumbing, then it could be in config (which includes env variables and --config) or a CLI flag (--verify test). The porcelain wrapper can control the policy in its preferred way.

If configuration verification is focused on porcelain, then any transient configuration can easily be forgotten between invocations which would favor putting it in a config file or the manifest.

[epage]

A reason that would limit people implementing this on their own is that we don't expose the details necessary for them to mix .crate files with crates.io, like the verify step does. That relies on an internal-only overlay Source that we don't expose to avoid dependency confusion attacks. Someone would have to vendor all of their dependencies to implement this.