Bug 94177 – [EFL] Update pixel test expectations for layout test fast/dom/

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKit EFL

[KwangYong Choi]

Layout test fast/dom/HTMLInputElement/input-image-alt-text.html should be pass.

[KwangYong Choi]

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.

[Raphael Kubo da Costa (:rakuco)]

Where's this test failing? None of the 3 bots running layout tests have reported it as failing.

[KwangYong Choi]

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

[Chris Dumez]

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.

[Chris Dumez]

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

[Chris Dumez]

Comment on attachment 158697 [details]
Patch

This particular test is failing locally for me as well with -p.

[Gyuyoung Kim]

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

[Raphael Kubo da Costa (:rakuco)]

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.

[Gyuyoung Kim]

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

[KwangYong Choi]

Created attachment 158720 [details]
Patch

[KwangYong Choi]

Created attachment 158724 [details]
Patch

Updated ChageLog.

[Chris Dumez]

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.

[Chris Dumez]

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

[Chris Dumez]

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

[KwangYong Choi]

Created attachment 158743 [details]
Patch

Modified TestExpectations.

[Chris Dumez]

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"

[Gyuyoung Kim]

Comment on attachment 158743 [details]
Patch

cq- because of typo

[KwangYong Choi]

Created attachment 158747 [details]
Patch

Fixed typo.

[Gyuyoung Kim]

Comment on attachment 158747 [details]
Patch

It looks there is no typo anymore. So, cq+ again.

[WebKit Review Bot]

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

[KwangYong Choi]

Created attachment 158767 [details]
Patch

Rebased.

[WebKit Review Bot]

Comment on attachment 158767 [details]
Patch

Clearing flags on attachment: 158767

Committed r125768: <http://trac.webkit.org/changeset/125768>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.