Bug 62425

Another example: http://build.webkit.org/builders/Chromium%20Win%20Release%20%28Tests%29/builds/15331 suggests that we have 3 test failures and 2 new passes. But the number don't match up on http://build.webkit.org/results/Chromium%20Win%20Release%20(Tests)/r88516%20(15331)/results.html I think the problem comes from the fact results.html 1. does not distinguish whether a teat has crash expectation or not when listing crashed tests 2. does not distinguish whether a test has pass expectation or not when listing unexpectedly passed tests

Hm. I may not have really looked at the results.html format since Ojan redid everything. I agree the output is highly confusing. It looks like the results.html file is showing tests that crash, have MISSING expectations, etc. even if those are expected? Moreover, it does not actually tell you that any of those things are expected, nor does it distinguish those sections from the "unexpected results" section, which is empty and hence not displaying. Frankly, I was surprised that we were generating a results.html at all when the test run is passing (i.e., there are no unexpected results), since that's not what ORWT does nor what NRWT used to do. Ojan, are these all intended changes? Can we fix things so that it's at least clearer that the stuff that is being displayed is expected (but admittedly bad)? I would actually argue that the *only* things that should be displayed by default are the unexpected results, and you should have to toggle the "expected crashes" into view, just like you have to toggle all of the other expected things. Also, there may be another issue causing confusion. I'll mention it here for continuity but it probably should be a separate bug. If I look at the chromium.webkit canary bot and find the most recent failing Linux build (which I was doing for comparison), I went from here: http://build.chromium.org/p/chromium.webkit/builders/Webkit%20Linux to: http://build.chromium.org/p/chromium.webkit/builders/Webkit%20Linux/builds/8949 which claimed that there was 1 unexpected failure and 1 unexpectedly flaky test (which matches the stdio output). However, if I click on the layout_test_results archive link, it takes me to: http://build.chromium.org/f/chromium/layout_test_results/Webkit_Linux/88633/ and downloading that .zip file gives me: Desktop $ shasum layout-test-results.zip e86ad69aff7b2dc53e3a5cf07a4eaff287d5700e layout-test-results.zip assuming I'm not getting some weird caching issue from Chrome, that zip file does not show the failures listed in the stdio log at all; is it possible that the wrong layout-test-results.zip file is getting archived?

Adding dimitri and koz since they're gardening and should probably be in the loop on this ...

(In reply to comment #2) > I would actually argue that the *only* things that should be displayed by default are the unexpected results, and you should have to toggle the "expected crashes" into view, just like you have to toggle all of the other expected things. I agree. > assuming I'm not getting some weird caching issue from Chrome, that zip file does not show the failures listed in the stdio log at all; is it possible that the wrong layout-test-results.zip file is getting archived? Looking at http://build.chromium.org/p/chromium.webkit/builders/Webkit%20Linux/builds/8949/steps/archive_webkit_tests_results/logs/stdio we seems to be archiving correctly (e.g. it's archiving filesystem-uri-origin-actual.txt/expected.txt). But you're right that zip file doesn't seem to contain what it claims to contain (e.g. results.html doesn't have any entry for filesystem-uri-origin.html and zip doesn't contain filesystem-uri-origin-actual.txt/expected.txt).

Infrastructure bug filed as http://crbug.com/85736 for fixing zip archiving.

Created attachment 96791 [details] fixes the bug

I took a quick look at this but it wasn't obvious to me how the patch actually would be changing what gets displayed. It sounds from the Changelog that you at least hide the expected crash/missing/timeouts by default, but how do they then get unhidden? Is it possible to clearly distinguish them from the unexpected results? It might be helpful to post a couple of screenshots or something. At any rate, you probably want Ojan to review this, not me, anyway.

(In reply to comment #7) > I took a quick look at this but it wasn't obvious to me how the patch actually would be changing what gets displayed. It sounds from the Changelog that you at least hide the expected crash/missing/timeouts by default, but how do they then get unhidden? Is it possible to clearly distinguish them from the unexpected results? They're hidden by default because of class=expected in tbody and table. It can be unhidden by unchecking a box in option on the upper right corner.

(In reply to comment #8) > (In reply to comment #7) > > I took a quick look at this but it wasn't obvious to me how the patch actually would be changing what gets displayed. It sounds from the Changelog that you at least hide the expected crash/missing/timeouts by default, but how do they then get unhidden? Is it possible to clearly distinguish them from the unexpected results? > > They're hidden by default because of class=expected in tbody and table. It can be unhidden by unchecking a box in option on the upper right corner. I'm guessing you're referring to the already existing "only show unexpected results" box? If you uncheck that, does that cause both the expected crashes/timeouts/missing *and* all of the other expected results to be displayed? It seemed like part of the reason Ojan changed the format to begin with was to be able to separate these two classes of results. Partially my concern was that the UI didn't really distinguish expected from unexpected as well as it could, period.

(In reply to comment #9) > I'm guessing you're referring to the already existing "only show unexpected results" box? If you uncheck that, does that cause both the expected crashes/timeouts/missing *and* all of the other expected results to be displayed? It seemed like part of the reason Ojan changed the format to begin with was to be able to separate these two classes of results. I talked about this with Ojan, and he told me this is a bug.

(In reply to comment #10) > (In reply to comment #9) > > I'm guessing you're referring to the already existing "only show unexpected results" box? If you uncheck that, does that cause both the expected crashes/timeouts/missing *and* all of the other expected results to be displayed? It seemed like part of the reason Ojan changed the format to begin with was to be able to separate these two classes of results. > > I talked about this with Ojan, and he told me this is a bug. "this" == displaying expected crashes/timeouts/missing separately from other expected failures?

(In reply to comment #11) > "this" == displaying expected crashes/timeouts/missing separately from other expected failures? No, the fact tests with pass expectations are showing up on the list of crashed and timeouted tests when "only show unexpected results" is checked.

Comment on attachment 96791 [details] fixes the bug Attachment 96791 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/8831083 New failing tests: fast/harness/results.html

Created attachment 96816 [details] Archive of layout-test-results from ec2-cr-linux-03 The attached test failures were seen while running run-webkit-tests on the chromium-ews. Bot: ec2-cr-linux-03 Port: Chromium Platform: Linux-2.6.35-28-virtual-x86_64-with-Ubuntu-10.10-maverick

(In reply to comment #13) > (From update of attachment 96791 [details]) > Attachment 96791 [details] did not pass chromium-ews (chromium-xvfb): > Output: http://queues.webkit.org/results/8831083 > > New failing tests: > fast/harness/results.html It seems like the bot didn't apply my patch properly :( The diff is identical to what I have in my patch.

Comment on attachment 96791 [details] fixes the bug 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.

Committed r88592: <http://trac.webkit.org/changeset/88592>