Move RunCommandExtractHashedFiles() to test_runner.py
Moves the logic for RunCommandExtractHashedFiles() to test_runner.py, as
_TestCaseRunner.TestPixel() is the only client. The new implementation
also leverages _TestCaseRunner.RunCommand() for better error handling.
Reusing RunCommand() required adding support for redirecting standard
output to an in-memory io.BytesIO.
Also makes pixel test output less noisy.
Fixed: pdfium:1964
Change-Id: I597ff6a54570885e89f444c4b283449cfed92c48
Reviewed-on: https://pdfium-review.googlesource.com/c/pdfium/+/102910
Commit-Queue: K. Moon <kmoon@chromium.org>
Reviewed-by: Nigi <nigi@chromium.org>
diff --git a/testing/tools/common.py b/testing/tools/common.py
index 37f2ed3..80b7815 100755
--- a/testing/tools/common.py
+++ b/testing/tools/common.py
@@ -23,14 +23,6 @@
raise Exception('Confused, can not determine OS, aborting.')
-def RunCommand(cmd):
- try:
- subprocess.check_call(cmd)
- return None
- except subprocess.CalledProcessError as e:
- return e
-
-
def RunCommandPropagateErr(cmd,
stdout_has_errors=False,
exit_status_on_error=None):
@@ -63,25 +55,6 @@
return output
-# RunCommandExtractHashedFiles returns a tuple: (raised_exception, hashed_files)
-# It runs the given command. If it fails it will return an exception and None.
-# If it succeeds it will return None and the list of processed files extracted
-# from the output of the command. It expects lines in this format:
-# MD5:<path_to_image_file>:<md5_hash_in_hex>
-# The returned hashed_files is a list of (file_path, MD5-hash) pairs.
-def RunCommandExtractHashedFiles(cmd):
- try:
- output = subprocess.check_output(cmd, universal_newlines=True)
- ret = []
- for line in output.split('\n'):
- line = line.strip()
- if line.startswith("MD5:"):
- ret.append([x.strip() for x in line.lstrip("MD5:").rsplit(":", 1)])
- return None, ret
- except subprocess.CalledProcessError as e:
- return e, None
-
-
class DirectoryFinder:
'''A class for finding directories and paths under either a standalone
checkout or a chromium checkout of PDFium.'''
diff --git a/testing/tools/pngdiffer.py b/testing/tools/pngdiffer.py
index 4c371ea..2ab2a3f 100755
--- a/testing/tools/pngdiffer.py
+++ b/testing/tools/pngdiffer.py
@@ -8,10 +8,9 @@
import itertools
import os
import shutil
+import subprocess
import sys
-import common
-
class PathMode:
"""PathMode indicates the available expected results' paths.
@@ -80,16 +79,23 @@
break
return actual_paths
+ def _RunCommand(self, cmd):
+ try:
+ subprocess.run(cmd, capture_output=True, check=True)
+ return None
+ except subprocess.CalledProcessError as e:
+ return e
+
def _RunImageCompareCommand(self, image_diff):
cmd = [self.pdfium_diff_path]
if self.reverse_byte_order:
cmd.append('--reverse-byte-order')
cmd.extend([image_diff.actual_path, image_diff.expected_path])
- return common.RunCommand(cmd)
+ return self._RunCommand(cmd)
def _RunImageDiffCommand(self, image_diff):
# TODO(crbug.com/pdfium/1925): Diff mode ignores --reverse-byte-order.
- return common.RunCommand([
+ return self._RunCommand([
self.pdfium_diff_path, '--subtract', image_diff.actual_path,
image_diff.expected_path, image_diff.diff_path
])
diff --git a/testing/tools/test_runner.py b/testing/tools/test_runner.py
index 8835981..a065820 100644
--- a/testing/tools/test_runner.py
+++ b/testing/tools/test_runner.py
@@ -6,6 +6,7 @@
import argparse
from dataclasses import dataclass, field
from datetime import timedelta
+from io import BytesIO
import multiprocessing
import os
import re
@@ -555,7 +556,18 @@
Returns:
The test result.
"""
+
+ # Standard output and error are directed to the test log. If `stdout` was
+ # provided, redirect standard output to it instead.
if stdout:
+ assert stdout != subprocess.PIPE
+ try:
+ stdout.fileno()
+ except OSError:
+ # `stdout` doesn't have a file descriptor, so it can't be passed to
+ # `subprocess.run()` directly.
+ original_stdout = stdout
+ stdout = subprocess.PIPE
stderr = subprocess.PIPE
else:
stdout = subprocess.PIPE
@@ -578,8 +590,13 @@
test_result.status = result_types.TIMEOUT
test_result.reason = 'Command {} timed out'.format(run_result.cmd)
+ if stdout == subprocess.PIPE and stderr == subprocess.PIPE:
+ # Copy captured standard output to the original `stdout`.
+ original_stdout.write(run_result.stdout)
+
if not test_result.IsPass():
- if stdout == subprocess.PIPE:
+ # On failure, report captured output to the test log.
+ if stderr == subprocess.STDOUT:
test_result.log = run_result.stdout
else:
test_result.log = run_result.stderr
@@ -703,17 +720,20 @@
cmd_to_run.append(self.pdf_path)
- raised_exception, results = common.RunCommandExtractHashedFiles(cmd_to_run)
- if raised_exception:
- return self.test_case.NewResult(
- result_types.FAIL, reason=str(raised_exception))
+ with BytesIO() as command_output:
+ test_result = self.RunCommand(cmd_to_run, stdout=command_output)
+ if not test_result.IsPass():
+ return test_result
- test_result = self.test_case.NewResult(
- result_types.PASS,
- image_artifacts=[
- self._NewImageArtifact(image_path=image_path, md5_hash=md5_hash)
- for image_path, md5_hash in results
- ])
+ test_result.image_artifacts = []
+ for line in command_output.getvalue().splitlines():
+ # Expect this format: MD5:<path to image file>:<hexadecimal MD5 hash>
+ line = bytes.decode(line).strip()
+ if line.startswith('MD5:'):
+ image_path, md5_hash = line[4:].rsplit(':', 1)
+ test_result.image_artifacts.append(
+ self._NewImageArtifact(
+ image_path=image_path.strip(), md5_hash=md5_hash.strip()))
if self.actual_images:
image_diffs = _per_process_state.image_differ.ComputeDifferences(
