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

cargo test --no-run should print paths to stdout #12932

Closed
juliusl opened this issue Nov 8, 2023 · 8 comments
Closed

cargo test --no-run should print paths to stdout #12932

juliusl opened this issue Nov 8, 2023 · 8 comments
Labels
C-feature-request Category: proposal for a feature. Before PR, ping rust-lang/cargo if this is not `Feature accepted` Command-test S-triage Status: This issue is waiting on initial triage.

Comments

@juliusl
Copy link

juliusl commented Nov 8, 2023

Problem

#10346 allowed printing the executable path from cargo test --no-run. It works however it prints to stderr and doesn't print the paths to stdout.

Proposed Solution

Print just the paths to stdout.

Notes

You would use --no-run for example if you need to execute a cross-arch binary. For example, qemu-aarch64 <path-to-binary>.
With this change it would be possible to do something like this.

cargo test --target aarch64-unknown-linux-gnu --no-run | xargs -L1 qemu-aarch64

Today the command looks like this,

cargo test --target aarch64-unknown-linux-gnu --no-run --message-format=json | jq  -r 'select(.reason=="compiler-artifact") | .executable | select(. != null)' | xargs -L1 qemu-aarch64
@juliusl juliusl added C-feature-request Category: proposal for a feature. Before PR, ping rust-lang/cargo if this is not `Feature accepted` S-triage Status: This issue is waiting on initial triage. labels Nov 8, 2023
@epage
Copy link
Contributor

epage commented Nov 8, 2023

Sounds like what you are wanting is for cargo test --no-run to provide machine-readable output of the test executables.

Looking over #9957, it seems that our intended way for you to do that is with cargo test --no-run --message-format=json, for example cargo test --no-run --message-format=json | jq 'select(.reason=="compiler-artifact") | .executable'.

@juliusl
Copy link
Author

juliusl commented Nov 8, 2023

@epage I updated my post as soon as you posted, but yes I'm aware of that solution. When I followed the original issue, they were also aware of this but chose to continue to output the paths. I think this was the right call but it feels like the current solution just added logging instead of addressing the original issue.

@weihanglo
Copy link
Member

While I understand the convenience of just reading stuff from stdout, this kind of approach is usually a bit too implicit. See #7280 about cargo vendor which prints to stdout for needed configurations but people are confused.

To me, I would prefer a more formal machine-readable format, like what epage recommended.

@epage
Copy link
Contributor

epage commented Nov 8, 2023

When I followed the original issue, they were also aware of this but chose to continue to output the paths

The original issue was targeted at providing the paths for a user which the PR resolved.

If we blindly printed paths, that would resolve this for you but would likely be confusing to others without additional context. It would likely need to be opt-in. This is what we have --message-format for. We prefer to have a more structured form for programmatic use (json) as the needs can vary more than "output one specific field per line". In your case, the field feels obvious for which field but that doesn't help with others. The "per line" can also be a problem which leads to people also wanting null-terminated delimiters, etc.

@juliusl
Copy link
Author

juliusl commented Nov 8, 2023

@weihanglo I feel that comparison is a bit of a different situation. In that case cargo vendor path is printing out an entire config.toml. (But I actually kind of like this and I personally don't think it's that confusing)

What I'm suggesting is for the command output to behave more like standard cli tools. For example, awk sed grep tar were designed around this type of behavior.

@epage Sorry, I wasn't implying that the Issue wasn't resolved, it was more after following the problem my expectations didn't match the solution. The opt-in is reasonable, even if it was just --message-format path-list I think that would be really helpful.

@epage
Copy link
Contributor

epage commented Nov 8, 2023

Sorry, I wasn't implying that the Issue wasn't resolved, it was more after following the problem my expectations didn't match the solution.

That was what I was trying to clarify. We had an existing solution for programmatic usage. The problem that needed solving was for human usage. The solution fits that quite well. The root of this is not with that issue or its solution but that you are questioning the existing programmatic usage.

. The opt-in is reasonable, even if it was just --message-format path-list I think that would be really helpful.

I mentioned that only in the context that it might not be as reasonable to provide.

In maintaining software, we have to balance user requests with the ability to continue to deliver on user requests. The more we add to cargo,

  • The more that needs documenting, the less people can find what they want, and the less of the provided features they use and the bigger the support burden in leading people to those features
  • The more we have to maintains and make sure works through all cases, slowing down overall progress.

@juliusl
Copy link
Author

juliusl commented Nov 8, 2023

@epage Okay sounds reasonable, I'll close the issue then.

@juliusl juliusl closed this as completed Nov 8, 2023
@ehuss
Copy link
Contributor

ehuss commented Nov 9, 2023

I thought I would mention, since I didn't see anyone reference it, is that the intended method for doing things like qemu for testing is to use a target runner.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-feature-request Category: proposal for a feature. Before PR, ping rust-lang/cargo if this is not `Feature accepted` Command-test S-triage Status: This issue is waiting on initial triage.
Projects
Development

No branches or pull requests

4 participants