Bug 61066 – Double-click on test queued by RebaselineServer to open test in main UI

• Component: Tools / Tests
• Status: UNCONFIRMED

[Tom Hudson]

Double-click on test queued by RebaselineServer to open test in main UI

[Tom Hudson]

Created attachment 93925 [details]
Patch

[Eric Seidel (no email)]

You should be sure to CC reviewers.

[Ojan Vafai]

Comment on attachment 93925 [details]
Patch

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

A few nits. Otherwise, code seems fine to me. I'm not familiar enough with the surrounding code code to be confident setSelectedTest is doing the right thing. Mihai, mind taking a look?

> Tools/Scripts/webkitpy/tool/commands/data/rebaselineserver/main.js:271
> +    var failureType = test.actual + ' (expected ' + test.expected + ')';

Duplicating this code from line 122 seems fragile to me. Could you create a helper function that takes a test and returns it's failureType?

> Tools/Scripts/webkitpy/tool/commands/data/rebaselineserver/queue.js:47
> +          setSelectedTest(
> +              this.options[this.selectedIndex].value)

Indentation here is off. Also, WebKit has no line-length limit. This would be easier to read as a single line.

> Tools/Scripts/webkitpy/tool/commands/data/rebaselineserver/util.js:60
> +

Perhaps this should throw an error (or at least log a warning to the console) if you pass it a value that's not in the select element?

[Mihai Parparita]

Comment on attachment 93925 [details]
Patch

I agree with Ojan's comments. An alternative guessing/duplicating the menu selections when setting the selected test would be to save them when adding the test to the queue so that they can be restored.