JUnit does not respect `CARGO_TARGET_DIR` environment variable.

[marmeladema]

Not a critical issue, but it seems that JUnit support does not respect CARGO_TARGET_DIR environment variable.

With the configuration specified in the documentation:

$ cat .config/nextest.toml 
[profile.ci.junit]
path = "junit.xml"

And then running the tests with:

$ CARGO_TARGET_DIR=/tmp/bug cargo nextest run --profile ci

The directory specified in `| does not contain a nextest directory:

$ ls /tmp/bug/
CACHEDIR.TAG  debug  tmp

However, the target directory in the current directory does:

$ ls target/nextest/ci/junit.xml 
target/nextest/ci/junit.xml

This can be reproduced using rust 1.70.0 installed with rustup and cargo-nextest version 0.9.53

[sunshowers]

Thanks for the report!

Sadly this was an oversight on my part when I first released nextest. It would be hard to fix by default in a backwards-compatible manner since there's already likely code in the wild that depends on it. However:

1. We can add a config option to store nextest's config in $CARGO_TARGET_DIR.
2. We can fix this if and when we do a breaking change to nextest in the future.

[poliorcetics]

The same problem exists for store.dir, which defaults to target/nextest

[janderholm]

An option to honor target-dir would be great. In some CI environments build.target-dir is used to separate the build and source directory completely. The source tree may for example be located somewhere that is not writable, like a network drive.

So I think it's important to not assume anything about how target-dir relates to the workspace directory.

Another potential solution would be to be able to override "store.dir" to another absolute path in runtime with something like a command line argument.

[sunshowers]

I don't expect to work on this any time soon but help would be greatly appreciated. At the very least we need to let people set store.dir to $CARGO_TARGET_DIR/nextest, which means letting people interpolate $CARGO_TARGET_DIR if it's at the beginning of the string.

Once that is done, we also need a plan to switch the default. The only place we actually use store.dir/target/nextest is in JUnit output. So I think a reasonable way forward would be:

1. Produce a warning

If all of the following are true:

1. JUnit output is requested
2. store.dir isn't explicitly set and the default value is used
3. The target dir isn't source-dir/target

Then, produce a warning saying that the location will be changed in 3 months (let's say we release the warning on 2024-01-01, then we should aim to switch the behavior on 2024-04-01).

In the message, explain how to silence the warning (either set it explicitly to target/nextest, or opt in by setting it to $CARGO_TARGET_DIR/nextest).

2. Wait for 3 months

We need to provide sufficient time for people depending on the directory to be notified. 3 months should be enough.

3. Switch the behavior

Switch the default behavior and make a nextest release (I'm happy to do this).

4. Warn for another month

After switching the default behavior, we should keep warning for another time period. Another month or so would be good.

5. Remove the warning

At this point, we've hopefully communicated this to nextest users and they've already switched their workflows.

I'm happy to do the release management, but don't have the time to implement the code. Maybe one of you who have expressed interest can help? @marmeladema @poliorcetics @janderholm

[janderholm]

There's a target_directory field returned by cargo metadata, and a method for this in guppy: https://docs.rs/guppy/latest/guppy/graph/struct.Workspace.html#method.target_directory

Is there a particular reason for using the environment variable $CARGO_TARGET_DIR instead of that? I've done a quick test and replacing workspace_root for that in config_impl.rs seems to work so it seems fairly easy to implement.

I could simply replace $CARGO_TARGET_DIR for whatever this variable is set to in store.dir but I feel that isn't right since it does not have anything to do with the environment variable.

[sunshowers]

Oh yeah, we should definitely use that field (and do use it in some other places, though it might get remapped with build archive/reuse). Maybe $CARGO_TARGET_DIR/nextest is not how it should be specified, and instead, something like

[store]
dir = { in-target-dir = true, path = "nextest" }

or similar. I think specifying it as $CARGO_TARGET_DIR/nextest is okay as well. We don't strictly use the env var as-is but we can communicate this to users.

I think we could go either way on this, I'll let you make a call here.

[sunshowers]

A thing I did realize is that if the target dir is created by us as part of running tests out of an archive, then the temporary directory will typically be deleted at the end of the run. This is almost certainly not what users want, so in that case I think we should make an exception and keep using <workspace-dir>/target/nextest just like today. We'll also want to add an info! message regarding this.

[sunshowers]

I've listed out a general way we can make behavior changes: https://nexte.st/book/stability.html#making-behavior-changes