Bug 97182 – REGRESSION: layout test results doesn't show diffs

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

[Ryosuke Niwa]

See http://build.webkit.org/results/EFL%20Linux%2064-bit%20Debug%20WK2/r129100%20%283410%29/results.html

Even though fast/text/atsui-multiple-renderers.html is failing, we can't see the diff. The diff is there, it's just that results page doesn't seem know how to handle new actual values.

[Ryosuke Niwa]

This is probably a regression from http://trac.webkit.org/changeset/129047.

[Ryosuke Niwa]

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.

[Dirk Pranke]

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

[Dirk Pranke]

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.

[Ojan Vafai]

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

[Dirk Pranke]

*** Bug 97238 has been marked as a duplicate of this bug. ***

[Alexey Proskuryakov]

Not seeing failure diffs basically blocks WebKit development. Can this be addressed today, or should r129047 be rolled out?

[Dirk Pranke]

Created attachment 164956 [details]
Patch

[Dirk Pranke]

Comment on attachment 164956 [details]
Patch

Clearing flags on attachment: 164956

Committed r129148: <http://trac.webkit.org/changeset/129148>

[Dirk Pranke]

All reviewed patches have been landed.  Closing bug.

[Dirk Pranke]

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