New issue
Advanced search Search tips

Issue 848926 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Jun 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Fuchsia
Pri: 3
Type: Bug



Sign in to add a comment

AudioOutputDeviceTest.CreateBitStreamStream frequently flakes on Fuchsia/x64 FYI bot

Project Member Reported by w...@chromium.org, Jun 1 2018

Issue description

In build https://ci.chromium.org/buildbot/chromium.fyi/Fuchsia/18176, for example the AudioOutputDeviceTest.CreateBitStreamStream test fails, and also results in CreateAndClose and OpenAndClose tests reporting "unknown" results.

 

Comment 1 by w...@chromium.org, Jun 1 2018

Cc: dalecur...@chromium.org w...@chromium.org
Owner: maxmorin@chromium.org
Looks like this test was added in https://chromium-review.googlesource.com/c/chromium/src/+/1023397 and has been flaking since then.
Project Member

Comment 2 by bugdroid1@chromium.org, Jun 1 2018

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

commit b013d8db08cb1da7eed40f0ebf96fc922d7d03bd
Author: Wez <wez@chromium.org>
Date: Fri Jun 01 22:50:46 2018

Disable AudioOutputDeviceTest.CreateBitStreamStream test on Fuchsia.

The test is verifying the behaviour of multi-threaded code, but resets
expectations with Mock::VerifyAndClear() mid-test, which seems likely to
be inherently flaky. However, it only flakes on Fuchsia in practice, at
present, so only disable it there.

