Bug 68254 – [Chromium] Update the test expectation file for media related tests.

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Chromium] Update the test expectation file for media related tests.

Created attachment 107675 [details]
Patch

[Ami Fischman]

Comment on attachment 107675 [details]
Patch

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

> LayoutTests/platform/chromium/test_expectations.txt:2006
> +BUGCR96861 WIN : media/audio-repaint.html = IMAGE

I don't understand this.  In the bug you linked you talk about a crash on win, not IMAGE.  Last I heard reed@ was still working on wk65203-related stuff, and was going to rebaseline as a group (and remove from t_e.txt).
I would wait till he's done before filing new bugs on this test (although if you think there's a new crasher problem it might be useful to loop him in).

> LayoutTests/platform/chromium/test_expectations.txt:2690
> +BUGWK67760 GPU : media/media-document-audio-repaint.html = IMAGE+TEXT IMAGE

Chasing down the related bugs didn't you already rebaseline this so it can be removed from this file? (I just closed wk67760 for you)

> LayoutTests/platform/chromium/test_expectations.txt:2696
> +BUGCR82033 LINUX GPU : media/video-controls-rendering.html = IMAGE

This should be pointing at wk58587 instead?  (and the comment is wrong, too)

Created attachment 107691 [details]
Patch

Comment on attachment 107675 [details]
Patch

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

>> LayoutTests/platform/chromium/test_expectations.txt:2006
>> +BUGCR96861 WIN : media/audio-repaint.html = IMAGE
> 
> I don't understand this.  In the bug you linked you talk about a crash on win, not IMAGE.  Last I heard reed@ was still working on wk65203-related stuff, and was going to rebaseline as a group (and remove from t_e.txt).
> I would wait till he's done before filing new bugs on this test (although if you think there's a new crasher problem it might be useful to loop him in).

OK. I will revert this change and create new crash entry.

>> LayoutTests/platform/chromium/test_expectations.txt:2690
>> +BUGWK67760 GPU : media/media-document-audio-repaint.html = IMAGE+TEXT IMAGE
> 
> Chasing down the related bugs didn't you already rebaseline this so it can be removed from this file? (I just closed wk67760 for you)

I will remove this once everything looks OK.

>> LayoutTests/platform/chromium/test_expectations.txt:2696
>> +BUGCR82033 LINUX GPU : media/video-controls-rendering.html = IMAGE
> 
> This should be pointing at wk58587 instead?  (and the comment is wrong, too)

Done.

[Ami Fischman]

Comment on attachment 107691 [details]
Patch

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

> LayoutTests/platform/chromium/test_expectations.txt:3757
> +BUGCR96861 WIN MAC DEBUG : media/audio-repaint.html = TIMEOUT IMAGE PASS

Did you mean to say CRASH instead of TIMEOUT?

[WebKit Review Bot]

Comment on attachment 107691 [details]
Patch

