[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();