From 317b581120c0402f7982803fde0c1c356b556ab4 Mon Sep 17 00:00:00 2001 From: Mukilan Thiyagarajan Date: Thu, 17 Aug 2023 19:33:39 +0530 Subject: [PATCH] Send bactrace to stderr and capture it in test runner Servo's panic hook writes backtraces to stdout. This patch changes it so they are written to stderr. The crash test executor for servo in WPT grouping formatter was also not capturing the output correctly for crashtests as the log events were being aggregated based on thread name which doesn't seem to match correctly in case of crashtests. This patch also fixes the log grouping logic to be based on test name. Co-authored-by: Martin Robinson Signed-off-by: Mukilan Thiyagarajan --- ports/winit/main2.rs | 16 ++++++++------ python/wpt/grouping_formatter.py | 6 ++---- tests/wpt/meta/MANIFEST.json | 2 +- .../wptrunner/executors/executorservo.py | 21 +++++++++++++------ 4 files changed, 28 insertions(+), 17 deletions(-) diff --git a/ports/winit/main2.rs b/ports/winit/main2.rs index 07d931c21abb6..2cccdb9550df5 100644 --- a/ports/winit/main2.rs +++ b/ports/winit/main2.rs @@ -117,11 +117,11 @@ pub fn main() { }; let current_thread = thread::current(); let name = current_thread.name().unwrap_or(""); - let stdout = std::io::stdout(); - let mut stdout = stdout.lock(); + let stderr = std::io::stderr(); + let mut stderr = stderr.lock(); if let Some(location) = info.location() { let _ = writeln!( - &mut stdout, + &mut stderr, "{} (thread {}, at {}:{})", msg, name, @@ -129,12 +129,16 @@ pub fn main() { location.line() ); } else { - let _ = writeln!(&mut stdout, "{} (thread {})", msg, name); + let _ = writeln!(&mut stderr, "{} (thread {})", msg, name); } if env::var("RUST_BACKTRACE").is_ok() { - let _ = backtrace::print(&mut stdout); + let _ = backtrace::print(&mut stderr); + } + drop(stderr); + + if opts::get().hard_fail && !opts::get().multiprocess { + std::process::exit(1); } - drop(stdout); error!("{}", msg); })); diff --git a/python/wpt/grouping_formatter.py b/python/wpt/grouping_formatter.py index fcf9fcbe69dd5..4e14d62b5735b 100644 --- a/python/wpt/grouping_formatter.py +++ b/python/wpt/grouping_formatter.py @@ -216,10 +216,8 @@ def test_status(self, data: dict): )) def process_output(self, data): - if data['thread'] not in self.running_tests: - return - test_name = self.running_tests[data['thread']] - self.test_output[test_name] += data['data'] + "\n" + if 'test' in data: + self.test_output[data['test']] += data['data'] + "\n" def log(self, _): pass diff --git a/tests/wpt/meta/MANIFEST.json b/tests/wpt/meta/MANIFEST.json index 833b804569bc7..045d0b4f0d609 100644 --- a/tests/wpt/meta/MANIFEST.json +++ b/tests/wpt/meta/MANIFEST.json @@ -465041,7 +465041,7 @@ [] ], "executorservo.py": [ - "a8ec38699616c5baf7a3b43b8149b89746308f35", + "0d22471e90e83fbfd80010001e8c60f509af8902", [] ], "executorservodriver.py": [ diff --git a/tests/wpt/tests/tools/wptrunner/wptrunner/executors/executorservo.py b/tests/wpt/tests/tools/wptrunner/wptrunner/executors/executorservo.py index a8ec38699616c..0d22471e90e83 100644 --- a/tests/wpt/tests/tools/wptrunner/wptrunner/executors/executorservo.py +++ b/tests/wpt/tests/tools/wptrunner/wptrunner/executors/executorservo.py @@ -83,6 +83,7 @@ def teardown(self): ProcessTestExecutor.teardown(self) def do_test(self, test): + self.test = test self.result_data = None self.result_flag = threading.Event() @@ -156,7 +157,7 @@ def on_output(self, line): else: self.logger.process_output(self.proc.pid, line, - " ".join(self.command)) + " ".join(self.command), self.test.url) def on_finish(self): self.result_flag.set() @@ -270,18 +271,19 @@ def screenshot(self, test, viewport_size, dpi, page_ranges): return True, [base64.b64encode(data).decode()] def do_test(self, test): + self.test = test result = self.implementation.run_test(test) return self.convert_result(test, result) - def on_output(self, line): + def on_output(line): line = line.decode("utf8", "replace") if self.interactive: print(line) else: self.logger.process_output(self.proc.pid, line, - " ".join(self.command)) + " ".join(self.command), self.test.url) class ServoTimedRunner(TimedRunner): @@ -342,7 +344,7 @@ def do_crashtest(self, protocol, url, timeout): env["HOST_FILE"] = self.hosts_path env["RUST_BACKTRACE"] = "1" - command = build_servo_command(self.test, + self.command = build_servo_command(self.test, self.test_url, self.browser, self.binary, @@ -351,12 +353,13 @@ def do_crashtest(self, protocol, url, timeout): extra_args=["-x"]) if not self.interactive: - self.proc = ProcessHandler(command, + self.proc = ProcessHandler(self.command, env=env, + processOutputLine=[self.on_output], storeOutput=False) self.proc.run() else: - self.proc = subprocess.Popen(command, env=env) + self.proc = subprocess.Popen(self.command, env=env) self.proc.wait() @@ -364,3 +367,9 @@ def do_crashtest(self, protocol, url, timeout): return {"status": "PASS", "message": None} return {"status": "CRASH", "message": None} + + def on_output(self, line): + line = line.decode("utf8", "replace") + self.logger.process_output(self.proc.pid, + line, + " ".join(self.command), self.test.url)