TBR: maxmorin
Bug:  848926 
Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;luci.chromium.try:linux_optional_gpu_tests_rel;luci.chromium.try:mac_optional_gpu_tests_rel;luci.chromium.try:win_optional_gpu_tests_rel
Change-Id: Iad403f88d1272ac1e7ed1a193aa41425288c39b1
Reviewed-on: https://chromium-review.googlesource.com/1083624
Reviewed-by: Wez <wez@chromium.org>
Commit-Queue: Wez <wez@chromium.org>
Cr-Commit-Position: refs/heads/master@{#563843}
[modify] https://crrev.com/b013d8db08cb1da7eed40f0ebf96fc922d7d03bd/media/audio/audio_output_device_unittest.cc

env.reader->Read() blocks until the read has been fulfilled, but it has a timeout. I guess the timeout is missed due to Fuchsia scheduling. Will fix, there's a setter for the timeout which was added for some other test.
Status: Started (was: Untriaged)
The "unknown" results are probably due to the large amount of failed expectations printed when the test fails. I'll fix that as well.
Project Member

Comment 5 by bugdroid1@chromium.org, Jun 4 2018

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

commit 8113bc9a78158f1c0663917a291b0b5cc3e349b5
Author: Max Morin <maxmorin@chromium.org>
Date: Mon Jun 04 12:01:22 2018

Deflake CreateBitStreamStream on Fuchsia.

Increase the read timeout to 500 ms, since the default timeout isn't
enough on Fuchsia. Also change an EXPECT to ASSERT to avoid spamming
the bots to hard if the test fails.

Bug:  848926 
Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;luci.chromium.try:linux_optional_gpu_tests_rel;luci.chromium.try:mac_optional_gpu_tests_rel;luci.chromium.try:win_optional_gpu_tests_rel
Change-Id: I0f927eb758da3436e82e23af2a4c3b13cfecca35
Reviewed-on: https://chromium-review.googlesource.com/1084834
Commit-Queue: Max Morin <maxmorin@chromium.org>
Reviewed-by: Olga Sharonova <olka@chromium.org>
Cr-Commit-Position: refs/heads/master@{#564061}
[modify] https://crrev.com/8113bc9a78158f1c0663917a291b0b5cc3e349b5/media/audio/audio_output_device_unittest.cc

Project Member

Comment 6 by bugdroid1@chromium.org, Jun 4 2018

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

commit 0c7d83697282e93f13be78b8ccc6e8380e1cc3be
Author: Max Morin <maxmorin@chromium.org>
Date: Mon Jun 04 12:25:28 2018

Revert "Disable AudioOutputDeviceTest.CreateBitStreamStream test on Fuchsia."

This reverts commit b013d8db08cb1da7eed40f0ebf96fc922d7d03bd.

Reason for revert: Test was fixed in https://crrev.com/c/1084834

Original change's description:
> Disable AudioOutputDeviceTest.CreateBitStreamStream test on Fuchsia.
> 
> The test is verifying the behaviour of multi-threaded code, but resets
> expectations with Mock::VerifyAndClear() mid-test, which seems likely to
> be inherently flaky. However, it only flakes on Fuchsia in practice, at
> present, so only disable it there.
> 
> TBR: maxmorin
> Bug:  848926 
> Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;luci.chromium.try:linux_optional_gpu_tests_rel;luci.chromium.try:mac_optional_gpu_tests_rel;luci.chromium.try:win_optional_gpu_tests_rel
> Change-Id: Iad403f88d1272ac1e7ed1a193aa41425288c39b1
> Reviewed-on: https://chromium-review.googlesource.com/1083624
> Reviewed-by: Wez <wez@chromium.org>
> Commit-Queue: Wez <wez@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#563843}

TBR=wez@chromium.org,maxmorin@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug:  848926 
Change-Id: Ie4f211e9317666f0dc1e57657984243e8885a236
Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;luci.chromium.try:linux_optional_gpu_tests_rel;luci.chromium.try:mac_optional_gpu_tests_rel;luci.chromium.try:win_optional_gpu_tests_rel
Reviewed-on: https://chromium-review.googlesource.com/1084887
Reviewed-by: Max Morin <maxmorin@chromium.org>
Commit-Queue: Max Morin <maxmorin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#564065}
[modify] https://crrev.com/0c7d83697282e93f13be78b8ccc6e8380e1cc3be/media/audio/audio_output_device_unittest.cc

Project Member

Comment 7 by bugdroid1@chromium.org, Jun 5 2018

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

commit 227e9498a3acc26c08ec01bf9b195ba931f227b5
Author: Wez <wez@chromium.org>
Date: Tue Jun 05 05:52:10 2018

Increase timeout for AudioOutputDeviceTests under Fuchsia.

Increase the max-wait timeout from 50ms to 250ms, to allow for extreme
scheduler jank under nested virtualization.

Bug:  838367 ,  848926 
Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;luci.chromium.try:linux_optional_gpu_tests_rel;luci.chromium.try:mac_optional_gpu_tests_rel;luci.chromium.try:win_optional_gpu_tests_rel
Change-Id: If70da6f568293f45e7375a8d4645c27883c34872
Reviewed-on: https://chromium-review.googlesource.com/1086287
Commit-Queue: Max Morin <maxmorin@chromium.org>
Reviewed-by: Max Morin <maxmorin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#564370}
[modify] https://crrev.com/227e9498a3acc26c08ec01bf9b195ba931f227b5/media/audio/audio_output_device_unittest.cc

The test has two threads, the main thread and the callback thread (the |audio_thread_| member https://cs.chromium.org/chromium/src/media/audio/audio_output_device.h?l=199). That thread isn't from the task scheduler and doesn't have a run loop, it has the following ThreadMain: https://cs.chromium.org/chromium/src/media/audio/audio_device_thread.cc?l=58. AudioOutputDevice also has a task runner for the render process io thread, but we use the main thread for that in these tests.

The synchronization in the test works like this:
EXPECT_CALL(env.callback, ...); // At this point, the AudioDeviceThread is blocked waiting for a RequestMoreData call.
env.reader->RequestMoreData(kDelay, env.time_stamp, kFramesSkipped); // Unblocks the AudioDeviceThread, callback can be called now.
env.reader->Read(test_bus.get()); // Blocks until the renderer signals that the write is finished, or until the deadline.
// |callback| won't be called again until RequestMoreData is called again, so we may safely VerifyAndClear (unless the deadline was missed).
Mock::VerifyAndClear(&env.callback);

Comment 9 by w...@chromium.org, Jun 5 2018

Status: Fixed (was: Started)
Re #8: Yup, I worked out the blocking in Read() and submitted the CL to increase the timeouts; will be optimistic and close this out Fixed for now.
[bulk-edit: disregard if N/A] Can the owner please set milestone to this bug if applicable?

Sign in to add a comment