Skip to content

Commit

Permalink
[rust] Update bluetooth-tests to prepare for toolchain roll
Browse files Browse the repository at this point in the history
A change to the language async/await semantics caused these tests to
hang. In particular, the lock guard returned by aux() is being saved
across `.await` when it wasn't before.

See rust-lang/rust#63832

Fixes fxb/40072

Change-Id: I456282b6a2e66166cfe65784c04468ed3a395303
  • Loading branch information
tmandry authored and CQ bot account: commit-bot@chromium.org committed Nov 1, 2019
1 parent a36b472 commit 0b2cd7c
Show file tree
Hide file tree
Showing 10 changed files with 118 additions and 69 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -778,7 +778,8 @@ async fn try_restore_bonds(
Some(data) => data,
None => return Ok(()),
};
let res = host_device.read().restore_bonds(data).await;
let fut = host_device.read().restore_bonds(data);
let res = fut.await;
res.map_err(|e| {
fx_log_err!("failed to restore bonding data for host: {:?}", e);
e
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -167,7 +167,7 @@ impl TestDevice {
let channel = fdio::clone_channel(&self.0)?;
let controller = ControllerProxy::new(fasync::Channel::from_channel(channel)?);

let _ = controller
controller
.bind(EMULATOR_DRIVER_PATH)
.map_err(Error::from)
.on_timeout(10.seconds().after_now(), || {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,8 @@ pub async fn new_control_harness() -> Result<ControlHarness, Error> {
let control_harness = ControlHarness::new(proxy);

// Store existing hosts in our state, as we won't get notified about them
let hosts = control_harness.aux().get_adapters().await?;
let fut = control_harness.aux().get_adapters();
let hosts = fut.await?;
if let Some(hosts) = hosts {
for host in hosts {
control_harness.write_state().hosts.insert(host.identifier.clone(), host);
Expand Down Expand Up @@ -198,7 +199,8 @@ pub async fn activate_fake_host(
.identifier
.to_string(); // We can safely unwrap here as this is guarded by the previous expectation

control.aux().set_active_adapter(&host).await?;
let fut = control.aux().set_active_adapter(&host);
fut.await?;
control
.when_satisfied(control_expectation::active_host_is(host.clone()), control_timeout())
.await?;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,11 +58,11 @@ pub fn expect_remote_device(

// Returns a future that resolves when a peer matching `id` is not present on the host.
pub async fn expect_no_peer(host: &HostDriverHarness, id: String) -> Result<(), Error> {
host.when_satisfied(
let fut = host.when_satisfied(
Predicate::<HostState>::new(move |host| host.peers.iter().all(|(i, _)| i != &id), None),
timeout_duration(),
)
.await?;
);
fut.await?;
Ok(())
}

Expand Down Expand Up @@ -115,21 +115,21 @@ pub async fn expect_host_peer(
host: &HostDriverHarness,
target: Predicate<RemoteDevice>,
) -> Result<HostState, Error> {
host.when_satisfied(
let fut = host.when_satisfied(
Predicate::<HostState>::new(
move |host| host.peers.iter().any(|(_, p)| target.satisfied(p)),
None,
),
timeout_duration(),
)
.await
);
fut.await
}

pub async fn expect_adapter_state(
host: &HostDriverHarness,
target: Predicate<AdapterState>,
) -> Result<HostState, Error> {
host.when_satisfied(
let fut = host.when_satisfied(
Predicate::<HostState>::new(
move |host| match &host.host_info.state {
Some(state) => target.satisfied(state),
Expand All @@ -138,8 +138,8 @@ pub async fn expect_adapter_state(
None,
),
timeout_duration(),
)
.await
);
fut.await
}

impl TestHarness for HostDriverHarness {
Expand Down
27 changes: 18 additions & 9 deletions src/connectivity/bluetooth/tests/integration/src/tests/bonding.rs
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,8 @@ async fn add_bonds(
state: &HostDriverHarness,
mut bonds: Vec<BondingData>,
) -> Result<(Status), Error> {
state.aux().proxy().add_bonded_devices(&mut bonds.iter_mut()).err_into().await
let fut = state.aux().proxy().add_bonded_devices(&mut bonds.iter_mut()).err_into();
fut.await
}

const TEST_ID1: &str = "1234";
Expand All @@ -70,7 +71,8 @@ const TEST_NAME2: &str = "Name2";
// Tests initializing bonded LE devices.
async fn test_add_bonded_devices_success(test_state: HostDriverHarness) -> Result<(), Error> {
// Devices should be initially empty.
let devices = test_state.aux().proxy().list_devices().await?;
let fut = test_state.aux().proxy().list_devices();
let devices = fut.await?;
expect_eq!(vec![], devices)?;

let bond_data1 = new_le_bond_data(TEST_ID1, TEST_ADDR1, TEST_NAME1, true /* has LTK */);
Expand All @@ -90,7 +92,8 @@ async fn test_add_bonded_devices_success(test_state: HostDriverHarness) -> Resul
expect_host_peer(&test_state, expected1).await?;
expect_host_peer(&test_state, expected2).await?;

let devices = test_state.aux().proxy().list_devices().await?;
let fut = test_state.aux().proxy().list_devices();
let devices = fut.await?;
expect_eq!(2, devices.len())?;
expect_true!(devices.iter().any(|dev| dev.address == TEST_ADDR1))?;
expect_true!(devices.iter().any(|dev| dev.address == TEST_ADDR2))?;
Expand All @@ -103,15 +106,17 @@ async fn test_add_bonded_devices_success(test_state: HostDriverHarness) -> Resul

async fn test_add_bonded_devices_no_ltk_fails(test_state: HostDriverHarness) -> Result<(), Error> {
// Devices should be initially empty.
let devices = test_state.aux().proxy().list_devices().await?;
let fut = test_state.aux().proxy().list_devices();
let devices = fut.await?;
expect_eq!(vec![], devices)?;

// Inserting a bonded device without a LTK should fail.
let bond_data = new_le_bond_data(TEST_ID1, TEST_ADDR1, TEST_NAME1, false /* no LTK */);
let status = add_bonds(&test_state, vec![bond_data]).await?;
expect_true!(status.error.is_some())?;

let devices = test_state.aux().proxy().list_devices().await?;
let fut = test_state.aux().proxy().list_devices();
let devices = fut.await?;
expect_eq!(vec![], devices)?;

Ok(())
Expand All @@ -121,7 +126,8 @@ async fn test_add_bonded_devices_duplicate_entry(
test_state: HostDriverHarness,
) -> Result<(), Error> {
// Devices should be initially empty.
let devices = test_state.aux().proxy().list_devices().await?;
let fut = test_state.aux().proxy().list_devices();
let devices = fut.await?;
expect_eq!(vec![], devices)?;

// Initialize one entry.
Expand All @@ -135,7 +141,8 @@ async fn test_add_bonded_devices_duplicate_entry(
.and(expectation::peer::bonded(true));

expect_host_peer(&test_state, expected.clone()).await?;
let devices = test_state.aux().proxy().list_devices().await?;
let fut = test_state.aux().proxy().list_devices();
let devices = fut.await?;
expect_eq!(1, devices.len())?;

// Adding an entry with the existing id should fail.
Expand All @@ -155,7 +162,8 @@ async fn test_add_bonded_devices_duplicate_entry(
// but reports an error.
async fn test_add_bonded_devices_invalid_entry(test_state: HostDriverHarness) -> Result<(), Error> {
// Devices should be initially empty.
let devices = test_state.aux().proxy().list_devices().await?;
let fut = test_state.aux().proxy().list_devices();
let devices = fut.await?;
expect_eq!(vec![], devices)?;

// Add one entry with no LTK (invalid) and one with (valid). This should create an entry for the
Expand All @@ -170,7 +178,8 @@ async fn test_add_bonded_devices_invalid_entry(test_state: HostDriverHarness) ->
.and(expectation::peer::bonded(true));

expect_host_peer(&test_state, expected.clone()).await?;
let devices = test_state.aux().proxy().list_devices().await?;
let fut = test_state.aux().proxy().list_devices();
let devices = fut.await?;
expect_eq!(1, devices.len())?;
expect_remote_device(&test_state, TEST_ADDR2, &expected)?;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,8 @@ async fn test_set_active_host(control: ControlHarness) -> Result<(), Error> {
.collect();

for (id, _) in state.hosts {
control.aux().set_active_adapter(&id).await?;
let fut = control.aux().set_active_adapter(&id);
fut.await?;
control.when_satisfied(control_expectation::active_host_is(id), control_timeout()).await?;
}

Expand Down Expand Up @@ -89,7 +90,8 @@ async fn test_disconnect(control: ControlHarness) -> Result<(), Error> {
.await?
.map_err(|e| format_err!("Failed to register fake peer: {:#?}", e))?;

control.aux().request_discovery(true).await?;
let fut = control.aux().request_discovery(true);
fut.await?;
let state = control
.when_satisfied(
control_expectation::peer_exists(expectation::peer::address(&peer_address_string)),
Expand All @@ -103,12 +105,14 @@ async fn test_disconnect(control: ControlHarness) -> Result<(), Error> {
// We can safely unwrap here as this is guarded by the previous expectation
let peer = state.peers.iter().find(|(_, d)| &d.address == &peer_address_string).unwrap().0;

control.aux().connect(peer).await?;
let fut = control.aux().connect(peer);
fut.await?;

control
.when_satisfied(control_expectation::peer_connected(peer, true), control_timeout())
.await?;
control.aux().disconnect(peer).await?;
let fut = control.aux().disconnect(peer);
fut.await?;

control
.when_satisfied(control_expectation::peer_connected(peer, false), control_timeout())
Expand Down
Loading

0 comments on commit 0b2cd7c

Please sign in to comment.