cargo-mutants icon indicating copy to clipboard operation
cargo-mutants copied to clipboard

Cargo mutants is not executed in workspace members

Open dmoka opened this issue 1 year ago • 2 comments

Description

cargo mutants command does not generate mutants for workspace members on a workspace project. Let's say I have a workspace project which contains 2 binary and 1 lib cargo crates. When I execute the cargo mutants on the root, there are no mutants generated, so the workspace member crates are not included in the generation of the mutants.

I prepared a GitHub repo for reproducing the problem:

Reproduction

Prerequisite

Clone the following workspace test project test-repo-for-cargo-mutants

Steps:

  • Go to the root folder of the cloned repo
  • Execute cargo mutants

Expected behavior:

Mutants are generated for all the projects in the workspace.

Note: I would expect this to happen because when for example I execute cargo test, then all the tests of workspace member crates are executed. The same for cargo build.

Actual behavior:

No mutants are generated

==PS== Anyway, awesome job with this crate, thank you!

dmoka avatar Jul 25 '22 17:07 dmoka

It seems the behavior we want is:

  • [x] Copy the whole workspace -- this should be already done
  • [x] List the contained packages out of cargo metadata
  • [x] Rewrite paths in all Cargo.toml in all packages
  • [x] Arguably in doing rewrites we should not rewrite relative paths inside the build directory, although perhaps it doesn't make a difference if we mutate one package at a time? This needs to take account of the position of the Cargo.toml within the overall sourcedir: e.g. a/Cargo.toml can have a path of ../b and it should not be made absolute.
  • [x] Discover and test all mutants in each contained package

One interesting question is whether, when we mutate a file in one package, we should run tests in all package or only in the mutated package. The first is probably typically much faster, and I'd guess normally each package should itself include enough tests to catch everything. However it seems possible people would have public-interface tests in another package...?

Assuming we want to test only the mutated package:

  • [x] Run cargo test telling it the name of the package to test

sourcefrog avatar Jul 28 '22 10:07 sourcefrog

~~As a workaround, you can run cargo mutants on each of the packages consecutively, and the previous fix will adjust the paths to find related crates. At least, this seems to work OK in the test repo.~~

Actually this doesn't work in many workspaces.

sourcefrog avatar Aug 01 '22 00:08 sourcefrog

Hey Martin!

Thanks for the update and the detailed plan! I am back from vacation so I can react only now.

The first is probably typically much faster

I thought that running only the package tests is faster than running all the package tests. But please correct me if I am wrong.

One interesting question is whether, when we mutate a file in one package, we should run tests in all package or only in the mutated package. The first is probably typically much faster, and I'd guess normally each package should itself include enough tests to catch everything. However it seems possible people would have public-interface tests in another package...?

This is a challenging one! Normally yes, each package should contain the corresponding tests with good coverage. Based on my experience, and also according to the official Rust book the integration and public API tests are placed under the tests directory, so by running only the package tests would cover most of the cases for the given package.

On the other hand, personally I have seen separate test packages (so with new Cargo.toml) only for pure integration tests with real dependencies. So such a thing can easily happen. But such tests can take much longer and I have a worry that by running them for each mutant would significantly increase the cargo mutants execution time.

Maybe we could start simple and exclude such integration tests for now then in the future we can see how to include it if really necessary? What do you think?

dmoka avatar Aug 05 '22 07:08 dmoka

Yep, I think the default should be to test just the affected package and then we could potentially add an option to run all tests.

sourcefrog avatar Aug 05 '22 14:08 sourcefrog

Actually this seems pretty important because many larger (or even medium size) projects do use workspaces in a way that cargo-mutants doesn't handle very well.

https://github.com/sourcefrog/cargo-mutants/wiki/Trials#apollo-rs shows one where at least it fails cleanly: Error: directory has no root package. But in some cases the packages won't build or test properly.

sourcefrog avatar Aug 06 '22 15:08 sourcefrog