Summary: | REGRESSION: layout test results doesn't show diffs | ||||||
---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Ryosuke Niwa <rniwa> | ||||
Component: | Tools / Tests | Assignee: | Dirk Pranke <dpranke> | ||||
Status: | RESOLVED FIXED | ||||||
Severity: | Blocker | CC: | ap, cdumez, dpranke, gyuyoung.kim, ojan, ossy, simon.fraser | ||||
Priority: | P1 | ||||||
Version: | 528+ (Nightly build) | ||||||
Hardware: | Unspecified | ||||||
OS: | Unspecified | ||||||
Attachments: |
|
Description
Ryosuke Niwa
2012-09-20 01:51:40 PDT
This is probably a regression from http://trac.webkit.org/changeset/129047. This bug is a little tricky to fix. We used to figure out which link (e.g wav diff, png diff, txt diff, etc...) to show based on actual failures. If we're going to just report "FAIL" in actual results, then we wouldn't know which file exist. Sigh. Yeah, I think perhaps the thing to do (at least for the moment) is to go back to using IMAGE+TEXT, TEXT, and AUDIO internally and still store those values in the JSON file, but keep Failure as the only expectation in the TestExpectations file, so that it will automatically address all of these. Longer term this means that we should probably consolidate IMAGE+TEXT, TEXT, and AUDIO with the TestFailures types so that we only have one list of detailed failures, and continue mapping those onto a smaller list of expectation types. It wouldn't be a bad thing if we were pushing more details about how exactly tests were failing into results.json (so you could see when checksums failed but diffs passed, etc.). An alternative is to have results.html probe for the various -actual files and -diff files and only display the ones it finds, but this feels less good to me, because we have the information, we're just throwing it away and then trying to recover from throwing it away. (In reply to comment #3) > Longer term this means that we should probably consolidate IMAGE+TEXT, TEXT, and AUDIO with the TestFailures types so that we only have one list of detailed failures, and continue mapping those onto a smaller list of expectation types. It wouldn't be a bad thing if we were pushing more details about how exactly tests were failing into results.json (so you could see when checksums failed but diffs passed, etc.). I agree. I think this is the better direction to go. *** Bug 97238 has been marked as a duplicate of this bug. *** Not seeing failure diffs basically blocks WebKit development. Can this be addressed today, or should r129047 be rolled out? Created attachment 164956 [details]
Patch
Comment on attachment 164956 [details] Patch Clearing flags on attachment: 164956 Committed r129148: <http://trac.webkit.org/changeset/129148> All reviewed patches have been landed. Closing bug. This should fix the issues. Unfortunately, we had unit tests for both sides of this (generating both sides of this) but not an integration test that the correct data propagated all the way through. It's not actually clear how to do such an integration test, unfortunately, but I'm going to keep thinking about it since it seems like a pretty important thing to ensure stays working ... |