New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 835642 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: May 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 1
Type: Bug



Sign in to add a comment

[ChromeOS] Enable VideoPlayerBrowserTests

Project Member Reported by noel@chromium.org, Apr 22 2018

Issue description

Video Player FileManagerBrowserTests had been disabled due to flakiness.
 
With that test suite in maintenance mode, other changes in chrome code over time have increased the likelihood of flakes in this test suite, resulting in blanket sheriff test disables (  issue 804413  ), often for causes unrelated to the original flakiness bug report, or disabled with no investigation at all.

The systemic cause of the browser test flakes in the FileManagerBrowserTest test suite was recently investigated / resolved (  issue 831074  ,   issue 804413  ,   issue 829310  ).

Hence, we do not expect any flakiness in this test suite anymore, and we should start working to re-enable this browser test suite.

This issue covers the Video Player tests in that test suite.

 
Project Member

Comment 1 by bugdroid1@chromium.org, Apr 22 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/8fc9e80a241c0d8e85f45399e42f9c8154e2ee81

commit 8fc9e80a241c0d8e85f45399e42f9c8154e2ee81
Author: Noel Gordon <noel@chromium.org>
Date: Sun Apr 22 13:58:38 2018

[ChromeOS] Re-enable VideoPlayer FileManagerBrowserTests

Flakiness cause in the FileManagerBrowserTests was resolved on issues:
 issue 831074   issue 804413   issue 829310   issue 618198 .

Re-enable the VideoPlayer tests in this browser test suite on all bots
RELEASE/DEBUG/MSAN/ASAN on ChromeOS.

Some tests were disabled in MSAN. Re-enabling them here to see if MSAN
is still a problem, issue 508949.

Mash doesn't support these tests: disable them in mash.

Tbr: yamaguchi@chromium.org
Tbr: kbr@chromium.org
Bug:  835642 ,508949
Change-Id: I6e7dc645b68c6fde73b195859c6bd6ccf0e71c16
Reviewed-on: https://chromium-review.googlesource.com/1023353
Commit-Queue: Noel Gordon <noel@chromium.org>
Reviewed-by: Noel Gordon <noel@chromium.org>
Cr-Commit-Position: refs/heads/master@{#552606}
[modify] https://crrev.com/8fc9e80a241c0d8e85f45399e42f9c8154e2ee81/chrome/browser/chromeos/file_manager/video_player_browsertest.cc
[modify] https://crrev.com/8fc9e80a241c0d8e85f45399e42f9c8154e2ee81/testing/buildbot/filters/mash.browser_tests.filter

Comment 3 by noel@chromium.org, May 7 2018

Status: Fixed (was: Started)
VideoPlayer tests are now going well, and are re-enabled on all bots [1].

They are always good in RELEASE which is good for the CQ. On other bots like ASAN, there's the occasional crash, but those are due to unrelated  issue 837950 .

[1] https://test-results.appspot.com/dashboards/flakiness_dashboard.html#testType=browser_tests%20(with%20patch)&tests=VideoPlayerBrowserTest*

Comment 4 by noel@chromium.org, May 8 2018

Status: Started (was: Fixed)
Re-opening to land a VideoPlayerChange here (use a fake audio layer to remove the test log spew about the Pulse Audio).
Project Member

Comment 5 by bugdroid1@chromium.org, May 9 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/2c490c99574601b697fbddaff50eb58212a11ecb

commit 2c490c99574601b697fbddaff50eb58212a11ecb
Author: Noel Gordon <noel@chromium.org>
Date: Wed May 09 00:21:45 2018

[ChromeOS] Cargo cult crrev.com/556704 into VideoPlayerBrowserTest

Again, PulseAudio layer is unsupported in a target_os="chromeos" build
and VideoPlayerBrowserTest produce LOG errors about that. Again, use a
fake audio layer to eliminate it, cargo cult crrev.com/556704.

Good enough until a future CL provides a more general fix, but step #1
is to kick the VideoPlayerBrowserTest's tyres on the bots.

Bug:  835626 ,835742, 835642 
Change-Id: I58cc1c188e95d39f8e75230a761e7b69ff99cabb
Reviewed-on: https://chromium-review.googlesource.com/1049768
Reviewed-by: Stuart Langley <slangley@chromium.org>
Commit-Queue: Noel Gordon <noel@chromium.org>
Cr-Commit-Position: refs/heads/master@{#557023}
[modify] https://crrev.com/2c490c99574601b697fbddaff50eb58212a11ecb/chrome/browser/chromeos/file_manager/video_player_browsertest.cc

Project Member

Comment 6 by bugdroid1@chromium.org, May 9 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/52c8de92c54cdf8f7afcc2d88ac09024dfb4fede

commit 52c8de92c54cdf8f7afcc2d88ac09024dfb4fede
Author: Noel Gordon <noel@chromium.org>
Date: Wed May 09 23:31:21 2018

FileManagerBrowserTestBase: use fake audio layer in all tests

Move the fake audio layer command-line setup from the AudioPlayer and
VideoPlayer tests to FileManagerBrowserTestBase so it enabled for all
the FileManager browser tests (see  crbug.com/835626#c12 ).

Bug:  835626 , 835642 ,836254,835742
Change-Id: Ib4794a753a4c080fa0544a55c4ead3447c897b68
Reviewed-on: https://chromium-review.googlesource.com/1052047
Reviewed-by: Stuart Langley <slangley@chromium.org>
Commit-Queue: Noel Gordon <noel@chromium.org>
Cr-Commit-Position: refs/heads/master@{#557375}
[modify] https://crrev.com/52c8de92c54cdf8f7afcc2d88ac09024dfb4fede/chrome/browser/chromeos/file_manager/audio_player_browsertest.cc
[modify] https://crrev.com/52c8de92c54cdf8f7afcc2d88ac09024dfb4fede/chrome/browser/chromeos/file_manager/file_manager_browsertest_base.cc
[modify] https://crrev.com/52c8de92c54cdf8f7afcc2d88ac09024dfb4fede/chrome/browser/chromeos/file_manager/video_player_browsertest.cc

Comment 7 by noel@chromium.org, May 10 2018

Status: Fixed (was: Started)

Sign in to add a comment