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

Compiletest: Custom differ #131181

Merged
merged 5 commits into from
Oct 23, 2024
Merged

Compiletest: Custom differ #131181

merged 5 commits into from
Oct 23, 2024

Conversation

dev-ardi
Copy link
Contributor

@dev-ardi dev-ardi commented Oct 3, 2024

This adds support for a custom differ for compiletests. It’s purely visual and helps produce cleaner output when UI tests fail.

I’m using an environment variable for now since it’s experimental and I don’t want to drill the cli arguments all the way down. Also did a bit of general cleanup while I was at it.

This is how it looks with debug info silenced (#131182)
COMPILETEST_DIFF_TOOL="/usr/bin/env difft --color always --background light --display side-by-side" ./x test tests/ui/parser
image

@rustbot
Copy link
Collaborator

rustbot commented Oct 3, 2024

r? @compiler-errors

rustbot has assigned @compiler-errors.
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 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. labels Oct 3, 2024
@rustbot
Copy link
Collaborator

rustbot commented Oct 3, 2024

Some changes occurred in src/tools/compiletest

cc @jieyouxu

@rustbot rustbot added the T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) label Oct 3, 2024
jieyouxu

This comment was marked as outdated.

@jieyouxu jieyouxu assigned jieyouxu and unassigned compiler-errors Oct 3, 2024
@jieyouxu jieyouxu added A-compiletest Area: The compiletest test runner and removed A-testsuite Area: The testsuite used to check the correctness of rustc labels Oct 3, 2024
@jieyouxu

This comment was marked as outdated.

@bors

This comment was marked as outdated.

@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 Oct 3, 2024
@jieyouxu

This comment was marked as outdated.

@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 Oct 3, 2024
@jieyouxu
Copy link
Member

jieyouxu commented Oct 3, 2024

Actually, instead of Yet Another Magic Env Var, can you add a new config.toml option in bootstrap so someone can specify their favorite diff tool without having to magically write the env var + provide a path everytime they want to use their favorite diff tool? So something like

# config.toml
[test]
display-diff-tool = "difft --color=always --background=light --display=side-by-side"

And then from bootstrap, pass that via a --display-diff-tool cli flag to compiletest. I specified "display-diff-tool" because that does not and cannot affect how the actual checked-in stderr diffs are generated.

@dev-ardi
Copy link
Contributor Author

dev-ardi commented Oct 3, 2024

Sure thing. Where should I document this option?

@jieyouxu
Copy link
Member

jieyouxu commented Oct 3, 2024

Sure thing. Where should I document this option?

For yourself, me, and everyone else who might like to use a custom diff tool, this should be documented in two places:

  1. In config.example.toml as an example config
  2. In rustc-dev-guide somewhere related to testing

If you need any help threading up the config from bootstrap side let me know.

@rustbot
Copy link
Collaborator

rustbot commented Oct 3, 2024

This PR modifies src/bootstrap/src/core/config.

If appropriate, please update CONFIG_CHANGE_HISTORY in src/bootstrap/src/utils/change_tracker.rs.

This PR modifies config.example.toml.

If appropriate, please update CONFIG_CHANGE_HISTORY in src/bootstrap/src/utils/change_tracker.rs.

@rustbot rustbot added the A-testsuite Area: The testsuite used to check the correctness of rustc label Oct 3, 2024
@rust-log-analyzer

This comment has been minimized.

@rust-log-analyzer

This comment has been minimized.

@rust-log-analyzer

This comment has been minimized.

matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Oct 19, 2024
Compiletest: Custom differ

This adds support for a custom differ for compiletests. It’s purely visual and helps produce cleaner output when UI tests fail.

I’m using an environment variable for now since it’s experimental and I don’t want to drill the cli arguments all the way down. Also did a bit of general cleanup while I was at it.

