[ChromeOS] Enable AudioPlayerBrowserTests |
|||||
Issue descriptionAudio 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.
,
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
,
Apr 22 2018
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.
,
Apr 22 2018
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?
,
Apr 22 2018
,
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
,
Apr 23 2018
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).
,
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.
,
Apr 23 2018
So far, so good, RELEASE/MSAN/ASAN results starting to appear: https://test-results.appspot.com/dashboards/flakiness_dashboard.html#testType=browser_tests%20(with%20patch)&tests=AudioPlayerBrowserTest*
,
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
,
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*
,
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?
,
May 7 2018
[1] Dale's --disable-audio-output patch https://chromium.googlesource.com/chromium/src/+/f29eb01290cd36a30177ecf8197f906c01088a0d
,
May 7 2018
Yep, using that would make sense.
,
May 7 2018
OK thx, and if I went ahead, do you have edge-case test coverage for your stuff?
,
May 7 2018
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.
,
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.
,
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
,
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
,
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
,
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
,
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
,
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
,
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
,
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
,
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?
,
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
,
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
,
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.
,
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
,
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
,
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.
,
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 ¯\(ツ)/¯
,
May 16 2018
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 |
|||||
Comment 1 by noel@chromium.org
, Apr 22 2018