Bug 94177

Summary: [EFL] Update pixel test expectations for layout test fast/dom/
Product: WebKit Reporter: KwangYong Choi <ky0.choi>
Component: WebKit EFLAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: cdumez, gyuyoung.kim, lucas.de.marchi, rakuco, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Other   
OS: Linux   
Bug Depends on:    
Bug Blocks: 94188    
Attachments:
Description Flags
Patch
none
Patch
none
Patch
none
Patch
gyuyoung.kim: commit-queue-
Patch
webkit.review.bot: commit-queue-
Patch none

Description KwangYong Choi 2012-08-15 20:07:10 PDT
Layout test fast/dom/HTMLInputElement/input-image-alt-text.html should be pass.
Comment 1 KwangYong Choi 2012-08-15 20:18:00 PDT
Created attachment 158697 [details]
Patch

I don't know why this test is failing, but it should be fixed.

The reason why first 'Success' is not displayed because the size of the input element is too small.
Comment 2 Raphael Kubo da Costa (:rakuco) 2012-08-15 21:20:46 PDT
Where's this test failing? None of the 3 bots running layout tests have reported it as failing.
Comment 3 KwangYong Choi 2012-08-15 21:59:23 PDT
(In reply to comment #2)
> Where's this test failing? None of the 3 bots running layout tests have reported it as failing.

I found it by my local test. I'm working on input type=range these days. I found it during I test fast/dom/HTMLInputElement/. Only one test reports fail, so, I try to fix it.

But, there are many failures on fast/dom/ when I tested fast/dom/. These failures seems they have line height issue on them. Line height of actual result is smaller than expected.

Regressions: Unexpected image-only failures : (30)
  fast/dom/34176.html = IMAGE
  fast/dom/Element/class-attribute-whitespace.html = IMAGE
  fast/dom/HTMLElement/bdo.html = IMAGE
  fast/dom/HTMLHeadElement/head-link-style-href-check.html = IMAGE
  fast/dom/HTMLImageElement/image-alt-text.html = IMAGE
  fast/dom/HTMLLinkElement/pending-stylesheet-count.html = IMAGE
  fast/dom/HTMLMeterElement/meter-appearances-capacity.html = IMAGE
  fast/dom/HTMLMeterElement/meter-appearances-rating-relevancy.html = IMAGE
  fast/dom/HTMLMeterElement/meter-boundary-values.html = IMAGE
  fast/dom/HTMLMeterElement/meter-optimums.html = IMAGE
  fast/dom/HTMLMeterElement/meter-styles-changing-pseudo.html = IMAGE
  fast/dom/HTMLMeterElement/meter-styles.html = IMAGE
  fast/dom/HTMLObjectElement/vspace-hspace-as-number.html = IMAGE
  fast/dom/HTMLTableColElement/resize-table-using-col-width.html = IMAGE
  fast/dom/HTMLTableElement/colSpan.html = IMAGE
  fast/dom/HTMLTableElement/createCaption.html = IMAGE
  fast/dom/HTMLTextAreaElement/reset-textarea.html = IMAGE
  fast/dom/Range/surroundContents-1.html = IMAGE
  fast/dom/Window/open-existing-pop-up-blocking.html = IMAGE
  fast/dom/blur-contenteditable.html = IMAGE
  fast/dom/children-nodes.html = IMAGE
  fast/dom/clone-contents-0-end-offset.html = IMAGE
  fast/dom/css-rule-functions.html = IMAGE
  fast/dom/focus-contenteditable.html = IMAGE
  fast/dom/gc-10.html = IMAGE
  fast/dom/importNodeHTML.html = IMAGE
  fast/dom/importNodeXML.xhtml = IMAGE
  fast/dom/row-inner-text.html = IMAGE
  fast/dom/scroll-reveal-left-overflow.html = IMAGE
  fast/dom/scroll-reveal-top-overflow.html = IMAGE

I think, I have to check my environment first.

Thank you.
Comment 4 Chris Dumez 2012-08-15 22:01:22 PDT
Comment on attachment 158697 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=158697&action=review

LGTM but:

> LayoutTests/ChangeLog:6
> +        Reviewed by NOBODY (OOPS!).

This could land unreviewed.
Comment 5 Chris Dumez 2012-08-15 22:03:17 PDT
(In reply to comment #2)
> Where's this test failing? None of the 3 bots running layout tests have reported it as failing.

The reason is that the bots are not running pixel tests. As you can see from his patch, only the expected image gets updated, not the text expectation.

Unfortunately, we have quite a lot of tests failing if you run tests with -p argument. It is good that something is fixing them even if they don't make the bot red :)
Comment 6 Chris Dumez 2012-08-15 22:05:54 PDT
Comment on attachment 158697 [details]
Patch

This particular test is failing locally for me as well with -p.
Comment 7 Gyuyoung Kim 2012-08-15 22:21:46 PDT
(In reply to comment #3)

> Regressions: Unexpected image-only failures : (30)
>   fast/dom/34176.html = IMAGE
>   fast/dom/Element/class-attribute-whitespace.html = IMAGE
>   fast/dom/HTMLElement/bdo.html = IMAGE
>   fast/dom/HTMLHeadElement/head-link-style-href-check.html = IMAGE
>   fast/dom/HTMLImageElement/image-alt-text.html = IMAGE
>   fast/dom/HTMLLinkElement/pending-stylesheet-count.html = IMAGE
>   fast/dom/HTMLMeterElement/meter-appearances-capacity.html = IMAGE
>   fast/dom/HTMLMeterElement/meter-appearances-rating-relevancy.html = IMAGE
>   fast/dom/HTMLMeterElement/meter-boundary-values.html = IMAGE
>   fast/dom/HTMLMeterElement/meter-optimums.html = IMAGE
>   fast/dom/HTMLMeterElement/meter-styles-changing-pseudo.html = IMAGE
>   fast/dom/HTMLMeterElement/meter-styles.html = IMAGE
>   fast/dom/HTMLObjectElement/vspace-hspace-as-number.html = IMAGE
>   fast/dom/HTMLTableColElement/resize-table-using-col-width.html = IMAGE
>   fast/dom/HTMLTableElement/colSpan.html = IMAGE
>   fast/dom/HTMLTableElement/createCaption.html = IMAGE
>   fast/dom/HTMLTextAreaElement/reset-textarea.html = IMAGE
>   fast/dom/Range/surroundContents-1.html = IMAGE
>   fast/dom/Window/open-existing-pop-up-blocking.html = IMAGE
>   fast/dom/blur-contenteditable.html = IMAGE
>   fast/dom/children-nodes.html = IMAGE
>   fast/dom/clone-contents-0-end-offset.html = IMAGE
>   fast/dom/css-rule-functions.html = IMAGE
>   fast/dom/focus-contenteditable.html = IMAGE
>   fast/dom/gc-10.html = IMAGE
>   fast/dom/importNodeHTML.html = IMAGE
>   fast/dom/importNodeXML.xhtml = IMAGE
>   fast/dom/row-inner-text.html = IMAGE
>   fast/dom/scroll-reveal-left-overflow.html = IMAGE
>   fast/dom/scroll-reveal-top-overflow.html = IMAGE
> 
> I think, I have to check my environment first.
> 
> Thank you.

As Christophe said, I also think nobody verified pixel-test for EFL layout test officially yet. IIRC, our png expected files came from other ports.(Maybe, gtk port? Kubo knows it well) So, IMO, we can also rebase above test cases for --pixel-test.
Comment 8 Raphael Kubo da Costa (:rakuco) 2012-08-15 22:43:56 PDT
The original PNG expectations for our port were partly imported from GTK where it made sense and partly generated when we were writing our DRT code.

I'm OK with cq+'ing this one, however keeping these baselines up-to-date will be an uphill battle until we start checking them on a regular basis.
Comment 9 Gyuyoung Kim 2012-08-15 23:01:12 PDT
(In reply to comment #8)
> The original PNG expectations for our port were partly imported from GTK where it made sense and partly generated when we were writing our DRT code.
> 
> I'm OK with cq+'ing this one, however keeping these baselines up-to-date will be an uphill battle until we start checking them on a regular basis.

Yes, this is one of arduous jobs. But, it looks now is that we start to support this. I file a meta bug for this.
Comment 10 KwangYong Choi 2012-08-15 23:34:58 PDT
Created attachment 158720 [details]
Patch
Comment 11 KwangYong Choi 2012-08-15 23:52:43 PDT
Created attachment 158724 [details]
Patch

Updated ChageLog.
Comment 12 Chris Dumez 2012-08-16 00:04:21 PDT
Comment on attachment 158724 [details]
Patch

Those look like failures to me:
fast/dom/HTMLImageElement/image-alt-text-expected.png
fast/dom/HTMLInputElement/input-image-alt-text-expected.png
fast/dom/clone-contents-0-end-offset-expected.png

They should probably be moved to TestExpectations after double-checking.
Comment 13 Chris Dumez 2012-08-16 00:20:38 PDT
(In reply to comment #12)
> (From update of attachment 158724 [details])
> Those look like failures to me:
> fast/dom/HTMLImageElement/image-alt-text-expected.png
> fast/dom/HTMLInputElement/input-image-alt-text-expected.png
> fast/dom/clone-contents-0-end-offset-expected.png
> 
> They should probably be moved to TestExpectations after double-checking.

After double-checking fast/dom/clone-contents-0-end-offset-expected.png looks correct after all.

I'm still looking into the 2 first ones but clearly we only display one "SUCCESS" and the text says it should be displayed twice.
Comment 14 Chris Dumez 2012-08-16 00:40:38 PDT
(In reply to comment #13)
> (In reply to comment #12)
> > (From update of attachment 158724 [details] [details])
> > Those look like failures to me:
> > fast/dom/HTMLImageElement/image-alt-text-expected.png
> > fast/dom/HTMLInputElement/input-image-alt-text-expected.png
> > fast/dom/clone-contents-0-end-offset-expected.png
> > 
> > They should probably be moved to TestExpectations after double-checking.
> 
> After double-checking fast/dom/clone-contents-0-end-offset-expected.png looks correct after all.
> 
> I'm still looking into the 2 first ones but clearly we only display one "SUCCESS" and the text says it should be displayed twice.

Please add the 2 first test cases to TestExpectations with the following bug ID:
 Bug 94198.
Comment 15 KwangYong Choi 2012-08-16 01:13:59 PDT
Created attachment 158743 [details]
Patch

Modified TestExpectations.
Comment 16 Chris Dumez 2012-08-16 01:18:06 PDT
Comment on attachment 158743 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=158743&action=review

LGTM with minor typos:

> LayoutTests/ChangeLog:8
> +        Some tests have worng pixel test results. They should be replaced.

"wrong"

> LayoutTests/ChangeLog:10
> +        And, two tests related image seems fail. Alt text should be displayed

"seems" -> "seem to"
Comment 17 Gyuyoung Kim 2012-08-16 01:19:55 PDT
Comment on attachment 158743 [details]
Patch

cq- because of typo
Comment 18 KwangYong Choi 2012-08-16 01:23:41 PDT
Created attachment 158747 [details]
Patch

Fixed typo.
Comment 19 Gyuyoung Kim 2012-08-16 01:36:19 PDT
Comment on attachment 158747 [details]
Patch

It looks there is no typo anymore. So, cq+ again.
Comment 20 WebKit Review Bot 2012-08-16 03:20:55 PDT
Comment on attachment 158747 [details]
Patch

Rejecting attachment 158747 [details] from commit-queue.

Failed to run "['/mnt/git/webkit-commit-queue/Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '-..." exit_code: 2

Last 500 characters of output:
p/animator.gyp
45>A    /mnt/git/webkit-commit-queue/Source/WebKit/chromium/third_party/skia/gyp/libjpeg.gyp
45> U   /mnt/git/webkit-commit-queue/Source/WebKit/chromium/third_party/skia/gyp
45>Checked out revision 5077.

________ running '/usr/bin/python tools/clang/scripts/update.py --mac-only' in '/mnt/git/webkit-commit-queue/Source/WebKit/chromium'

________ running '/usr/bin/python gyp_webkit' in '/mnt/git/webkit-commit-queue/Source/WebKit/chromium'
Updating webkit projects from gyp files...

Full output: http://queues.webkit.org/results/13517297
Comment 21 KwangYong Choi 2012-08-16 03:29:59 PDT
Created attachment 158767 [details]
Patch

Rebased.
Comment 22 WebKit Review Bot 2012-08-16 04:55:30 PDT
Comment on attachment 158767 [details]
Patch

Clearing flags on attachment: 158767

Committed r125768: <http://trac.webkit.org/changeset/125768>
Comment 23 WebKit Review Bot 2012-08-16 04:55:36 PDT
All reviewed patches have been landed.  Closing bug.