Commit 5af51966 authored by Brian Sheedy's avatar Brian Sheedy Committed by Commit Bot

Minor Gold Python code refactor

Does the following minor refactoring, prompted by the fact that ANGLE
is in the process of adopting Gold and can make use of some existing
code if it's moved into the common implementation.

1. Moves //content/test/gpu/gpu_tests/skia_gold/gpu_skia_gold_session.py
   and its associated tests to
   //build/skia_gold_common/output_managerless_skia_gold_session.py.
2. Makes 'chrome' the default instance in SkiaGoldSessionManager since
   the GPU tests have moved to using that instance and thus all tests
   use the 'chrome' instance.
3. Makes _GetSessionClass() no longer protected and uses its return
   value in test runners instead of manually importing the class.

Bug: angleproject:4090
Change-Id: I1c2743d2dbe10e168f00e5f169f25fe9568c7c94
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2427354
Auto-Submit: Brian Sheedy <bsheedy@chromium.org>
Reviewed-by: default avatarJamie Madill <jmadill@chromium.org>
Reviewed-by: default avatarTibor Goldschwendt <tiborg@chromium.org>
Commit-Queue: Brian Sheedy <bsheedy@chromium.org>
Cr-Commit-Position: refs/heads/master@{#810055}
parent f29b8e7c
...@@ -1091,7 +1091,8 @@ class LocalDeviceInstrumentationTestRun( ...@@ -1091,7 +1091,8 @@ class LocalDeviceInstrumentationTestRun(
failure_log = ( failure_log = (
'Skia Gold reported failure for RenderTest %s. See ' 'Skia Gold reported failure for RenderTest %s. See '
'RENDER_TESTS.md for how to fix this failure.' % render_name) 'RENDER_TESTS.md for how to fix this failure.' % render_name)
status_codes = gold_utils.AndroidSkiaGoldSession.StatusCodes status_codes =\
self._skia_gold_session_manager.GetSessionClass().StatusCodes
if status == status_codes.AUTH_FAILURE: if status == status_codes.AUTH_FAILURE:
_AppendToLog(results, _AppendToLog(results,
'Gold authentication failed with output %s' % error) 'Gold authentication failed with output %s' % error)
......
...@@ -68,11 +68,7 @@ class AndroidSkiaGoldSession(skia_gold_session.SkiaGoldSession): ...@@ -68,11 +68,7 @@ class AndroidSkiaGoldSession(skia_gold_session.SkiaGoldSession):
class AndroidSkiaGoldSessionManager( class AndroidSkiaGoldSessionManager(
skia_gold_session_manager.SkiaGoldSessionManager): skia_gold_session_manager.SkiaGoldSessionManager):
@staticmethod @staticmethod
def _GetDefaultInstance(): def GetSessionClass():
return 'chrome'
@staticmethod
def _GetSessionClass():
return AndroidSkiaGoldSession return AndroidSkiaGoldSession
......
# Copyright 2020 The Chromium Authors. All rights reserved. # Copyright 2020 The Chromium Authors. All rights reserved.
# Use of this source code is governed by a BSD-style license that can be # Use of this source code is governed by a BSD-style license that can be
# found in the LICENSE file. # found in the LICENSE file.
"""GPU implementation of //testing/skia_gold_common/skia_gold_session.py.""" """Implementation of skia_gold_session.py without output managers.
Diff output is instead stored in a directory and pointed to with file:// URLs.
"""
import os import os
import subprocess import subprocess
import tempfile import tempfile
from gpu_tests import path_util
path_util.AddDirToPathIfNeeded(path_util.GetChromiumSrcDir(), 'build')
from skia_gold_common import skia_gold_session from skia_gold_common import skia_gold_session
class GpuSkiaGoldSession(skia_gold_session.SkiaGoldSession): class OutputManagerlessSkiaGoldSession(skia_gold_session.SkiaGoldSession):
def RunComparison( # pylint: disable=too-many-arguments def RunComparison( # pylint: disable=too-many-arguments
self, self,
name, name,
...@@ -23,7 +24,7 @@ class GpuSkiaGoldSession(skia_gold_session.SkiaGoldSession): ...@@ -23,7 +24,7 @@ class GpuSkiaGoldSession(skia_gold_session.SkiaGoldSession):
# Passing True for the output manager is a bit of a hack, as we don't # Passing True for the output manager is a bit of a hack, as we don't
# actually need an output manager and just need to get past the truthy # actually need an output manager and just need to get past the truthy
# check. # check.
return super(GpuSkiaGoldSession, self).RunComparison( return super(OutputManagerlessSkiaGoldSession, self).RunComparison(
name=name, name=name,
png_file=png_file, png_file=png_file,
output_manager=output_manager, output_manager=output_manager,
......
...@@ -11,13 +11,10 @@ import unittest ...@@ -11,13 +11,10 @@ import unittest
import mock import mock
from gpu_tests import path_util
from gpu_tests.skia_gold import gpu_skia_gold_properties
from gpu_tests.skia_gold import gpu_skia_gold_session
from pyfakefs import fake_filesystem_unittest from pyfakefs import fake_filesystem_unittest
path_util.AddDirToPathIfNeeded(path_util.GetChromiumSrcDir(), 'build') from skia_gold_common import output_managerless_skia_gold_session as omsgs
from skia_gold_common import skia_gold_properties
from skia_gold_common import unittest_utils from skia_gold_common import unittest_utils
createSkiaGoldArgs = unittest_utils.createSkiaGoldArgs createSkiaGoldArgs = unittest_utils.createSkiaGoldArgs
...@@ -34,17 +31,17 @@ class GpuSkiaGoldSessionDiffTest(fake_filesystem_unittest.TestCase): ...@@ -34,17 +31,17 @@ class GpuSkiaGoldSessionDiffTest(fake_filesystem_unittest.TestCase):
self._working_dir = tempfile.mkdtemp() self._working_dir = tempfile.mkdtemp()
self._json_keys = tempfile.NamedTemporaryFile(delete=False).name self._json_keys = tempfile.NamedTemporaryFile(delete=False).name
@mock.patch.object(gpu_skia_gold_session.GpuSkiaGoldSession, @mock.patch.object(omsgs.OutputManagerlessSkiaGoldSession,
'_RunCmdForRcAndOutput') '_RunCmdForRcAndOutput')
def test_commandCommonArgs(self, cmd_mock): def test_commandCommonArgs(self, cmd_mock):
cmd_mock.return_value = (None, None) cmd_mock.return_value = (None, None)
args = createSkiaGoldArgs(git_revision='a', local_pixel_tests=False) args = createSkiaGoldArgs(git_revision='a', local_pixel_tests=False)
sgp = gpu_skia_gold_properties.GpuSkiaGoldProperties(args) sgp = skia_gold_properties.SkiaGoldProperties(args)
session = gpu_skia_gold_session.GpuSkiaGoldSession(self._working_dir, session = omsgs.OutputManagerlessSkiaGoldSession(self._working_dir,
sgp, sgp,
self._json_keys, self._json_keys,
'corpus', 'corpus',
instance='instance') instance='instance')
session.Diff('name', 'png_file', None) session.Diff('name', 'png_file', None)
call_args = cmd_mock.call_args[0][0] call_args = cmd_mock.call_args[0][0]
self.assertIn('diff', call_args) self.assertIn('diff', call_args)
...@@ -67,7 +64,8 @@ class GpuSkiaGoldSessionDiffTest(fake_filesystem_unittest.TestCase): ...@@ -67,7 +64,8 @@ class GpuSkiaGoldSessionDiffTest(fake_filesystem_unittest.TestCase):
self.assertNotIn(self._working_dir, call_args[i + 1]) self.assertNotIn(self._working_dir, call_args[i + 1])
class GpuSkiaGoldSessionStoreDiffLinksTest(fake_filesystem_unittest.TestCase): class OutputManagerlessSkiaGoldSessionStoreDiffLinksTest(
fake_filesystem_unittest.TestCase):
def setUp(self): def setUp(self):
self.setUpPyfakefs() self.setUpPyfakefs()
self._working_dir = tempfile.mkdtemp() self._working_dir = tempfile.mkdtemp()
...@@ -75,10 +73,10 @@ class GpuSkiaGoldSessionStoreDiffLinksTest(fake_filesystem_unittest.TestCase): ...@@ -75,10 +73,10 @@ class GpuSkiaGoldSessionStoreDiffLinksTest(fake_filesystem_unittest.TestCase):
def test_outputManagerNotNeeded(self): def test_outputManagerNotNeeded(self):
args = createSkiaGoldArgs(git_revision='a', local_pixel_tests=True) args = createSkiaGoldArgs(git_revision='a', local_pixel_tests=True)
sgp = gpu_skia_gold_properties.GpuSkiaGoldProperties(args) sgp = skia_gold_properties.SkiaGoldProperties(args)
session = gpu_skia_gold_session.GpuSkiaGoldSession(self._working_dir, sgp, session = omsgs.OutputManagerlessSkiaGoldSession(self._working_dir, sgp,
self._json_keys, None, self._json_keys, None,
None) None)
input_filepath = os.path.join(self._working_dir, 'input-inputhash.png') input_filepath = os.path.join(self._working_dir, 'input-inputhash.png')
with open(input_filepath, 'w') as f: with open(input_filepath, 'w') as f:
f.write('') f.write('')
......
...@@ -52,8 +52,8 @@ class SkiaGoldSessionManager(object): ...@@ -52,8 +52,8 @@ class SkiaGoldSessionManager(object):
if not session: if not session:
working_dir = tempfile.mkdtemp(dir=self._working_dir) working_dir = tempfile.mkdtemp(dir=self._working_dir)
keys_file = _GetKeysAsJson(keys_input, working_dir) keys_file = _GetKeysAsJson(keys_input, working_dir)
session = self._GetSessionClass()(working_dir, self._gold_properties, session = self.GetSessionClass()(working_dir, self._gold_properties,
keys_file, corpus, instance) keys_file, corpus, instance)
self._sessions[instance][corpus][keys_string] = session self._sessions[instance][corpus][keys_string] = session
return session return session
...@@ -64,10 +64,10 @@ class SkiaGoldSessionManager(object): ...@@ -64,10 +64,10 @@ class SkiaGoldSessionManager(object):
Returns: Returns:
A string containing the default instance. A string containing the default instance.
""" """
raise NotImplementedError return 'chrome'
@staticmethod @staticmethod
def _GetSessionClass(): def GetSessionClass():
"""Gets the SkiaGoldSession class to use for session creation. """Gets the SkiaGoldSession class to use for session creation.
Returns: Returns:
......
...@@ -29,7 +29,7 @@ class SkiaGoldSessionManagerGetSessionTest(fake_filesystem_unittest.TestCase): ...@@ -29,7 +29,7 @@ class SkiaGoldSessionManagerGetSessionTest(fake_filesystem_unittest.TestCase):
self.setUpPyfakefs() self.setUpPyfakefs()
self._working_dir = tempfile.mkdtemp() self._working_dir = tempfile.mkdtemp()
self._patcher = mock.patch.object( self._patcher = mock.patch.object(
skia_gold_session_manager.SkiaGoldSessionManager, '_GetSessionClass') skia_gold_session_manager.SkiaGoldSessionManager, 'GetSessionClass')
self._session_class_mock = self._patcher.start() self._session_class_mock = self._patcher.start()
self._session_class_mock.return_value = skia_gold_session.SkiaGoldSession self._session_class_mock.return_value = skia_gold_session.SkiaGoldSession
self.addCleanup(self._patcher.stop) self.addCleanup(self._patcher.stop)
......
...@@ -4,17 +4,13 @@ ...@@ -4,17 +4,13 @@
"""GPU impl of //testing/skia_gold_common/skia_gold_session_manager.py.""" """GPU impl of //testing/skia_gold_common/skia_gold_session_manager.py."""
from gpu_tests import path_util from gpu_tests import path_util
from gpu_tests.skia_gold import gpu_skia_gold_session
path_util.AddDirToPathIfNeeded(path_util.GetChromiumSrcDir(), 'build') path_util.AddDirToPathIfNeeded(path_util.GetChromiumSrcDir(), 'build')
from skia_gold_common import output_managerless_skia_gold_session
from skia_gold_common import skia_gold_session_manager as sgsm from skia_gold_common import skia_gold_session_manager as sgsm
class GpuSkiaGoldSessionManager(sgsm.SkiaGoldSessionManager): class GpuSkiaGoldSessionManager(sgsm.SkiaGoldSessionManager):
@staticmethod @staticmethod
def _GetDefaultInstance(): def GetSessionClass():
return 'chrome' return output_managerless_skia_gold_session.OutputManagerlessSkiaGoldSession
@staticmethod
def _GetSessionClass():
return gpu_skia_gold_session.GpuSkiaGoldSession
...@@ -15,7 +15,6 @@ from gpu_tests import common_browser_args as cba ...@@ -15,7 +15,6 @@ from gpu_tests import common_browser_args as cba
from gpu_tests import gpu_integration_test from gpu_tests import gpu_integration_test
from gpu_tests import path_util from gpu_tests import path_util
from gpu_tests.skia_gold import gpu_skia_gold_properties from gpu_tests.skia_gold import gpu_skia_gold_properties
from gpu_tests.skia_gold import gpu_skia_gold_session
from gpu_tests.skia_gold import gpu_skia_gold_session_manager from gpu_tests.skia_gold import gpu_skia_gold_session_manager
from py_utils import cloud_storage from py_utils import cloud_storage
...@@ -358,7 +357,8 @@ class SkiaGoldIntegrationTestBase(gpu_integration_test.GpuIntegrationTest): ...@@ -358,7 +357,8 @@ class SkiaGoldIntegrationTestBase(gpu_integration_test.GpuIntegrationTest):
if not status: if not status:
return return
status_codes = gpu_skia_gold_session.GpuSkiaGoldSession.StatusCodes status_codes =\
self.GetSkiaGoldSessionManager().GetSessionClass().StatusCodes
if status == status_codes.AUTH_FAILURE: if status == status_codes.AUTH_FAILURE:
logging.error('Gold authentication failed with output %s', error) logging.error('Gold authentication failed with output %s', error)
elif status == status_codes.INIT_FAILURE: elif status == status_codes.INIT_FAILURE:
......
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