Skip to content

Commit 8e455db

Browse files
borsbors
committed
Auto merge of rust-lang#115706 - RalfJung:compiletest-truncation, r=wesleywiser
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#115675 Fixes rust-lang#96229 Fixes rust-lang#94322 Fixes rust-lang#92211
2 parents 367db83 + 9cf2798 commit 8e455db

File tree

3 files changed

+74
-70
lines changed

3 files changed

+74
-70
lines changed

‎src/tools/compiletest/src/read2.rs

+34-29
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,16 @@ use std::io::{self, Write};
99
use std::mem::replace;
1010
use std::process::{Child, Output};
1111

12-
pub fn read2_abbreviated(mut child: Child, filter_paths_from_len: &[String]) -> io::Result<Output> {
12+
#[derive(Copy, Clone, Debug)]
13+
pub enum Truncated {
14+
Yes,
15+
No,
16+
}
17+
18+
pub fn read2_abbreviated(
19+
mut child: Child,
20+
filter_paths_from_len: &[String],
21+
) -> io::Result<(Output, Truncated)> {
1322
let mut stdout = ProcOutput::new();
1423
let mut stderr = ProcOutput::new();
1524

@@ -24,11 +33,12 @@ pub fn read2_abbreviated(mut child: Child, filter_paths_from_len: &[String]) ->
2433
)?;
2534
let status = child.wait()?;
2635

27-
Ok(Output { status, stdout: stdout.into_bytes(), stderr: stderr.into_bytes() })
36+
let truncated =
37+
if stdout.truncated() || stderr.truncated() { Truncated::Yes } else { Truncated::No };
38+
Ok((Output { status, stdout: stdout.into_bytes(), stderr: stderr.into_bytes() }, truncated))
2839
}
2940

30-
const HEAD_LEN: usize = 160 * 1024;
31-
const TAIL_LEN: usize = 256 * 1024;
41+
const MAX_OUT_LEN: usize = 512 * 1024;
3242

3343
// Whenever a path is filtered when counting the length of the output, we need to add some
3444
// placeholder length to ensure a compiler emitting only filtered paths doesn't cause a OOM.
@@ -39,14 +49,18 @@ const FILTERED_PATHS_PLACEHOLDER_LEN: usize = 32;
3949

4050
enum ProcOutput {
4151
Full { bytes: Vec<u8>, filtered_len: usize },
42-
Abbreviated { head: Vec<u8>, skipped: usize, tail: Box<[u8]> },
52+
Abbreviated { head: Vec<u8>, skipped: usize },
4353
}
4454

