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

Improve compiletest expected/not found formatting #125890

Merged
merged 1 commit into from
Jun 3, 2024

Conversation

Nilstrieb
Copy link
Member

compiletest, oh compiletest, you are truly one of the tools in this repository. You're the omnipresent gatekeeper, ensuring that every new change works, doesn't break the world, and is nice. We thank you for your work, for your tests, for your test runs, for your features that help writing tests, for all the stability and and good you have caused. Without you, Rust wouldn't exist as it does, without you, nothing would work, without you, we would all go insane from having changes break and having to test them all by hand. Thank you, compiletest.

but holy shit i fucking hate your stupid debug output so much i simply cannot take this anymore aaaaaaaaaaaaaaaaaaaaaaaaaaaaaa

By changing a few magic lines in this file called "runtest.rs", we can cause compiletest to emit nicer messages. This is widely regarded as a good thing. We stop wasting vertical space, allowing more errors to be displayed at once. Additionally, we add colors, which make it so much more pretty and gay, both of which are very good and useful.

There's a bit of fuckery needed to get the colors to work. colored checks whether stdout is a terminal. We also print to stdout, so that works well.
But.... for some stupid reason that I absolutely refuse to even attempt to debug, stdout is not a terminal when executing tests in a terminal.
But stderr is >:).
So this just checks whether stderr is a terminal.
If you have a use case where you dump compiletest stdout into a place where colors are not supported while having stderr be a terminal, then I'm sorry for you, but you are gonna get colors and you're gonna like it. Stop it with the usual environment variable, which colored also respects by default.

before (bad, hurts your brain, makes you want to cry)

image

after (good, gay, makes you want to cry)

image

r? jieyouxu said he wants to review the PR

@rustbot rustbot added A-testsuite Area: The testsuite used to check the correctness of rustc S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) labels Jun 2, 2024
@rustbot
Copy link
Collaborator

rustbot commented Jun 2, 2024

Some changes occurred in src/tools/compiletest

cc @jieyouxu

@jyn514
Copy link
Member

jyn514 commented Jun 2, 2024

But.... for some stupid reason that I absolutely refuse to even attempt to debug, stdout is not a terminal when executing tests in a terminal.

bootstrap tells compiletest to use libtest to capture stdout and convert it to JSON so it can render the output itself
image

Copy link
Contributor

@jieyouxu jieyouxu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, this makes it much nicer to read :3

src/tools/compiletest/src/main.rs Outdated Show resolved Hide resolved
src/tools/compiletest/src/runtest.rs Show resolved Hide resolved
Copy link
Contributor

@jieyouxu jieyouxu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, the changes look good, r=me after CI is green.

@rust-log-analyzer

This comment has been minimized.

compiletest, oh compiletest, you are truly one of the tools in this
repository. You're the omnipresent gatekeeper, ensuring that every new
change works, doesn't break the world, and is nice. We thank you for
your work, for your tests, for your test runs, for your features that
help writing tests, for all the stability and and good you have caused.
Without you, Rust wouldn't exist as it does, without you, nothing would
work, without you, we would all go insane from having changes break and
having to test them all by hand. Thank you, compiletest.

but holy shit i fucking hate your stupid debug output so much i simply
cannot take this anymore aaaaaaaaaaaaaaaaaaaaaaaaaaaaaa

By changing a few magic lines in this file called "runtest.rs", we can
cause compiletest to emit nicer messages. This is widely regarded as a
good thing. We stop wasting vertical space, allowing more errors to be
displayed at once. Additionally, we add colors, which make it so much
more pretty *and* gay, both of which are very good and useful.

There's a bit of fuckery needed to get the colors to work. `colored`
checks whether stdout is a terminal. We also print to stdout, so that
works well.
But.... for some stupid reason that I absolutely refuse to even attempt
to debug, stdout is *not* a terminal when executing tests *in a
terminal*.
But stderr is >:).
So this just checks whether stderr is a terminal.
If you have a use case where you dump compiletest stdout into a place
where colors are not supported while having stderr be a terminal, then
I'm sorry for you, but you are gonna get colors and you're gonna like
it. Stop it with the usual environment variable, which `colored` also
respects by default.
@Nilstrieb
Copy link
Member Author

@bors r=jieyouxu

@bors
Copy link
Contributor

bors commented Jun 2, 2024

📌 Commit 3aefc4a has been approved by jieyouxu

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jun 2, 2024
@jieyouxu
Copy link
Contributor

jieyouxu commented Jun 2, 2024

@bors r- (for the more specific VarError::NotPresent check).

@bors bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Jun 2, 2024
@jieyouxu
Copy link
Contributor

jieyouxu commented Jun 2, 2024

@bors r+ rollup

@bors
Copy link
Contributor

bors commented Jun 2, 2024

📌 Commit 3aefc4a has been approved by jieyouxu

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jun 2, 2024
@WaffleLapkin
Copy link
Member

@bors r=jieyouxu

It's not that important to block this clear improvement. Although I do wish we had some function somewhere for consistent behavior of parsing env vars as bools.

@bors
Copy link
Contributor

bors commented Jun 2, 2024

💡 This pull request was already approved, no need to approve it again.

@bors
Copy link
Contributor

bors commented Jun 2, 2024

📌 Commit 3aefc4a has been approved by jieyouxu

It is now in the queue for this repository.

@jieyouxu
Copy link
Contributor

jieyouxu commented Jun 2, 2024

It's not that important to block this clear improvement. Although I do wish we had some function somewhere for consistent behavior of parsing env vars as bools.

This seems like a good general compiletest robustness improvement, so I'll file an issue to track this.
EDIT: filed #125895.

jieyouxu added a commit to jieyouxu/rust that referenced this pull request Jun 2, 2024
Improve compiletest expected/not found formatting

