Blessing clippy with x.py does not work the same way as other tools #102093
Labels
A-clippy
Area: Clippy
A-contributor-roadblock
Area: Makes things more difficult for new contributors to rust itself
A-testsuite
Area: The testsuite used to check the correctness of rustc
Comments
|
does it work if you cd into the clippy dir root and run cargo dev bless ? |
|
The For Clippy it first runs all UI tests, which will give you the above output, then compile |
|
Or maybe I misinterpreted the output? I got a clippy test failure, scrolled up across a screen worth of noise in the output, and then found that error. Maybe part of the problem is that I first ran it without |
This part of the output shows that And this line: Shows that the file was also updated by the |
|
The odd thing is that I did see the file changed in vscode. (I didn't realize that that was the case but now I think that is what happened.) But the tests still failed until I manually edited the file... |
inquisitivecrystal
added
T-bootstrap
That sounds like you accidentally overwrote the blessed file at first. I agree the UI here is confusing but I'm not yet sure that bless is actually broken, just that it's hard to use. |
|
How can anything I do in an editor lead to |
|
Or maybe the UI is so confusing that I still don't know how to use it. 🤷 I would assume that |
|
Oh are you saying that with I don't know if I tried that. I was working on rustc so I did not read any clippy-specific documentation (and I don't think I should have to), but rustc CI fails when clippy tests don't pass so I was forced to try and get them to pass again, which I expected to work like it does for rustc and Miri tests. (It's already bad enough that clippy does not work with |
Yes, this is how the flag works for clippy |
|
Okay, that would probably explain what I saw. So this issue is then tracking to make |
RalfJung
changed the title
|
This isn't something bootstrap controls. It would need to be changed in clippy's version of compiletest (or I guess we could try and guess which output on stderr will be hidden on the next run, but that seems very error prone). Compiletest for clippy is kind of in limbo right now ... @flip1995 @matthiaskrgr @oli-obk do you know the current state of it? are you planning to use oli's rewrite or try and sync it with the rust-lang/rust compiletest? |
jyn514
added
A-testsuite
|
I don't think we have active plans to use oli's rewrite but we're open to it if it supports everything we need. We have proposed multiple times that rustc compiletest be published out of tree (perhaps with the plugin API exposed so rustc can have add modes without doing a dance) and it's been shot down by compiler team members because they want it to be easy to tweak the test suite. |
|
I still need to implement rustfix support. After that I just need to rebase my clippy PR and open it for the clippy team to discuss. |
|
Great! |
I think this is a good reason for Clippy to also switch to oli's implementation. Trying to maintain a compiletest copy out of tree is probably more annoying than something build for out of tree tools. |
|
I got it working with clippy, so now there's a PR up at rust-lang/rust-clippy#10426 |
Merged
bors
added a commit
to rust-lang-ci/rust
that referenced
this issue
Jul 2, 2023
Update Clippy r? `@Manishearth` cc `@oli-obk` (for the ui-test changes in de69d55 and the valtree fix in 0cbe963) Fixes rust-lang#102093 This must've been the most involved Clippy sync ever.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
I can this command:
I expected this to bless the test files. Instead I got output like this:
Ironically, the output tells me to add
--bless, which I already did!Cc @rust-lang/clippy
The text was updated successfully, but these errors were encountered: