Commit d81e3afc authored by Brian Sheedy's avatar Brian Sheedy Committed by Commit Bot

Support diff images for local GPU pixel tests

Supports outputting the closest image known to Gold and its diff with
the produced image when running the GPU pixel tests locally. This should
improve the usability of the tests when debugging locally, as before
they only output the produced image and a link to all images known to
Gold for the test.

Bug: 1030349
Change-Id: I55cd724541f86edf8d6de534bd921d70d03d6eda
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1949727Reviewed-by: default avatarKenneth Russell <kbr@chromium.org>
Commit-Queue: Brian Sheedy <bsheedy@chromium.org>
Cr-Commit-Position: refs/heads/master@{#721626}
parent f1e4d06c
...@@ -39,6 +39,7 @@ else: ...@@ -39,6 +39,7 @@ else:
goldctl_bin = os.path.join(goldctl_bin, 'linux', 'goldctl') goldctl_bin = os.path.join(goldctl_bin, 'linux', 'goldctl')
SKIA_GOLD_INSTANCE = 'chrome-gpu' SKIA_GOLD_INSTANCE = 'chrome-gpu'
SKIA_GOLD_CORPUS = SKIA_GOLD_INSTANCE
@contextlib.contextmanager @contextlib.contextmanager
...@@ -127,8 +128,7 @@ class SkiaGoldIntegrationTestBase(gpu_integration_test.GpuIntegrationTest): ...@@ -127,8 +128,7 @@ class SkiaGoldIntegrationTestBase(gpu_integration_test.GpuIntegrationTest):
@classmethod @classmethod
def TearDownProcess(cls): def TearDownProcess(cls):
super(SkiaGoldIntegrationTestBase, cls).TearDownProcess() super(SkiaGoldIntegrationTestBase, cls).TearDownProcess()
if not cls.GetParsedCommandLineOptions().local_run: shutil.rmtree(cls._skia_gold_temp_dir)
shutil.rmtree(cls._skia_gold_temp_dir)
@classmethod @classmethod
def AddCommandlineArgs(cls, parser): def AddCommandlineArgs(cls, parser):
...@@ -427,6 +427,12 @@ class SkiaGoldIntegrationTestBase(gpu_integration_test.GpuIntegrationTest): ...@@ -427,6 +427,12 @@ class SkiaGoldIntegrationTestBase(gpu_integration_test.GpuIntegrationTest):
build_id_args + extra_imgtest_args) build_id_args + extra_imgtest_args)
subprocess.check_output(cmd, stderr=subprocess.STDOUT) subprocess.check_output(cmd, stderr=subprocess.STDOUT)
except CalledProcessError as e: except CalledProcessError as e:
# We don't want to bother printing out triage links for local runs.
# Instead, we print out local filepaths for debugging. However, we want
# these to be at the bottom of the output so they're easier to find, so
# that is handled later.
if self._IsLocalRun():
pass
# The triage link for the image is output to the failure file, so report # The triage link for the image is output to the failure file, so report
# that if it's available so it shows up in Milo. If for whatever reason # that if it's available so it shows up in Milo. If for whatever reason
# the file is not present or malformed, the triage link will still be # the file is not present or malformed, the triage link will still be
...@@ -434,7 +440,7 @@ class SkiaGoldIntegrationTestBase(gpu_integration_test.GpuIntegrationTest): ...@@ -434,7 +440,7 @@ class SkiaGoldIntegrationTestBase(gpu_integration_test.GpuIntegrationTest):
# If we're running on a trybot, instead generate a link to all results # If we're running on a trybot, instead generate a link to all results
# for the CL so that the user can visit a single page instead of # for the CL so that the user can visit a single page instead of
# clicking on multiple links on potentially multiple bots. # clicking on multiple links on potentially multiple bots.
if parsed_options.review_patch_issue: elif parsed_options.review_patch_issue:
cl_images = ('https://%s-gold.skia.org/search?' cl_images = ('https://%s-gold.skia.org/search?'
'issue=%s&new_clstore=true' % ( 'issue=%s&new_clstore=true' % (
SKIA_GOLD_INSTANCE, parsed_options.review_patch_issue)) SKIA_GOLD_INSTANCE, parsed_options.review_patch_issue))
...@@ -447,15 +453,36 @@ class SkiaGoldIntegrationTestBase(gpu_integration_test.GpuIntegrationTest): ...@@ -447,15 +453,36 @@ class SkiaGoldIntegrationTestBase(gpu_integration_test.GpuIntegrationTest):
logging.error('Failed to read contents of goldctl failure file') logging.error('Failed to read contents of goldctl failure file')
logging.error('goldctl failed with output: %s', e.output) logging.error('goldctl failed with output: %s', e.output)
if parsed_options.local_run: if self._IsLocalRun():
logging.error( # Intentionally not cleaned up so the user can look at its contents.
'Image produced by %s: file://%s', image_name, png_temp_file) diff_dir = tempfile.mkdtemp()
gold_images = ('https://%s-gold.skia.org/search?' cmd = [goldctl_bin, 'diff',
'match=name&metric=combined&pos=true&' '--corpus', SKIA_GOLD_CORPUS,
'query=name%%3D%s&unt=false' % ( '--instance', SKIA_GOLD_INSTANCE,
SKIA_GOLD_INSTANCE, image_name)) '--input', png_temp_file,
logging.error( '--test', image_name,
'Approved images for %s in Gold: %s', image_name, gold_images) '--work-dir', self._skia_gold_temp_dir,
'--out-dir', diff_dir,
]
try:
subprocess.check_output(cmd, stderr=subprocess.STDOUT)
except CalledProcessError as e:
logging.error('Failed to generate diffs from Gold: %s', e)
# The directory should contain "input-<hash>.png", "closest-<hash>.png",
# and "diff.png".
for f in os.listdir(diff_dir):
filepath = os.path.join(diff_dir, f)
if f.startswith("input-"):
logging.error("Image produced by %s: file://%s",
image_name, filepath)
elif f.startswith("closest-"):
logging.error("Closest image for %s: file://%s",
image_name, filepath)
elif f == "diff.png":
logging.error("Diff image for %s: file://%s",
image_name, filepath)
if self._ShouldReportGoldFailure(page): if self._ShouldReportGoldFailure(page):
raise Exception('goldctl command failed, see above for details') raise Exception('goldctl command failed, see above for details')
...@@ -529,7 +556,7 @@ class SkiaGoldIntegrationTestBase(gpu_integration_test.GpuIntegrationTest): ...@@ -529,7 +556,7 @@ class SkiaGoldIntegrationTestBase(gpu_integration_test.GpuIntegrationTest):
else: else:
with RunInChromiumSrc(): with RunInChromiumSrc():
try: try:
subprocess.check_call(['git', 'status'], shell=IsWin()) subprocess.check_output(['git', 'status'], shell=IsWin())
logging.warning( logging.warning(
'Automatically determined that test is running on a workstation') 'Automatically determined that test is running on a workstation')
cls._local_run = True cls._local_run = True
......
...@@ -280,9 +280,9 @@ reason, you can manually pass some flags to force the same behavior: ...@@ -280,9 +280,9 @@ reason, you can manually pass some flags to force the same behavior:
In order to get around the local run issues, simply pass the `--local-run=1` In order to get around the local run issues, simply pass the `--local-run=1`
flag to the tests. This will disable uploading, but otherwise go through the flag to the tests. This will disable uploading, but otherwise go through the
same steps as a test normally would. Each test will also print out a `file://` same steps as a test normally would. Each test will also print out `file://`
URL to the image it produces and a link to all approved images for that test in URLs to the produced image, the closest image for the test known to Gold, and
Gold. the diff between the two.
Because the image produced by the test locally is likely slightly different from Because the image produced by the test locally is likely slightly different from
any of the approved images in Gold, local test runs are likely to fail during any of the approved images in Gold, local test runs are likely to fail during
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment