Let user add directories for auto-discovering tests/bins/...

[LukasKalbertodt]

^{Please excuse if this has been asked before -- I couldn't find anything.}

Hi there!

Cargo auto-discovers certain things: integration tests, binaries, benchmarks and examples. For each of those categories, Cargo looks in a specific folder (tests/, ...). Thanks to #5330 we can now disable or enable auto-discovery explicitly.

I think it would be really useful if the user can specify custom directories that are used for auto discovery. For example, consider my use case: I am working on a proc-macro and want to make sure that certain programs compile fine with my macro (run-pass) and that other programs will result in a compiler error (compile-fail). compile-fail tests are not supported by Cargo, but at least I can write my own script (with harness=false) that will execute my custom tests. run-pass tests are of course normal integration tests supported by Cargo.

All my compile-fail tests are in tests/compile-fail/. It would be neat if I could place my run-pass tests into tests/run-pass/ ... to have some kind of symmetry. Sadly, as I understand it, it's not currently possible without listing every single run-pass test with its own [[test]] section in Cargo.toml.

To be specific, what I'd like to have is the following. Folder layout:

• Cargo.toml
• src/
• tests/
  • compile-fail/
    • ... a bunch of .rs files
  • run-pass
    • ... a bunch of .rs files
  • compile_fail.rs

The compile_fail.rs script uses all files in compile-fail/ as a single test and executes them accordingly with my own logic.

Cargo.toml:

# ...

[[test]]
name = "compile_fail"
path = "tests/compile_fail.rs"
harness = false

[[test]]
name = "run-pass"
path = "tests/run-pass/*"    # <-- note the wildcard!

---

The only way to do this right now is:

• Put all run-pass tests into tests/ directly which leads to asymmetry
• List all run-pass tests in Cargo.toml as a seperate [[test]] section
• Or write my own test-script (like compile-fail)

All possibilities are sub-optimal, I think.

Please note that I don't care about the syntax. I think the wildcard above is pretty natural, but I'm also fine with other solutions, such as auto-discover = "tests/run-pass/".

---

So....

• Has this been discussed before?
• What do others think about this idea?
• Are there plans to do this?

Thanks!

[alexcrichton]

This seems plausible to me! I don't believe we've had previous requests to change the directories for auto-inference, but I don't see why not!

Recently we added keys like autotests = false to disable automatic inference of test cases, and that seems like we could possibly reuse it for your use case? Something like autotests = 'my-test-dir' could look in a different directory for tests, and something like autotests = ['tests/run-pass', 'tests/compile-fail'] could search multiple dirs perhaps?

[LukasKalbertodt]

@alexcrichton Sounds great! I wasn't sure whether we could have different types for one key (bool vs. string vs. array). But if that's possible, that would be great! Then I'd suggest that autotests might have the following values:

• false (equivalent to [])
• true (equivalent to ['tests/'])
• Array of strings

How would we go about implementing this? Does this need an RFC? How are "unstable" things handled in Cargo? Would it need a -Z custom-auto-paths flag? I could certainly try implementing this!

[alexcrichton]

Seems plausible to me!

For implementation that can basically happen at any time, so long as it's a nightly feature. We've got a system for handling unstable nightly-only feautres in Cargo (similar to rustc) and the UI would basically require something like:

# at the top of Cargo.toml
cargo-features = ['this-features-name']

[package]
# ...
autotests = [...]

[LukasKalbertodt]

@alexcrichton I have a question. So I started working on this and already spotted the place where most changes are necessary: src/cargo/util/toml/* in particular targets.rs. I tried to understand what this file is doing ... which wasn't trivial thanks to the almost complete lack of comments. So I thought: when I have to invest the time to understand all of this anyway, I can write down my knowledge in form of doc-comments for the functions in targets.rs. That's already mostly done.

But now after having a rough understanding of the module, I feel like it could use some cleanup. And I don't mean small things, but basically a rewrite of this module. Why? I think the whole code can be significantly shortened and made faster. For example, right now, many functions return Vecs that are only used to extend another Vec from it. In many places things are cloned to make the borrow checker happy.

So my question is: what is the policy on cleanup/partial rewrite PRs? So of course, cleanup commits are annoying for files that are frequently changed by many people. This targets.rs file is not changed that often. Furthermore, rewrites can of course introduce new bugs that were "fixed" before. But of course, shorter and faster code is also good. So yeah, that's my question: would you accept such a PR? Is this something you'd like to get? Or should I rather just concentrate on implementing the feature I requested?

(if I were to rewrite that module, I would of course write a lot of documentation/comments to clarify things)

[alexcrichton]

Fixes, cleanup, and documentation are all more than welcome! I know @matklad may have some thoughts about this module, but IIRC it's historically a relatively unloved module that would definitely appreciate some love

[matklad]

Yes, we love rewrites for readability! I would be especially thrilled if the targets.rs module could be simplified: it's a shame to spend 700 lines of code on something which in theory should be relatively uninvolved. The major reason for this complexity is backwards compatibility: Cargo used to accept pretty random things as crates, and the code goes to the great lengths to preserve that. This is something to keep an eye on during rewrite; luckily, we have a pretty good test coverage of weird cases cases in this area.

[LukasKalbertodt]

Two things. First, my work on this will be a bit delayed. I have to get other stuff finished before working on this. Edit: I won't work on this anytime soon, so please grab this issue if someone is interested!

Secondly: I initially liked the autotests = ["path/to/"] approach, but I noticed that it could be more useful/powerful. In particular, consider this:

[[test]]
name = "foo"
path = "tests/foo/*"

[[test]]
name = "bar"
path = "tests/bar/*"
harness = false

So if we instead let the user add the path to all tests in a [[test]] section, other configurations (like harness) can be set for all tests in that directory. I think that would actually be useful.

[alecmocatta]

A project I'm working on has dozens of tests that need to be run without the harness. Currently they're exhaustively listed with harness = false in the Cargo.toml, which is less than ideal due to the added need to keep them matched up.

So if we instead let the user add the path to all tests in a [[test]] section, other configurations (like harness) can be set for all tests in that directory. I think that would actually be useful.

This would address my use case certainly.

As I see it, the expansion to Cargo.toml [[test]] section definition is:

• No name as this typically doesn't make sense when there'll be multiple names inferred from the multiple resolved paths (all test targets must have unique names);
• A path that can have limited glob functionality.

The name could be allowed when if the glob path resolves to exactly one location?

The logic to special-case ./src/bin, ./examples, ./tests, etc could presumably be moved atop or merged with the work to achieve the above.

The other option that would also address my use case is adding another key at the [package] level (i.e. alongside autotests) that makes the harness opt-in rather than the default opt-out. This is (like autotests IMO) a bit inelegant however.

[alexcrichton]

@LukasKalbertodt I don't think we have a great way to interpret multiple paths per test though, right now each test in target is a crate (aka one rustc invocation), but multiple paths necessitate multiple crates?

@alecmocatta your use case definitely makes sense! We may be able to do something like @LukasKalbertodt is mentioning though one way or another to solve that? I'm not entirely sure what it might look like though

[mimoo]

This would be great for bins too, right now automatic discovery is only available for src/bin

[dansanders]

This would be great for bins too, right now automatic discovery is only available for src/bin

To provide a concrete example, I am writing Project Euler solutions and as a result have hundreds of small bins. For organizational reasons I'd prefer they were not in a flat tree, and for stylistic reasons I'd rather they were not in src/bin.

In my case a glob could be effective, it would be something like ???/???/*.rs.

For now I have a small script to generate Cargo.toml, but it would be more convenient to have them be discovered.

[epage]

For the original project, one workaround isn't mentioned: have a run-pass/main.rs that mods all of the test files. This would improve consistency in making the two test styles a single binary.

[epage]

For making it easier to configure a bunch of tests, we have another issue about setting defaults for crate kinds (bin, test, etc).

For a lot of bins, I suspect "cargo script", maybe combined with workspace support, would be a better fit.

For the original request here, it doesn't sound too bad. However, we've had related requests that mostly exist to have projects deviate from the standard (#2725, #12672) and we've closed those due to the challenges that a lack of consistency would cause end-users contributing. Since we closed those, I'm going to go ahead and close this. If there is a reason we should re-evaluate, let us know!