Skip to content

Conversation

@GuillaumeGomez
Copy link
Member

Last part of #11421.

Now all ui tests require annotations.

The change in ui_test is to add ICE: errors.

changelog: Make internals ui tests annotations mandatory

r? @flip1995

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label Mar 12, 2025
Copy link
Member

@flip1995 flip1995 left a comment

Choose a reason for hiding this comment

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

Nice! Was the assert->span_delayed_bug change necessary for this to work?

Comment on lines 12 to +13
fn it_looks_like_you_are_trying_to_kill_clippy() {}
//~^ ice: Would you like some help with that?
Copy link
Member

Choose a reason for hiding this comment

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

I love that the error message is now also there. This makes the joke complete 🎉

@GuillaumeGomez
Copy link
Member Author

Nice! Was the assert->span_delayed_bug change necessary for this to work?

Yep because otherwise it's not emitted as JSON, preventing ui_test to "catch it" and therefore prevent us to have annotations. :)

@GuillaumeGomez
Copy link
Member Author

Fixed failing test. :)

@flip1995
Copy link
Member

Yep because otherwise it's not emitted as JSON, preventing ui_test to "catch it" and therefore prevent us to have annotations. :)

Nice! Happy that my guess was useful 🎉

@flip1995 flip1995 added this pull request to the merge queue Mar 12, 2025
@GuillaumeGomez
Copy link
Member Author

It definitely saved me time investigating what was going on. :)

Merged via the queue into rust-lang:master with commit 0730678 Mar 12, 2025
11 checks passed
@GuillaumeGomez GuillaumeGomez deleted the mandatory-annotations branch March 12, 2025 12:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

S-waiting-on-review Status: Awaiting review from the assignee but also interested parties

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants