Bug 49626 – Rebaseline server: display test results

• Component: Tools / Tests
• Status: RESOLVED FIXED

[Mihai Parparita]

Rebaseline server: display test results

[Mihai Parparita]

Created attachment 74043 [details]
Patch

[Mihai Parparita]

Running at http://bearberry.mtv.corp.google.com:8127/ if you'd like to see it in action.

[Mihai Parparita]

Created attachment 74062 [details]
Patch

Forgot util.js

[Tony Chang]

Comment on attachment 74062 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=74062&action=review

> WebKitTools/Scripts/webkitpy/tool/commands/data/rebaselineserver/main.js:48
> +          return;

Nit: 4 space indent

> WebKitTools/Scripts/webkitpy/tool/commands/data/rebaselineserver/main.js:53
> +          case 'Left': event.preventDefault(); previousTest(); break;
> +          case 'Right': event.preventDefault(); nextTest(); break;

Nit: 0 or 4 space indent

> WebKitTools/Scripts/webkitpy/tool/commands/data/rebaselineserver/main.js:79
> +          testsByFailureType[failureType] = [];
> +          failureTypes.push(failureType);

Nit: 4 space indent

> WebKitTools/Scripts/webkitpy/tool/commands/rebaselineserver.py:117
> +        (test_name, test_extension) = os.path.splitext(self.query['test'][0])

Nit: you don't need the ()s on the left side of the =.  Also, unused variables are normally just called _. E.g.:
  test_name, _ = os.path.splitext(...

> WebKitTools/Scripts/webkitpy/tool/commands/rebaselineserver.py:189
> +        results_json_file = file(results_json_path)
> +        results_json = simplejson.load(results_json_file)
> +        results_json_file.close()

You could use 'with' here to close the file implicitly.

[Mihai Parparita]

(In reply to comment #4)
> (From update of attachment 74062 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=74062&action=review
> 
> > WebKitTools/Scripts/webkitpy/tool/commands/data/rebaselineserver/main.js:48
> > +          return;
> 
> Nit: 4 space indent

Fixed.

> > WebKitTools/Scripts/webkitpy/tool/commands/data/rebaselineserver/main.js:53
> > +          case 'Left': event.preventDefault(); previousTest(); break;
> > +          case 'Right': event.preventDefault(); nextTest(); break;
> 
> Nit: 0 or 4 space indent

Switched to WebKit style (0-space)

> > WebKitTools/Scripts/webkitpy/tool/commands/data/rebaselineserver/main.js:79
> > +          testsByFailureType[failureType] = [];
> > +          failureTypes.push(failureType);
> 
> Nit: 4 space indent

Fixed.

> > WebKitTools/Scripts/webkitpy/tool/commands/rebaselineserver.py:117
> > +        (test_name, test_extension) = os.path.splitext(self.query['test'][0])
> 
> Nit: you don't need the ()s on the left side of the =.  Also, unused variables are normally just called _. E.g.:
>   test_name, _ = os.path.splitext(...

Fixed.
 
> > WebKitTools/Scripts/webkitpy/tool/commands/rebaselineserver.py:189
> > +        results_json_file = file(results_json_path)
> > +        results_json = simplejson.load(results_json_file)
> > +        results_json_file.close()
> 
> You could use 'with' here to close the file implicitly.

Done.

Will land with the above changes.

[Mihai Parparita]

Committed r72157: <http://trac.webkit.org/changeset/72157>