[pkg] Use new MockPaver Hooks.
This replaces usage of the `config_hook`, `config_status_hook`,
`firmware_hook`, and `read_hook` methods of MockPaver with equivalent
calls to the `insert_hook` method introduced in fxrev.dev/448895.
Change-Id: Id9d2651a230cb3b7ad2076ed9f616540700b6931
Reviewed-on: https://fuchsia-review.googlesource.com/c/fuchsia/+/450134
Reviewed-by: Ben Keller <galbanum@google.com>
Testability-Review: Ben Keller <galbanum@google.com>
Commit-Queue: Hunter Freyer <hjfreyer@google.com>
diff --git a/src/sys/pkg/bin/system-updater/src/update/history/version.rs b/src/sys/pkg/bin/system-updater/src/update/history/version.rs
index e443c72..02d534a 100644
--- a/src/sys/pkg/bin/system-updater/src/update/history/version.rs
+++ b/src/sys/pkg/bin/system-updater/src/update/history/version.rs
@@ -246,7 +246,7 @@
fidl_fuchsia_paver::Configuration,
fuchsia_pkg_testing::TestUpdatePackage,
fuchsia_zircon::Vmo,
- mock_paver::{MockPaverServiceBuilder, PaverEvent},
+ mock_paver::{hooks as mphooks, MockPaverServiceBuilder},
pretty_assertions::assert_eq,
std::sync::Arc,
};
@@ -293,18 +293,13 @@
let paver = Arc::new(
MockPaverServiceBuilder::new()
.active_config(Configuration::A)
- .read_hook(|event| match event {
- PaverEvent::ReadAsset { configuration, asset } => {
- assert_eq!(configuration, &Configuration::A);
- match asset {
- Asset::Kernel => Ok(vec![0x7a, 0x62, 0x69]),
- Asset::VerifiedBootMetadata => {
- Ok(vec![0x76, 0x62, 0x6d, 0x65, 0x74, 0x61])
- }
- }
+ .insert_hook(mphooks::read_asset(|configuration, asset| {
+ assert_eq!(configuration, Configuration::A);
+ match asset {
+ Asset::Kernel => Ok(vec![0x7a, 0x62, 0x69]),
+ Asset::VerifiedBootMetadata => Ok(vec![0x76, 0x62, 0x6d, 0x65, 0x74, 0x61]),
}
- _ => panic!("Unexpected event: {:?}", event),
- })
+ }))
.build(),
);
let data_sink = paver.spawn_data_sink_service();
diff --git a/src/sys/pkg/bin/system-updater/src/update/paver.rs b/src/sys/pkg/bin/system-updater/src/update/paver.rs
index fe49cfe..8619c78 100644
--- a/src/sys/pkg/bin/system-updater/src/update/paver.rs
+++ b/src/sys/pkg/bin/system-updater/src/update/paver.rs
@@ -511,7 +511,7 @@
super::*,
fidl_fuchsia_paver::DataSinkMarker,
matches::assert_matches,
- mock_paver::{MockPaverServiceBuilder, PaverEvent},
+ mock_paver::{hooks as mphooks, MockPaverServiceBuilder, PaverEvent},
std::sync::Arc,
};
@@ -639,7 +639,7 @@
let paver = Arc::new(
MockPaverServiceBuilder::new()
.current_config(current_config)
- .config_status_hook(move |_| status)
+ .insert_hook(mphooks::config_status(move |_| Ok(status)))
.build(),
);
let boot_manager = paver.spawn_boot_manager_service();
@@ -788,10 +788,9 @@
async fn write_image_buffer_ignores_unsupported_firmware() {
let paver = Arc::new(
MockPaverServiceBuilder::new()
- .firmware_hook(|event| match event {
- PaverEvent::WriteFirmware { .. } => WriteFirmwareResult::Unsupported(true),
- _ => panic!("Unexpected event: {:?}", event),
- })
+ .insert_hook(mphooks::write_firmware(|_, _, _| {
+ WriteFirmwareResult::Unsupported(true)
+ }))
.build(),
);
let data_sink = paver.spawn_data_sink_service();
@@ -819,12 +818,9 @@
async fn write_image_buffer_forwards_other_firmware_errors() {
let paver = Arc::new(
MockPaverServiceBuilder::new()
- .firmware_hook(|event| match event {
- PaverEvent::WriteFirmware { .. } => {
- WriteFirmwareResult::Status(Status::INTERNAL.into_raw())
- }
- _ => panic!("Unexpected event: {:?}", event),
- })
+ .insert_hook(mphooks::write_firmware(|_, _, _| {
+ WriteFirmwareResult::Status(Status::INTERNAL.into_raw())
+ }))
.build(),
);
let data_sink = paver.spawn_data_sink_service();
@@ -910,10 +906,10 @@
async fn write_image_buffer_write_asset_forwards_errors() {
let paver = Arc::new(
MockPaverServiceBuilder::new()
- .call_hook(|event| match event {
+ .insert_hook(mphooks::return_error(|event| match event {
PaverEvent::WriteAsset { .. } => Status::INTERNAL,
_ => panic!("Unexpected event: {:?}", event),
- })
+ }))
.build(),
);
let data_sink = paver.spawn_data_sink_service();
@@ -1066,7 +1062,7 @@
use {
super::*,
matches::assert_matches,
- mock_paver::{MockPaverServiceBuilder, PaverEvent},
+ mock_paver::{hooks as mphooks, MockPaverServiceBuilder, PaverEvent},
std::sync::Arc,
};
@@ -1204,14 +1200,14 @@
async fn write_image_buffer_forwards_config_a_error() {
let paver = Arc::new(
MockPaverServiceBuilder::new()
- .call_hook(|event| match event {
+ .insert_hook(mphooks::return_error(|event| match event {
PaverEvent::WriteAsset {
configuration: Configuration::A,
asset: Asset::Kernel,
payload: _,
} => Status::INTERNAL,
_ => Status::OK,
- })
+ }))
.build(),
);
let data_sink = paver.spawn_data_sink_service();
@@ -1241,14 +1237,14 @@
async fn write_image_buffer_ignores_config_b_not_supported_error() {
let paver = Arc::new(
MockPaverServiceBuilder::new()
- .call_hook(|event| match event {
+ .insert_hook(mphooks::return_error(|event| match event {
PaverEvent::WriteAsset {
configuration: Configuration::B,
asset: Asset::Kernel,
payload: _,
} => Status::NOT_SUPPORTED,
_ => Status::OK,
- })
+ }))
.build(),
);
let data_sink = paver.spawn_data_sink_service();
@@ -1283,14 +1279,10 @@
async fn write_image_buffer_write_firmware_ignores_unsupported_config_b() {
let paver = Arc::new(
MockPaverServiceBuilder::new()
- .firmware_hook(|event| match event {
- PaverEvent::WriteFirmware {
- configuration: Configuration::B,
- firmware_type: _,
- payload: _,
- } => WriteFirmwareResult::Unsupported(true),
+ .insert_hook(mphooks::write_firmware(|configuration, _, _| match configuration {
+ Configuration::B => WriteFirmwareResult::Unsupported(true),
_ => WriteFirmwareResult::Status(Status::OK.into_raw()),
- })
+ }))
.build(),
);
let data_sink = paver.spawn_data_sink_service();
@@ -1325,14 +1317,14 @@
async fn write_image_buffer_forwards_other_config_b_errors() {
let paver = Arc::new(
MockPaverServiceBuilder::new()
- .call_hook(|event| match event {
+ .insert_hook(mphooks::return_error(|event| match event {
PaverEvent::WriteAsset {
configuration: Configuration::B,
asset: Asset::Kernel,
..
} => Status::INTERNAL,
_ => Status::OK,
- })
+ }))
.build(),
);
let data_sink = paver.spawn_data_sink_service();
diff --git a/src/sys/pkg/lib/isolated-swd/src/omaha.rs b/src/sys/pkg/lib/isolated-swd/src/omaha.rs
index b8e4935..f325447 100644
--- a/src/sys/pkg/lib/isolated-swd/src/omaha.rs
+++ b/src/sys/pkg/lib/isolated-swd/src/omaha.rs
@@ -156,7 +156,7 @@
fuchsia_component::server::{ServiceFs, ServiceObj},
fuchsia_pkg_testing::PackageBuilder,
fuchsia_zircon as zx,
- mock_paver::PaverEvent,
+ mock_paver::{hooks as mphooks, PaverEvent},
omaha_client::http_request::mock::MockHttpRequest,
serde_json::json,
};
@@ -266,7 +266,7 @@
.context("Building test_package")?;
let updater = UpdaterBuilder::new()
.await
- .paver(|p| p.call_hook(hook))
+ .paver(|p| p.insert_hook(mphooks::return_error(hook)))
.repo_url(TEST_REPO_URL)
.add_package(test_package)
.add_image("zbi.signed", &data)
diff --git a/src/sys/pkg/lib/system-health-check/src/lib.rs b/src/sys/pkg/lib/system-health-check/src/lib.rs
index 4966fc4..8590227 100644
--- a/src/sys/pkg/lib/system-health-check/src/lib.rs
+++ b/src/sys/pkg/lib/system-health-check/src/lib.rs
@@ -180,7 +180,7 @@
fuchsia_async as fasync,
fuchsia_zircon::Status,
matches::assert_matches,
- mock_paver::{MockPaverServiceBuilder, PaverEvent},
+ mock_paver::{hooks as mphooks, MockPaverServiceBuilder, PaverEvent},
std::sync::Arc,
};
@@ -191,7 +191,7 @@
let paver = Arc::new(
MockPaverServiceBuilder::new()
.current_config(current_config)
- .config_status_hook(|_| paver::ConfigurationStatus::Healthy)
+ .insert_hook(mphooks::config_status(|_| Ok(paver::ConfigurationStatus::Healthy)))
.build(),
);
check_and_set_system_health(&paver.spawn_boot_manager_service()).await.unwrap();
@@ -224,7 +224,7 @@
let paver = Arc::new(
MockPaverServiceBuilder::new()
.current_config(current_config)
- .config_status_hook(|_| paver::ConfigurationStatus::Pending)
+ .insert_hook(mphooks::config_status(|_| Ok(paver::ConfigurationStatus::Pending)))
.build(),
);
check_and_set_system_health(&paver.spawn_boot_manager_service()).await.unwrap();
@@ -255,7 +255,7 @@
let paver = Arc::new(
MockPaverServiceBuilder::new()
.current_config(current_config)
- .config_status_hook(|_| paver::ConfigurationStatus::Unbootable)
+ .insert_hook(mphooks::config_status(|_| Ok(paver::ConfigurationStatus::Unbootable)))
.build(),
);
assert_matches!(
@@ -299,7 +299,7 @@
let paver = Arc::new(
MockPaverServiceBuilder::new()
.current_config(Configuration::Recovery)
- .config_status_hook(|_| paver::ConfigurationStatus::Healthy)
+ .insert_hook(mphooks::config_status(|_| Ok(paver::ConfigurationStatus::Healthy)))
.build(),
);
check_and_set_system_health(&paver.spawn_boot_manager_service()).await.unwrap();
@@ -310,10 +310,10 @@
async fn test_fails_when_set_healthy_fails() {
let paver = Arc::new(
MockPaverServiceBuilder::new()
- .call_hook(|e| match e {
+ .insert_hook(mphooks::return_error(|e| match e {
PaverEvent::SetConfigurationHealthy { .. } => Status::OUT_OF_RANGE,
_ => Status::OK,
- })
+ }))
.build(),
);
diff --git a/src/sys/pkg/testing/mock-paver/src/lib.rs b/src/sys/pkg/testing/mock-paver/src/lib.rs
index 57f61c3..07a6485 100644
--- a/src/sys/pkg/testing/mock-paver/src/lib.rs
+++ b/src/sys/pkg/testing/mock-paver/src/lib.rs
@@ -331,44 +331,6 @@
self
}
- /// TODO: Remove and replace with direct calls to insert_hook.
- pub fn call_hook<F>(self, call_hook: F) -> Self
- where
- F: Fn(&PaverEvent) -> Status + Send + Sync + 'static,
- {
- self.insert_hook(hooks::return_error(call_hook))
- }
-
- /// TODO: Remove and replace with direct calls to insert_hook.
- pub fn config_status_hook<F>(self, config_status_hook: F) -> Self
- where
- F: Fn(&PaverEvent) -> paver::ConfigurationStatus + Send + Sync + 'static,
- {
- self.insert_hook(hooks::config_status(move |configuration| {
- Ok(config_status_hook(&PaverEvent::QueryConfigurationStatus { configuration }))
- }))
- }
-
- /// TODO: Remove and replace with direct calls to insert_hook.
- pub fn firmware_hook<F>(self, firmware_hook: F) -> Self
- where
- F: Fn(&PaverEvent) -> paver::WriteFirmwareResult + Send + Sync + 'static,
- {
- self.insert_hook(hooks::write_firmware(move |configuration, firmware_type, payload| {
- firmware_hook(&PaverEvent::WriteFirmware { configuration, firmware_type, payload })
- }))
- }
-
- /// TODO: Remove and replace with direct calls to insert_hook.
- pub fn read_hook<F>(self, read_hook: F) -> Self
- where
- F: Fn(&PaverEvent) -> Result<Vec<u8>, Status> + Send + Sync + 'static,
- {
- self.insert_hook(hooks::read_asset(move |configuration, asset| {
- read_hook(&PaverEvent::ReadAsset { configuration, asset })
- }))
- }
-
// Provide a callback which will be called for every paver event.
// Useful for logging or interaction assertions.
pub fn event_hook<F>(mut self, event_hook: F) -> Self
diff --git a/src/sys/pkg/tests/isolated-ota/src/lib.rs b/src/sys/pkg/tests/isolated-ota/src/lib.rs
index f7331f3..fb29f2f 100644
--- a/src/sys/pkg/tests/isolated-ota/src/lib.rs
+++ b/src/sys/pkg/tests/isolated-ota/src/lib.rs
@@ -22,7 +22,7 @@
isolated_ota::{download_and_apply_update, OmahaConfig, UpdateError},
matches::assert_matches,
mock_omaha_server::{OmahaResponse, OmahaServer},
- mock_paver::{MockPaverService, MockPaverServiceBuilder, PaverEvent},
+ mock_paver::{hooks as mphooks, MockPaverService, MockPaverServiceBuilder, PaverEvent},
serde_json::json,
std::{
collections::{BTreeSet, HashMap},
@@ -376,7 +376,7 @@
};
let env = TestEnvBuilder::new()
- .paver(|p| p.call_hook(paver_hook))
+ .paver(|p| p.insert_hook(mphooks::return_error(paver_hook)))
.add_package(test_package)
.add_image("zbi.signed", "FAIL".as_bytes())
.add_image("fuchsia.vbmeta", "FAIL".as_bytes())
@@ -610,7 +610,7 @@
.add_package(package)
.add_image("zbi.signed", "ZBI".as_bytes())
.blobfs(ClientEnd::from(client))
- .paver(|p| p.call_hook(paver_hook))
+ .paver(|p| p.insert_hook(mphooks::return_error(paver_hook)))
.build()
.await
.context("Building TestEnv")?;
diff --git a/src/sys/pkg/tests/omaha-client/src/lib.rs b/src/sys/pkg/tests/omaha-client/src/lib.rs
index 286ffd7..0dc1257 100644
--- a/src/sys/pkg/tests/omaha-client/src/lib.rs
+++ b/src/sys/pkg/tests/omaha-client/src/lib.rs
@@ -32,7 +32,7 @@
matches::assert_matches,
mock_installer::MockUpdateInstallerService,
mock_omaha_server::{OmahaResponse, OmahaServer},
- mock_paver::{MockPaverService, MockPaverServiceBuilder, PaverEvent},
+ mock_paver::{hooks as mphooks, MockPaverService, MockPaverServiceBuilder, PaverEvent},
mock_reboot::MockRebootService,
mock_resolver::MockResolverService,
parking_lot::Mutex,
@@ -366,10 +366,10 @@
let paver = MockPaverServiceBuilder::new()
.current_config(Configuration::B)
.event_hook(move |e| send.unbounded_send(e.to_owned()).expect("channel stayed open"))
- .call_hook(move |event| match event {
+ .insert_hook(mphooks::return_error(|event| match event {
PaverEvent::SetConfigurationHealthy { .. } => Status::INTERNAL,
_ => Status::OK,
- })
+ }))
.build();
let env = TestEnvBuilder::new().paver(paver).build();
diff --git a/src/sys/pkg/tests/system-update-checker/src/lib.rs b/src/sys/pkg/tests/system-update-checker/src/lib.rs
index 3cdd74e..0e7eea1e 100644
--- a/src/sys/pkg/tests/system-update-checker/src/lib.rs
+++ b/src/sys/pkg/tests/system-update-checker/src/lib.rs
@@ -20,7 +20,7 @@
fuchsia_zircon::Status,
futures::{channel::mpsc, prelude::*},
mock_installer::MockUpdateInstallerService,
- mock_paver::{MockPaverService, MockPaverServiceBuilder, PaverEvent},
+ mock_paver::{hooks as mphooks, MockPaverService, MockPaverServiceBuilder, PaverEvent},
mock_resolver::MockResolverService,
std::{fs::File, sync::Arc},
tempfile::TempDir,
@@ -224,7 +224,9 @@
#[fasync::run_singlethreaded(test)]
// Test will hang if system-update-checker does not call paver service
async fn test_channel_provider_get_current_works_after_paver_service_fails() {
- let env = TestEnvBuilder::new().paver_init(|p| p.call_hook(|_| Status::INTERNAL)).build();
+ let env = TestEnvBuilder::new()
+ .paver_init(|p| p.insert_hook(mphooks::return_error(|_| Status::INTERNAL)))
+ .build();
assert_eq!(
env.proxies.paver_events.take(1).collect::<Vec<PaverEvent>>().await,
@@ -240,7 +242,9 @@
#[fasync::run_singlethreaded(test)]
// Test will hang if system-update-checker does not call paver service
async fn test_update_manager_check_now_works_after_paver_service_fails() {
- let env = TestEnvBuilder::new().paver_init(|p| p.call_hook(|_| Status::INTERNAL)).build();
+ let env = TestEnvBuilder::new()
+ .paver_init(|p| p.insert_hook(mphooks::return_error(|_| Status::INTERNAL)))
+ .build();
assert_eq!(
env.proxies.paver_events.take(1).collect::<Vec<PaverEvent>>().await,
diff --git a/src/sys/pkg/tests/system-updater/src/channel.rs b/src/sys/pkg/tests/system-updater/src/channel.rs
index 95a6f2c..ba619a6 100644
--- a/src/sys/pkg/tests/system-updater/src/channel.rs
+++ b/src/sys/pkg/tests/system-updater/src/channel.rs
@@ -44,8 +44,9 @@
#[fasync::run_singlethreaded(test)]
async fn does_not_promote_target_channel_on_failure() {
- let env =
- TestEnv::builder().paver_service(|builder| builder.call_hook(|_| Status::INTERNAL)).build();
+ let env = TestEnv::builder()
+ .paver_service(|builder| builder.insert_hook(mphooks::return_error(|_| Status::INTERNAL)))
+ .build();
env.set_target_channel("target-channel");
diff --git a/src/sys/pkg/tests/system-updater/src/lib.rs b/src/sys/pkg/tests/system-updater/src/lib.rs
index 2b8e099..7f53cf8 100644
--- a/src/sys/pkg/tests/system-updater/src/lib.rs
+++ b/src/sys/pkg/tests/system-updater/src/lib.rs
@@ -21,7 +21,7 @@
fuchsia_zircon::Status,
futures::prelude::*,
matches::assert_matches,
- mock_paver::{MockPaverService, MockPaverServiceBuilder, PaverEvent},
+ mock_paver::{hooks as mphooks, MockPaverService, MockPaverServiceBuilder, PaverEvent},
mock_reboot::MockRebootService,
mock_resolver::MockResolverService,
parking_lot::Mutex,
@@ -144,8 +144,6 @@
let system_updater_builder =
system_updater_app_builder(&data_path, &build_info_path, &misc_path, mount_data);
- // Set up the paver service to push events to our interactions buffer by overriding
- // call_hook and firmware_hook.
let interactions_paver_clone = Arc::clone(&interactions);
let paver_service = Arc::new(
paver_service_builder
diff --git a/src/sys/pkg/tests/system-updater/src/writes_firmware.rs b/src/sys/pkg/tests/system-updater/src/writes_firmware.rs
index 7ea855d7..a54db87 100644
--- a/src/sys/pkg/tests/system-updater/src/writes_firmware.rs
+++ b/src/sys/pkg/tests/system-updater/src/writes_firmware.rs
@@ -184,7 +184,9 @@
async fn skips_unsupported_firmware_type() {
let env = TestEnv::builder()
.paver_service(|builder| {
- builder.firmware_hook(|_| paver::WriteFirmwareResult::Unsupported(true))
+ builder.insert_hook(mphooks::write_firmware(|_, _, _| {
+ paver::WriteFirmwareResult::Unsupported(true)
+ }))
})
.build();
@@ -241,8 +243,9 @@
async fn fails_on_firmware_write_error() {
let env = TestEnv::builder()
.paver_service(|builder| {
- builder
- .firmware_hook(|_| paver::WriteFirmwareResult::Status(Status::INTERNAL.into_raw()))
+ builder.insert_hook(mphooks::write_firmware(|_, _, _| {
+ paver::WriteFirmwareResult::Status(Status::INTERNAL.into_raw())
+ }))
})
.build();
diff --git a/src/sys/pkg/tests/system-updater/src/writes_images.rs b/src/sys/pkg/tests/system-updater/src/writes_images.rs
index d441674..6c7977b 100644
--- a/src/sys/pkg/tests/system-updater/src/writes_images.rs
+++ b/src/sys/pkg/tests/system-updater/src/writes_images.rs
@@ -34,10 +34,10 @@
async fn fails_on_image_write_error() {
let env = TestEnv::builder()
.paver_service(|builder| {
- builder.call_hook(|event| match event {
+ builder.insert_hook(mphooks::return_error(|event| match event {
PaverEvent::WriteAsset { .. } => Status::INTERNAL,
_ => Status::OK,
- })
+ }))
})
.build();
@@ -199,7 +199,7 @@
let env = TestEnv::builder()
.paver_service(|builder| {
builder
- .config_status_hook(|_| paver::ConfigurationStatus::Pending)
+ .insert_hook(mphooks::config_status(|_| Ok(paver::ConfigurationStatus::Pending)))
.current_config(current_config)
})
.build();
@@ -249,10 +249,10 @@
let env = TestEnv::builder()
.paver_service(|builder| {
builder
- .call_hook(|event| match event {
+ .insert_hook(mphooks::return_error(|event| match event {
PaverEvent::SetConfigurationUnbootable { .. } => Status::INTERNAL,
_ => Status::OK,
- })
+ }))
.current_config(current_config)
})
.build();
