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's "abbreviated" output can be very misleading #92211

Closed
Aaron1011 opened this issue Dec 22, 2021 · 0 comments · Fixed by #115706
Closed

Compiletest's "abbreviated" output can be very misleading #92211

Aaron1011 opened this issue Dec 22, 2021 · 0 comments · Fixed by #115706
Labels
C-bug Category: This is a bug. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-dev-tools Relevant to the dev-tools subteam, which will review and decide on the PR/issue.

Comments

@Aaron1011
Copy link
Member

Under certain circumstances, compiletest will 'abbreviate' the output of tests:

write!(&mut head, "\n\n<<<<<< SKIPPED {} BYTES >>>>>>\n\n", skipped).unwrap();

However, the <<<<<< SKIPPED {} BYTES >>>>>> can easily be missed. I spent quite a while trying to figure out how the incremental compilation system was getting into a certain state without hitting any debug! statements, only to find out that a large chunk of the output was getting discarded.

Ideally, we would remove this 'abbreviation' logic entirely, and just ensure that our tests don't produce too much output. If it's really necessary, then we should require tests to opt in to this behavior, and fail the test if abbreviation is required when the test hasn't explicitly opted in.

@Aaron1011 Aaron1011 added C-bug Category: This is a bug. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-dev-tools Relevant to the dev-tools subteam, which will review and decide on the PR/issue. labels Dec 22, 2021
bors added a commit to rust-lang-ci/rust that referenced this issue Jun 6, 2022
…iating, r=Mark-Simulacrum

[compiletest] Ignore known paths when abbreviating output

To prevent out of memory conditions, compiletest limits the amount of output a test can generate, abbreviating it if the test emits more than a threshold. While the behavior is desirable, it also causes some issues (like rust-lang#96229, rust-lang#94322 and rust-lang#92211).

The latest one happened recently, when the `src/test/ui/numeric/numeric-cast.rs` test started to fail on systems where the path of the rust-lang/rust checkout is too long. This includes my own development machine and [LLVM's CI](rust-lang#96362 (comment)). Rust's CI uses a pretty short directory name for the checkout, which hides these sort of problems until someone runs the test suite on their own computer.

When developing the fix I tried to find the most targeted fix that would prevent this class of failures from happening in the future, deferring the decision on if/how to redesign abbreviation to a later date. The solution I came up with was to ignore known base paths when calculating whether the output exceeds the abbreviation threshold, which removes this kind of nondeterminism.

This PR is best reviewed commit-by-commit.
@bors bors closed this as completed in 8e455db Sep 13, 2023
github-actions bot pushed a commit to rust-lang/miri that referenced this issue Sep 15, 2023
Make compiletest output truncation less disruptive

When the test output becomes too large, compiletest stops recording all of it. However:
- this can lead to invalid JSON, which then causes compiletest itself to throw further errors
- the note that output was truncated is in the middle of the output, with >100kb of text on each side; that makes it almost impossible to actually see that note in the terminal

So assuming that we do need to keep the output truncation, I propose that we only ever do a cut at the end, so that it is very clear by looking at the end of the log that truncation happened. I added a message at the beginning of the output as well. Also I added some logic to make it less likely that we'll cut things off in the middle of a JSON record. (I tested that successfully by reducing the output limit to something very low and running a few ui tests.) Furthermore I increased the max buffer size to 512KB; that's really not a lot of memory compared to how much RAM it takes to build rustc (it's ~25% more than the previous maximum HEAD+TAIL length). And finally, the information that things got truncated is now propagated to the higher levels, so that we can fail the test instead of comparing the truncated output with the reference.

Fixes rust-lang/rust#115675
Fixes rust-lang/rust#96229
Fixes rust-lang/rust#94322
Fixes rust-lang/rust#92211
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Category: This is a bug. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-dev-tools Relevant to the dev-tools subteam, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant