[webkit-reviews] review granted: [Bug 62425] new-run-webkit-tests: results.html don't list the same list of failed tests as bots do : [Attachment 96791] fixes the bug

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Fri Jun 10 17:48:32 PDT 2011


Ojan Vafai <ojan at chromium.org> has granted Ryosuke Niwa <rniwa at webkit.org>'s
request for review:
Bug 62425: new-run-webkit-tests: results.html don't list the same list of
failed tests as bots do
https://bugs.webkit.org/show_bug.cgi?id=62425

Attachment 96791: fixes the bug
https://bugs.webkit.org/attachment.cgi?id=96791&action=review

------- Additional Comments from Ojan Vafai <ojan at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=96791&action=review

Thanks for fixing this!

> LayoutTests/fast/harness/results.html:563
> +    for (var i = 0; i < tests.length; i++) {
> +	   if (!tests[i].isExpected)
> +	       return true;
> +    }

https://developer.mozilla.org/en/JavaScript/Reference/Global_Objects/Array/some


Something like: return tests[i].some(function(element) { return
!element.isExpected;});

> LayoutTests/fast/harness/results.html:576
>	   html += '<thead><th>test</th><th>expected failure</th></thead>';

Can you add a FIXME to add the expected failure column for all the test lists
if globalState().results.uses_expectations_file?

> LayoutTests/fast/harness/resources/results-test.js:435
> +    function enclosingNodeWithTagNameHasClassName(node, name, className) {

s/name/tagName

> LayoutTests/fast/harness/resources/results-test.js:436
> +	   while (node && (!node.tagName || node.tagName.toLowerCase() !=
name))

Why not just pass in the upperCase name? That or use localName, which is
lowercase.

> LayoutTests/ChangeLog:12
> +	   (runTests):
> +	   ():

Normally I just remove this cruft. It doesn't work well for JS code.


More information about the webkit-reviews mailing list