Issue 9959028: Convert the media and audio UI test to a browser_test. browser_tests are sharded and run quicker, a…

Issue 9959028: Convert the media and audio UI test to a browser_test. browser_tests are sharded and run quicker, a… (Closed)

Created:
8 years, 8 months ago by jam

Modified:
8 years, 8 months ago

Reviewers:
scherkus (not reviewing)

CC:
chromium-reviews, joi+watch-content_chromium.org, darin-cc_chromium.org, jam, feature-media-reviews_chromium.org

Base URL:
svn://chrome-svn/chrome/trunk/src/

Visibility:
Public.

More Reviews

Description

Convert the media and audio UI test to a browser_test. browser_tests are sharded and run quicker, and are generally less flaky than ui tests. They'll also be portable to content_browsertest once we have it. BUG=90448, 117828 Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=129958

Patch Set 1 : #

Patch Set 2 : sync before bad v8 roll and fix failing test #

Total comments: 4

Patch Set 3 : #

Created: 8 years, 8 months ago

Download [raw] [tar.bz2]

	Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+116 lines, -374 lines)			Patch
D	chrome/browser/audio_layout_uitest.cc	View	1	1 chunk	+0 lines, -86 lines	0 comments	Download
D	chrome/browser/media_uitest.cc	View	1	1 chunk	+0 lines, -165 lines	0 comments	Download
M	chrome/chrome_tests.gypi	View	1	4 chunks	+2 lines, -2 lines	0 comments	Download
M	chrome/test/data/media/player.html	View	1 2	1 chunk	+1 line, -2 lines	0 comments	Download
A +	content/browser/audio_browsertest.cc	View	2	2 chunks	+36 lines, -50 lines	0 comments	Download
A +	content/browser/media_browsertest.cc	View	1	3 chunks	+67 lines, -68 lines	0 comments	Download
M	content/test/layout_browsertest.cc	View	1 2	2 chunks	+10 lines, -1 line	0 comments	Download

Messages

Total messages: 4 (0 generated)

Expand Messages | Collapse Messages

scherkus (not reviewing)

LGTM w/ nit + q thanks! http://codereview.chromium.org/9959028/diff/4003/chrome/test/data/media/player.html File chrome/test/data/media/player.html (right): http://codereview.chromium.org/9959028/diff/4003/chrome/test/data/media/player.html#newcode31 chrome/test/data/media/player.html:31: InstallEventHandler('error', nit: fit ...

8 years, 8 months ago (2012-03-30 19:53:08 UTC) #2

scherkus (not reviewing)

also it looks like we have a more accurate bug if you want to update ...

8 years, 8 months ago (2012-03-30 20:09:16 UTC) #3

jam

8 years, 8 months ago (2012-03-30 20:19:06 UTC) #4

http://codereview.chromium.org/9959028/diff/4003/chrome/test/data/media/playe...
File chrome/test/data/media/player.html (right):

http://codereview.chromium.org/9959028/diff/4003/chrome/test/data/media/playe...
chrome/test/data/media/player.html:31: InstallEventHandler('error',
On 2012/03/30 19:53:08, scherkus wrote:
> nit: fit on one line?

Done.

http://codereview.chromium.org/9959028/diff/4003/content/browser/audio_browse...
File content/browser/audio_browsertest.cc (right):

http://codereview.chromium.org/9959028/diff/4003/content/browser/audio_browse...
content/browser/audio_browsertest.cc:48: set_wait_until_done(false);  // This
test just ensures no crash.
On 2012/03/30 19:53:08, scherkus wrote:
> what's up with this?

you know, i thought that the ui_test version was timing out (and we don't check
for timeout as an error) and it still passed because the html matched. but
because of your question, I looked into this deeper. turns out that fake layout
test controller we have for browser_tests need to be smarter to match the
ui_test one, i'll update it now.

Expand Messages | Collapse Messages