about summary refs log tree commit diff
path: root/src/bootstrap
diff options
context:
space:
mode:
authorMatthias Krüger <matthias.krueger@famsik.de>2023-03-25 03:37:10 +0100
committerGitHub <noreply@github.com>2023-03-25 03:37:10 +0100
commit65220b5ba9a055d7dbf66859f862a8fa6b274d64 (patch)
treec3f48e6261481b3ac29c7b8b65cae187c7825b1d /src/bootstrap
parentf3d3f350cceb6ce8d20ec4bace032f4a62a43c4e (diff)
parent0d1a0540e46b504597497c1db3e2a4d0d89ee550 (diff)
downloadrust-65220b5ba9a055d7dbf66859f862a8fa6b274d64.tar.gz
rust-65220b5ba9a055d7dbf66859f862a8fa6b274d64.zip
Rollup merge of #109484 - fortanix:raoul/bugfix_libtest_json_output, r=pietroalbini
Bugfix: avoid panic on invalid json output from libtest

#108659 introduces a custom test display implementation. It does so by using libtest to output json. The stdout is read and parsed; The code trims the line read and checks whether it starts with a `{` and ends with a `}`. If so, it concludes that it must be a json encoded `Message`. Unfortunately, this does not work in all cases:

- This assumes that tests running with `--nocapture` will never start and end lines with `{` and `}` characters
- Output is generated by issuing multiple `write_message` [statements](https://github.com/rust-lang/rust/blob/master/library/test/src/formatters/json.rs#L33-L60). Where only the last one issues a `\n`. This likely results in a race condition as we see multiple json outputs on the same line when running tests for the `x86_64-fortanix-unknown-sgx` target:
```
10:21:04      Running tests/run-time-detect.rs (build/x86_64-unknown-linux-gnu/stage1-std/x86_64-fortanix-unknown-sgx/release/deps/run_time_detect-8c66026bd4b1871a)
10:21:04
10:21:04 running 1 tests
10:21:04 test x86_all ... ok
10:21:04      Running tests/thread.rs (build/x86_64-unknown-linux-gnu/stage1-std/x86_64-fortanix-unknown-sgx/release/deps/thread-ed5456a7d80a6193)
10:21:04 thread 'main' panicked at 'failed to parse libtest json output; error: trailing characters at line 1 column 135, line: "{ \"type\": \"suite\", \"event\": \"ok\", \"passed\": 1, \"failed\": 0, \"ignored\": 0, \"measured\": 0, \"filtered_out\": 0, \"exec_time\": 0.000725911 }{ \"type\": \"suite\", \"event\": \"started\", \"test_count\": 1 }\n"', render_tests.rs:108:25
```

This PR implements a partial fix by being much more conservative of what it asserts is a valid json encoded `Message`. This prevents panics, but still does not resolve the race condition. A discussion is needed where this race condition comes from exactly and how it best can be avoided.

cc: `@jethrogb,` `@pietroalbini`
Diffstat (limited to 'src/bootstrap')
-rw-r--r--src/bootstrap/render_tests.rs19
1 files changed, 7 insertions, 12 deletions
diff --git a/src/bootstrap/render_tests.rs b/src/bootstrap/render_tests.rs
index af2370d439e..19019ad2c08 100644
--- a/src/bootstrap/render_tests.rs
+++ b/src/bootstrap/render_tests.rs
@@ -100,18 +100,13 @@ impl<'a> Renderer<'a> {
                 break;
             }
 
-            let trimmed = line.trim();
-            if trimmed.starts_with("{") && trimmed.ends_with("}") {
-                self.render_message(match serde_json::from_str(&trimmed) {
-                    Ok(parsed) => parsed,
-                    Err(err) => {
-                        panic!("failed to parse libtest json output; error: {err}, line: {line:?}");
-                    }
-                });
-            } else {
-                // Handle non-JSON output, for example when --nocapture is passed.
-                print!("{line}");
-                let _ = std::io::stdout().flush();
+            match serde_json::from_str(&line) {
+                Ok(parsed) => self.render_message(parsed),
+                Err(_err) => {
+                    // Handle non-JSON output, for example when --nocapture is passed.
+                    print!("{line}");
+                    let _ = std::io::stdout().flush();
+                }
             }
         }
     }