From 3eb77e4d5a381fa55197f6bd03c599e709146069 Mon Sep 17 00:00:00 2001 From: "commit-bot@chromium.org" Date: Fri, 4 Apr 2014 16:40:25 +0000 Subject: [PATCH] teach rebaseline_server how to compare results of multiple render_pictures runs BUG=skia:2230,skia:1942 NOTRY=True R=rmistry@google.com Author: epoger@google.com Review URL: https://codereview.chromium.org/216103004 git-svn-id: http://skia.googlecode.com/svn/trunk@14062 2bbb7eff-a529-9590-31e7-b0007b416f81 --- .../compare_rendered_pictures.py | 208 ++++++++++++++++++ .../compare_rendered_pictures_test.py | 61 +++++ gm/rebaseline_server/results.py | 30 +++ gm/rebaseline_server/results_test.py | 58 +++++ .../render_pictures_output/.gitattributes | 1 + .../bitmap-64bitMD5_11092453015575919668.png | Bin 0 -> 1970 bytes .../bitmap-64bitMD5_2520753504544298264.png | Bin 0 -> 1971 bytes .../after_patch/builder1/output.json | 8 + .../bitmap-64bitMD5_11092453015575919668.png | Bin 0 -> 1970 bytes .../bitmap-64bitMD5_8891695120562235492.png | Bin 0 -> 1971 bytes .../before_patch/builder1/output.json | 8 + .../compare_rendered_pictures.json | 90 ++++++++ 12 files changed, 464 insertions(+) create mode 100755 gm/rebaseline_server/compare_rendered_pictures.py create mode 100755 gm/rebaseline_server/compare_rendered_pictures_test.py create mode 100755 gm/rebaseline_server/results_test.py create mode 100644 gm/rebaseline_server/testdata/inputs/render_pictures_output/.gitattributes create mode 100644 gm/rebaseline_server/testdata/inputs/render_pictures_output/after_patch/builder1/bitmap-64bitMD5_11092453015575919668.png create mode 100644 gm/rebaseline_server/testdata/inputs/render_pictures_output/after_patch/builder1/bitmap-64bitMD5_2520753504544298264.png create mode 100644 gm/rebaseline_server/testdata/inputs/render_pictures_output/after_patch/builder1/output.json create mode 100644 gm/rebaseline_server/testdata/inputs/render_pictures_output/before_patch/builder1/bitmap-64bitMD5_11092453015575919668.png create mode 100644 gm/rebaseline_server/testdata/inputs/render_pictures_output/before_patch/builder1/bitmap-64bitMD5_8891695120562235492.png create mode 100644 gm/rebaseline_server/testdata/inputs/render_pictures_output/before_patch/builder1/output.json create mode 100644 gm/rebaseline_server/testdata/outputs/expected/compare_rendered_pictures_test.CompareRenderedPicturesTest.test_endToEnd/compare_rendered_pictures.json diff --git a/gm/rebaseline_server/compare_rendered_pictures.py b/gm/rebaseline_server/compare_rendered_pictures.py new file mode 100755 index 0000000000..14b1fb1d80 --- /dev/null +++ b/gm/rebaseline_server/compare_rendered_pictures.py @@ -0,0 +1,208 @@ +#!/usr/bin/python + +""" +Copyright 2014 Google Inc. + +Use of this source code is governed by a BSD-style license that can be +found in the LICENSE file. + +Compare results of two render_pictures runs. +""" + +# System-level imports +import logging +import os +import re +import sys +import time + +# Imports from within Skia +# +# TODO(epoger): Once we move the create_filepath_url() function out of +# download_actuals into a shared utility module, we won't need to import +# download_actuals anymore. +# +# We need to add the 'gm' directory, so that we can import gm_json.py within +# that directory. That script allows us to parse the actual-results.json file +# written out by the GM tool. +# Make sure that the 'gm' dir is in the PYTHONPATH, but add it at the *end* +# so any dirs that are already in the PYTHONPATH will be preferred. +PARENT_DIRECTORY = os.path.dirname(os.path.realpath(__file__)) +GM_DIRECTORY = os.path.dirname(PARENT_DIRECTORY) +TRUNK_DIRECTORY = os.path.dirname(GM_DIRECTORY) +if GM_DIRECTORY not in sys.path: + sys.path.append(GM_DIRECTORY) +import download_actuals +import gm_json +import imagediffdb +import imagepair +import imagepairset +import results + +# Characters we don't want popping up just anywhere within filenames. +DISALLOWED_FILEPATH_CHAR_REGEX = re.compile('[^\w\-]') + +# URL under which all render_pictures images can be found in Google Storage. +# TODO(epoger): Move this default value into +# https://skia.googlesource.com/buildbot/+/master/site_config/global_variables.json +DEFAULT_IMAGE_BASE_URL = 'http://chromium-skia-gm.commondatastorage.googleapis.com/render_pictures/images' + + +class RenderedPicturesComparisons(results.BaseComparisons): + """Loads results from two different render_pictures runs into an ImagePairSet. + """ + + def __init__(self, subdirs, actuals_root, + generated_images_root=results.DEFAULT_GENERATED_IMAGES_ROOT, + image_base_url=DEFAULT_IMAGE_BASE_URL, + diff_base_url=None): + """ + Args: + actuals_root: root directory containing all render_pictures-generated + JSON files + subdirs: (string, string) tuple; pair of subdirectories within + actuals_root to compare + generated_images_root: directory within which to create all pixel diffs; + if this directory does not yet exist, it will be created + image_base_url: URL under which all render_pictures result images can + be found; this will be used to read images for comparison within + this code, and included in the ImagePairSet so its consumers know + where to download the images from + diff_base_url: base URL within which the client should look for diff + images; if not specified, defaults to a "file:///" URL representation + of generated_images_root + """ + time_start = int(time.time()) + self._image_diff_db = imagediffdb.ImageDiffDB(generated_images_root) + self._image_base_url = image_base_url + self._diff_base_url = ( + diff_base_url or + download_actuals.create_filepath_url(generated_images_root)) + self._load_result_pairs(actuals_root, subdirs) + self._timestamp = int(time.time()) + logging.info('Results complete; took %d seconds.' % + (self._timestamp - time_start)) + + def _load_result_pairs(self, actuals_root, subdirs): + """Loads all JSON files found within two subdirs in actuals_root, + compares across those two subdirs, and stores the summary in self._results. + + Args: + actuals_root: root directory containing all render_pictures-generated + JSON files + subdirs: (string, string) tuple; pair of subdirectories within + actuals_root to compare + """ + logging.info( + 'Reading actual-results JSON files from %s subdirs within %s...' % ( + subdirs, actuals_root)) + subdirA, subdirB = subdirs + subdirA_builder_dicts = results.BaseComparisons._read_dicts_from_root( + os.path.join(actuals_root, subdirA)) + subdirB_builder_dicts = results.BaseComparisons._read_dicts_from_root( + os.path.join(actuals_root, subdirB)) + logging.info('Comparing subdirs %s and %s...' % (subdirA, subdirB)) + + all_image_pairs = imagepairset.ImagePairSet( + descriptions=subdirs, + diff_base_url=self._diff_base_url) + failing_image_pairs = imagepairset.ImagePairSet( + descriptions=subdirs, + diff_base_url=self._diff_base_url) + + all_image_pairs.ensure_extra_column_values_in_summary( + column_id=results.KEY__EXTRACOLUMN__RESULT_TYPE, values=[ + results.KEY__RESULT_TYPE__FAILED, + results.KEY__RESULT_TYPE__NOCOMPARISON, + results.KEY__RESULT_TYPE__SUCCEEDED, + ]) + failing_image_pairs.ensure_extra_column_values_in_summary( + column_id=results.KEY__EXTRACOLUMN__RESULT_TYPE, values=[ + results.KEY__RESULT_TYPE__FAILED, + results.KEY__RESULT_TYPE__NOCOMPARISON, + ]) + + builders = sorted(set(subdirA_builder_dicts.keys() + + subdirB_builder_dicts.keys())) + num_builders = len(builders) + builder_num = 0 + for builder in builders: + builder_num += 1 + logging.info('Generating pixel diffs for builder #%d of %d, "%s"...' % + (builder_num, num_builders, builder)) + # TODO(epoger): This will fail if we have results for this builder in + # subdirA but not subdirB (or vice versa). + subdirA_results = results.BaseComparisons.combine_subdicts( + subdirA_builder_dicts[builder][gm_json.JSONKEY_ACTUALRESULTS]) + subdirB_results = results.BaseComparisons.combine_subdicts( + subdirB_builder_dicts[builder][gm_json.JSONKEY_ACTUALRESULTS]) + image_names = sorted(set(subdirA_results.keys() + + subdirB_results.keys())) + for image_name in image_names: + # The image name may contain funny characters or be ridiculously long + # (see https://code.google.com/p/skia/issues/detail?id=2344#c10 ), + # so make sure we sanitize it before using it in a URL path. + # + # TODO(epoger): Rather than sanitizing/truncating the image name here, + # do it in render_pictures instead. + # Reason: we will need to be consistent in applying this rule, so that + # the process which uploads the files to GS using these paths will + # match the paths created by downstream processes. + # So, we should make render_pictures write out images to paths that are + # "ready to upload" to Google Storage, like gm does. + sanitized_test_name = DISALLOWED_FILEPATH_CHAR_REGEX.sub( + '_', image_name)[:30] + + subdirA_image_relative_url = ( + results.BaseComparisons._create_relative_url( + hashtype_and_digest=subdirA_results.get(image_name), + test_name=sanitized_test_name)) + subdirB_image_relative_url = ( + results.BaseComparisons._create_relative_url( + hashtype_and_digest=subdirB_results.get(image_name), + test_name=sanitized_test_name)) + + # If we have images for at least one of these two subdirs, + # add them to our list. + if subdirA_image_relative_url or subdirB_image_relative_url: + if subdirA_image_relative_url == subdirB_image_relative_url: + result_type = results.KEY__RESULT_TYPE__SUCCEEDED + elif not subdirA_image_relative_url: + result_type = results.KEY__RESULT_TYPE__NOCOMPARISON + elif not subdirB_image_relative_url: + result_type = results.KEY__RESULT_TYPE__NOCOMPARISON + else: + result_type = results.KEY__RESULT_TYPE__FAILED + + extra_columns_dict = { + results.KEY__EXTRACOLUMN__RESULT_TYPE: result_type, + results.KEY__EXTRACOLUMN__BUILDER: builder, + results.KEY__EXTRACOLUMN__TEST: image_name, + # TODO(epoger): Right now, the client UI crashes if it receives + # results that do not include a 'config' column. + # Until we fix that, keep the client happy. + results.KEY__EXTRACOLUMN__CONFIG: 'TODO', + } + + try: + image_pair = imagepair.ImagePair( + image_diff_db=self._image_diff_db, + base_url=self._image_base_url, + imageA_relative_url=subdirA_image_relative_url, + imageB_relative_url=subdirB_image_relative_url, + extra_columns=extra_columns_dict) + all_image_pairs.add_image_pair(image_pair) + if result_type != results.KEY__RESULT_TYPE__SUCCEEDED: + failing_image_pairs.add_image_pair(image_pair) + except (KeyError, TypeError): + logging.exception( + 'got exception while creating ImagePair for image_name ' + '"%s", builder "%s"' % (image_name, builder)) + + self._results = { + results.KEY__HEADER__RESULTS_ALL: all_image_pairs.as_dict(), + results.KEY__HEADER__RESULTS_FAILURES: failing_image_pairs.as_dict(), + } + + +# TODO(epoger): Add main() so this can be called by vm_run_skia_try.sh diff --git a/gm/rebaseline_server/compare_rendered_pictures_test.py b/gm/rebaseline_server/compare_rendered_pictures_test.py new file mode 100755 index 0000000000..c1eebb794b --- /dev/null +++ b/gm/rebaseline_server/compare_rendered_pictures_test.py @@ -0,0 +1,61 @@ +#!/usr/bin/python + +""" +Copyright 2014 Google Inc. + +Use of this source code is governed by a BSD-style license that can be +found in the LICENSE file. + +Test compare_rendered_pictures.py + +TODO(epoger): Create a command to update the expected results (in +self._output_dir_expected) when appropriate. For now, you should: +1. examine the results in self._output_dir_actual and make sure they are ok +2. rm -rf self._output_dir_expected +3. mv self._output_dir_actual self._output_dir_expected +Although, if you're using an SVN checkout, this will blow away .svn directories +within self._output_dir_expected, which wouldn't be good... + +""" + +import os +import sys + +# Imports from within Skia +import base_unittest +import compare_rendered_pictures +import results +import gm_json # must import results first, so that gm_json will be in sys.path + + +class CompareRenderedPicturesTest(base_unittest.TestCase): + + def test_endToEnd(self): + """Compare results of two render_pictures runs.""" + # TODO(epoger): Specify image_base_url pointing at the directory on local + # disk containing our test images, so that we can actually compute pixel + # diffs. For now, this test attempts to download images from + # DEFAULT_IMAGE_BASE_URL, and there aren't any there yet. + results_obj = compare_rendered_pictures.RenderedPicturesComparisons( + actuals_root=os.path.join(self._input_dir, 'render_pictures_output'), + subdirs=('before_patch', 'after_patch'), + generated_images_root=self._temp_dir, + diff_base_url='/static/generated-images') + results_obj.get_timestamp = mock_get_timestamp + gm_json.WriteToFile( + results_obj.get_packaged_results_of_type( + results.KEY__HEADER__RESULTS_ALL), + os.path.join(self._output_dir_actual, 'compare_rendered_pictures.json')) + + +def mock_get_timestamp(): + """Mock version of BaseComparisons.get_timestamp() for testing.""" + return 12345678 + + +def main(): + base_unittest.main(CompareRenderedPicturesTest) + + +if __name__ == '__main__': + main() diff --git a/gm/rebaseline_server/results.py b/gm/rebaseline_server/results.py index 7cf57453ca..49e32519a7 100755 --- a/gm/rebaseline_server/results.py +++ b/gm/rebaseline_server/results.py @@ -197,3 +197,33 @@ class BaseComparisons(object): test_name=test_name, hash_type=hashtype_and_digest[0], hash_digest=hashtype_and_digest[1]) + + @staticmethod + def combine_subdicts(input_dict): + """ Flatten out a dictionary structure by one level. + + Input: + { + "failed" : { + "changed.png" : [ "bitmap-64bitMD5", 8891695120562235492 ], + }, + "no-comparison" : { + "unchanged.png" : [ "bitmap-64bitMD5", 11092453015575919668 ], + } + } + + Output: + { + "changed.png" : [ "bitmap-64bitMD5", 8891695120562235492 ], + "unchanged.png" : [ "bitmap-64bitMD5", 11092453015575919668 ], + } + + If this would result in any repeated keys, it will raise an Exception. + """ + output_dict = {} + for key, subdict in input_dict.iteritems(): + for subdict_key, subdict_value in subdict.iteritems(): + if subdict_key in output_dict: + raise Exception('duplicate key %s in combine_subdicts' % subdict_key) + output_dict[subdict_key] = subdict_value + return output_dict diff --git a/gm/rebaseline_server/results_test.py b/gm/rebaseline_server/results_test.py new file mode 100755 index 0000000000..a2f4073dcf --- /dev/null +++ b/gm/rebaseline_server/results_test.py @@ -0,0 +1,58 @@ +#!/usr/bin/python + +""" +Copyright 2014 Google Inc. + +Use of this source code is governed by a BSD-style license that can be +found in the LICENSE file. + +Test results.py + +""" + +# Imports from within Skia +import base_unittest +import results + + +class ResultsTest(base_unittest.TestCase): + + def test_combine_subdicts_typical(self): + """Test combine_subdicts() with no merge conflicts. """ + input_dict = { + "failed" : { + "changed.png" : [ "bitmap-64bitMD5", 8891695120562235492 ], + }, + "no-comparison" : { + "unchanged.png" : [ "bitmap-64bitMD5", 11092453015575919668 ], + } + } + expected_output_dict = { + "changed.png" : [ "bitmap-64bitMD5", 8891695120562235492 ], + "unchanged.png" : [ "bitmap-64bitMD5", 11092453015575919668 ], + } + actual_output_dict = results.BaseComparisons.combine_subdicts( + input_dict=input_dict) + self.assertEqual(actual_output_dict, expected_output_dict) + + def test_combine_subdicts_with_merge_conflict(self): + """Test combine_subdicts() with a merge conflict. """ + input_dict = { + "failed" : { + "changed.png" : [ "bitmap-64bitMD5", 8891695120562235492 ], + }, + "no-comparison" : { + "changed.png" : [ "bitmap-64bitMD5", 11092453015575919668 ], + } + } + with self.assertRaises(Exception): + actual_output_dict = results.BaseComparisons.combine_subdicts( + input_dict=input_dict) + + +def main(): + base_unittest.main(ResultsTest) + + +if __name__ == '__main__': + main() diff --git a/gm/rebaseline_server/testdata/inputs/render_pictures_output/.gitattributes b/gm/rebaseline_server/testdata/inputs/render_pictures_output/.gitattributes new file mode 100644 index 0000000000..947f120640 --- /dev/null +++ b/gm/rebaseline_server/testdata/inputs/render_pictures_output/.gitattributes @@ -0,0 +1 @@ +*.png binary diff --git a/gm/rebaseline_server/testdata/inputs/render_pictures_output/after_patch/builder1/bitmap-64bitMD5_11092453015575919668.png b/gm/rebaseline_server/testdata/inputs/render_pictures_output/after_patch/builder1/bitmap-64bitMD5_11092453015575919668.png new file mode 100644 index 0000000000000000000000000000000000000000..349c961988de33b9f16aeac51d34f28ebb668727 GIT binary patch literal 1970 zcmeAS@N?(olHy`uVBq!ia0y~yU}|7sV4T3g1{AUTd{Grhu@pObhHwBu4M$1`0|R@e zr;B4q#hkadHu5$Y@Gv;CZ~TAi!sQFB3-@c!J@)kUsaZ3#85mAj?2BVyXxPi5z~Ios zz{1dBIiFdciIL%7?%qAS8JHN3a5ykDNHB6R2q-f!G9()bqxbT-Vv;=N-NE30Mm=c)I$ztaD0e F0ssTWCWZh2 literal 0 HcmV?d00001 diff --git a/gm/rebaseline_server/testdata/inputs/render_pictures_output/after_patch/builder1/output.json b/gm/rebaseline_server/testdata/inputs/render_pictures_output/after_patch/builder1/output.json new file mode 100644 index 0000000000..6b9acc0450 --- /dev/null +++ b/gm/rebaseline_server/testdata/inputs/render_pictures_output/after_patch/builder1/output.json @@ -0,0 +1,8 @@ +{ + "actual-results" : { + "no-comparison" : { + "changed.png" : [ "bitmap-64bitMD5", 2520753504544298264 ], + "unchanged.png" : [ "bitmap-64bitMD5", 11092453015575919668 ] + } + } +} diff --git a/gm/rebaseline_server/testdata/inputs/render_pictures_output/before_patch/builder1/bitmap-64bitMD5_11092453015575919668.png b/gm/rebaseline_server/testdata/inputs/render_pictures_output/before_patch/builder1/bitmap-64bitMD5_11092453015575919668.png new file mode 100644 index 0000000000000000000000000000000000000000..349c961988de33b9f16aeac51d34f28ebb668727 GIT binary patch literal 1970 zcmeAS@N?(olHy`uVBq!ia0y~yU}|7sV4T3g1{AUTd{Grhu@pObhHwBu4M$1`0|R@e zr;B4q#hkadHu5$Y@Gv;CZ~TAi!sQFB3-@c!J@)kUsaZ3#85mAj?2BVyXxPi5z~Ios zz{1dBIiFdciIL%7?%qAS8JHN3a5ykDNHB6R2q-f!G9()bqxb+}GLUSBrg!1=hk0p00i_>zopr E08!&5a{vGU literal 0 HcmV?d00001 diff --git a/gm/rebaseline_server/testdata/inputs/render_pictures_output/before_patch/builder1/output.json b/gm/rebaseline_server/testdata/inputs/render_pictures_output/before_patch/builder1/output.json new file mode 100644 index 0000000000..16e1316bda --- /dev/null +++ b/gm/rebaseline_server/testdata/inputs/render_pictures_output/before_patch/builder1/output.json @@ -0,0 +1,8 @@ +{ + "actual-results" : { + "no-comparison" : { + "changed.png" : [ "bitmap-64bitMD5", 8891695120562235492 ], + "unchanged.png" : [ "bitmap-64bitMD5", 11092453015575919668 ] + } + } +} diff --git a/gm/rebaseline_server/testdata/outputs/expected/compare_rendered_pictures_test.CompareRenderedPicturesTest.test_endToEnd/compare_rendered_pictures.json b/gm/rebaseline_server/testdata/outputs/expected/compare_rendered_pictures_test.CompareRenderedPicturesTest.test_endToEnd/compare_rendered_pictures.json new file mode 100644 index 0000000000..c60c7873f9 --- /dev/null +++ b/gm/rebaseline_server/testdata/outputs/expected/compare_rendered_pictures_test.CompareRenderedPicturesTest.test_endToEnd/compare_rendered_pictures.json @@ -0,0 +1,90 @@ +{ + "extraColumnHeaders": { + "builder": { + "headerText": "builder", + "isFilterable": true, + "isSortable": true, + "valuesAndCounts": { + "builder1": 2 + } + }, + "config": { + "headerText": "config", + "isFilterable": true, + "isSortable": true, + "valuesAndCounts": { + "TODO": 2 + } + }, + "resultType": { + "headerText": "resultType", + "isFilterable": true, + "isSortable": true, + "valuesAndCounts": { + "failed": 1, + "no-comparison": 0, + "succeeded": 1 + } + }, + "test": { + "headerText": "test", + "isFilterable": true, + "isSortable": true, + "valuesAndCounts": { + "changed.png": 1, + "unchanged.png": 1 + } + } + }, + "header": { + "dataHash": "3972200251153667246", + "isEditable": false, + "isExported": true, + "schemaVersion": 2, + "timeNextUpdateAvailable": null, + "timeUpdated": 12345678, + "type": "all" + }, + "imagePairs": [ + { + "extraColumns": { + "builder": "builder1", + "config": "TODO", + "resultType": "failed", + "test": "changed.png" + }, + "imageAUrl": "bitmap-64bitMD5/changed_png/8891695120562235492.png", + "imageBUrl": "bitmap-64bitMD5/changed_png/2520753504544298264.png", + "isDifferent": true + }, + { + "extraColumns": { + "builder": "builder1", + "config": "TODO", + "resultType": "succeeded", + "test": "unchanged.png" + }, + "imageAUrl": "bitmap-64bitMD5/unchanged_png/11092453015575919668.png", + "imageBUrl": "bitmap-64bitMD5/unchanged_png/11092453015575919668.png", + "isDifferent": false + } + ], + "imageSets": { + "diffs": { + "baseUrl": "/static/generated-images/diffs", + "description": "color difference per channel" + }, + "imageA": { + "baseUrl": "http://chromium-skia-gm.commondatastorage.googleapis.com/render_pictures/images", + "description": "before_patch" + }, + "imageB": { + "baseUrl": "http://chromium-skia-gm.commondatastorage.googleapis.com/render_pictures/images", + "description": "after_patch" + }, + "whiteDiffs": { + "baseUrl": "/static/generated-images/whitediffs", + "description": "differing pixels in white" + } + } +} \ No newline at end of file -- 2.34.1