compiletest, oh compiletest, you are truly one of the tools in this repository. You're the omnipresent gatekeeper, ensuring that every new change works, doesn't break the world, and is nice. We thank you for your work, for your tests, for your test runs, for your features that help writing tests, for all the stability and and good you have caused. Without you, Rust wouldn't exist as it does, without you, nothing would work, without you, we would all go insane from having changes break and having to test them all by hand. Thank you, compiletest.

but holy shit i fucking hate your stupid debug output so much i simply cannot take this anymore aaaaaaaaaaaaaaaaaaaaaaaaaaaaaa

By changing a few magic lines in this file called "runtest.rs", we can cause compiletest to emit nicer messages. This is widely regarded as a good thing. We stop wasting vertical space, allowing more errors to be displayed at once. Additionally, we add colors, which make it so much more pretty *and* gay, both of which are very good and useful.

There's a bit of fuckery needed to get the colors to work. `colored` checks whether stdout is a terminal. We also print to stdout, so that works well.
But.... for some stupid reason that I absolutely refuse to even attempt to debug, stdout is *not* a terminal when executing tests *in a terminal*.
But stderr is >:).
So this just checks whether stderr is a terminal.
If you have a use case where you dump compiletest stdout into a place where colors are not supported while having stderr be a terminal, then I'm sorry for you, but you are gonna get colors and you're gonna like it. Stop it with the usual environment variable, which `colored` also respects by default.

### before (bad, hurts your brain, makes you want to cry)
![image](https://github.com/rust-lang/rust/assets/48135649/cbeecb5d-fc25-460b-b192-9808f8fa2079)

## after (good, gay, makes you want to cry)
![image](https://github.com/rust-lang/rust/assets/48135649/a655b220-8841-443e-a825-72a835d56882)

r? jieyouxu said he wants to review the PR
bors added a commit to rust-lang-ci/rust that referenced this pull request Jun 2, 2024
Rollup of 5 pull requests

Successful merges:

 - rust-lang#121062 (Change f32::midpoint to upcast to f64)
 - rust-lang#125808 (Migrate `run-make/c-link-to-rust-dylib` to `rmake.rs`)
 - rust-lang#125886 (Migrate run make issue 15460)
 - rust-lang#125890 (Improve compiletest expected/not found formatting)
 - rust-lang#125896 (compiletest: fix outdated rmake.rs comment)

r? `@ghost`
`@rustbot` modify labels: rollup
bors added a commit to rust-lang-ci/rust that referenced this pull request Jun 2, 2024
…kingjubilee

Rollup of 6 pull requests

Successful merges:

 - rust-lang#121062 (Change f32::midpoint to upcast to f64)
 - rust-lang#125808 (Migrate `run-make/c-link-to-rust-dylib` to `rmake.rs`)
 - rust-lang#125884 (Implement feature `integer_sign_cast`)
 - rust-lang#125890 (Improve compiletest expected/not found formatting)
 - rust-lang#125896 (compiletest: fix outdated rmake.rs comment)
 - rust-lang#125898 (typo: depending from -> on)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 6c92908 into rust-lang:master Jun 3, 2024
6 checks passed
@rustbot rustbot added this to the 1.80.0 milestone Jun 3, 2024
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Jun 3, 2024
Rollup merge of rust-lang#125890 - Nilstrieb:gay-compiletest, r=jieyouxu

Improve compiletest expected/not found formatting

compiletest, oh compiletest, you are truly one of the tools in this repository. You're the omnipresent gatekeeper, ensuring that every new change works, doesn't break the world, and is nice. We thank you for your work, for your tests, for your test runs, for your features that help writing tests, for all the stability and and good you have caused. Without you, Rust wouldn't exist as it does, without you, nothing would work, without you, we would all go insane from having changes break and having to test them all by hand. Thank you, compiletest.

but holy shit i fucking hate your stupid debug output so much i simply cannot take this anymore aaaaaaaaaaaaaaaaaaaaaaaaaaaaaa

By changing a few magic lines in this file called "runtest.rs", we can cause compiletest to emit nicer messages. This is widely regarded as a good thing. We stop wasting vertical space, allowing more errors to be displayed at once. Additionally, we add colors, which make it so much more pretty *and* gay, both of which are very good and useful.

There's a bit of fuckery needed to get the colors to work. `colored` checks whether stdout is a terminal. We also print to stdout, so that works well.
But.... for some stupid reason that I absolutely refuse to even attempt to debug, stdout is *not* a terminal when executing tests *in a terminal*.
But stderr is >:).
So this just checks whether stderr is a terminal.
If you have a use case where you dump compiletest stdout into a place where colors are not supported while having stderr be a terminal, then I'm sorry for you, but you are gonna get colors and you're gonna like it. Stop it with the usual environment variable, which `colored` also respects by default.

### before (bad, hurts your brain, makes you want to cry)
![image](https://github.com/rust-lang/rust/assets/48135649/cbeecb5d-fc25-460b-b192-9808f8fa2079)

## after (good, gay, makes you want to cry)
![image](https://github.com/rust-lang/rust/assets/48135649/a655b220-8841-443e-a825-72a835d56882)

r? jieyouxu said he wants to review the PR
@Nilstrieb Nilstrieb deleted the gay-compiletest branch June 3, 2024 04:47
@BoxyUwU
Copy link
Member

BoxyUwU commented Jun 4, 2024

Thank you for making compiletest gay.

@Nilstrieb
Copy link
Member Author

you're gay

@jyn514
Copy link
Member

jyn514 commented Jun 4, 2024

no u

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-testsuite Area: The testsuite used to check the correctness of rustc S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap)
Projects
Development

Successfully merging this pull request may close these issues.

None yet

8 participants