Bug 49626 - Rebaseline server: display test results
Summary: Rebaseline server: display test results
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Mihai Parparita
URL:
Keywords:
Depends on:
Blocks: 47761
  Show dependency treegraph
 
Reported: 2010-11-16 14:47 PST by Mihai Parparita
Modified: 2010-11-16 18:37 PST (History)
1 user (show)

See Also:


Attachments
Patch (18.58 KB, patch)
2010-11-16 14:52 PST, Mihai Parparita
no flags Details | Formatted Diff | Diff
Patch (21.10 KB, patch)
2010-11-16 16:48 PST, Mihai Parparita
tony: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Mihai Parparita 2010-11-16 14:47:49 PST
Rebaseline server: display test results
Comment 1 Mihai Parparita 2010-11-16 14:52:13 PST
Created attachment 74043 [details]
Patch
Comment 2 Mihai Parparita 2010-11-16 14:53:56 PST
Running at http://bearberry.mtv.corp.google.com:8127/ if you'd like to see it in action.
Comment 3 Mihai Parparita 2010-11-16 16:48:38 PST
Created attachment 74062 [details]
Patch

Forgot util.js
Comment 4 Tony Chang 2010-11-16 17:18:29 PST
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.
Comment 5 Mihai Parparita 2010-11-16 18:35:32 PST
(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.
Comment 6 Mihai Parparita 2010-11-16 18:37:42 PST
Committed r72157: <http://trac.webkit.org/changeset/72157>