From 54aa9e8ba2efe68bfcf5c407a5a9cf1a8d17b354 Mon Sep 17 00:00:00 2001 From: Charles Celerier Date: Sat, 6 Jul 2024 05:15:38 +0000 Subject: [PATCH 1/3] [fuchsia-test-runner] Remove runner logs from stdout and stderr Many tests use stdout and stderr to validate whether the test emitted the correct output. Because fuchsia-test-runner.py was sending all logs, including test output, to stdout, tests could not validate output properly. This change removes the runner logs from stdout and stderr entirely with the exception of output from the test. All runner logs are still available in the "log" file. Fixed: https://fxbug.dev/351356417 --- src/ci/docker/scripts/fuchsia-test-runner.py | 11 ++--------- 1 file changed, 2 insertions(+), 9 deletions(-) diff --git a/src/ci/docker/scripts/fuchsia-test-runner.py b/src/ci/docker/scripts/fuchsia-test-runner.py index 6db25ff1a809e..1a2e69712a92f 100755 --- a/src/ci/docker/scripts/fuchsia-test-runner.py +++ b/src/ci/docker/scripts/fuchsia-test-runner.py @@ -271,13 +271,6 @@ def setup_logging(self, log_to_file=False): logfile_handler.setLevel(logging.DEBUG) logfile_handler.setFormatter(fs) logging.getLogger().addHandler(logfile_handler) - stream_handler = logging.StreamHandler(sys.stdout) - stream_handler.setFormatter(fs) - if self.verbose: - stream_handler.setLevel(logging.DEBUG) - else: - stream_handler.setLevel(logging.INFO) - logging.getLogger().addHandler(stream_handler) logging.getLogger().setLevel(logging.DEBUG) @property @@ -927,7 +920,7 @@ def run(self, args): ) else: with open(stdout_path, encoding="utf-8", errors="ignore") as f: - runner_logger.info(f.read()) + sys.stdout.write(f.read()) if stderr_path is not None: if not os.path.exists(stderr_path): runner_logger.error( @@ -935,7 +928,7 @@ def run(self, args): ) else: with open(stderr_path, encoding="utf-8", errors="ignore") as f: - runner_logger.error(f.read()) + sys.stderr.write(f.read()) runner_logger.info("Done!") return return_code From 3d5b4d8f7cd721ded6589edc2e6e350a8575848f Mon Sep 17 00:00:00 2001 From: Charles Celerier Date: Sun, 7 Jul 2024 15:33:31 +0000 Subject: [PATCH 2/3] [fuchsia-test-runner] Reformat fuchsia-test-runner.py Applied formatting suggestions from isort and black via pylsp. --- src/ci/docker/scripts/fuchsia-test-runner.py | 52 ++++++-------------- 1 file changed, 15 insertions(+), 37 deletions(-) diff --git a/src/ci/docker/scripts/fuchsia-test-runner.py b/src/ci/docker/scripts/fuchsia-test-runner.py index 1a2e69712a92f..00269e68422d3 100755 --- a/src/ci/docker/scripts/fuchsia-test-runner.py +++ b/src/ci/docker/scripts/fuchsia-test-runner.py @@ -8,8 +8,6 @@ """ import argparse -from concurrent.futures import ThreadPoolExecutor -from dataclasses import dataclass import glob import io import json @@ -20,6 +18,8 @@ import shutil import subprocess import sys +from concurrent.futures import ThreadPoolExecutor +from dataclasses import dataclass from pathlib import Path from typing import ClassVar, List, Optional @@ -42,12 +42,8 @@ def exhaust_pipe(handler, pipe): for line in pipe: handler(line.rstrip()) - executor_out = executor.submit( - exhaust_pipe, stdout_handler, process.stdout - ) - executor_err = executor.submit( - exhaust_pipe, stderr_handler, process.stderr - ) + executor_out = executor.submit(exhaust_pipe, stdout_handler, process.stdout) + executor_err = executor.submit(exhaust_pipe, stderr_handler, process.stderr) executor_out.result() executor_err.result() retcode = process.poll() @@ -203,9 +199,7 @@ def build_id(self, binary): raise Exception(f"Unreadable build-id for binary {binary}") data = json.loads(process.stdout) if len(data) != 1: - raise Exception( - f"Unreadable output from llvm-readelf for binary {binary}" - ) + raise Exception(f"Unreadable output from llvm-readelf for binary {binary}") notes = data[0]["Notes"] for note in notes: note_section = note["NoteSection"] @@ -265,9 +259,7 @@ def write_to_file(self): def setup_logging(self, log_to_file=False): fs = logging.Formatter("%(asctime)s %(levelname)s:%(name)s:%(message)s") if log_to_file: - logfile_handler = logging.FileHandler( - self.tmp_dir().joinpath("log") - ) + logfile_handler = logging.FileHandler(self.tmp_dir().joinpath("log")) logfile_handler.setLevel(logging.DEBUG) logfile_handler.setFormatter(fs) logging.getLogger().addHandler(logfile_handler) @@ -447,9 +439,7 @@ def start(self): # Initialize temp directory os.makedirs(self.tmp_dir(), exist_ok=True) if len(os.listdir(self.tmp_dir())) != 0: - raise Exception( - f"Temp directory is not clean (in {self.tmp_dir()})" - ) + raise Exception(f"Temp directory is not clean (in {self.tmp_dir()})") self.setup_logging(log_to_file=True) os.mkdir(self.output_dir) @@ -486,9 +476,7 @@ def start(self): shutil.rmtree(self.local_pb_path, ignore_errors=True) # Look up the product bundle transfer manifest. - self.env_logger.info( - "Looking up the product bundle transfer manifest..." - ) + self.env_logger.info("Looking up the product bundle transfer manifest...") product_name = "minimal." + self.triple_to_arch(self.target) sdk_version = self.read_sdk_version() @@ -510,9 +498,7 @@ def start(self): ) try: - transfer_manifest_url = json.loads(output)[ - "transfer_manifest_url" - ] + transfer_manifest_url = json.loads(output)["transfer_manifest_url"] except Exception as e: print(e) raise Exception("Unable to parse transfer manifest") from e @@ -762,9 +748,7 @@ def run(self, args): # Use /tmp as the test temporary directory env_vars += '\n "RUST_TEST_TMPDIR=/tmp",' - cml.write( - self.CML_TEMPLATE.format(env_vars=env_vars, exe_name=exe_name) - ) + cml.write(self.CML_TEMPLATE.format(env_vars=env_vars, exe_name=exe_name)) runner_logger.info("Compiling CML...") @@ -915,17 +899,13 @@ def run(self, args): if stdout_path is not None: if not os.path.exists(stdout_path): - runner_logger.error( - f"stdout file {stdout_path} does not exist." - ) + runner_logger.error(f"stdout file {stdout_path} does not exist.") else: with open(stdout_path, encoding="utf-8", errors="ignore") as f: sys.stdout.write(f.read()) if stderr_path is not None: if not os.path.exists(stderr_path): - runner_logger.error( - f"stderr file {stderr_path} does not exist." - ) + runner_logger.error(f"stderr file {stderr_path} does not exist.") else: with open(stderr_path, encoding="utf-8", errors="ignore") as f: sys.stderr.write(f.read()) @@ -1030,7 +1010,7 @@ def debug(self, args): f"--symbol-path={self.rust_dir}/lib/rustlib/{self.target}/lib", ] - # Add rust source if it's available + # Add rust source if it's available rust_src_map = None if args.rust_src is not None: # This matches the remapped prefix used by compiletest. There's no @@ -1203,7 +1183,7 @@ def print_help(args): start_parser.add_argument( "--use-local-product-bundle-if-exists", help="if the product bundle already exists in the local path, use " - "it instead of downloading it again", + "it instead of downloading it again", action="store_true", ) start_parser.set_defaults(func=start) @@ -1239,9 +1219,7 @@ def print_help(args): ) cleanup_parser.set_defaults(func=cleanup) - syslog_parser = subparsers.add_parser( - "syslog", help="prints the device syslog" - ) + syslog_parser = subparsers.add_parser("syslog", help="prints the device syslog") syslog_parser.set_defaults(func=syslog) debug_parser = subparsers.add_parser( From 479b0cdb8957255716fc8158f06a1cb0aba5ad1a Mon Sep 17 00:00:00 2001 From: Charles Celerier Date: Sun, 7 Jul 2024 05:03:59 +0000 Subject: [PATCH 3/3] Ignore fuchsia tests implicitly relying on a signal upon abort Both test-panic-abort-nocapture.rs and test-panic-abort.rs assert the stderr output of the test. On Fuchsia, if a test fails an assertion, this output will contain a line noting the process returned the code -1028 (ZX_TASK_RETCODE_EXCEPTION_KILL). But the asserted stderr output lacks this note. Presumably this is because other platforms implement -Cpanic=abort by killing the process instead of returned a status code. --- tests/ui/test-attrs/test-panic-abort-nocapture.rs | 1 + tests/ui/test-attrs/test-panic-abort-nocapture.run.stderr | 4 ++-- tests/ui/test-attrs/test-panic-abort.rs | 1 + tests/ui/test-attrs/test-panic-abort.run.stdout | 2 +- 4 files changed, 5 insertions(+), 3 deletions(-) diff --git a/tests/ui/test-attrs/test-panic-abort-nocapture.rs b/tests/ui/test-attrs/test-panic-abort-nocapture.rs index 03c175a2a49ff..8c9e222a40d3e 100644 --- a/tests/ui/test-attrs/test-panic-abort-nocapture.rs +++ b/tests/ui/test-attrs/test-panic-abort-nocapture.rs @@ -10,6 +10,7 @@ //@ ignore-wasm no panic or subprocess support //@ ignore-emscripten no panic or subprocess support //@ ignore-sgx no subprocess support +//@ ignore-fuchsia code returned as ZX_TASK_RETCODE_EXCEPTION_KILL, FIXME (#127539) #![cfg(test)] diff --git a/tests/ui/test-attrs/test-panic-abort-nocapture.run.stderr b/tests/ui/test-attrs/test-panic-abort-nocapture.run.stderr index 16001b3eecd4d..4c94518d4d197 100644 --- a/tests/ui/test-attrs/test-panic-abort-nocapture.run.stderr +++ b/tests/ui/test-attrs/test-panic-abort-nocapture.run.stderr @@ -1,9 +1,9 @@ -thread 'main' panicked at $DIR/test-panic-abort-nocapture.rs:34:5: +thread 'main' panicked at $DIR/test-panic-abort-nocapture.rs:35:5: assertion `left == right` failed left: 2 right: 4 note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace -thread 'main' panicked at $DIR/test-panic-abort-nocapture.rs:28:5: +thread 'main' panicked at $DIR/test-panic-abort-nocapture.rs:29:5: assertion `left == right` failed left: 2 right: 4 diff --git a/tests/ui/test-attrs/test-panic-abort.rs b/tests/ui/test-attrs/test-panic-abort.rs index 77efaf05bbc0d..c9f6439ef8946 100644 --- a/tests/ui/test-attrs/test-panic-abort.rs +++ b/tests/ui/test-attrs/test-panic-abort.rs @@ -10,6 +10,7 @@ //@ ignore-wasm no panic or subprocess support //@ ignore-emscripten no panic or subprocess support //@ ignore-sgx no subprocess support +//@ ignore-fuchsia code returned as ZX_TASK_RETCODE_EXCEPTION_KILL, FIXME (#127539) #![cfg(test)] #![feature(test)] diff --git a/tests/ui/test-attrs/test-panic-abort.run.stdout b/tests/ui/test-attrs/test-panic-abort.run.stdout index f5d14e77da963..25105f38fcf78 100644 --- a/tests/ui/test-attrs/test-panic-abort.run.stdout +++ b/tests/ui/test-attrs/test-panic-abort.run.stdout @@ -17,7 +17,7 @@ hello, world testing123 ---- it_fails stderr ---- testing321 -thread 'main' panicked at $DIR/test-panic-abort.rs:39:5: +thread 'main' panicked at $DIR/test-panic-abort.rs:40:5: assertion `left == right` failed left: 2 right: 5