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: show affected architecture in error summary #78517

Closed
RalfJung opened this issue Oct 29, 2020 · 4 comments · Fixed by #78705
Closed

compiletest: show affected architecture in error summary #78517

RalfJung opened this issue Oct 29, 2020 · 4 comments · Fixed by #78705
Labels
A-testsuite Area: The testsuite used to check the correctness of rustc C-enhancement Category: An issue proposing an enhancement or a PR with one.

Comments

@RalfJung
Copy link
Member

When compiletest fails, it prints an error summary like this at the end:

failures:
    [mir-opt] mir-opt/issue-73223.rs

test result: FAILED. 139 passed; 1 failed; 3 ignored; 0 measured; 0 filtered out

However, there is one very important bit of information that I cannot gleam from this information: what are the host and target architectures that were tested! Figuring this out requires scrolling up quite a bit to where this test beings (and carefully not scrolling up too far because then one might look at the wrong test).

See #78343 for an example where this confused people. I have also been confused by this multiple times in the past, e.g. using the name of the CI runner to incorrectly deduce what the architecture(s) are (and not realizing that some of our runners run tests for quite a few different targets).

So I think in case of a failure, that summary at the end should repeat the important information that is printed at the top of the test run:

compiletest suite=mir-opt mode=mir-opt (x86_64-unknown-linux-gnu -> armv5te-unknown-linux-gnueabi)

@RalfJung
Copy link
Member Author

Cc @Mark-Simulacrum

@jyn514 jyn514 added A-testsuite Area: The testsuite used to check the correctness of rustc C-enhancement Category: An issue proposing an enhancement or a PR with one. labels Oct 29, 2020
@camelid
Copy link
Member

camelid commented Oct 29, 2020

And also it would be nice if when it ignored a test because it was the wrong platform, it showed some indication of that, because otherwise it looks like it ran the test and it passed. Perhaps show a question mark (?)? (We already use i for tests that were ignored because they were already run and passed.)

@RalfJung
Copy link
Member Author

AFAIK i should already be used for all kinds of "ignored", including // ignore-XX directives.

@RalfJung
Copy link
Member Author

RalfJung commented Nov 1, 2020

Turns out these messages are printed by bootstrap, not compiletest:

"Check compiletest suite={} mode={} ({} -> {})",

However, detecting failures and exiting is done a few layers deeper:

std::process::exit(1);

In case of failure, try_run never returns or unwinds (except if fail-fast is off).

I am not sure how to best adjust this to print an error before exiting (or to always print an error after the process finished, whether it succeeded or failed). @Mark-Simulacrum any advice? If we just want this to happen when fail-fast is off, that would be easy, but I am not sure if that ever applies.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-testsuite Area: The testsuite used to check the correctness of rustc C-enhancement Category: An issue proposing an enhancement or a PR with one.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants