-
Notifications
You must be signed in to change notification settings - Fork 13k
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
fix(libtest): Deprecate '--logfile' #134283
base: master
Are you sure you want to change the base?
Conversation
r? @Noratrieb rustbot has assigned @Noratrieb. Use |
This comment has been minimized.
This comment has been minimized.
r? libs-api |
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 (rust-lang@5cc050b, rust-lang#2127) and the history gives the impression this more of slipped through rather than being an intended feature (see also rust-lang#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 rust-lang#3455](https://rust-lang.github.io/rfcs/3455-t-test.html), this is still subject to final approval from T-libs-api.
eedd0a3
to
450aaa4
Compare
Some changes occurred in src/tools/cargo cc @ehuss |
This was discussed in the libs-api call today. People on the call were a bit unclear about the context, the motivation, and the broader plan, so it would help if those things might be clarified. In particular:
Thanks for making the proposal here and for filling in the other details that might help the team to build the context needed to make a decision on this. |
Unsure if it was a formatting / presentation issue but most answers to these questions were in the PR description. Not all answers were included, in part because its hard to predict what all may be relevant and in part because this is the first lib This post re-formats the PR description around the questions and fills in some of the missing context.
It will tee test output to a file using an undocumented, custom textual format Example of a test run
Example when passing
This was added Pre-1.0, see #2127 For the motivation, quoting from #2127
When ehuss was writing end-user documentation for lib
(while the format wasn't specified in that PR, the existence of the flag was)
Back at the time of #82350, ehuss was not aware of people using it. T-testing-devex is also not aware of people using it todqay. It is difficult to evaluate how widely it is used as
Looking over those, I'm not seeing any situation where the person is successful with it, usually from #105424, meaning those posts likely do not represent someone actively using it. The only desired use cases were
For this teeing use case, the Cargo team has rejected it for now in rust-lang/cargo#14555.
The current format is undocumented, bespoke format with limited data. There are challenges with evolving this format or switching to an alternative. These could be worked through with enough effort but the value seems low while this puts more of a burden on custom test harnesses to support and is a responsibility that would generally belong in a test runner (e.g. By deprecating this,
A full removal was not broached as that would be a breaking change. We had not considered a mitigation strategy but I'm unsure if it is worth coming up with one vs freezing the code. The rest was covered in the previous answer but I'll repeat for ease of discover The larger plan is to shift the focus of feature development in lib |
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 theimpression 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, this is still subject to final approval from T-libs-api.
Closes rust-lang/testing-devex-team#9