Skip to content

Commit

Permalink
[bluetooth] Fix test flake
Browse files Browse the repository at this point in the history
The hypothesis for why these tests flake is the following:

- A local component (for example: `mock_avrcp_client` connects to some
  capabilities.
- The local component returns (exits).
- component_manager shuts down the local component
- Before the component's namespace vfs has a chance to process the Open
  requests for #1, component_manager drops the vfs.
- Because the Open requests were dropped, we never run routing tasks for
  them.
- Because we never run routing tasks, the component on the other end is
  never started.
- The test stalls waiting for events that will never come.

To fix, change these tasks to never exit.

We should eventually fix component_manager to guarantee processing of
these requests, but it's a more complicated fix (b/303919602).

Multiply: bt-avrcp-smoke-test
Multiply: bt-a2dp-smoke-test
Multiply: bt-hfp-audio-gateway-smoke-test
Multiply: bt-avrcp-target-smoke-test
Multiply: bt-init-smoke-test
Multiply: bt-gap-smoke-test

Fixed: 133235
Change-Id: I47e8995430a15648372539ad6d26f2d68fdce9fd
Reviewed-on: https://fuchsia-review.googlesource.com/c/fuchsia/+/928705
Reviewed-by: Marie Janssen <jamuraa@google.com>
Commit-Queue: Gary Bressler <geb@google.com>
Reviewed-by: Ani Ramakrishnan <aniramakri@google.com>
  • Loading branch information
gebressler authored and Rebase bot committed Oct 9, 2023
1 parent 3b24391 commit 222bea5
Show file tree
Hide file tree
Showing 7 changed files with 35 additions and 18 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ use {
fuchsia_component_test::{
Capability, ChildOptions, LocalComponentHandles, RealmBuilder, Ref, Route,
},
futures::{channel::mpsc, SinkExt, StreamExt},
futures::{channel::mpsc, pending, SinkExt, StreamExt},
realmbuilder_mock_helpers::add_fidl_service_handler,
std::{collections::HashSet, iter::FromIterator},
tracing::info,
Expand Down Expand Up @@ -145,6 +145,9 @@ async fn mock_a2dp_client(
.send(Event::A2dpMediaStream(Some(a2dp_media_stream_svc)))
.await
.expect("failed sending ack to test");
// TODO(fxbug.dev/303919602): pending! is a workaround to never exit this component so
// we don't trigger this bug, which can cause a flake.
pending!();
Ok(())
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ use {
fuchsia_component_test::{
Capability, ChildOptions, LocalComponentHandles, RealmBuilder, Ref, Route,
},
futures::{channel::mpsc, SinkExt, StreamExt},
futures::{channel::mpsc, pending, SinkExt, StreamExt},
realmbuilder_mock_helpers::mock_component,
tracing::info,
};
Expand Down Expand Up @@ -52,6 +52,9 @@ async fn mock_avrcp_client(
.send(Event::PeerManagerExt(Some(peer_manager_ext_svc)))
.await
.expect("failed sending ack to test");
// TODO(fxbug.dev/303919602): pending! is a workaround to never exit this component so
// we don't trigger this bug, which can cause a flake.
pending!();
Ok(())
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -80,19 +80,16 @@ async fn mock_avrcp_target_client(
handles: LocalComponentHandles,
) -> Result<(), Error> {
let lifecycle_svc = handles.connect_to_protocol::<LifecycleMarker>()?;
fasync::Task::local(async move {
let lifecycle = lifecycle_svc.clone();
loop {
match lifecycle_svc.get_state().await.unwrap() {
LifecycleState::Initializing => {}
LifecycleState::Ready => break,
}
fasync::Timer::new(fasync::Time::after(1_i64.millis())).await;
let lifecycle = lifecycle_svc.clone();
loop {
match lifecycle_svc.get_state().await.unwrap() {
LifecycleState::Initializing => {}
LifecycleState::Ready => break,
}
info!("Client successfully connected to Lifecycle service");
sender.send(Event::Lifecycle(Some(lifecycle))).await.expect("failed sending ack to test");
})
.detach();
fasync::Timer::new(fasync::Time::after(1_i64.millis())).await;
}
info!("Client successfully connected to Lifecycle service");
sender.send(Event::Lifecycle(Some(lifecycle))).await.expect("failed sending ack to test");
Ok(())
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ use {
fuchsia_component_test::{
Capability, ChildOptions, LocalComponentHandles, RealmBuilder, Ref, Route,
},
futures::{channel::mpsc, SinkExt, StreamExt},
futures::{channel::mpsc, pending, SinkExt, StreamExt},
realmbuilder_mock_helpers::{mock_dev, provide_bt_gap_uses},
std::sync::Arc,
tracing::info,
Expand Down Expand Up @@ -130,6 +130,9 @@ async fn mock_client(
let pairing_svc = handles.connect_to_protocol::<PairingMarker>()?;
sender.send(Event::Pairing(Some(pairing_svc))).await.expect("failed sending ack to test");

// TODO(fxbug.dev/303919602): pending! is a workaround to never exit this component so
// we don't trigger this bug, which can cause a flake.
pending!();
Ok(())
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ use {
fuchsia_component_test::{
Capability, ChildOptions, LocalComponentHandles, RealmBuilder, Ref, Route,
},
futures::{channel::mpsc, SinkExt, StreamExt},
futures::{channel::mpsc, pending, SinkExt, StreamExt},
realmbuilder_mock_helpers::{add_fidl_service_handler, mock_component, mock_dev},
std::{collections::HashSet, iter::FromIterator},
tracing::info,
Expand Down Expand Up @@ -91,6 +91,10 @@ async fn mock_hfp_client(

let hfp_test_svc = handles.connect_to_protocol::<HfpTestMarker>()?;
sender.send(Event::HfpTest(Some(hfp_test_svc))).await.expect("failed sending ack to test");

// TODO(fxbug.dev/303919602): pending! is a workaround to never exit this component so
// we don't trigger this bug, which can cause a flake.
pending!();
Ok(())
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ use {
fuchsia_component_test::{
Capability, ChildOptions, LocalComponentHandles, RealmBuilder, Ref, Route,
},
futures::{channel::mpsc, SinkExt, StreamExt},
futures::{channel::mpsc, pending, SinkExt, StreamExt},
realmbuilder_mock_helpers::{add_fidl_service_handler, mock_dev, provide_bt_gap_uses},
std::sync::Arc,
tracing::info,
Expand Down Expand Up @@ -135,6 +135,9 @@ async fn mock_client(
.await
.expect("failed sending ack to test");

// TODO(fxbug.dev/303919602): pending! is a workaround to never exit this component so
// we don't trigger this bug, which can cause a flake.
pending!();
Ok(())
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ use {
fuchsia_component_test::{
Capability, ChildOptions, LocalComponentHandles, RealmBuilder, Ref, Route,
},
futures::{channel::mpsc, SinkExt, StreamExt},
futures::{channel::mpsc, pending, SinkExt, StreamExt},
realmbuilder_mock_helpers::add_fidl_service_handler,
tracing::info,
};
Expand Down Expand Up @@ -47,6 +47,10 @@ async fn mock_rfcomm_client(

let test_svc = handles.connect_to_protocol::<RfcommTestMarker>()?;
sender.send(Event::Test(Some(test_svc))).await.expect("failed sending ack to test");

// TODO(fxbug.dev/303919602): pending! is a workaround to never exit this component so
// we don't trigger this bug, which can cause a flake.
pending!();
Ok(())
}

Expand Down

0 comments on commit 222bea5

Please sign in to comment.