Bug 61877 – TestFailures page doesn't show testers that use new-run-webkit-tests

• Status: RESOLVED
• Resolution: FIXED
• Component: Tools / Tests

[Adam Roben (:aroben)]

The TestFailures page doesn't show information about testers that use new-run-webkit-tests. It should!

[Adam Roben (:aroben)]

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.

[Adam Roben (:aroben)]

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

[Adam Roben (:aroben)]

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

[Adam Barth]

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.

[Adam Roben (:aroben)]

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.

[Adam Roben (:aroben)]

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

[Adam Barth]

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.

[Adam Roben (:aroben)]

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.

[Adam Roben (:aroben)]

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

[Ojan Vafai]

(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.

[Adam Roben (:aroben)]

(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.

[Adam Barth]

> 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.  :)

[Ojan Vafai]

(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.

[Adam Barth]

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