Skip to content

Make annotations mandatory for internal ui tests #14393

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

Merged
merged 4 commits into from
Mar 12, 2025

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