Rejecting attachment 107691 [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:
tch file(s).
patching file LayoutTests/ChangeLog
Hunk #1 succeeded at 1 with fuzz 3.
patching file LayoutTests/platform/chromium/test_expectations.txt
Hunk #1 succeeded at 2072 (offset 3 lines).
Hunk #2 succeeded at 2689 (offset 3 lines).
Hunk #3 FAILED at 3749.
1 out of 3 hunks FAILED -- saving rejects to file LayoutTests/platform/chromium/test_expectations.txt.rej

Failed to run "[u'/mnt/git/webkit-commit-queue/Tools/Scripts/svn-apply', u'--reviewer', u'David Levin', u'--force']" exit_code: 1

Full output: http://queues.webkit.org/results/9734088

Created attachment 108383 [details]
Patch

[Ami Fischman]

Comment on attachment 108383 [details]
Patch

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

> LayoutTests/ChangeLog:4
> +        It includes updataing bug numbers, removing

typo

> LayoutTests/platform/chromium/test_expectations.txt:2698
> +// BUGWK67760: we made sure rebaselines are correct.

I don't understand this comment.

> LayoutTests/platform/chromium/test_expectations.txt:2699
> +BUGWK67760 GPU : media/media-document-audio-repaint.html = IMAGE+TEXT IMAGE

This bug is RESOLVED and your last comment in it is that the "rebaseline is good" so why is this not deletable?

> LayoutTests/platform/chromium/test_expectations.txt:2704
> +BUGWK58587 LINUX GPU : media/video-controls-rendering.html = IMAGE

DEBUG only?

Comment on attachment 108383 [details]
Patch

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

>> LayoutTests/ChangeLog:4
>> +        It includes updataing bug numbers, removing
> 
> typo

Fixed.

>> LayoutTests/platform/chromium/test_expectations.txt:2698
>> +// BUGWK67760: we made sure rebaselines are correct.
> 
> I don't understand this comment.

removed.

>> LayoutTests/platform/chromium/test_expectations.txt:2699
>> +BUGWK67760 GPU : media/media-document-audio-repaint.html = IMAGE+TEXT IMAGE
> 
> This bug is RESOLVED and your last comment in it is that the "rebaseline is good" so why is this not deletable?

I will remove this line once everything looks OK.

>> LayoutTests/platform/chromium/test_expectations.txt:2704
>> +BUGWK58587 LINUX GPU : media/video-controls-rendering.html = IMAGE
> 
> DEBUG only?

http://test-results.appspot.com/dashboards/flakiness_dashboard.html#group=%40ToT%20GPU%20Mesa%20-%20chromium.org&tests=media/video-controls-rendering.html shows it does fail on release

Created attachment 108391 [details]
Patch

[Ami Fischman]

> >> LayoutTests/platform/chromium/test_expectations.txt:2699
> >> +BUGWK67760 GPU : media/media-document-audio-repaint.html = IMAGE+TEXT IMAGE
> > This bug is RESOLVED and your last comment in it is that the "rebaseline is good" so why is this not deletable?
> I will remove this line once everything looks OK.

I don't understand that response.  What do you expect to look OK after this submission that doesn't look OK now?

> >> LayoutTests/platform/chromium/test_expectations.txt:2704
> >> +BUGWK58587 LINUX GPU : media/video-controls-rendering.html = IMAGE
> > DEBUG only?
> http://test-results.appspot.com/dashboards/flakiness_dashboard.html#group=%40ToT%20GPU%20Mesa%20-%20chromium.org&tests=media/video-controls-rendering.html shows it does fail on release

Does that mean a rebaseline is in order?  The bug you're pointing at is about differences between debug & release.  It doesn't make sense to point at it as a cause for *both* debug and release failing.

(In reply to comment #11)
> > >> LayoutTests/platform/chromium/test_expectations.txt:2699
> > >> +BUGWK67760 GPU : media/media-document-audio-repaint.html = IMAGE+TEXT IMAGE
> > > This bug is RESOLVED and your last comment in it is that the "rebaseline is good" so why is this not deletable?
> > I will remove this line once everything looks OK.
> 
> I don't understand that response.  What do you expect to look OK after this submission that doesn't look OK now?

It is deletable. However, I would like to wait until it passes on the bot after this submission. I just want to play safe.

> > >> LayoutTests/platform/chromium/test_expectations.txt:2704
> > >> +BUGWK58587 LINUX GPU : media/video-controls-rendering.html = IMAGE
> > > DEBUG only?
> > http://test-results.appspot.com/dashboards/flakiness_dashboard.html#group=%40ToT%20GPU%20Mesa%20-%20chromium.org&tests=media/video-controls-rendering.html shows it does fail on release
> 
> Does that mean a rebaseline is in order?  The bug you're pointing at is about differences between debug & release.  It doesn't make sense to point at it as a cause for *both* debug and release failing.

(In reply to comment #11)
> > >> LayoutTests/platform/chromium/test_expectations.txt:2699
> > >> +BUGWK67760 GPU : media/media-document-audio-repaint.html = IMAGE+TEXT IMAGE
> > > This bug is RESOLVED and your last comment in it is that the "rebaseline is good" so why is this not deletable?
> > I will remove this line once everything looks OK.
> 
> I don't understand that response.  What do you expect to look OK after this submission that doesn't look OK now?
> 
> > >> LayoutTests/platform/chromium/test_expectations.txt:2704
> > >> +BUGWK58587 LINUX GPU : media/video-controls-rendering.html = IMAGE
> > > DEBUG only?
> > http://test-results.appspot.com/dashboards/flakiness_dashboard.html#group=%40ToT%20GPU%20Mesa%20-%20chromium.org&tests=media/video-controls-rendering.html shows it does fail on release
> 
> Does that mean a rebaseline is in order?  The bug you're pointing at is about differences between debug & release.  It doesn't make sense to point at it as a cause for *both* debug and release failing.

I will file another bug to track release failing.

Created attachment 108407 [details]
Patch

fischman, PTAL? Thanks!

[Ami Fischman]

lgtm

[WebKit Review Bot]

Comment on attachment 108407 [details]
Patch

Rejecting attachment 108407 [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:
hing file LayoutTests/ChangeLog
Hunk #1 succeeded at 1 with fuzz 3.
patching file LayoutTests/platform/chromium/test_expectations.txt
Hunk #1 succeeded at 2700 (offset 5 lines).
Hunk #2 succeeded at 3556 (offset -38 lines).
Hunk #3 succeeded at 3693 (offset -38 lines).
Hunk #4 FAILED at 3827.
1 out of 4 hunks FAILED -- saving rejects to file LayoutTests/platform/chromium/test_expectations.txt.rej

Failed to run "[u'/mnt/git/webkit-commit-queue/Tools/Scripts/svn-apply', u'--force']" exit_code: 1

Full output: http://queues.webkit.org/results/9843891

Created attachment 108689 [details]
Patch

[David Levin]

Comment on attachment 108689 [details]
Patch

Need an r+ due to the "Reviewed by NOBODY (OOPS!)."

It looks like this has been sufficiently reviewed now.

[WebKit Review Bot]

Comment on attachment 108689 [details]
Patch

Rejecting attachment 108689 [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:
eLog
Hunk #1 succeeded at 1 with fuzz 3.
patching file LayoutTests/platform/chromium/test_expectations.txt
Hunk #1 succeeded at 2705 (offset 5 lines).
Hunk #2 succeeded at 3561 (offset 5 lines).
Hunk #3 succeeded at 3698 (offset 5 lines).
Hunk #4 FAILED at 3820.
1 out of 4 hunks FAILED -- saving rejects to file LayoutTests/platform/chromium/test_expectations.txt.rej

Failed to run "[u'/mnt/git/webkit-commit-queue/Tools/Scripts/svn-apply', u'--reviewer', u'David Levin', u'--force']" exit_code: 1

Full output: http://queues.webkit.org/results/9879070

[Mihai Parparita]

It looks like this landed as http://trac.webkit.org/changeset/96013. Note that the change introduced duplicate expectations for media/audio-repaint.html (see http://trac.webkit.org/browser/trunk/LayoutTests/platform/chromium/test_expectations.txt?rev=96013, lines 2015, 3831-3832). This causes test runs to fail immediately:

http://build.chromium.org/p/chromium.webkit/builders/Webkit%20Win%20%28dbg%29%281%29/builds/6906/steps/webkit_tests/logs/stdio

I removed the earlier one with http://trac.webkit.org/changeset/96047, but for future reference you can use run-webkit-tests --chromium --lint-test-files to make sure you check in a valid test_expectations.txt file.