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

Issue 835626 link

Starred by 1 user

Issue metadata

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



Sign in to add a comment

[ChromeOS] Enable AudioPlayerBrowserTests

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

Issue description

Audio 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 Audio Player tests in that test suite. Getting them going again would help other Chromium developers get on with their work too [1]

[1] https://chromium-review.googlesource.com/c/chromium/src/+/1023570 for example.
 

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

Synced my git client to crrev.com/551314 and then re-nabled all the AudioPlayer FileManagerBrowserTests in:

  chrome/browser/chromeos/file_manager/audio_player_browsertest.cc

Did clean builds of browser_tests in DEBUG and RELEASE, and then ran the tests, with multiple repeats, DEBUG in one window, and RELEASE in another:

  DISPLAY=:0 ./out/Debug/browser_tests \
      --gtest_filter="AudioPlayerBrowserTest*" --gtest_repeat=100
  DISPLAY=:0 ./out/Release/browser_tests \
      --gtest_filter="AudioPlayerBrowserTest*" --gtest_repeat=100

Let them run for 40 minutes: no failures observed. Creating a patch to re-enable the AudioPlayer tests on the bots therefore ...
Project Member

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

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

commit b44ed3c60b2d1558f6149dfc962e73aff3ee0afc
Author: Noel Gordon <noel@chromium.org>
Date: Sun Apr 22 09:59:28 2018

[ChromeOS] Re-enable AudioPlayer FileManagerBrowserTests

Cause of the flakinessin the FileManagerBrowserTest suite was resolved
on  issue 831074   issue 804413   issue 829310   issue 618198 .

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

OpenAudioOnDrive, OpenAudioOnDownloads were disabled on MSAN (in issue
508949). Re-enabling here to see if MSAN is still a problem.

Mash doesn't support these tests (see mash browser tests patch set #1)
so disable these AudioPlayer FileManagerBrowserTests in mash.

