Skip to content

Commit

Permalink
fix(libtest): Deprecate '--logfile'
Browse files Browse the repository at this point in the history
rust-lang/testing-devex-team#9 proposed changing the behavior of `--logfile`.
The given reasons were:

(1) Bazel can't programmatically process stdout.  This seems like a
limitation in Bazel and we recommend focusing on that.  If we look at
the wider Rust ecosystem, Rustc and Cargo don't support any such
mechanism and the Cargo team rejected having one.  Expecting this in
libtest when its not supported elsewhere seems too specialized.

(2) Tests that leak out non-programmatic output that intermixes with
programmatic output.  We acknowledge this is a problem to be evaluated
but we need to make sure we are stepping back and gathering
requirements, rather than assuming `--logfile` will fit the needs.

Independent of the motive, regarding using or changing  `--logfile`

(1) Most ways to do it would be a breaking change, like if we respect
any stable `--format`.  As suggested above, we could specialize this to
new `--format` values but that would be confusing for some values to
apply but not others.

(2) Other ways of solving this add new features to lib`test` when we are
instead wanting to limit the feature set it has to minimize the
compatibility surface that has to be maintained and the burden it would
put on third party harnesses which are a focus area.  Examples include
`--format compact` or a `--log-format` flag

(3) The existence of `--logfile` dates back quite a ways
(5cc050b,
#2127) and the history gives the
impression this more of slipped through rather than being an intended
feature (see also
#82350 (comment)).
Deprecation would better match to how it has been treated.
By deprecating this, we do not expect custom test harnesses
(rust-lang/testing-devex-team#2) to implement this.

T-testing-devex held an FCP for deprecating in rust-lang/testing-devex-team#9
though according to
[RFC #3455](https://rust-lang.github.io/rfcs/3455-t-test.html),
this is still subject to final approval from T-libs-api.
  • Loading branch information
epage committed Jan 7, 2025
1 parent fb546ee commit 502f647
Show file tree
Hide file tree
Showing 3 changed files with 17 additions and 2 deletions.
8 changes: 6 additions & 2 deletions library/test/src/cli.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
//! Module converting command-line arguments into test configuration.
use std::env;
use std::io::{self, IsTerminal};
use std::io::{self, IsTerminal, Write};
use std::path::PathBuf;

use super::options::{ColorConfig, Options, OutputFormat, RunIgnored};
Expand Down Expand Up @@ -58,7 +58,7 @@ fn optgroups() -> getopts::Options {
.optflag("", "bench", "Run benchmarks instead of tests")
.optflag("", "list", "List all tests and benchmarks")
.optflag("h", "help", "Display this message")
.optopt("", "logfile", "Write logs to the specified file", "PATH")
.optopt("", "logfile", "Write logs to the specified file (deprecated)", "PATH")
.optflag(
"",
"nocapture",
Expand Down Expand Up @@ -281,6 +281,10 @@ fn parse_opts_impl(matches: getopts::Matches) -> OptRes {

let options = Options::new().display_output(matches.opt_present("show-output"));

if logfile.is_some() && !format.is_programmatic() {
let _ = write!(io::stdout(), "warning: `--logfile` is deprecated");
}

let test_opts = TestOpts {
list,
filters,
Expand Down
9 changes: 9 additions & 0 deletions library/test/src/options.rs
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,15 @@ pub enum OutputFormat {
Junit,
}

impl OutputFormat {
pub(crate) fn is_programmatic(&self) -> bool {
match self {
Self::Pretty | Self::Terse => false,
Self::Json | Self::Junit => true,
}
}
}

/// Whether ignored test should be run or not
#[derive(Copy, Clone, Debug, PartialEq, Eq)]
pub enum RunIgnored {
Expand Down
2 changes: 2 additions & 0 deletions src/doc/rustc/src/tests/index.md
Original file line number Diff line number Diff line change
Expand Up @@ -268,6 +268,8 @@ Controls the format of the output. Valid options:

Writes the results of the tests to the given file.

This option is deprecated.

#### `--report-time`

⚠️ 🚧 This option is [unstable](#unstable-options), and requires the `-Z
Expand Down

0 comments on commit 502f647

Please sign in to comment.