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

make individual errors in a failed test more clearly visible #167

Merged
merged 4 commits into from
Sep 26, 2023

Conversation

RalfJung
Copy link
Collaborator

In large errors I feel it is easy to miss when somewhere at the top it also says "got exit status: 0, but expected 1", or so.

Ideally all errors would do this consistently, e.g. start with a underlined title and then perhaps have more details. However that doesn't really work for the errors that show a span... but maybe ut's worth doing that at least for these two otherwise-easy-to-miss errors?

@oli-obk
Copy link
Owner

oli-obk commented Sep 25, 2023

We could also use the snippets logic we use for spanned errors, so you'd get it in the same format and style (red error followed by the message)

@RalfJung
Copy link
Collaborator Author

RalfJung commented Sep 25, 2023

Yeah that would be the other option I guess. We need to be careful to make this still clearly "smaller" than the separator that introduces the messages for each test failure:

tests/fail/function_pointers/abi_mismatch_simple.rs FAILED:

Maybe if it looks like this that is enough?

<underline><bold><red>FAILED TEST:</red> tests/fail/function_pointers/abi_mismatch_simple.rs</bold></underline>

<bold><red>error:</red> bad exit status</bold>

<bold><red>error:</red> substring not found</bold>
{span printed}

<bold><red>error:</red> unmatched diagnostics</bold>
{span printed}

The all-caps and the underline then hopefully clearly separate multiple failed tests.

@oli-obk
Copy link
Owner

oli-obk commented Sep 26, 2023

Yea that sounds good to me.

@RalfJung
Copy link
Collaborator Author

This looks better I think, but it's not entirely the same tone of red somehow...

image

@RalfJung
Copy link
Collaborator Author

Though rustc and cargo are not consistent either.^^

image

Anyway I made it all bright_red, now it is consistent within ui_test. I left diff at red though, not sure which one git diff uses.

@RalfJung
Copy link
Collaborator Author

I think git diff uses the darker red.

@RalfJung RalfJung changed the title make exit status messages more more visible make individual errors in a failed test more clearly visible Sep 26, 2023
src/status_emitter.rs Outdated Show resolved Hide resolved
@RalfJung
Copy link
Collaborator Author

Updated screenshot:

image

@oli-obk
Copy link
Owner

oli-obk commented Sep 26, 2023

I love it, thanks!

@oli-obk oli-obk merged commit 9086891 into oli-obk:main Sep 26, 2023
5 checks passed
@RalfJung RalfJung deleted the error-render branch September 26, 2023 17:43
@RalfJung
Copy link
Collaborator Author

Yeah I'm quite happy with it. Can't wait to see it in Miri. Just needs a release now. nudge wink ;)

@oli-obk
Copy link
Owner

oli-obk commented Sep 26, 2023

I can do releases from gha, I just can't do the version bump from it 😃

I'll do both tomorrow

@RalfJung
Copy link
Collaborator Author

No worries, not urgent at all. :)

bors added a commit to rust-lang/miri that referenced this pull request Sep 28, 2023
update ui_test

In particular this includes oli-obk/ui_test#167 :)
RalfJung pushed a commit to RalfJung/rust that referenced this pull request Sep 30, 2023
RalfJung pushed a commit to RalfJung/rust that referenced this pull request Oct 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants