Bug 61877

The most immediate issue is that new-run-webkit-tests bots don't seem to upload results to build.webkit.org/results. But the TestFailures code also doesn't know anything about NRWT's output format. We'll probably need to teach it to deal with NRWT's JSON files.

Created attachment 99861 [details] Extract code to parse ORWT's results.html file into its own class

Created attachment 99865 [details] Teach TestFailures how to load, parse, and interpret NRWT test results

Comment on attachment 99865 [details] Teach TestFailures how to load, parse, and interpret NRWT test results View in context: https://bugs.webkit.org/attachment.cgi?id=99865&action=review > Tools/BuildSlaveSupport/build.webkit.org-config/public_html/TestFailures/NRWTResultsParser.js:74 > + result.tests[test.name] = { failureType: convertNRWTResultString(test.actual) }; A couple things: 1) You'll need to split test.actual on " " because there can be multiple actual results for a given test in a given run. 2) You'll need to compare against test.expected otherwise you'll get a bunch of bogus failure reports about tests that we expect to fail.

Comment on attachment 99865 [details] Teach TestFailures how to load, parse, and interpret NRWT test results View in context: https://bugs.webkit.org/attachment.cgi?id=99865&action=review >> Tools/BuildSlaveSupport/build.webkit.org-config/public_html/TestFailures/NRWTResultsParser.js:74 >> + result.tests[test.name] = { failureType: convertNRWTResultString(test.actual) }; > > A couple things: > > 1) You'll need to split test.actual on " " because there can be multiple actual results for a given test in a given run. > 2) You'll need to compare against test.expected otherwise you'll get a bunch of bogus failure reports about tests that we expect to fail. 1) You can see in convertNRWTResultString that I explicitly handle the " " case and just call the test "flaky". 2) We're loading unexpected_results.json, so I don't think we'll see any tests where we expect failure.

Created attachment 99869 [details] Teach TestFailures how to load, parse, and interpret NRWT test results

Comment on attachment 99869 [details] Teach TestFailures how to load, parse, and interpret NRWT test results View in context: https://bugs.webkit.org/attachment.cgi?id=99869&action=review > Tools/BuildSlaveSupport/build.webkit.org-config/public_html/TestFailures/NRWTResultsParser.js:65 > + if (expectedArray.contains('FAIL') && ['IMAGE', 'TEXT', 'IMAGE+TEXT'].contains(actualValue)) FAIL includes all of these: http://trac.webkit.org/browser/trunk/Tools/Scripts/webkitpy/tool/servers/data/gardeningserver/results.js#L17 > Tools/BuildSlaveSupport/build.webkit.org-config/public_html/TestFailures/NRWTResultsParser.js:88 > + return 'unknown failure type ' + nrwtResult; AUDIO will fall into this category, for example.

Comment on attachment 99869 [details] Teach TestFailures how to load, parse, and interpret NRWT test results View in context: https://bugs.webkit.org/attachment.cgi?id=99869&action=review >> Tools/BuildSlaveSupport/build.webkit.org-config/public_html/TestFailures/NRWTResultsParser.js:65 >> + if (expectedArray.contains('FAIL') && ['IMAGE', 'TEXT', 'IMAGE+TEXT'].contains(actualValue)) > > FAIL includes all of these: > http://trac.webkit.org/browser/trunk/Tools/Scripts/webkitpy/tool/servers/data/gardeningserver/results.js#L17 http://trac.webkit.org/browser/trunk/Tools/Scripts/webkitpy/layout_tests/layout_package/test_expectations.py#L53 and http://trac.webkit.org/browser/trunk/LayoutTests/fast/harness/results.html#L396 seem to disagree with you. >> Tools/BuildSlaveSupport/build.webkit.org-config/public_html/TestFailures/NRWTResultsParser.js:88 >> + return 'unknown failure type ' + nrwtResult; > > AUDIO will fall into this category, for example. Right. This is just a first pass, trying to get NRWT basically to the level of ORWT in TestFailures.

Committed r90489: <http://trac.webkit.org/changeset/90489>

(In reply to comment #5) > 2) We're loading unexpected_results.json, so I don't think we'll see any tests where we expect failure. This is fine. I've been trying to get everyone to use full_results.json instead. That way we could kill unexpected_results.json and have one fewer json file to maintain.

(In reply to comment #10) > (In reply to comment #5) > > 2) We're loading unexpected_results.json, so I don't think we'll see any tests where we expect failure. > > This is fine. I've been trying to get everyone to use full_results.json instead. That way we could kill unexpected_results.json and have one fewer json file to maintain. The version of this patch that landed uses full_results.json, not unexpected_results.json.

> This is fine. I've been trying to get everyone to use full_results.json instead. That way we could kill unexpected_results.json and have one fewer json file to maintain. IMHO, we should just remove unexpected_results.json now before the situation gets worse. If folks are using it, this will encourage them to stop. :)

(In reply to comment #11) > The version of this patch that landed uses full_results.json, not unexpected_results.json. Whoops. Sorry. Should have ready more closely. (In reply to comment #12) > > This is fine. I've been trying to get everyone to use full_results.json instead. That way we could kill unexpected_results.json and have one fewer json file to maintain. > > IMHO, we should just remove unexpected_results.json now before the situation gets worse. If folks are using it, this will encourage them to stop. :) I agree. I've been slowly doing that over time, but haven't gotten rid of the last few usages of it and I don't want to break existing tooling: http://codesearch.google.com/codesearch#search/&exact_package=chromium&q=unexpected_results.json(%5C%22%7C%5C')&type=cs They're all very easy to do, it just takes adding some extra code to filter out the expected results.

I think the only real one of those is the rebaseline server, which is going to be scrapped for parts in relatively short order.