Bug 61877 - TestFailures page doesn't show testers that use new-run-webkit-tests
Summary: TestFailures page doesn't show testers that use new-run-webkit-tests
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Nobody
URL: http://build.webkit.org/TestFailures/
Keywords: ToolsHitList
Depends on:
Blocks:
 
Reported: 2011-06-01 12:19 PDT by Adam Roben (:aroben)
Modified: 2011-07-06 15:49 PDT (History)
2 users (show)

See Also:


Attachments
Extract code to parse ORWT's results.html file into its own class (10.26 KB, patch)
2011-07-06 12:52 PDT, Adam Roben (:aroben)
abarth: review+
Details | Formatted Diff | Diff
Teach TestFailures how to load, parse, and interpret NRWT test results (13.28 KB, patch)
2011-07-06 12:58 PDT, Adam Roben (:aroben)
no flags Details | Formatted Diff | Diff
Teach TestFailures how to load, parse, and interpret NRWT test results (14.34 KB, patch)
2011-07-06 13:26 PDT, Adam Roben (:aroben)
abarth: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Adam Roben (:aroben) 2011-06-01 12:19:56 PDT
The TestFailures page doesn't show information about testers that use new-run-webkit-tests. It should!
Comment 1 Adam Roben (:aroben) 2011-06-01 12:20:51 PDT
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.
Comment 2 Adam Roben (:aroben) 2011-07-06 12:52:49 PDT
Created attachment 99861 [details]
Extract code to parse ORWT's results.html file into its own class
Comment 3 Adam Roben (:aroben) 2011-07-06 12:58:53 PDT
Created attachment 99865 [details]
Teach TestFailures how to load, parse, and interpret NRWT test results
Comment 4 Adam Barth 2011-07-06 13:04:17 PDT
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 5 Adam Roben (:aroben) 2011-07-06 13:05:28 PDT
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.
Comment 6 Adam Roben (:aroben) 2011-07-06 13:26:30 PDT
Created attachment 99869 [details]
Teach TestFailures how to load, parse, and interpret NRWT test results
Comment 7 Adam Barth 2011-07-06 13:31:23 PDT
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 8 Adam Roben (:aroben) 2011-07-06 13:38:53 PDT
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.
Comment 9 Adam Roben (:aroben) 2011-07-06 13:42:39 PDT
Committed r90489: <http://trac.webkit.org/changeset/90489>
Comment 10 Ojan Vafai 2011-07-06 15:23:26 PDT
(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.
Comment 11 Adam Roben (:aroben) 2011-07-06 15:24:08 PDT
(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.
Comment 12 Adam Barth 2011-07-06 15:30:05 PDT
> 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.  :)
Comment 13 Ojan Vafai 2011-07-06 15:47:09 PDT
(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.
Comment 14 Adam Barth 2011-07-06 15:49:19 PDT
I think the only real one of those is the rebaseline server, which is going to be scrapped for parts in relatively short order.