Skip to content

Respect NO_COLOR and CLICOLOR_FORCE in libtest #139864

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

Closed
wants to merge 1 commit into from

Conversation

SabrinaJewson
Copy link
Contributor

@rustbot
Copy link
Collaborator

rustbot commented Apr 15, 2025

r? @tgross35

rustbot has assigned @tgross35.
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 Apr 15, 2025
@jieyouxu jieyouxu added A-libtest Area: `#[test]` / the `test` library T-testing-devex Relevant to the testing devex team (testing DX), which will review and decide on the PR/issue. labels Apr 15, 2025
@tgross35
Copy link
Contributor

I'm not sure who is best to make a decision on this since it is user-facing. Maybe @rust-lang/testing-devex?

@epage
Copy link
Contributor

epage commented Apr 15, 2025

Speaking of myself on testing-devex interests

Overall, we've been exploring shifting of responsibilities from libtest to its callers. One of the reasons for this is reducing the expected behavior custom test harnesses need to implement. rust-lang/cargo#1983 is the issue for Cargo being in charge of telling libtest when it should use colors.

This "spec" is unversioned, without a changelog, and has undergone breaking changes in the past. Technically, Cargo supports these but that is an implementation side effect and is not specified behavior within Cargo.

@tgross35 tgross35 assigned epage and unassigned tgross35 Apr 26, 2025
@tgross35
Copy link
Contributor

@epage I'm going to assign this to you since you have a much better picture of how this should interact.

@epage
Copy link
Contributor

epage commented May 1, 2025

We discussed this within testing-devex (zulip. We agree that policy like this should belong in the test runner rather than in test harnesses.

Cargo's control over this is being tracked in rust-lang/cargo#1983. After finding I found out that cargo does some argument forwarding (rust-lang/cargo#15465), I'm going to put an update on the color issue exploring us doing a similar short term hack until we have better defined interactions. So there might be an opportunity for you to improve the state of things in that issue.

btw I just found some past discussion on the topic of this feature at #27867.

@epage epage closed this May 1, 2025
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-testing-devex Relevant to the testing devex team (testing DX), which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants