rebaseline.py: add --bugs and --unreviewed flags
BUG=skia:1569
R=borenet@google.com
Review URL: https://codereview.chromium.org/23478011
git-svn-id: http://skia.googlecode.com/svn/trunk@11058 2bbb7eff-a529-9590-31e7-b0007b416f81
diff --git a/gm/gm_json.py b/gm/gm_json.py
index 44ec5ea..c2a01f1 100644
--- a/gm/gm_json.py
+++ b/gm/gm_json.py
@@ -21,31 +21,57 @@
# These constants must be kept in sync with the kJsonKey_ constants in
# gm_expectations.cpp !
+
JSONKEY_ACTUALRESULTS = 'actual-results'
+
# Tests whose results failed to match expectations.
JSONKEY_ACTUALRESULTS_FAILED = 'failed'
+
# Tests whose results failed to match expectations, but IGNOREFAILURE causes
# us to take them less seriously.
JSONKEY_ACTUALRESULTS_FAILUREIGNORED = 'failure-ignored'
+
# Tests for which we do not have any expectations. They may be new tests that
# we haven't had a chance to check in expectations for yet, or we may have
# consciously decided to leave them without expectations because we are unhappy
# with the results (although we should try to move away from that, and instead
# check in expectations with the IGNOREFAILURE flag set).
JSONKEY_ACTUALRESULTS_NOCOMPARISON = 'no-comparison'
+
# Tests whose results matched their expectations.
JSONKEY_ACTUALRESULTS_SUCCEEDED = 'succeeded'
+
JSONKEY_EXPECTEDRESULTS = 'expected-results'
+
# One or more [HashType/DigestValue] pairs representing valid results for this
# test. Typically, there will just be one pair, but we allow for multiple
# expectations, and the test will pass if any one of them is matched.
JSONKEY_EXPECTEDRESULTS_ALLOWEDDIGESTS = 'allowed-digests'
+
+# Optional: one or more integers listing Skia bugs (under
+# https://code.google.com/p/skia/issues/list ) that pertain to this expectation.
+JSONKEY_EXPECTEDRESULTS_BUGS = 'bugs'
+
# If IGNOREFAILURE is set to True, a failure of this test will be reported
# within the FAILUREIGNORED section (thus NOT causing the buildbots to go red)
# rather than the FAILED section (which WOULD cause the buildbots to go red).
JSONKEY_EXPECTEDRESULTS_IGNOREFAILURE = 'ignore-failure'
+# Optional: a free-form text string with human-readable information about
+# this expectation.
+JSONKEY_EXPECTEDRESULTS_NOTES = 'notes'
+
+# Optional: boolean indicating whether this expectation was reviewed/approved
+# by a human being.
+# If True: a human looked at this image and approved it.
+# If False: this expectation was committed blind. (In such a case, please
+# add notes indicating why!)
+# If absent: this expectation was committed by a tool that didn't enforce human
+# review of expectations.
+JSONKEY_EXPECTEDRESULTS_REVIEWED = 'reviewed-by-human'
+
+
# Allowed hash types for test expectations.
JSONKEY_HASHTYPE_BITMAP_64BITMD5 = 'bitmap-64bitMD5'
diff --git a/tools/rebaseline.py b/tools/rebaseline.py
index 5129281..bf21e87 100755
--- a/tools/rebaseline.py
+++ b/tools/rebaseline.py
@@ -160,10 +160,18 @@
# rebaseline whatever configs the JSON results summary file tells
# us to
# add_new: if True, add expectations for tests which don't have any yet
+ # bugs: optional list of bug numbers which pertain to these expectations
+ # notes: free-form text notes to add to all updated expectations
+ # mark_unreviewed: if True, mark these expectations as NOT having been
+ # reviewed by a human; otherwise, leave that field blank.
+ # Currently, there is no way to make this script mark
+ # expectations as reviewed-by-human=True.
+ # TODO(epoger): Add that capability to a review tool.
def __init__(self, expectations_root, expectations_input_filename,
expectations_output_filename, actuals_base_url,
actuals_filename, exception_handler,
- tests=None, configs=None, add_new=False):
+ tests=None, configs=None, add_new=False, bugs=None, notes=None,
+ mark_unreviewed=None):
self._expectations_root = expectations_root
self._expectations_input_filename = expectations_input_filename
self._expectations_output_filename = expectations_output_filename
@@ -173,6 +181,9 @@
self._actuals_filename = actuals_filename
self._exception_handler = exception_handler
self._add_new = add_new
+ self._bugs = bugs
+ self._notes = notes
+ self._mark_unreviewed = mark_unreviewed
self._image_filename_re = re.compile(gm_json.IMAGE_FILENAME_PATTERN)
self._using_svn = os.path.isdir(os.path.join(expectations_root, '.svn'))
@@ -246,11 +257,12 @@
# results we need to update.
actuals_url = '/'.join([self._actuals_base_url,
builder, self._actuals_filename])
- # In most cases, we won't need to re-record results that are already
- # succeeding, but including the SUCCEEDED results will allow us to
- # re-record expectations if they somehow get out of sync.
- sections = [gm_json.JSONKEY_ACTUALRESULTS_FAILED,
- gm_json.JSONKEY_ACTUALRESULTS_SUCCEEDED]
+ # Only update results for tests that are currently failing.
+ # We don't want to rewrite results for tests that are already succeeding,
+ # because we don't want to add annotation fields (such as
+ # JSONKEY_EXPECTEDRESULTS_BUGS) except for tests whose expectations we
+ # are actually modifying.
+ sections = [gm_json.JSONKEY_ACTUALRESULTS_FAILED]
if self._add_new:
sections.append(gm_json.JSONKEY_ACTUALRESULTS_NOCOMPARISON)
results_to_update = self._GetActualResults(json_url=actuals_url,
@@ -278,8 +290,21 @@
continue
if not expected_results.get(image_name):
expected_results[image_name] = {}
- expected_results[image_name][gm_json.JSONKEY_EXPECTEDRESULTS_ALLOWEDDIGESTS] = \
- [image_results]
+ expected_results[image_name]\
+ [gm_json.JSONKEY_EXPECTEDRESULTS_ALLOWEDDIGESTS]\
+ = [image_results]
+ if self._mark_unreviewed:
+ expected_results[image_name]\
+ [gm_json.JSONKEY_EXPECTEDRESULTS_REVIEWED]\
+ = False
+ if self._bugs:
+ expected_results[image_name]\
+ [gm_json.JSONKEY_EXPECTEDRESULTS_BUGS]\
+ = self._bugs
+ if self._notes:
+ expected_results[image_name]\
+ [gm_json.JSONKEY_EXPECTEDRESULTS_NOTES]\
+ = self._notes
# Write out updated expectations.
expectations_output_filepath = os.path.join(
@@ -296,57 +321,71 @@
parser = argparse.ArgumentParser()
parser.add_argument('--actuals-base-url',
- help='base URL from which to read files containing JSON ' +
- 'summaries of actual GM results; defaults to %(default)s',
+ help=('base URL from which to read files containing JSON '
+ 'summaries of actual GM results; defaults to '
+ '%(default)s'),
default='http://skia-autogen.googlecode.com/svn/gm-actual')
parser.add_argument('--actuals-filename',
- help='filename (within builder-specific subdirectories ' +
- 'of ACTUALS_BASE_URL) to read a summary of results from; ' +
- 'defaults to %(default)s',
+ help=('filename (within builder-specific subdirectories '
+ 'of ACTUALS_BASE_URL) to read a summary of results '
+ 'from; defaults to %(default)s'),
default='actual-results.json')
# TODO(epoger): Add test that exercises --add-new argument.
parser.add_argument('--add-new', action='store_true',
- help='in addition to the standard behavior of ' +
- 'updating expectations for failing tests, add ' +
- 'expectations for tests which don\'t have expectations ' +
- 'yet.')
+ help=('in addition to the standard behavior of '
+ 'updating expectations for failing tests, add '
+ 'expectations for tests which don\'t have '
+ 'expectations yet.'))
+parser.add_argument('--bugs', metavar='BUG', type=int, nargs='+',
+ help=('Skia bug numbers (under '
+ 'https://code.google.com/p/skia/issues/list ) which '
+ 'pertain to this set of rebaselines.'))
parser.add_argument('--builders', metavar='BUILDER', nargs='+',
- help='which platforms to rebaseline; ' +
- 'if unspecified, rebaseline all platforms, same as ' +
- '"--builders %s"' % ' '.join(sorted(TEST_BUILDERS)))
+ help=('which platforms to rebaseline; '
+ 'if unspecified, rebaseline all platforms, same as '
+ '"--builders %s"' % ' '.join(sorted(TEST_BUILDERS))))
# TODO(epoger): Add test that exercises --configs argument.
parser.add_argument('--configs', metavar='CONFIG', nargs='+',
- help='which configurations to rebaseline, e.g. ' +
- '"--configs 565 8888", as a filter over the full set of ' +
- 'results in ACTUALS_FILENAME; if unspecified, rebaseline ' +
- '*all* configs that are available.')
+ help=('which configurations to rebaseline, e.g. '
+ '"--configs 565 8888", as a filter over the full set '
+ 'of results in ACTUALS_FILENAME; if unspecified, '
+ 'rebaseline *all* configs that are available.'))
parser.add_argument('--expectations-filename',
- help='filename (under EXPECTATIONS_ROOT) to read ' +
- 'current expectations from, and to write new ' +
- 'expectations into (unless a separate ' +
- 'EXPECTATIONS_FILENAME_OUTPUT has been specified); ' +
- 'defaults to %(default)s',
+ help=('filename (under EXPECTATIONS_ROOT) to read '
+ 'current expectations from, and to write new '
+ 'expectations into (unless a separate '
+ 'EXPECTATIONS_FILENAME_OUTPUT has been specified); '
+ 'defaults to %(default)s'),
default='expected-results.json')
parser.add_argument('--expectations-filename-output',
- help='filename (under EXPECTATIONS_ROOT) to write ' +
- 'updated expectations into; by default, overwrites the ' +
- 'input file (EXPECTATIONS_FILENAME)',
+ help=('filename (under EXPECTATIONS_ROOT) to write '
+ 'updated expectations into; by default, overwrites '
+ 'the input file (EXPECTATIONS_FILENAME)'),
default='')
parser.add_argument('--expectations-root',
- help='root of expectations directory to update-- should ' +
- 'contain one or more builder subdirectories. Defaults to ' +
- '%(default)s',
+ help=('root of expectations directory to update-- should '
+ 'contain one or more builder subdirectories. '
+ 'Defaults to %(default)s'),
default=os.path.join('expectations', 'gm'))
parser.add_argument('--keep-going-on-failure', action='store_true',
- help='instead of halting at the first error encountered, ' +
- 'keep going and rebaseline as many tests as possible, ' +
- 'and then report the full set of errors at the end')
+ help=('instead of halting at the first error encountered, '
+ 'keep going and rebaseline as many tests as '
+ 'possible, and then report the full set of errors '
+ 'at the end'))
+parser.add_argument('--notes',
+ help=('free-form text notes to add to all updated '
+ 'expectations'))
# TODO(epoger): Add test that exercises --tests argument.
parser.add_argument('--tests', metavar='TEST', nargs='+',
- help='which tests to rebaseline, e.g. ' +
- '"--tests aaclip bigmatrix", as a filter over the full ' +
- 'set of results in ACTUALS_FILENAME; if unspecified, ' +
- 'rebaseline *all* tests that are available.')
+ help=('which tests to rebaseline, e.g. '
+ '"--tests aaclip bigmatrix", as a filter over the '
+ 'full set of results in ACTUALS_FILENAME; if '
+ 'unspecified, rebaseline *all* tests that are '
+ 'available.'))
+parser.add_argument('--unreviewed', action='store_true',
+ help=('mark all expectations modified by this run as '
+ '"%s": False' %
+ gm_json.JSONKEY_EXPECTEDRESULTS_REVIEWED))
args = parser.parse_args()
exception_handler = ExceptionHandler(
keep_going_on_failure=args.keep_going_on_failure)
@@ -374,7 +413,8 @@
actuals_base_url=args.actuals_base_url,
actuals_filename=args.actuals_filename,
exception_handler=exception_handler,
- add_new=args.add_new)
+ add_new=args.add_new, bugs=args.bugs, notes=args.notes,
+ mark_unreviewed=args.unreviewed)
try:
rebaseliner.RebaselineSubdir(builder=builder)
except BaseException as e:
diff --git a/tools/tests/rebaseline/output/marked-unreviewed/output-expected/command_line b/tools/tests/rebaseline/output/marked-unreviewed/output-expected/command_line
new file mode 100644
index 0000000..b6a2c4d
--- /dev/null
+++ b/tools/tests/rebaseline/output/marked-unreviewed/output-expected/command_line
@@ -0,0 +1 @@
+python tools/rebaseline.py --expectations-root tools/tests/rebaseline/output/marked-unreviewed/output-actual/gm-expectations --actuals-base-url tools/tests/rebaseline/input/json1 --bugs 1234 5678 --builders Test-Android-GalaxyNexus-SGX540-Arm7-Debug Test-Win7-ShuttleA-HD2000-x86-Release --notes notes_content --unreviewed
diff --git a/tools/tests/rebaseline/output/marked-unreviewed/output-expected/gm-expectations/Test-Android-GalaxyNexus-SGX540-Arm7-Debug/expected-results.json b/tools/tests/rebaseline/output/marked-unreviewed/output-expected/gm-expectations/Test-Android-GalaxyNexus-SGX540-Arm7-Debug/expected-results.json
new file mode 100644
index 0000000..a9f0248
--- /dev/null
+++ b/tools/tests/rebaseline/output/marked-unreviewed/output-expected/gm-expectations/Test-Android-GalaxyNexus-SGX540-Arm7-Debug/expected-results.json
@@ -0,0 +1,90 @@
+{
+ "expected-results": {
+ "3x3bitmaprect_565.png": {
+ "allowed-digests": null,
+ "ignore-failure": false
+ },
+ "3x3bitmaprect_8888.png": {
+ "allowed-digests": null,
+ "ignore-failure": false
+ },
+ "aaclip_gpu.png": {
+ "allowed-digests": [
+ [
+ "bitmap-64bitMD5",
+ 11899819492385205974
+ ]
+ ],
+ "ignore-failure": false,
+ "unknown-extra-field": "make sure that rebaseline.py maintains this unknown field within a record whose allowed-digest IS NOT modified"
+ },
+ "aarectmodes_565.png": {
+ "allowed-digests": [
+ [
+ "bitmap-64bitMD5",
+ 14760033689012826769
+ ]
+ ],
+ "ignore-failure": false
+ },
+ "imageblur_565.png": {
+ "allowed-digests": [
+ [
+ "bitmap-64bitMD5",
+ 3359963596899141322
+ ]
+ ],
+ "bugs": [
+ 1234,
+ 5678
+ ],
+ "ignore-failure": false,
+ "notes": "notes_content",
+ "reviewed-by-human": false,
+ "unknown-extra-field": "make sure that rebaseline.py maintains this unknown field within a record whose allowed-digest IS modified"
+ },
+ "imageblur_8888.png": {
+ "allowed-digests": [
+ [
+ "bitmap-64bitMD5",
+ 4217923806027861152
+ ]
+ ],
+ "bugs": [
+ 1234,
+ 5678
+ ],
+ "ignore-failure": false,
+ "notes": "notes_content",
+ "reviewed-by-human": false
+ },
+ "shadertext3_8888.png": {
+ "allowed-digests": [
+ [
+ "bitmap-64bitMD5",
+ 3713708307125704716
+ ]
+ ],
+ "bugs": [
+ 1234,
+ 5678
+ ],
+ "ignore-failure": false,
+ "notes": "notes_content",
+ "reviewed-by-human": false
+ },
+ "xfermodeimagefilter_pdf.png": {
+ "allowed-digests": null,
+ "ignore-failure": false
+ },
+ "xfermodes_pdf.png": {
+ "allowed-digests": [
+ [
+ "bitmap-64bitMD5",
+ 9151974350149210736
+ ]
+ ],
+ "ignore-failure": false
+ }
+ }
+}
\ No newline at end of file
diff --git a/tools/tests/rebaseline/output/marked-unreviewed/output-expected/gm-expectations/Test-Mac10.6-MacMini4.1-GeForce320M-x86-Release/expected-results.json b/tools/tests/rebaseline/output/marked-unreviewed/output-expected/gm-expectations/Test-Mac10.6-MacMini4.1-GeForce320M-x86-Release/expected-results.json
new file mode 100644
index 0000000..79af0cf
--- /dev/null
+++ b/tools/tests/rebaseline/output/marked-unreviewed/output-expected/gm-expectations/Test-Mac10.6-MacMini4.1-GeForce320M-x86-Release/expected-results.json
@@ -0,0 +1,26 @@
+{
+ "expected-results" : {
+ "aaclip_565.png" : {
+ "allowed-digests" : [
+ [ "bitmap-64bitMD5", 12345 ]
+ ],
+ "ignore-failure" : false
+ },
+ "aaclip_8888.png" : {
+ "allowed-digests" : [
+ [ "bitmap-64bitMD5", 67890 ]
+ ],
+ "ignore-failure" : false
+ },
+ "aaclip_gpu.png" : {
+ "allowed-digests" : [
+ [ "bitmap-64bitMD5", 11899819492385205974 ]
+ ],
+ "ignore-failure" : false
+ },
+ "aaclip_pdf.png" : {
+ "allowed-digests" : null,
+ "ignore-failure" : false
+ }
+ }
+}
diff --git a/tools/tests/rebaseline/output/marked-unreviewed/output-expected/gm-expectations/Test-Win7-ShuttleA-HD2000-x86-Release/expected-results.json b/tools/tests/rebaseline/output/marked-unreviewed/output-expected/gm-expectations/Test-Win7-ShuttleA-HD2000-x86-Release/expected-results.json
new file mode 100644
index 0000000..20721f3
--- /dev/null
+++ b/tools/tests/rebaseline/output/marked-unreviewed/output-expected/gm-expectations/Test-Win7-ShuttleA-HD2000-x86-Release/expected-results.json
@@ -0,0 +1,70 @@
+{
+ "expected-results": {
+ "3x3bitmaprect_565.png": {
+ "allowed-digests": null,
+ "ignore-failure": false
+ },
+ "3x3bitmaprect_8888.png": {
+ "allowed-digests": null,
+ "ignore-failure": false
+ },
+ "aaclip_gpu.png": {
+ "allowed-digests": [
+ [
+ "bitmap-64bitMD5",
+ 11899819492385205974
+ ]
+ ],
+ "ignore-failure": false
+ },
+ "aarectmodes_565.png": {
+ "allowed-digests": [
+ [
+ "bitmap-64bitMD5",
+ 14760033689012826769
+ ]
+ ],
+ "ignore-failure": false
+ },
+ "imageblur_565.png": {
+ "allowed-digests": [
+ [
+ "bitmap-64bitMD5",
+ 3359963596899141322
+ ]
+ ],
+ "ignore-failure": false
+ },
+ "imageblur_8888.png": {
+ "allowed-digests": [
+ [
+ "bitmap-64bitMD5",
+ 4217923806027861152
+ ]
+ ],
+ "ignore-failure": false
+ },
+ "shadertext3_8888.png": {
+ "allowed-digests": [
+ [
+ "bitmap-64bitMD5",
+ 3713708307125704716
+ ]
+ ],
+ "ignore-failure": false
+ },
+ "xfermodeimagefilter_pdf.png": {
+ "allowed-digests": null,
+ "ignore-failure": false
+ },
+ "xfermodes_pdf.png": {
+ "allowed-digests": [
+ [
+ "bitmap-64bitMD5",
+ 9151974350149210736
+ ]
+ ],
+ "ignore-failure": false
+ }
+ }
+}
\ No newline at end of file
diff --git a/tools/tests/rebaseline/output/marked-unreviewed/output-expected/return_value b/tools/tests/rebaseline/output/marked-unreviewed/output-expected/return_value
new file mode 100644
index 0000000..573541a
--- /dev/null
+++ b/tools/tests/rebaseline/output/marked-unreviewed/output-expected/return_value
@@ -0,0 +1 @@
+0
diff --git a/tools/tests/rebaseline/output/marked-unreviewed/output-expected/stdout b/tools/tests/rebaseline/output/marked-unreviewed/output-expected/stdout
new file mode 100644
index 0000000..e69de29
--- /dev/null
+++ b/tools/tests/rebaseline/output/marked-unreviewed/output-expected/stdout
diff --git a/tools/tests/run.sh b/tools/tests/run.sh
index 16c00d7..6a41a8b 100755
--- a/tools/tests/run.sh
+++ b/tools/tests/run.sh
@@ -243,6 +243,7 @@
REBASELINE_INPUT=tools/tests/rebaseline/input
REBASELINE_OUTPUT=tools/tests/rebaseline/output
rebaseline_test "$REBASELINE_INPUT/json1" "--actuals-base-url $REBASELINE_INPUT/json1 --builders Test-Android-GalaxyNexus-SGX540-Arm7-Debug Test-Win7-ShuttleA-HD2000-x86-Release" "$REBASELINE_OUTPUT/using-json1-expectations"
+rebaseline_test "$REBASELINE_INPUT/json1" "--actuals-base-url $REBASELINE_INPUT/json1 --bugs 1234 5678 --builders Test-Android-GalaxyNexus-SGX540-Arm7-Debug Test-Win7-ShuttleA-HD2000-x86-Release --notes notes_content --unreviewed" "$REBASELINE_OUTPUT/marked-unreviewed"
#
# Test jsondiff.py ...