4555
impl ProcOutput {
4656
fn new() -> Self {
4757
ProcOutput::Full { bytes: Vec::new(), filtered_len: 0 }
4858
}
4959

60+
fn truncated(&self) -> bool {
61+
matches!(self, Self::Abbreviated { .. })
62+
}
63+
5064
fn extend(&mut self, data: &[u8], filter_paths_from_len: &[String]) {
5165
let new_self = match *self {
5266
ProcOutput::Full { ref mut bytes, ref mut filtered_len } => {
@@ -83,24 +97,21 @@ impl ProcOutput {
8397
}
8498

8599
let new_len = bytes.len();
86-
if (*filtered_len).min(new_len) <= HEAD_LEN + TAIL_LEN {
100+
if (*filtered_len).min(new_len) <= MAX_OUT_LEN {
87101
return;
88102
}
89103

90104
let mut head = replace(bytes, Vec::new());
91-
let mut middle = head.split_off(HEAD_LEN);
92-
let tail = middle.split_off(middle.len() - TAIL_LEN).into_boxed_slice();
93-
let skipped = new_len - HEAD_LEN - TAIL_LEN;
94-
ProcOutput::Abbreviated { head, skipped, tail }
105+
// Don't truncate if this as a whole line.
106+
// That should make it less likely that we cut a JSON line in half.
107+
if head.last() != Some(&('\n' as u8)) {
108+
head.truncate(MAX_OUT_LEN);
109+
}
110+
let skipped = new_len - head.len();
111+
ProcOutput::Abbreviated { head, skipped }
95112
}
96-
ProcOutput::Abbreviated { ref mut skipped, ref mut tail, .. } => {
113+
ProcOutput::Abbreviated { ref mut skipped, .. } => {
97114
*skipped += data.len();
98-
if data.len() <= TAIL_LEN {
99-
tail[..data.len()].copy_from_slice(data);
100-
tail.rotate_left(data.len());
101-
} else {
102-
tail.copy_from_slice(&data[(data.len() - TAIL_LEN)..]);
103-
}
104115
return;
105116
}
106117
};
@@ -110,18 +121,12 @@ impl ProcOutput {
110121
fn into_bytes(self) -> Vec<u8> {
111122
match self {
112123
ProcOutput::Full { bytes, .. } => bytes,
113-
ProcOutput::Abbreviated { mut head, mut skipped, tail } => {
114-
let mut tail = &*tail;
115-
116-
// Skip over '{' at the start of the tail, so we don't later wrongfully consider this as json.
117-
// See <https://rust-lang.zulipchat.com/#narrow/stream/182449-t-compiler.2Fhelp/topic/Weird.20CI.20failure/near/321797811>
118-
while tail.get(0) == Some(&b'{') {
119-
tail = &tail[1..];
120-
skipped += 1;
121-
}
122-
123-
write!(&mut head, "\n\n<<<<<< SKIPPED {} BYTES >>>>>>\n\n", skipped).unwrap();
124-
head.extend_from_slice(tail);
124+
ProcOutput::Abbreviated { mut head, skipped } => {
125+
let head_note =
126+
format!("<<<<<< TRUNCATED, SHOWING THE FIRST {} BYTES >>>>>>\n\n", head.len());
127+
head.splice(0..0, head_note.into_bytes());
128+
write!(&mut head, "\n\n<<<<<< TRUNCATED, DROPPED {} BYTES >>>>>>", skipped)
129+
.unwrap();
125130
head
126131
}
127132
}

‎src/tools/compiletest/src/read2/tests.rs

+14-36
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,6 @@
1-
use crate::read2::{ProcOutput, FILTERED_PATHS_PLACEHOLDER_LEN, HEAD_LEN, TAIL_LEN};
1+
use std::io::Write;
2+
3+
use crate::read2::{ProcOutput, FILTERED_PATHS_PLACEHOLDER_LEN, MAX_OUT_LEN};
24

35
#[test]
46
fn test_abbreviate_short_string() {
@@ -21,35 +23,13 @@ fn test_abbreviate_short_string_multiple_steps() {
2123
fn test_abbreviate_long_string() {
2224
let mut out = ProcOutput::new();
2325

24-
let data = vec![b'.'; HEAD_LEN + TAIL_LEN + 16];
26+
let data = vec![b'.'; MAX_OUT_LEN + 16];
2527
out.extend(&data, &[]);
2628

27-
let mut expected = vec![b'.'; HEAD_LEN];
28-
expected.extend_from_slice(b"\n\n<<<<<< SKIPPED 16 BYTES >>>>>>\n\n");
29-
expected.extend_from_slice(&vec![b'.'; TAIL_LEN]);
30-
31-
// We first check the length to avoid endless terminal output if the length differs, since
32-
// `out` is hundreds of KBs in size.
33-
let out = out.into_bytes();
34-
assert_eq!(expected.len(), out.len());
35-
assert_eq!(expected, out);
36-
}
37-
38-
#[test]
39-
fn test_abbreviate_long_string_multiple_steps() {
40-
let mut out = ProcOutput::new();
41-
42-
out.extend(&vec![b'.'; HEAD_LEN], &[]);
43-
out.extend(&vec![b'.'; TAIL_LEN], &[]);
44-
// Also test whether the rotation works
45-
out.extend(&vec![b'!'; 16], &[]);
46-
out.extend(&vec![b'?'; 16], &[]);
47-
48-
let mut expected = vec![b'.'; HEAD_LEN];
49-
expected.extend_from_slice(b"\n\n<<<<<< SKIPPED 32 BYTES >>>>>>\n\n");
50-
expected.extend_from_slice(&vec![b'.'; TAIL_LEN - 32]);
51-
expected.extend_from_slice(&vec![b'!'; 16]);
52-
expected.extend_from_slice(&vec![b'?'; 16]);
29+
let mut expected = Vec::new();
30+
write!(expected, "<<<<<< TRUNCATED, SHOWING THE FIRST {MAX_OUT_LEN} BYTES >>>>>>\n\n").unwrap();
31+
expected.extend_from_slice(&[b'.'; MAX_OUT_LEN]);
32+
expected.extend_from_slice(b"\n\n<<<<<< TRUNCATED, DROPPED 16 BYTES >>>>>>");
5333

5434
// We first check the length to avoid endless terminal output if the length differs, since
5535
// `out` is hundreds of KBs in size.
@@ -86,9 +66,8 @@ fn test_abbreviate_filters_avoid_abbreviations() {
8666
let mut out = ProcOutput::new();
8767
let filters = &[std::iter::repeat('a').take(64).collect::<String>()];
8868

89-
let mut expected = vec![b'.'; HEAD_LEN - FILTERED_PATHS_PLACEHOLDER_LEN as usize];
69+
let mut expected = vec![b'.'; MAX_OUT_LEN - FILTERED_PATHS_PLACEHOLDER_LEN as usize];
9070
expected.extend_from_slice(filters[0].as_bytes());
91-
expected.extend_from_slice(&vec![b'.'; TAIL_LEN]);
9271

9372
out.extend(&expected, filters);
9473

@@ -104,14 +83,13 @@ fn test_abbreviate_filters_can_still_cause_abbreviations() {
10483
let mut out = ProcOutput::new();
10584
let filters = &[std::iter::repeat('a').take(64).collect::<String>()];
10685

107-
let mut input = vec![b'.'; HEAD_LEN];
108-
input.extend_from_slice(&vec![b'.'; TAIL_LEN]);
86+
let mut input = vec![b'.'; MAX_OUT_LEN];
10987
input.extend_from_slice(filters[0].as_bytes());
11088

111-
let mut expected = vec![b'.'; HEAD_LEN];
112-
expected.extend_from_slice(b"\n\n<<<<<< SKIPPED 64 BYTES >>>>>>\n\n");
113-
expected.extend_from_slice(&vec![b'.'; TAIL_LEN - 64]);
114-
expected.extend_from_slice(&vec![b'a'; 64]);
89+
let mut expected = Vec::new();
90+
write!(expected, "<<<<<< TRUNCATED, SHOWING THE FIRST {MAX_OUT_LEN} BYTES >>>>>>\n\n").unwrap();
91+
expected.extend_from_slice(&[b'.'; MAX_OUT_LEN]);
92+
expected.extend_from_slice(b"\n\n<<<<<< TRUNCATED, DROPPED 64 BYTES >>>>>>");
11593

11694
out.extend(&input, filters);
11795

‎src/tools/compiletest/src/runtest.rs

+26-5
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ use crate::compute_diff::{write_diff, write_filtered_diff};
1212
use crate::errors::{self, Error, ErrorKind};
1313
use crate::header::TestProps;
1414
use crate::json;
15-
use crate::read2::read2_abbreviated;
15+
use crate::read2::{read2_abbreviated, Truncated};
1616
use crate::util::{add_dylib_path, dylib_env_var, logv, PathBufExt};
1717
use crate::ColorConfig;
1818
use regex::{Captures, Regex};
@@ -701,6 +701,7 @@ impl<'test> TestCx<'test> {
701701
status: output.status,
702702
stdout: String::from_utf8(output.stdout).unwrap(),
703703
stderr: String::from_utf8(output.stderr).unwrap(),
704+
truncated: Truncated::No,
704705
cmdline: format!("{cmd:?}"),
705706
};
706707
self.dump_output(&proc_res.stdout, &proc_res.stderr);
@@ -1275,6 +1276,7 @@ impl<'test> TestCx<'test> {
12751276
status,
12761277
stdout: String::from_utf8(stdout).unwrap(),
12771278
stderr: String::from_utf8(stderr).unwrap(),
1279+
truncated: Truncated::No,
12781280
cmdline,
12791281
};
12801282
if adb.kill().is_err() {
@@ -1566,7 +1568,13 @@ impl<'test> TestCx<'test> {
15661568
};
15671569

15681570
self.dump_output(&out, &err);
1569-
ProcRes { status, stdout: out, stderr: err, cmdline: format!("{:?}", cmd) }
1571+
ProcRes {
1572+
status,
1573+
stdout: out,
1574+
stderr: err,
1575+
truncated: Truncated::No,
1576+
cmdline: format!("{:?}", cmd),
1577+
}
15701578
}
15711579

15721580
fn cleanup_debug_info_options(&self, options: &Vec<String>) -> Vec<String> {
@@ -2227,7 +2235,7 @@ impl<'test> TestCx<'test> {
22272235
dylib
22282236
}
22292237

2230-
fn read2_abbreviated(&self, child: Child) -> Output {
2238+
fn read2_abbreviated(&self, child: Child) -> (Output, Truncated) {
22312239
let mut filter_paths_from_len = Vec::new();
22322240
let mut add_path = |path: &Path| {
22332241
let path = path.display().to_string();
@@ -2274,12 +2282,13 @@ impl<'test> TestCx<'test> {
22742282
child.stdin.as_mut().unwrap().write_all(input.as_bytes()).unwrap();
22752283
}
22762284

2277-
let Output { status, stdout, stderr } = self.read2_abbreviated(child);
2285+
let (Output { status, stdout, stderr }, truncated) = self.read2_abbreviated(child);
22782286

22792287
let result = ProcRes {
22802288
status,
22812289
stdout: String::from_utf8_lossy(&stdout).into_owned(),
22822290
stderr: String::from_utf8_lossy(&stderr).into_owned(),
2291+
truncated,
22832292
cmdline,
22842293
};
22852294

@@ -3610,12 +3619,14 @@ impl<'test> TestCx<'test> {
36103619
}
36113620
}
36123621

3613-
let output = self.read2_abbreviated(cmd.spawn().expect("failed to spawn `make`"));
3622+
let (output, truncated) =
3623+
self.read2_abbreviated(cmd.spawn().expect("failed to spawn `make`"));
36143624
if !output.status.success() {
36153625
let res = ProcRes {
36163626
status: output.status,
36173627
stdout: String::from_utf8_lossy(&output.stdout).into_owned(),
36183628
stderr: String::from_utf8_lossy(&output.stderr).into_owned(),
3629+
truncated,
36193630
cmdline: format!("{:?}", cmd),
36203631
};
36213632
self.fatal_proc_rec("make failed", &res);
@@ -3777,6 +3788,15 @@ impl<'test> TestCx<'test> {
37773788
let emit_metadata = self.should_emit_metadata(pm);
37783789
let proc_res = self.compile_test(should_run, emit_metadata);
37793790
self.check_if_test_should_compile(&proc_res, pm);
3791+
if matches!(proc_res.truncated, Truncated::Yes)
3792+
&& !self.props.dont_check_compiler_stdout
3793+
&& !self.props.dont_check_compiler_stderr
3794+
{
3795+
self.fatal_proc_rec(
3796+
&format!("compiler output got truncated, cannot compare with reference file"),
3797+
&proc_res,
3798+
);
3799+
}
37803800

37813801
// if the user specified a format in the ui test
37823802
// print the output to the stderr file, otherwise extract
@@ -4468,6 +4488,7 @@ pub struct ProcRes {
44684488
status: ExitStatus,
44694489
stdout: String,
44704490
stderr: String,
4491+
truncated: Truncated,
44714492
cmdline: String,
44724493
}
44734494

0 commit comments

Comments
 (0)