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

fix(libtest): Deprecate '--logfile' #134283

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

epage
Copy link
Contributor

@epage epage commented Dec 13, 2024

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 libtest 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, this is still subject to final approval from T-libs-api.

Closes rust-lang/testing-devex-team#9

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.
@epage epage added the A-libtest Area: `#[test]` / the `test` library label Dec 13, 2024
@rustbot
Copy link
Collaborator

rustbot commented Dec 13, 2024

r? @Noratrieb

rustbot has assigned @Noratrieb.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Dec 13, 2024
@rust-log-analyzer

This comment has been minimized.

@Noratrieb
Copy link
Member

r? libs-api

@rustbot rustbot added the T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. label Dec 13, 2024
@rustbot rustbot assigned Amanieu and unassigned Noratrieb Dec 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-libtest Area: `#[test]` / the `test` library S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-libs Relevant to the library team, which will review and decide on the PR/issue. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Export machine-readable test results to a file
5 participants