This is how it looks [with debug info silenced](rust-lang#131182) (rust-lang#131182)
`COMPILETEST_DIFF_TOOL="/usr/bin/env difft --color always --background light --display side-by-side" ./x test tests/ui/parser`
![image](https://github.com/user-attachments/assets/f740ce50-7564-4469-be0a-86e24bc50eb8)
@bors
Copy link
Contributor

bors commented Oct 19, 2024

☔ The latest upstream changes (presumably #131934) made this pull request unmergeable. Please resolve the merge conflicts.

@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 Oct 19, 2024
bors added a commit to rust-lang-ci/rust that referenced this pull request Oct 19, 2024
…iaskrgr

Rollup of 12 pull requests

Successful merges:

 - rust-lang#116863 (warn less about non-exhaustive in ffi)
 - rust-lang#127675 (Remove invalid help diagnostics for const pointer)
 - rust-lang#131772 (Remove `const_refs_to_static` TODO in proc_macro)
 - rust-lang#131789 (Make sure that outer opaques capture inner opaques's lifetimes even with precise capturing syntax)
 - rust-lang#131795 (Stop inverting expectation in normalization errors)
 - rust-lang#131920 (Add codegen test for branchy bool match)
 - rust-lang#131921 (replace STATX_ALL with (STATX_BASIC_STATS | STATX_BTIME) as former is deprecated)
 - rust-lang#131925 (Warn on redundant `--cfg` directive when revisions are used)
 - rust-lang#131931 (Remove unnecessary constness from `lower_generic_args_of_path`)
 - rust-lang#131932 (use tracked_path in rustc_fluent_macro)
 - rust-lang#131936 (feat(rustdoc-json-types): introduce rustc-hash feature)
 - rust-lang#131939 (Get rid of `OnlySelfBounds`)

Failed merges:

 - rust-lang#131181 (Compiletest: Custom differ)

r? `@ghost`
`@rustbot` modify labels: rollup
@jieyouxu
Copy link
Member

jieyouxu commented Oct 20, 2024

Unfortunately this needs a rebase.
@bors r-

bors added a commit to rust-lang-ci/rust that referenced this pull request Oct 20, 2024
…iaskrgr

Rollup of 3 pull requests

Successful merges:

 - rust-lang#131926 (Align boolean option descriptions in `configure.py`)
 - rust-lang#131961 (compiletest: tidy up how `tidy` and `tidy` (html version) are disambiguated)
 - rust-lang#131962 (Make `llvm::set_section` take a `&CStr`)

Failed merges:

 - rust-lang#131181 (Compiletest: Custom differ)

r? `@ghost`
`@rustbot` modify labels: rollup
@dev-ardi
Copy link
Contributor Author

done. @rustbot ready

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Oct 22, 2024
@jieyouxu
Copy link
Member

Thanks.
@bors r+

@bors
Copy link
Contributor

bors commented Oct 23, 2024

📌 Commit 28095bc 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 Oct 23, 2024
fmease added a commit to fmease/rust that referenced this pull request Oct 23, 2024
Compiletest: Custom differ

This adds support for a custom differ for compiletests. It’s purely visual and helps produce cleaner output when UI tests fail.

I’m using an environment variable for now since it’s experimental and I don’t want to drill the cli arguments all the way down. Also did a bit of general cleanup while I was at it.

This is how it looks [with debug info silenced](rust-lang#131182) (rust-lang#131182)
`COMPILETEST_DIFF_TOOL="/usr/bin/env difft --color always --background light --display side-by-side" ./x test tests/ui/parser`
![image](https://github.com/user-attachments/assets/f740ce50-7564-4469-be0a-86e24bc50eb8)
bors added a commit to rust-lang-ci/rust that referenced this pull request Oct 23, 2024
Rollup of 5 pull requests

Successful merges:

 - rust-lang#131043 (Refactor change detection for rustdoc and download-rustc)
 - rust-lang#131181 (Compiletest: Custom differ)
 - rust-lang#131487 (Add wasm32v1-none target (compiler-team/rust-lang#791))
 - rust-lang#132054 (do not remove `.cargo` directory)
 - rust-lang#132058 (CI: rfl: use rust-next temporary commit)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit f267500 into rust-lang:master Oct 23, 2024
6 checks passed
@rustbot rustbot added this to the 1.84.0 milestone Oct 23, 2024
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Oct 23, 2024
Rollup merge of rust-lang#131181 - dev-ardi:custom-differ, r=jieyouxu

Compiletest: Custom differ

This adds support for a custom differ for compiletests. It’s purely visual and helps produce cleaner output when UI tests fail.

I’m using an environment variable for now since it’s experimental and I don’t want to drill the cli arguments all the way down. Also did a bit of general cleanup while I was at it.

This is how it looks [with debug info silenced](rust-lang#131182) (rust-lang#131182)
`COMPILETEST_DIFF_TOOL="/usr/bin/env difft --color always --background light --display side-by-side" ./x test tests/ui/parser`
![image](https://github.com/user-attachments/assets/f740ce50-7564-4469-be0a-86e24bc50eb8)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-compiletest Area: The compiletest test runner 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.

8 participants