Tbr: yamaguchi@chromium.org
Tbr: maxmorin@chromium.org
Tbr: kbr@chromium.org
Bug:  835626 ,508949,787806, 830493 
Change-Id: I966bc9123b15fec453fe85fec09059af091c916f
Reviewed-on: https://chromium-review.googlesource.com/1023072
Reviewed-by: Noel Gordon <noel@chromium.org>
Commit-Queue: Noel Gordon <noel@chromium.org>
Cr-Commit-Position: refs/heads/master@{#552603}
[modify] https://crrev.com/b44ed3c60b2d1558f6149dfc962e73aff3ee0afc/chrome/browser/chromeos/file_manager/audio_player_browsertest.cc
[modify] https://crrev.com/b44ed3c60b2d1558f6149dfc962e73aff3ee0afc/testing/buildbot/filters/mash.browser_tests.filter

Comment 3 by noel@chromium.org, Apr 22 2018

Cc: yamaguchi@chromium.org mtomasz@chromium.org fukino@chromium.org
On test AudioPlayerBrowserTest.ChangeTracks was added by fukino-san@ in code review https://codereview.chromium.org/1526923002

The test reliably PASSES for me in DEBUG and RELEASE, but the log is interesting:

[220199:220199:0422/180957.311130:INFO:CONSOLE(4325)] "Received the result of openAudioPlayer", source: chrome-extension://hhaomjibdihmijegdhdafkllkbggdgoj/background/js/background_common_scripts.js (4325)
[220199:220199:0422/180957.612563:INFO:CONSOLE(0)] "Styling master document from stylesheets defined in HTML Imports is deprecated. Please refer to https://goo.gl/EGXzpw for possible migration paths.", source:  (0)
[220199:220595:0422/180959.419957:ERROR:pulse_util.cc(454)] Invalid PulseAudio stream state
[220199:220595:0422/180959.420017:WARNING:pulse_util.cc(238)] Operation is NULL
[220199:220595:0422/180959.420915:ERROR:audio_output_resampler.cc(396)] Unable to open audio device in high latency mode.  Falling back to fake audio output.
[220199:220199:0422/180959.941873:INFO:CONSOLE(0)] "Uncaught (in promise) AbortError: The play() request was interrupted by a call to pause(). https://goo.gl/LdLk22", source: chrome-extension://cjbfomnbifhcdnihkgipgfcihmgjfhbf/audio_player.html (0)
[220199:220199:0422/181000.274214:INFO:CONSOLE(129)] "Uncaught TypeError: Cannot read property 'appendChild' of null", source: chrome://resources/polymer/v1_0/paper-ripple/paper-ripple-extracted.js (129)
[220199:220199:0422/181000.436198:INFO:CONSOLE(0)] "Uncaught (in promise) AbortError: The play() request was interrupted by a call to pause(). https://goo.gl/LdLk22", source: chrome-extension://cjbfomnbifhcdnihkgipgfcihmgjfhbf/audio_player.html (0)
[220199:220199:0422/181000.436944:INFO:CONSOLE(0)] "Uncaught (in promise) AbortError: The play() request was interrupted by a call to pause(). https://goo.gl/LdLk22", source: chrome-extension://cjbfomnbifhcdnihkgipgfcihmgjfhbf/audio_player.html (0)
[220199:220199:0422/181000.505421:INFO:CONSOLE(129)] "Uncaught TypeError: Cannot read property 'appendChild' of null", source: chrome://resources/polymer/v1_0/paper-ripple/paper-ripple-extracted.js (129)
[220199:220199:0422/181000.638867:INFO:CONSOLE(0)] "[SUCCESS] undefined", source: chrome-extension://ddabbgbggambiildohfagdkliahiecfl/_generated_background_page.html (0)
[220199:220600:0422/181001.195685:WARNING:discardable_shared_memory_manager.cc(436)] Some MojoDiscardableSharedMemoryManagerImpls are still alive. They will be leaked.
[220199:221281:0422/181001.532458:WARNING:internal_linux.cc(64)] Failed to read /proc/220598/stat
[220199:220199:0422/181001.551658:WARNING:pref_notifier_impl.cc(23)] Pref observer found at shutdown.
[220199:220199:0422/181001.551776:WARNING:pref_notifier_impl.cc(23)] Pref observer found at shutdown.
[       OK ] AudioPlayerBrowserTest.ChangeTracks (15728 ms)
[----------] 1 test from AudioPlayerBrowserTest (15728 ms total)

[----------] Global test environment tear-down
[==========] 1 test from 1 test case ran. (15729 ms total)
[  PASSED  ] 1 test.



Comment 4 by noel@chromium.org, Apr 22 2018

Status: Started (was: Assigned)
From that log, it seems AudioPlayerBrowserTest.ChangeTracks has a few "Uncaught Errors" dealing with the polymer paper-ripple element.

@fukino-san: is this something we should worry about?

Comment 5 by noel@chromium.org, Apr 22 2018

Summary: [ChromeOS] Enable AudioPlayerBrowserTests (was: [ChromeOS] Enable Audio Player FileManagerBrowserTests)
Project Member

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

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

commit bca65ccdbf1442e03d2b381e63f1f872274827f3
Author: Noel Gordon <noel@chromium.org>
Date: Sun Apr 22 13:09:58 2018

Retain AudioPlayerBrowserTestInGuestMode name

crrev.com/552603 changed the name. AudioPlayerBrowserTestInGuestMode
is better (easier when reading flakiness dashboard history).

Restore the InGuestMode name, and in the mash filter as well.

Tbr: yamaguchi@chromium.org
Tbr: kbr@chromium.org
Bug:  835626 
Change-Id: Id1cea92cf253b912624fe6c401087329aa68789a
Reviewed-on: https://chromium-review.googlesource.com/1023253
Reviewed-by: Noel Gordon <noel@chromium.org>
Commit-Queue: Noel Gordon <noel@chromium.org>
Cr-Commit-Position: refs/heads/master@{#552605}
[modify] https://crrev.com/bca65ccdbf1442e03d2b381e63f1f872274827f3/chrome/browser/chromeos/file_manager/audio_player_browsertest.cc
[modify] https://crrev.com/bca65ccdbf1442e03d2b381e63f1f872274827f3/testing/buildbot/filters/mash.browser_tests.filter

Logs in comment 3 indicate that Chromium is trying to use PulseAudio. This won't work on Chrome OS (which uses CRAS), so I guess the build flags are messed up somehow (and it's a bit suspicious that the test is passing).

Comment 8 by noel@chromium.org, Apr 23 2018

Thanks Max.  These tests are testing the front-end JS/HTML of the audio player by the looks, not the low-level audio, best I can tell.

I'm leaving as is for now to support your audio work: saw issue 835742 and agree, if we "fix" AudioPlayer here, then be aware of issue 835742.


Comment 10 by noel@chromium.org, Apr 24 2018

One flake in ASAN, AudioPlayerBrowserTest.ChangeVolumeLevel in build

https://ci.chromium.org/buildbot/tryserver.chromium.linux/linux_chromium_chromeos_asan_rel_ng/15567

[16492:16492:0423/152424.323715:FATAL:extension_function.cc(504)] Check failed: !browser_client || browser_client->IsShuttingDown() || did_respond() || ignore_all_did_respond_for_testing_do_not_use. test.sendMessage

This flake is known  issue 668680 

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

Current AudiPlayer tests are going well [1], no evidence of  issue 668680  (landed a patch there to fix it), and are re-enabled on all bots.

I'd love to close here, but there is unanswered questions ...

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

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

Question #4 for fukino-san.
 Any thoughts / tips?

Question #7,#8 for Max.
 The build is chrome-os on linux.  Not sure what the right audio flags should
 be.  Noticed dale's patch [1].  Should we run --disable-audio-output to fake
 the audio layer?
Yep, using that would make sense.

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

OK thx, and if I went ahead, do you have edge-case test coverage for your stuff?
There isn't a full coverage of browser tests for all the audio-related error cases yet, and there probably won't be for M68. There are still browser tests for many error cases as well as comprehensive unit tests though.

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

Ok, posted https://chromium-review.googlesource.com/c/chromium/src/+/1047045 if we want go ahead using --disable-audio-output ... let me know.
Project Member

Comment 18 by bugdroid1@chromium.org, May 8 2018

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

commit 8c1f585122a8365d2662a4f35c785bacacb93d23
Author: Noel Gordon <noel@chromium.org>
Date: Tue May 08 02:13:14 2018

Re-enable SortColumns/FileManagerBrowserTest all bots

Enabled in RELEASE, now re-enable this test in DEBUG/MSAN/ASAN.

No-Presubmit: true
No-Try: true
Bug:  835626 
Change-Id: I079d77a41435f188b2286460f0ee072f1ce6c87c
Reviewed-on: https://chromium-review.googlesource.com/1049185
Commit-Queue: Noel Gordon <noel@chromium.org>
Reviewed-by: Stuart Langley <slangley@chromium.org>
Cr-Commit-Position: refs/heads/master@{#556649}
[modify] https://crrev.com/8c1f585122a8365d2662a4f35c785bacacb93d23/chrome/browser/chromeos/file_manager/file_manager_browsertest.cc

Project Member

Comment 19 by bugdroid1@chromium.org, May 8 2018

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

commit f4b0f66553dcc0c1b5d0514a20925e77f55f1403
Author: Noel Gordon <noel@chromium.org>
Date: Tue May 08 03:16:33 2018

Re-enable DirectoryTreeContextMenuGear/FileManagerBrowserTest all bots

Enabled in RELEASE, now re-enable this test in DEBUG/MSAN/ASAN.

Bug:  835626 
Change-Id: I45957a882b6f3209cc06f6eb2f70d1e6d12f4012
Reviewed-on: https://chromium-review.googlesource.com/1049087
Reviewed-by: Stuart Langley <slangley@chromium.org>
Commit-Queue: Noel Gordon <noel@chromium.org>
Cr-Commit-Position: refs/heads/master@{#556668}
[modify] https://crrev.com/f4b0f66553dcc0c1b5d0514a20925e77f55f1403/chrome/browser/chromeos/file_manager/file_manager_browsertest.cc

Project Member

Comment 20 by bugdroid1@chromium.org, May 8 2018

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

commit 8e639c09b7d599e5238119609cf09fa209b16b6d
Author: Noel Gordon <noel@chromium.org>
Date: Tue May 08 05:32:36 2018

Re-enable ShowGridView/FileManagerBrowserTest all bots

Enabled in RELEASE, now re-enable this test in DEBUG/MSAN/ASAN.

Bug:  835626 
Change-Id: I1f2037fe9bebc3005b561440f6ead2dd3eadf80c
Reviewed-on: https://chromium-review.googlesource.com/1049090
Reviewed-by: Tatsuhisa Yamaguchi <yamaguchi@chromium.org>
Commit-Queue: Noel Gordon <noel@chromium.org>
Cr-Commit-Position: refs/heads/master@{#556699}
[modify] https://crrev.com/8e639c09b7d599e5238119609cf09fa209b16b6d/chrome/browser/chromeos/file_manager/file_manager_browsertest.cc

Project Member

Comment 21 by bugdroid1@chromium.org, May 8 2018

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

commit 9855fd4c1b9a2bb2e14f93aeed278b97f7e0c396
Author: Noel Gordon <noel@chromium.org>
Date: Tue May 08 05:55:24 2018

[ChromeOS] AudioPlayerBrowserTest: use faked audio layer

These AudioPlayerBrowserTest run on a target_os="chromeos" linux build
which attempts to use PulseAudio for audio, and always fails [1].

The tests still PASS, since they don't need to test lower-layer audio,
but they do output PulseAudio LOG failure messages. To avoid that, use
a fake audio layer instead, which can be requested by a media command-
line switch kDisableAudioOutput after crrev.com/539638.

[1] PulseAudio won't work in this case and the LOG output of the tests
indicate that (see  bug 835626  comments #3 and #7).

Bug:  835626 ,835742
Change-Id: I8113c15a123bfb8048141e7a20c8d02cf3341e13
Reviewed-on: https://chromium-review.googlesource.com/1047045
Reviewed-by: Tatsuhisa Yamaguchi <yamaguchi@chromium.org>
Reviewed-by: Naoki Fukino <fukino@chromium.org>
Reviewed-by: Max Morin <maxmorin@chromium.org>
Commit-Queue: Noel Gordon <noel@chromium.org>
Cr-Commit-Position: refs/heads/master@{#556704}
[modify] https://crrev.com/9855fd4c1b9a2bb2e14f93aeed278b97f7e0c396/chrome/browser/chromeos/file_manager/audio_player_browsertest.cc

Project Member

Comment 22 by bugdroid1@chromium.org, May 8 2018

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

commit be33e329cfd1e3d11288f0370d09ea71b26247dd
Author: Noel Gordon <noel@chromium.org>
Date: Tue May 08 08:12:05 2018

Re-enable DefaultTaskDialog/FileManagerBrowserTest all bots

Enabled in RELEASE, now re-enable this test in DEBUG/MSAN/ASAN.

Bug:  835626 
Change-Id: Ifd9d394f5cf2f4dd25146e0a0422b7bcd00b4103
Reviewed-on: https://chromium-review.googlesource.com/1049415
Reviewed-by: Naoki Fukino <fukino@chromium.org>
Commit-Queue: Noel Gordon <noel@chromium.org>
Cr-Commit-Position: refs/heads/master@{#556721}
[modify] https://crrev.com/be33e329cfd1e3d11288f0370d09ea71b26247dd/chrome/browser/chromeos/file_manager/file_manager_browsertest.cc

Project Member

Comment 23 by bugdroid1@chromium.org, May 8 2018

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

commit bd01410b63f05af2e278f369fb4282735684fef5
Author: Noel Gordon <noel@chromium.org>
Date: Tue May 08 08:12:56 2018

Re-enable TabindexFocusDirectorySelected/FileManagerBrowserTest all bots

Enabled in RELEASE, now re-enable this test in DEBUG/MSAN/ASAN.

Bug:  835626 
Change-Id: I404db91904dab92834a7efca5a567b2ad66d84e4
Reviewed-on: https://chromium-review.googlesource.com/1049474
Reviewed-by: Naoki Fukino <fukino@chromium.org>
Commit-Queue: Noel Gordon <noel@chromium.org>
Cr-Commit-Position: refs/heads/master@{#556722}
[modify] https://crrev.com/bd01410b63f05af2e278f369fb4282735684fef5/chrome/browser/chromeos/file_manager/file_manager_browsertest.cc

Project Member

Comment 24 by bugdroid1@chromium.org, May 8 2018

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

commit be4b2b171e946872c085babaf87724849c2e7b22
Author: Noel Gordon <noel@chromium.org>
Date: Tue May 08 08:17:42 2018

Re-enable ExecuteDefaultTaskOnDownloads/FileManagerBrowserTest all bots

Enabled in RELEASE, now re-enable this test in DEBUG/MSAN/ASAN.

Bug:  835626 
Change-Id: I848da4a1571db55d1bbf4795eaa469ff564a55e2
Reviewed-on: https://chromium-review.googlesource.com/1049416
Reviewed-by: Naoki Fukino <fukino@chromium.org>
Commit-Queue: Noel Gordon <noel@chromium.org>
Cr-Commit-Position: refs/heads/master@{#556724}
[modify] https://crrev.com/be4b2b171e946872c085babaf87724849c2e7b22/chrome/browser/chromeos/file_manager/file_manager_browsertest.cc

Project Member

Comment 25 by bugdroid1@chromium.org, May 8 2018

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

commit 9a152bcab51b28e9318d1dbf3e2181c10eab599a
Author: Noel Gordon <noel@chromium.org>
Date: Tue May 08 09:50:43 2018

Re-enable Transfer/FileManagerBrowserTest all bots

Enabled in RELEASE, now re-enable this test in DEBUG/MSAN/ASAN.

Bug:  835626 
Change-Id: Ie214d9ef7241ab084772010b2a854e50ef051b67
Reviewed-on: https://chromium-review.googlesource.com/1049347
Reviewed-by: Naoki Fukino <fukino@chromium.org>
Commit-Queue: Noel Gordon <noel@chromium.org>
Cr-Commit-Position: refs/heads/master@{#556741}
[modify] https://crrev.com/9a152bcab51b28e9318d1dbf3e2181c10eab599a/chrome/browser/chromeos/file_manager/file_manager_browsertest.cc

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

Ok, the fake audio layer for the AudioPlayer tests in landed (#21).

Fukino-san mentioned to me off-line, re: the other issues:

1) paper-ripple

220199:220199:0422/181000.274214:INFO:CONSOLE(129)] "Uncaught TypeError: Cannot read property 'appendChild' of null", source: chrome://resources/polymer/v1_0/paper-ripple/paper-ripple-extracted.js (129)

"We use a forked version of paper-ripple. I guess there is an issue in the forked version."

2) The play() interrupted by pause() error

[220199:220199:0422/180959.941873:INFO:CONSOLE(0)] "Uncaught (in promise) AbortError: The play() request was interrupted by a call to pause(). https://goo.gl/LdLk22",

https://developers.google.com/web/updates/2017/06/play-request-was-interrupted
looks like it'd help here.

fukino-san: you mentioned "I can reproduce the error log by simply clicking the track row of audio player (and we don't see the ripple effect on the row). This is an issue, but not the testing issue."

You did that with the live Files.app on ChromeOS?  Seems like we have a real- live Files.app bug if so?
Project Member

Comment 27 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 28 by bugdroid1@chromium.org, May 9 2018

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

commit ead7790aab6fd1469949730e207dcf826b2f3dbd
Author: Noel Gordon <noel@chromium.org>
Date: Wed May 09 03:01:30 2018

Update AudioPlayerBrowserTest togglePlayState test

Add more documentation. Add an extra step to toggle the play button to
state play again at the end of the test: re-test the label, and player
state, to confirm that the UI indicates audio is playing.

For the record, this test does not exhibit to "Uncaught exception play
interrupted by pause" problem per this test's LOG in a DEBUG build. So
that's maybe a clue about that error ( bug 835626  comment #3).

Test: browser_test --gtest_filter="AudioPlayerBrowserTest/TogglePlay*"
No-Presubmit: true
Bug:  835626 
Cq-Include-Trybots: master.tryserver.chromium.linux:closure_compilation
Change-Id: I8af5abcdddfe9b19a22ac257327b359511e2d263
Reviewed-on: https://chromium-review.googlesource.com/1050165
Commit-Queue: Noel Gordon <noel@chromium.org>
Reviewed-by: Naoki Fukino <fukino@chromium.org>
Cr-Commit-Position: refs/heads/master@{#557073}
[modify] https://crrev.com/ead7790aab6fd1469949730e207dcf826b2f3dbd/ui/file_manager/integration_tests/audio_player/click_control_buttons.js

Comment 29 by noel@chromium.org, May 9 2018

Per #28, "Uncaught exception play interrupted by pause" was due to not waiting for the Play button aria-label state change.

We could wait the aria-label state change in the changeTracks test and eliminate it from that test too.  The changeTracks tests multiple things though, so I think we can break it up into 3 separate changeTracks sub-tests.
Project Member

Comment 30 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

Project Member

Comment 31 by bugdroid1@chromium.org, May 10 2018

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

commit d7043c37f69048b011ad9d8a99d685bebeff4a19
Author: Noel Gordon <noel@chromium.org>
Date: Thu May 10 02:44:45 2018

Break AudioPlayerBrowserTest changeTracks into separate tests

Break test changeTracks into three separate tests, rather than do it
all in one test. And hopefully, improve test speed and robustness by
eliminating the interrupted play() -> pause() problem, which was due
due to not waiting for the play button aria-label state change.

changeTracks
  Clicks on the "next" button and tests that the "next" track plays.
  Move what was formerly in this test, to the new tests.

changeTracksPlayList
  Expands the track list by clicking on the the play-list button and
  clicks track 0. Test that the expected audio track (0) plays.

changeTracksPlayListIcon
  Expands the track list by clicking on the the play-list button and
  clicks the track 1 'Play' icon. Test that the expected audio track
  (1) plays.

Add more comments to the togglePlayState and changeVolumeLevel tests
and comments describing the new tests.

Test: browser_tests --gtest_filter=AudioPlayerBrowserTest.ChangeTr*
Bug:  835626 
Cq-Include-Trybots: master.tryserver.chromium.linux:closure_compilation
Change-Id: Ibfb771ab7b276fbb64e2f40c5d17221a95244d15
Reviewed-on: https://chromium-review.googlesource.com/1051306
Reviewed-by: Naoki Fukino <fukino@chromium.org>
Commit-Queue: Noel Gordon <noel@chromium.org>
Cr-Commit-Position: refs/heads/master@{#557434}
[modify] https://crrev.com/d7043c37f69048b011ad9d8a99d685bebeff4a19/chrome/browser/chromeos/file_manager/audio_player_browsertest.cc
[modify] https://crrev.com/d7043c37f69048b011ad9d8a99d685bebeff4a19/ui/file_manager/integration_tests/audio_player/click_control_buttons.js

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

The new tests above do not exhibit the play() interrupted by pause() problem anymore in test.

The last two tests (changeTracksPlayList, changeTracksPlayListICon) still exhibit the paper ripple problem though.

Fukino-san reproduced that paper ripple problem in the real Files.app and filed a new issue 841654 about it so we can move on here.

Comment 33 by noel@chromium.org, May 16 2018

These tests now run fine in RELEASE/DEBUG/MSAN/ASAN, so marking fixed.

One thing note: we disabled audio and used a fake layer #30. The logging about PulseAudio  disappeared from FileManager browser test logs (good).

A week or so later, it re-appeared and proceeds the FileManager browser test logs.  Looks like it got re-enabled earlier in the process, somehow ¯\(ツ)/¯

Comment 34 by noel@chromium.org, May 16 2018

Status: Fixed (was: Started)
The actual log I'm seeing #33 is:

ALSA lib confmisc.c:768:(parse_card) cannot find card '0'
ALSA lib conf.c:4248:(_snd_config_evaluate) function snd_func_card_driver returned error: No such file or directory
ALSA lib confmisc.c:392:(snd_func_concat) error evaluating strings
ALSA lib conf.c:4248:(_snd_config_evaluate) function snd_func_concat returned error: No such file or directory
ALSA lib confmisc.c:1251:(snd_func_refer) error evaluating name
ALSA lib conf.c:4248:(_snd_config_evaluate) function snd_func_refer returned error: No such file or directory
ALSA lib conf.c:4727:(snd_config_expand) Evaluate error: No such file or directory
ALSA lib pcm.c:2239:(snd_pcm_open_noupdate) Unknown PCM default
[21402:21632:0516/064204.898261:WARNING:alsa_util.cc(24)] PcmOpen: default,No such file or directory
ALSA lib confmisc.c:768:(parse_card) cannot find card '0'
ALSA lib conf.c:4248:(_snd_config_evaluate) function snd_func_card_driver returned error: No such file or directory
ALSA lib confmisc.c:392:(snd_func_concat) error evaluating strings
ALSA lib conf.c:4248:(_snd_config_evaluate) function snd_func_concat returned error: No such file or directory
ALSA lib confmisc.c:1251:(snd_func_refer) error evaluating name
ALSA lib conf.c:4248:(_snd_config_evaluate) function snd_func_refer returned error: No such file or directory
ALSA lib conf.c:4727:(snd_config_expand) Evaluate error: No such file or directory
ALSA lib pcm.c:2239:(snd_pcm_open_noupdate) Unknown PCM default
[21402:21632:0516/064204.898419:WARNING:alsa_util.cc(24)] PcmOpen: plug:default,No such file or directory

Might be audio, not really sure.

Sign in to add a comment