[Webkit-unassigned] [Bug 60964] Allow sorting in RebaselineServer based on 'metric' field in unexpected_results.json

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=60964





--- Comment #7 from Tom Hudson <tomhudson at google.com>  2011-05-31 08:08:04 PST ---
(In reply to comment #6)
> (From update of attachment 93776 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=93776&action=review
> 
> > Tools/Scripts/webkitpy/tool/commands/data/rebaselineserver/index.html:54
> > +    <span id="toggle-sort" class="link">Sort Tests</span>
> 
> Call this "Sort tests by metric"?

Sure, for now. When we have support for multiple metrics, I expect to replace this with a drop-down box to choose which metric; then it makes sense to me to have the label be "Sort tests by:" and a default value of "alphabetical order".

> > Tools/Scripts/webkitpy/tool/commands/data/rebaselineserver/main.js:50
> > +var wasSortingImages = false;
> 
> Do you actually need this variable? If you only check shouldSortTestsByMetric in the selectedTypeIsSortable branch, you don't need to clear it to false and restore it.

The intent was to have a bit of UI memory; if we change from looking at IMAGE to TEXT failure cases, and then back, we restore the sortedness/unsortedness from before.

I'll at least rename to wasSortingTestsByMetric to be consistent with shouldSortImages -> shouldSortTestsByMetric.

> > Tools/Scripts/webkitpy/tool/commands/data/rebaselineserver/main.js:157
> > +    $('toggle-sort').style.color = '';
> 
> Rather than manually setting the color style, can you add/remove a 'disabled' class and then control the disabled appearance from the CSS?

My Javascript-fu is presumably far weaker than Dirk's; sounds like I need to learn a little CSS as well. This was just extrapolation from selectTest()/displayImageResults()/...

> > Tools/Scripts/webkitpy/tool/commands/data/rebaselineserver/main.js:263
> > +            testsByState[state].sort(function(a, b) {
> 
> Given that this modifies testsByState, does turning off sorting actually work? It seems like you'd need to re-sort alphabetically in that case.

testsByState is being regenerated (in its default alphabetical order) every time selectDirectory() is called, and the $('toggle-sort').onclick calls selectDirectory() in part to make sure we end up alpha afterwards. Since that's a non-obvious reason I've added a comment to explain.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.