From 39b125eb726ebe6910b82198b351a7624ee26a53 Mon Sep 17 00:00:00 2001 From: Dennis Kempin Date: Thu, 18 Aug 2022 21:15:26 +0000 Subject: [PATCH] Revert "test_runner: Support multiple profiles per test" This reverts commit 9ec4afc0ae23e739da343e4f661ac849ac2807cd. Reason for revert: Breaks post-submit https://ci.chromium.org/ui/p/crosvm/builders/ci/linux_x86_64/b8805461167965542065/overview Original change's description: > test_runner: Support multiple profiles per test > > Tests may spawn multiple processes (e.g. for running crosvm in > integration_tests). We want those to generate a separate profile > file to collect them as well. > > This should add coverage from integration tests into the coverage > data. > > BUG=b:239255082 > TEST=./tools/run_tests --generate-lcov coverage.lcov > > Change-Id: Ic6c6b0801b676c96c4692069c1cd6111edea6fc4 > Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/3840311 > Reviewed-by: Daniel Verkamp > Commit-Queue: Dennis Kempin > Tested-by: Dennis Kempin Bug: b:239255082 Change-Id: I958792f86a5d5bc24910c057c61242fe405db20d No-Presubmit: true No-Tree-Checks: true No-Try: true Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/3840169 Tested-by: Dennis Kempin Commit-Queue: Dennis Kempin Bot-Commit: Rubber Stamper Auto-Submit: Dennis Kempin --- tools/impl/test_runner.py | 30 +++++++++++++++------------- tools/impl/test_target.py | 41 ++++++++++++--------------------------- 2 files changed, 28 insertions(+), 43 deletions(-) diff --git a/tools/impl/test_runner.py b/tools/impl/test_runner.py index 221dc4f11c..e34481e78d 100644 --- a/tools/impl/test_runner.py +++ b/tools/impl/test_runner.py @@ -71,14 +71,14 @@ class ExecutableResults(object): success: bool, test_log: str, previous_attempts: List["ExecutableResults"], - profile_files: List[Path], + profile_file: Optional[Path], ): self.name = name self.binary_file = binary_file self.success = success self.test_log = test_log self.previous_attempts = previous_attempts - self.profile_files = profile_files + self.profile_file = profile_file class Executable(NamedTuple): @@ -250,7 +250,7 @@ def build_all_binaries(target: TestTarget, crosvm_direct: bool, instrument_cover print("Building crosvm workspace") features = BUILD_FEATURES[str(target.build_triple)] - extra_args: List[str] = [] + extra_args = [] if crosvm_direct: features += ",direct" extra_args.append("--no-default-features") @@ -316,22 +316,24 @@ def execute_test(target: TestTarget, attempts: int, collect_coverage: bool, exec print(f"Running test {executable.name} on {target}... (attempt {i}/{attempts})") try: + profile_file = None + if collect_coverage: + profile_file = binary_path.with_suffix(".profraw") + # Pipe stdout/err to be printed in the main process if needed. test_process = test_target.exec_file_on_target( target, binary_path, args=args, timeout=get_test_timeout(target, executable), - generate_profile=True, + profile_file=profile_file, stdout=subprocess.PIPE, stderr=subprocess.STDOUT, ) - profile_files: List[Path] = [] - if collect_coverage: - profile_files = [*test_target.list_profile_files(binary_path)] - if not profile_files: - print() - print(f"Warning: Running {binary_path} did not produce a profile file.") + if profile_file and not profile_file.exists(): + print() + print(f"Warning: Running {binary_path} did not produce profile file.") + profile_file = None result = ExecutableResults( executable.name, @@ -339,7 +341,7 @@ def execute_test(target: TestTarget, attempts: int, collect_coverage: bool, exec test_process.returncode == 0, test_process.stdout, previous_attempts, - profile_files, + profile_file, ) except subprocess.TimeoutExpired as e: # Append a note about the timeout to the stdout of the process. @@ -350,7 +352,7 @@ def execute_test(target: TestTarget, attempts: int, collect_coverage: bool, exec False, e.stdout.decode("utf-8") + msg, previous_attempts, - [], + None, ) if result.success: break @@ -423,7 +425,7 @@ def find_crosvm_binary(executables: List[Executable]): def generate_lcov(results: List[ExecutableResults], lcov_file: str): print("Merging profiles") merged_file = testvm.cargo_target_dir() / "merged.profraw" - profiles = [str(p) for r in results if r.profile_files for p in r.profile_files] + profiles = [str(r.profile_file) for r in results if r.profile_file] subprocess.check_call(["rust-profdata", "merge", "-sparse", *profiles, "-o", str(merged_file)]) print("Exporting report") @@ -432,7 +434,7 @@ def generate_lcov(results: List[ExecutableResults], lcov_file: str): "rust-cov", "export", "--format=lcov", - "--ignore-filename-regex='/registry/'", + "--ignore-filename-regex='/.cargo/registry'", f"--instr-profile={merged_file}", *(f"--object={r.binary_file}" for r in results), ], diff --git a/tools/impl/test_target.py b/tools/impl/test_target.py index c8a0f32748..43e5cbba36 100755 --- a/tools/impl/test_target.py +++ b/tools/impl/test_target.py @@ -110,12 +110,12 @@ class Ssh: ] return subprocess.run(scp_cmd, check=True) - def download_files( - self, remote_file: str, target_dir: Path, quiet: bool = False, check: bool = True + def download_file( + self, remote_file: str, local_file: Path, quiet: bool = False, check: bool = True ): """Wrapper around SCP.""" flags = ["-q"] if quiet else [] - scp_cmd = ["scp", *flags, *self.opts, f"{self.hostname}:{remote_file}", str(target_dir)] + scp_cmd = ["scp", *flags, *self.opts, f"{self.hostname}:{remote_file}", str(local_file)] return subprocess.run(scp_cmd, check=check) @@ -344,17 +344,13 @@ def set_target(target: TestTarget): print(f"Target Architecture: {target.build_triple}") -def list_profile_files(binary_path: Path): - return binary_path.parent.glob(f"{binary_path.name}.profraw.*") - - def exec_file_on_target( target: TestTarget, filepath: Path, timeout: int, args: List[str] = [], extra_files: List[Path] = [], - generate_profile: bool = False, + profile_file: Optional[Path] = None, **kwargs: Any, ): """Executes a file on the test target. @@ -367,19 +363,9 @@ def exec_file_on_target( Timeouts will trigger a subprocess.TimeoutExpired exception, which contanins any output produced by the subprocess until the timeout. - - Coverage profiles can be generated by setting `generate_profile` and will be written to - "$filepath.profraw.$PID". Existing profiles are deleted. """ env = os.environ.copy() prefix = target.emulator_cmd if target.emulator_cmd else [] - - # Delete existing profile files - profile_prefix = filepath.with_suffix(".profraw") - if generate_profile: - for path in list_profile_files(filepath): - path.unlink() - if not target.ssh: # Allow test binaries to find rust's test libs. if os.name == "posix": @@ -391,8 +377,8 @@ def exec_file_on_target( env["PATH"] += ";" + str(find_rust_lib_dir()) else: raise Exception(f"Unsupported build target: {os.name}") - if generate_profile: - env["LLVM_PROFILE_FILE"] = f"{profile_prefix}.%p" + if profile_file: + env["LLVM_PROFILE_FILE"] = str(profile_file) cmd_line = [*prefix, str(filepath), *args] return subprocess.run( @@ -407,10 +393,9 @@ def exec_file_on_target( target.ssh.upload_files([filepath] + extra_files, quiet=True) cmd_line = [*prefix, f"./{filename}", *args] - remote_profile_prefix = f"/tmp/{filename}.profraw" - if generate_profile: - target.ssh.check_output(f"sudo rm -f {remote_profile_prefix}*") - cmd_line = [f"LLVM_PROFILE_FILE={remote_profile_prefix}.%p", *cmd_line] + remote_profile_file = f"/tmp/{filename}.profraw" + if profile_file: + cmd_line = [f"LLVM_PROFILE_FILE={remote_profile_file}", *cmd_line] try: result = target.ssh.run( @@ -423,12 +408,10 @@ def exec_file_on_target( # Remove uploaded files regardless of test result all_filenames = [filename] + [f.name for f in extra_files] target.ssh.check_output(f"sudo rm {' '.join(all_filenames)}") - if generate_profile: + if profile_file: # Fail silently. Some tests don't write a profile file. - target.ssh.download_files( - f"{remote_profile_prefix}*", profile_prefix.parent, check=False, quiet=True - ) - target.ssh.check_output(f"sudo rm -f {remote_profile_prefix}*") + target.ssh.download_file(remote_profile_file, profile_file, check=False, quiet=True) + target.ssh.check_output(f"sudo rm -f {remote_profile_file}") return result