Rename test option --nocapture to --no-capture

[zaeleus]

Rust's built-in unit testing framework supports not capturing the output
of tests. The current flag --nocapture is inconsistent with the other
long name options, i.e, they are written in spinal-case (a.k.a.
kebab-case).

Codegen options no-prepopulate-passes, no-vectorize-loops,
no-vectorize-slp, no-integrated-as, and no-redzone are hyphenated
between "no" and the name. Cargo has --no-default-features and
--no-deps.

This change renames the test option --nocapture to --no-capture to
be consistent with Rust's naming of long name options. The ENV variable
RUST_TEST_NOCAPTURE is also renamed to RUST_TEST_NO_CAPTURE.

Because this is a public facing option, it is a [breaking-change].

[rust-highfive]

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @brson (or someone else) soon.

If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. The way Github handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes.

Please see CONTRIBUTING.md for more information.

[pnkfelix]

i would suggest that we start by adding --no-capture and deprecating --nocapture, but not removing the latter outright. (It can be a real pain for clients to migrate infrastructure atomically.)

[zaeleus]

@pnkfelix, that's doable.

Is there a standard way to convey runtime warnings to the user? I would think to use log::warn!, but it does require RUST_LOG to be set, which may make the deprecation be non-obvious if never used. Otherwise, a message can be written directly to stderr.

[pnkfelix]

@zaeleus I don't think you'd have access to log::warn from the code generated via --test, since the code being compiled does not necessarily have extern crate log; set up.

I would not object to issuing a warning to stderr; to me, that seems a better first step than just removing --nocapture outright. (But others might disagree; an unexpected message to stderr could be just as disruptive as removing --nocapture entirely would have been...)

[zaeleus]

Using the old option names now emits a warning to stderr telling the user to use the new ones. This makes the changes in this patch backward compatible.

[pnkfelix]

The PR looks good to me now.

@brson : I'd like for at least one other team member to weigh in about:

1. whether we should indeed do this, and
2. assuming we should do it, should we also cherry-pick it back to the beta channel?

[bors]

☔ The latest upstream changes (presumably #25048) made this pull request unmergeable. Please resolve the merge conflicts.

[huonw]

I'm in favour (of both 1 and 2).

Maybe @alexcrichton has an opinion?

[alexcrichton]

This is quite a commonly used option when running tests (especially through Cargo), so I do think that this change will have quite an impact, and I totally agree with @pnkfelix that deprecation here is the best way forward. I'm not so certain about the WARN: {} syntax here, but we don't have any prior work here outside of compiler diagnostics.

I would personally feel it's a little too late to make a change like this, but I agree that if we do this it should make its way into the beta channel. If this does go in it would also be nice for the --help of test binaries to not show --nocapture at all by default.

[brson]

This slipped through the cracks.

[brson]

I think let's defer to @alexcrichton's opinion that it's too late for this. At this point it has to have a long deprecation path so it hardly matters if we do it now, then or never.

[brson]

This was discussed in last week's minutes and the outcome appeared to be to keep this, but no rush needed.

Maybe the only remaining step here is update the --help output.

[pnkfelix]

@brson my memory of how that discussion concluded was that we do need to establish an evolutionary (or "migration" / "deprecation") path for changes like this, but since we do not yet have a good roadmap for how such changes will be handled, that we would hold off on landing this change until such a roadmap had been established.

Perhaps this is a sign that an issue should be opened on the RFC's repo (for establishing the migration path) and then this PR closed (but with the expectation that it will be re-opened after the path has been established) ?

[bors]

☔ The latest upstream changes (presumably #26192) made this pull request unmergeable. Please resolve the merge conflicts.

[alexcrichton]

I'm going to close this out of inactivity now, and at this point it definitely seems like there's not a lot of steam behind the rename and it may not be worth the deprecation path.