From d2018efecf6057a8094727a7aacdf068b1edac10 Mon Sep 17 00:00:00 2001 From: Eliza Weisman Date: Fri, 6 Feb 2026 11:16:27 -0800 Subject: [PATCH 1/6] [WIP] put phd test output in their own dirs --- phd-tests/README.md | 2 +- phd-tests/framework/src/disk/crucible.rs | 5 +- phd-tests/framework/src/disk/mod.rs | 3 + phd-tests/framework/src/lib.rs | 197 +++++++++++------- phd-tests/framework/src/lifecycle.rs | 6 +- phd-tests/framework/src/test_vm/config.rs | 15 +- .../framework/src/test_vm/environment.rs | 11 +- phd-tests/framework/src/test_vm/mod.rs | 16 +- phd-tests/framework/src/test_vm/server.rs | 10 +- phd-tests/runner/src/execute.rs | 5 +- phd-tests/testcase/src/lib.rs | 5 +- phd-tests/tests/src/boot_order.rs | 10 +- phd-tests/tests/src/cpuid.rs | 10 +- phd-tests/tests/src/crucible/migrate.rs | 4 +- phd-tests/tests/src/crucible/mod.rs | 4 +- phd-tests/tests/src/crucible/smoke.rs | 10 +- phd-tests/tests/src/disk.rs | 6 +- phd-tests/tests/src/framework.rs | 4 +- phd-tests/tests/src/hw.rs | 2 +- phd-tests/tests/src/hyperv.rs | 8 +- phd-tests/tests/src/migrate.rs | 28 +-- phd-tests/tests/src/server_state_machine.rs | 12 +- phd-tests/tests/src/smoke.rs | 8 +- phd-tests/tests/src/stats.rs | 2 +- 24 files changed, 221 insertions(+), 162 deletions(-) diff --git a/phd-tests/README.md b/phd-tests/README.md index 43fc95f8a..5fbef1927 100644 --- a/phd-tests/README.md +++ b/phd-tests/README.md @@ -227,7 +227,7 @@ the adapter to provide the correct credentials. ## Authoring tests PHD's test cases live in the `tests` crate. To write a new test, add a function -of the form `fn my_test(ctx: &Framework)` and tag it with the +of the form `fn my_test(ctx: &TestCtx)` and tag it with the `#[phd_testcase]` attribute macro. The framework will automatically register the test into the crate's test inventory for the runner to discover. diff --git a/phd-tests/framework/src/disk/crucible.rs b/phd-tests/framework/src/disk/crucible.rs index 62efad9bf..6b9694200 100644 --- a/phd-tests/framework/src/disk/crucible.rs +++ b/phd-tests/framework/src/disk/crucible.rs @@ -92,6 +92,7 @@ impl CrucibleDisk { read_only_parent: Option<&impl AsRef>, guest_os: Option, log_config: LogConfig, + output_dir: &impl AsRef, ) -> anyhow::Result { Ok(Self { device_name, @@ -105,6 +106,7 @@ impl CrucibleDisk { data_dir_root, read_only_parent, log_config, + output_dir, )?), }) } @@ -209,6 +211,7 @@ impl Inner { data_dir_root: &impl AsRef, read_only_parent: Option<&impl AsRef>, log_config: LogConfig, + output_dir: &impl AsRef, ) -> anyhow::Result { // To create a region, Crucible requires a block size, an extent size // given as a number of blocks, and an extent count. Compute the latter @@ -339,7 +342,7 @@ impl Inner { // nice to connect this more directly to the output desire expressed // by the test runner. let (stdout, stderr) = log_config.output_mode.get_handles( - data_dir_root, + output_dir, &format!("crucible_{disk_uuid}_{port}"), )?; diff --git a/phd-tests/framework/src/disk/mod.rs b/phd-tests/framework/src/disk/mod.rs index 8444feecd..e7325b0c0 100644 --- a/phd-tests/framework/src/disk/mod.rs +++ b/phd-tests/framework/src/disk/mod.rs @@ -8,6 +8,7 @@ //! They can then pass these disks to the VM factory to connect them to a //! specific guest VM. +use std::path::Path; use std::sync::Arc; use anyhow::Context; @@ -278,6 +279,7 @@ impl DiskFactory { source: &DiskSource<'_>, mut min_disk_size_gib: u64, block_size: BlockSize, + output_dir: &impl AsRef, ) -> Result, DiskError> { const BYTES_PER_GIB: u64 = 1024 * 1024 * 1024; @@ -326,6 +328,7 @@ impl DiskFactory { artifact_path.as_ref(), guest_os, self.log_config, + output_dir, ) .map(Arc::new) .map_err(Into::into) diff --git a/phd-tests/framework/src/lib.rs b/phd-tests/framework/src/lib.rs index e0373e466..96cb2177b 100644 --- a/phd-tests/framework/src/lib.rs +++ b/phd-tests/framework/src/lib.rs @@ -58,6 +58,13 @@ mod serial; pub mod test_vm; pub(crate) mod zfs; +/// A test context for an individual PHD test, containing a `Framework` plus +/// test specific information. +pub struct TestCtx { + pub(crate) framework: Arc, + pub(crate) output_dir: Utf8PathBuf, +} + /// An instance of the PHD test framework. pub struct Framework { pub(crate) tmp_directory: Utf8PathBuf, @@ -123,6 +130,117 @@ pub enum BasePropolisSource<'a> { Local(&'a Utf8PathBuf), } +impl TestCtx { + /// Creates a new VM configuration builder using the default configuration + /// from this framework instance. + pub fn vm_config_builder(&self, vm_name: &str) -> VmConfig<'_> { + self.framework.vm_config_builder(vm_name) + } + + /// Yields an environment builder with default settings (run the VM on the + /// test runner's machine using the default Propolis from the command line). + pub fn environment_builder(&self) -> EnvironmentSpec { + self.framework.environment_builder() + } + + /// Yields this framework instance's default guest OS artifact name. This + /// can be used to configure boot disks with different parameters than the + /// builder defaults. + pub fn default_guest_os_artifact(&self) -> &str { + self.framework.default_guest_os_artifact() + } + + /// Yields the guest OS adapter corresponding to the default guest OS + /// artifact. + pub async fn default_guest_os_kind(&self) -> anyhow::Result { + self.framework.default_guest_os_kind().await + } + + /// Indicates whether the disk factory in this framework supports the + /// creation of Crucible disks. This can be used to skip tests that require + /// Crucible support. + pub fn crucible_enabled(&self) -> bool { + self.framework.crucible_enabled + } + + /// Indicates whether a "migration base" Propolis server artifact is + /// available for migration-from-base tests. + pub fn migration_base_enabled(&self) -> bool { + self.framework.migration_base_enabled + } + /// Spawns a test VM using the default configuration returned from + /// `vm_builder` and the default environment returned from + /// `environment_builder`. + pub async fn spawn_default_vm( + &self, + vm_name: &str, + ) -> anyhow::Result { + self.spawn_vm(&self.vm_config_builder(vm_name), None).await + } + + /// Spawns a new test VM using the supplied `config`. If `environment` is + /// `Some`, the VM is spawned using the supplied environment; otherwise it + /// is spawned using the default `environment_builder`. + pub async fn spawn_vm( + &self, + config: &VmConfig<'_>, + environment: Option<&EnvironmentSpec>, + ) -> anyhow::Result { + self.spawn_vm_with_spec( + config + .vm_spec(self) + .await + .context("building VM spec from VmConfig")?, + environment, + ) + .await + } + + /// Spawns a new test VM using the supplied `spec`. If `environment` is + /// `Some`, the VM is spawned using the supplied environment; otherwise it + /// is spawned using the default `environment_builder`. + pub async fn spawn_vm_with_spec( + &self, + spec: VmSpec, + environment: Option<&EnvironmentSpec>, + ) -> anyhow::Result { + TestVm::new( + self, + spec, + environment.unwrap_or(&self.environment_builder()), + ) + .await + .context("constructing test VM") + } + + /// Spawns a "successor" to the supplied `vm`. The successor has the same + /// configuration and takes additional references to all of its + /// predecessor's backing objects (e.g. disk handles). If `environment` is + /// `None`, the successor is launched using the predecessor's environment + /// spec. + pub async fn spawn_successor_vm( + &self, + vm_name: &str, + vm: &TestVm, + environment: Option<&EnvironmentSpec>, + ) -> anyhow::Result { + let mut vm_spec = vm.vm_spec().clone(); + vm_spec.set_vm_name(vm_name.to_owned()); + + // Create new metadata for an instance based on this predecessor. It + // should have the same project and silo IDs, but the sled identifiers + // will be different. + vm_spec.refresh_sled_identifiers(); + + TestVm::new( + self, + vm_spec, + environment.unwrap_or(&vm.environment_spec()), + ) + .await + } +} + // The framework implementation includes some "runner-only" functions // (constructing and resetting a framework) that are marked `pub`. This could be // improved by splitting the "test case" functions into a trait and giving test @@ -209,6 +327,12 @@ impl Framework { }) } + pub fn test_ctx(self: &Arc, fully_qualified_name: String) -> TestCtx { + let output_dir = + self.tmp_directory.as_path().join(&fully_qualified_name); + TestCtx { framework: self.clone(), output_dir } + } + /// Resets the state of any stateful objects in the framework to prepare it /// to run a new test case. pub async fn reset(&self) { @@ -227,85 +351,12 @@ impl Framework { &self.default_guest_os_artifact, ) } - /// Yields an environment builder with default settings (run the VM on the /// test runner's machine using the default Propolis from the command line). pub fn environment_builder(&self) -> EnvironmentSpec { EnvironmentSpec::new(VmLocation::Local, DEFAULT_PROPOLIS_ARTIFACT) } - /// Spawns a test VM using the default configuration returned from - /// `vm_builder` and the default environment returned from - /// `environment_builder`. - pub async fn spawn_default_vm( - &self, - vm_name: &str, - ) -> anyhow::Result { - self.spawn_vm(&self.vm_config_builder(vm_name), None).await - } - - /// Spawns a new test VM using the supplied `config`. If `environment` is - /// `Some`, the VM is spawned using the supplied environment; otherwise it - /// is spawned using the default `environment_builder`. - pub async fn spawn_vm( - &self, - config: &VmConfig<'_>, - environment: Option<&EnvironmentSpec>, - ) -> anyhow::Result { - self.spawn_vm_with_spec( - config - .vm_spec(self) - .await - .context("building VM spec from VmConfig")?, - environment, - ) - .await - } - - /// Spawns a new test VM using the supplied `spec`. If `environment` is - /// `Some`, the VM is spawned using the supplied environment; otherwise it - /// is spawned using the default `environment_builder`. - pub async fn spawn_vm_with_spec( - &self, - spec: VmSpec, - environment: Option<&EnvironmentSpec>, - ) -> anyhow::Result { - TestVm::new( - self, - spec, - environment.unwrap_or(&self.environment_builder()), - ) - .await - .context("constructing test VM") - } - - /// Spawns a "successor" to the supplied `vm`. The successor has the same - /// configuration and takes additional references to all of its - /// predecessor's backing objects (e.g. disk handles). If `environment` is - /// `None`, the successor is launched using the predecessor's environment - /// spec. - pub async fn spawn_successor_vm( - &self, - vm_name: &str, - vm: &TestVm, - environment: Option<&EnvironmentSpec>, - ) -> anyhow::Result { - let mut vm_spec = vm.vm_spec().clone(); - vm_spec.set_vm_name(vm_name.to_owned()); - - // Create new metadata for an instance based on this predecessor. It - // should have the same project and silo IDs, but the sled identifiers - // will be different. - vm_spec.refresh_sled_identifiers(); - - TestVm::new( - self, - vm_spec, - environment.unwrap_or(&vm.environment_spec()), - ) - .await - } - /// Yields this framework instance's default guest OS artifact name. This /// can be used to configure boot disks with different parameters than the /// builder defaults. diff --git a/phd-tests/framework/src/lifecycle.rs b/phd-tests/framework/src/lifecycle.rs index 1ce4ae977..da94b17c4 100644 --- a/phd-tests/framework/src/lifecycle.rs +++ b/phd-tests/framework/src/lifecycle.rs @@ -7,7 +7,7 @@ use futures::future::BoxFuture; use tracing::info; use uuid::Uuid; -use crate::{test_vm::MigrationTimeout, Framework, TestVm}; +use crate::{test_vm::MigrationTimeout, TestCtx, TestVm}; /// The set of actions that can be taken on a VM undergoing lifecycle testing. pub enum Action<'a> { @@ -31,11 +31,11 @@ pub enum Action<'a> { MigrateToPropolis(&'a str), } -impl Framework { +impl TestCtx { /// Runs a lifecycle test on the supplied `vm` by iterating over the /// `actions`, performing the specified action, and then calling `check_fn` /// on the resulting VM to verify invariants. - pub async fn lifecycle_test<'a>( + pub async fn lifecycle_test( &self, vm: TestVm, actions: &[Action<'_>], diff --git a/phd-tests/framework/src/test_vm/config.rs b/phd-tests/framework/src/test_vm/config.rs index 3be06080c..6e17c84e3 100644 --- a/phd-tests/framework/src/test_vm/config.rs +++ b/phd-tests/framework/src/test_vm/config.rs @@ -20,7 +20,7 @@ use uuid::Uuid; use crate::{ disk::{DeviceName, DiskConfig, DiskSource}, test_vm::spec::VmSpec, - Framework, + TestCtx, }; /// The disk interface to use for a given guest disk. @@ -207,10 +207,7 @@ impl<'dr> VmConfig<'dr> { self } - pub async fn vm_spec( - &self, - framework: &Framework, - ) -> anyhow::Result { + pub async fn vm_spec(&self, ctx: &TestCtx) -> anyhow::Result { let VmConfig { vm_name, cpus, @@ -222,7 +219,7 @@ impl<'dr> VmConfig<'dr> { migration_failure, guest_hv_interface, } = self; - + let framework = &ctx.framework; let bootrom_path = framework .artifact_store .get_bootrom(bootrom_artifact) @@ -272,7 +269,7 @@ impl<'dr> VmConfig<'dr> { let mut disk_handles = Vec::new(); for disk in disks.iter() { disk_handles.push( - make_disk(disk.name.to_owned(), framework, disk) + make_disk(disk.name.to_owned(), ctx, disk) .await .context("creating disk")?, ); @@ -404,10 +401,11 @@ impl<'dr> VmConfig<'dr> { async fn make_disk( device_name: String, - framework: &Framework, + ctx: &TestCtx, req: &DiskRequest<'_>, ) -> anyhow::Result> { let device_name = DeviceName::new(device_name); + let framework = &ctx.framework; Ok(match req.backend { DiskBackend::File => framework @@ -424,6 +422,7 @@ async fn make_disk( &req.source, min_disk_size_gib, block_size, + &ctx.output_dir, ) .await .with_context(|| { diff --git a/phd-tests/framework/src/test_vm/environment.rs b/phd-tests/framework/src/test_vm/environment.rs index a5375aebe..e587ee84d 100644 --- a/phd-tests/framework/src/test_vm/environment.rs +++ b/phd-tests/framework/src/test_vm/environment.rs @@ -6,7 +6,7 @@ use std::net::{Ipv4Addr, SocketAddrV4}; use anyhow::Context; -use crate::{test_vm::server::ServerProcessParameters, Framework}; +use crate::{test_vm::server::ServerProcessParameters, TestCtx}; /// Specifies where the framework should start a new test VM. #[derive(Clone, Copy, Debug)] @@ -60,9 +60,9 @@ impl EnvironmentSpec { pub(crate) async fn build<'a>( &self, - framework: &'a Framework, + ctx: &'a TestCtx, ) -> anyhow::Result> { - Environment::from_builder(self, framework).await + Environment::from_builder(self, ctx).await } } @@ -79,8 +79,9 @@ pub(crate) enum Environment<'a> { impl<'a> Environment<'a> { async fn from_builder( builder: &EnvironmentSpec, - framework: &'a Framework, + ctx: &'a TestCtx, ) -> anyhow::Result { + let framework = &ctx.framework; match builder.location { VmLocation::Local => { let propolis_server = framework @@ -110,7 +111,7 @@ impl<'a> Environment<'a> { }); let params = ServerProcessParameters { server_path: propolis_server, - data_dir: framework.tmp_directory.as_path(), + output_dir: ctx.output_dir.as_path(), server_addr: SocketAddrV4::new( Ipv4Addr::new(127, 0, 0, 1), server_port, diff --git a/phd-tests/framework/src/test_vm/mod.rs b/phd-tests/framework/src/test_vm/mod.rs index 132483077..50d971cae 100644 --- a/phd-tests/framework/src/test_vm/mod.rs +++ b/phd-tests/framework/src/test_vm/mod.rs @@ -20,7 +20,7 @@ use crate::{ test_vm::{ environment::Environment, server::ServerProcessParameters, spec::VmSpec, }, - Framework, + TestCtx, }; use anyhow::{anyhow, Context, Result}; @@ -165,7 +165,7 @@ pub struct TestVm { metrics: Option, spec: VmSpec, environment_spec: EnvironmentSpec, - data_dir: Utf8PathBuf, + output_dir: Utf8PathBuf, guest_os: Box, @@ -198,7 +198,7 @@ impl TestVm { /// - guest_os_kind: The kind of guest OS this VM will host. #[instrument(skip_all)] pub(crate) async fn new( - framework: &Framework, + ctx: &TestCtx, spec: VmSpec, environment: &EnvironmentSpec, ) -> Result { @@ -217,7 +217,7 @@ impl TestVm { info!(%vm_name, ?guest_os_kind, ?environment); match environment - .build(framework) + .build(ctx) .await .context("building environment for new VM")? { @@ -226,7 +226,7 @@ impl TestVm { spec, environment.clone(), params, - framework.cleanup_task_channel(), + ctx.framework.cleanup_task_channel(), ), } } @@ -250,7 +250,7 @@ impl TestVm { } }); - let data_dir = params.data_dir.to_path_buf(); + let output_dir = params.output_dir.to_path_buf(); let server_addr = params.server_addr; let server = server::PropolisServer::new( &vm_spec.vm_name, @@ -267,7 +267,7 @@ impl TestVm { metrics, spec: vm_spec, environment_spec, - data_dir, + output_dir, guest_os, state: VmState::New, cleanup_task_tx, @@ -997,7 +997,7 @@ impl TestVm { /// can log serial console output. fn serial_log_file_path(&self) -> Utf8PathBuf { let filename = format!("{}.serial.log", self.spec.vm_name); - let mut path = self.data_dir.clone(); + let mut path = self.output_dir.clone(); path.push(filename); path } diff --git a/phd-tests/framework/src/test_vm/server.rs b/phd-tests/framework/src/test_vm/server.rs index 5b2006592..38ad53e7a 100644 --- a/phd-tests/framework/src/test_vm/server.rs +++ b/phd-tests/framework/src/test_vm/server.rs @@ -24,9 +24,9 @@ pub struct ServerProcessParameters<'a> { /// The path to the server binary to launch. pub server_path: Utf8PathBuf, - /// The directory in which to find or place files that are read or written - /// by this server process. - pub data_dir: &'a Utf8Path, + /// The directory in which toplace files that are written by this server + /// process. + pub output_dir: &'a Utf8Path, /// The address at which the server should serve. pub server_addr: SocketAddrV4, @@ -54,7 +54,7 @@ impl PropolisServer { ) -> Result { let ServerProcessParameters { server_path, - data_dir, + output_dir, server_addr, metrics_addr, vnc_addr, @@ -69,7 +69,7 @@ impl PropolisServer { ); let (server_stdout, server_stderr) = - log_config.output_mode.get_handles(&data_dir, vm_name)?; + log_config.output_mode.get_handles(&output_dir, vm_name)?; let mut args = vec![server_path.into_string(), "run".to_string()]; diff --git a/phd-tests/runner/src/execute.rs b/phd-tests/runner/src/execute.rs index 3126e3a31..f7d878c39 100644 --- a/phd-tests/runner/src/execute.rs +++ b/phd-tests/runner/src/execute.rs @@ -98,10 +98,11 @@ pub async fn run_tests_with_ctx( } stats.tests_not_run -= 1; - let test_ctx = ctx.clone(); + let framework = ctx.clone(); let tc = execution.tc; let mut sigint_rx_task = sigint_rx.clone(); let test_outcome = tokio::spawn(async move { + let test_ctx = framework.test_ctx(tc.fully_qualified_name()); tokio::select! { // Ensure interrupt signals are always handled instead of // continuing to run the test. @@ -116,7 +117,7 @@ pub async fn run_tests_with_ctx( Some("test interrupted by SIGINT".to_string()) ) } - outcome = tc.run(test_ctx.as_ref()) => outcome + outcome = tc.run(&test_ctx) => outcome } }) .await diff --git a/phd-tests/testcase/src/lib.rs b/phd-tests/testcase/src/lib.rs index befe35820..318f5d2d3 100644 --- a/phd-tests/testcase/src/lib.rs +++ b/phd-tests/testcase/src/lib.rs @@ -9,6 +9,7 @@ use thiserror::Error; pub use phd_framework::Framework; pub use phd_framework::FrameworkParameters; +pub use phd_framework::TestCtx; #[derive(Debug, Error)] pub enum TestSkippedError { @@ -34,7 +35,7 @@ pub enum TestOutcome { /// A wrapper for test functions. This is needed to allow [`TestCase`] to have a /// `const` constructor for the inventory crate. pub struct TestFunction { - pub f: fn(&Framework) -> futures::future::BoxFuture<'_, TestOutcome>, + pub f: fn(&TestCtx) -> futures::future::BoxFuture<'_, TestOutcome>, } /// A description of a single test case. @@ -74,7 +75,7 @@ impl TestCase { /// Runs the test case's body with the supplied test context and returns its /// outcome. - pub async fn run(&self, ctx: &Framework) -> TestOutcome { + pub async fn run(&self, ctx: &TestCtx) -> TestOutcome { (self.function.f)(ctx).await } } diff --git a/phd-tests/tests/src/boot_order.rs b/phd-tests/tests/src/boot_order.rs index 6cb3ef1bb..a048ae6cf 100644 --- a/phd-tests/tests/src/boot_order.rs +++ b/phd-tests/tests/src/boot_order.rs @@ -49,7 +49,7 @@ use efi_utils::{ // Unlike later tests, this test does not manipulate boot configuration from // inside the guest OS. #[phd_testcase] -async fn configurable_boot_order(ctx: &Framework) { +async fn configurable_boot_order(ctx: &TestCtx) { let mut cfg = ctx.vm_config_builder("configurable_boot_order"); // Create a second disk backed by the same artifact as the default @@ -115,7 +115,7 @@ async fn configurable_boot_order(ctx: &Framework) { // system booting means that boot order is respected and a non-bootable disk // does not wedge startup. #[phd_testcase] -async fn unbootable_disk_skipped(ctx: &Framework) { +async fn unbootable_disk_skipped(ctx: &TestCtx) { let mut cfg = ctx.vm_config_builder("unbootable_disk_skipped"); cfg.data_disk( @@ -234,7 +234,7 @@ async fn unbootable_disk_skipped(ctx: &Framework) { // so that next boot we'll boot from `unbootable` first. Then reboot and verify // that the boot order is still "boot-disk" first. #[phd_testcase] -async fn guest_can_adjust_boot_order(ctx: &Framework) { +async fn guest_can_adjust_boot_order(ctx: &TestCtx) { let mut cfg = ctx.vm_config_builder("guest_can_adjust_boot_order"); cfg.data_disk( @@ -395,7 +395,7 @@ async fn guest_can_adjust_boot_order(ctx: &Framework) { // store of NvVar variables is the source of boot order, and guests can control // their boot fates. #[phd_testcase] -async fn boot_order_source_priority(ctx: &Framework) { +async fn boot_order_source_priority(ctx: &TestCtx) { let mut cfg = ctx.vm_config_builder("boot_order_source_priority"); cfg.data_disk( @@ -502,7 +502,7 @@ async fn boot_order_source_priority(ctx: &Framework) { } #[phd_testcase] -async fn nvme_boot_option_description(ctx: &Framework) { +async fn nvme_boot_option_description(ctx: &TestCtx) { let mut cfg = ctx.vm_config_builder("nvme_boot_option_description"); cfg.data_disk( diff --git a/phd-tests/tests/src/cpuid.rs b/phd-tests/tests/src/cpuid.rs index bfec563e3..0c76cf136 100644 --- a/phd-tests/tests/src/cpuid.rs +++ b/phd-tests/tests/src/cpuid.rs @@ -22,7 +22,7 @@ fn cpuid_entry( } #[phd_testcase] -async fn cpuid_instance_spec_round_trip_test(ctx: &Framework) { +async fn cpuid_instance_spec_round_trip_test(ctx: &TestCtx) { // The guest isn't actually going to boot with these nonsense settings. The // goal is simply to verify that the ensure API properly records these // options and reflects them back out on request. @@ -106,7 +106,7 @@ async fn verify_guest_brand_string(vm: &TestVm) -> anyhow::Result<()> { /// Launches a test VM with a synthetic brand string injected into its CPUID /// leaves. async fn launch_cpuid_smoke_test_vm( - ctx: &Framework, + ctx: &TestCtx, vm_name: &str, ) -> anyhow::Result { let mut host_cpuid = cpuid_utils::host::query_complete( @@ -139,13 +139,13 @@ async fn launch_cpuid_smoke_test_vm( } #[phd_testcase] -async fn cpuid_boot_test(ctx: &Framework) { +async fn cpuid_boot_test(ctx: &TestCtx) { let vm = launch_cpuid_smoke_test_vm(ctx, "cpuid_boot_test").await?; verify_guest_brand_string(&vm).await?; } #[phd_testcase] -async fn cpuid_migrate_smoke_test(ctx: &Framework) { +async fn cpuid_migrate_smoke_test(ctx: &TestCtx) { let vm = launch_cpuid_smoke_test_vm(ctx, "cpuid_boot_test").await?; verify_guest_brand_string(&vm).await?; @@ -276,7 +276,7 @@ impl<'a> LinuxGuestTopo<'a> { } #[phd_testcase] -async fn guest_cpu_topo_test(ctx: &Framework) { +async fn guest_cpu_topo_test(ctx: &TestCtx) { let vm = launch_cpuid_smoke_test_vm(ctx, "guest_cpu_topo_test").await?; // The topology-checking is Linux-specific, though it should be appropriate diff --git a/phd-tests/tests/src/crucible/migrate.rs b/phd-tests/tests/src/crucible/migrate.rs index b43470cc6..05c6cab30 100644 --- a/phd-tests/tests/src/crucible/migrate.rs +++ b/phd-tests/tests/src/crucible/migrate.rs @@ -8,7 +8,7 @@ use tracing::info; use uuid::Uuid; #[phd_testcase] -async fn smoke_test(ctx: &Framework) { +async fn smoke_test(ctx: &TestCtx) { let mut config = ctx.vm_config_builder("crucible_migrate_smoke_source"); super::add_default_boot_disk(ctx, &mut config)?; let mut source = ctx.spawn_vm(&config, None).await?; @@ -37,7 +37,7 @@ async fn smoke_test(ctx: &Framework) { } #[phd_testcase] -async fn load_test(ctx: &Framework) { +async fn load_test(ctx: &TestCtx) { let mut config = ctx.vm_config_builder("crucible_load_test_source"); super::add_default_boot_disk(ctx, &mut config)?; let mut source = ctx.spawn_vm(&config, None).await?; diff --git a/phd-tests/tests/src/crucible/mod.rs b/phd-tests/tests/src/crucible/mod.rs index ffcf1e062..ce6ba856e 100644 --- a/phd-tests/tests/src/crucible/mod.rs +++ b/phd-tests/tests/src/crucible/mod.rs @@ -14,7 +14,7 @@ mod migrate; mod smoke; fn add_crucible_boot_disk_or_skip<'a>( - ctx: &Framework, + ctx: &TestCtx, config: &mut VmConfig<'a>, artifact: &'a str, interface: DiskInterface, @@ -37,7 +37,7 @@ fn add_crucible_boot_disk_or_skip<'a>( } fn add_default_boot_disk<'a>( - ctx: &'a Framework, + ctx: &'a TestCtx, config: &mut VmConfig<'a>, ) -> phd_testcase::Result<()> { add_crucible_boot_disk_or_skip( diff --git a/phd-tests/tests/src/crucible/smoke.rs b/phd-tests/tests/src/crucible/smoke.rs index a69ac2f5b..0532ac0a2 100644 --- a/phd-tests/tests/src/crucible/smoke.rs +++ b/phd-tests/tests/src/crucible/smoke.rs @@ -12,7 +12,7 @@ use phd_testcase::*; use propolis_client::types::InstanceState; #[phd_testcase] -async fn boot_test(ctx: &Framework) { +async fn boot_test(ctx: &TestCtx) { let mut config = ctx.vm_config_builder("crucible_boot_test"); super::add_default_boot_disk(ctx, &mut config)?; let mut vm = ctx.spawn_vm(&config, None).await?; @@ -21,7 +21,7 @@ async fn boot_test(ctx: &Framework) { } #[phd_testcase] -async fn api_reboot_test(ctx: &Framework) { +async fn api_reboot_test(ctx: &TestCtx) { let mut config = ctx.vm_config_builder("crucible_guest_reboot_test"); super::add_default_boot_disk(ctx, &mut config)?; @@ -33,7 +33,7 @@ async fn api_reboot_test(ctx: &Framework) { } #[phd_testcase] -async fn guest_reboot_test(ctx: &Framework) { +async fn guest_reboot_test(ctx: &TestCtx) { let mut config = ctx.vm_config_builder("crucible_guest_reboot_test"); super::add_default_boot_disk(ctx, &mut config)?; @@ -45,7 +45,7 @@ async fn guest_reboot_test(ctx: &Framework) { } #[phd_testcase] -async fn shutdown_persistence_test(ctx: &Framework) { +async fn shutdown_persistence_test(ctx: &TestCtx) { let mut config = ctx.vm_config_builder("crucible_shutdown_persistence_test"); super::add_default_boot_disk(ctx, &mut config)?; @@ -88,7 +88,7 @@ async fn shutdown_persistence_test(ctx: &Framework) { } #[phd_testcase] -async fn vcr_replace_during_start_test(ctx: &Framework) { +async fn vcr_replace_during_start_test(ctx: &TestCtx) { if !ctx.crucible_enabled() { phd_skip!("Crucible backends not enabled (no downstairs path)"); } diff --git a/phd-tests/tests/src/disk.rs b/phd-tests/tests/src/disk.rs index bcf5f9238..9ef94ee13 100644 --- a/phd-tests/tests/src/disk.rs +++ b/phd-tests/tests/src/disk.rs @@ -17,7 +17,7 @@ use uuid::Uuid; /// Returns a tuple containing the created VM and the path to the guest disk /// device representing the in-memory disk. async fn launch_vm_and_find_in_memory_disk( - ctx: &Framework, + ctx: &TestCtx, vm_name: &str, data: DiskSource<'_>, readonly: bool, @@ -113,7 +113,7 @@ async fn mount_in_memory_disk( } #[phd_testcase] -async fn in_memory_backend_smoke_test(ctx: &Framework) { +async fn in_memory_backend_smoke_test(ctx: &TestCtx) { if ctx.default_guest_os_kind().await?.is_windows() { phd_skip!( "in-memory disk tests use mount options not supported by Cygwin" @@ -144,7 +144,7 @@ async fn in_memory_backend_smoke_test(ctx: &Framework) { } #[phd_testcase] -async fn in_memory_backend_migration_test(ctx: &Framework) { +async fn in_memory_backend_migration_test(ctx: &TestCtx) { // A blank disk is fine for this test: the rest of the test will address the // disk device directly instead of assuming it has a file system. This works // around #824 for Windows guests (which may not recognize the FAT diff --git a/phd-tests/tests/src/framework.rs b/phd-tests/tests/src/framework.rs index 4fc88f249..d162b6a53 100644 --- a/phd-tests/tests/src/framework.rs +++ b/phd-tests/tests/src/framework.rs @@ -8,7 +8,7 @@ use phd_framework::guest_os::GuestOsKind; use phd_testcase::*; #[phd_testcase] -async fn multiline_serial_test(ctx: &Framework) { +async fn multiline_serial_test(ctx: &TestCtx) { let mut vm = ctx.spawn_default_vm("multiline_test").await?; vm.launch().await?; vm.wait_to_boot().await?; @@ -18,7 +18,7 @@ async fn multiline_serial_test(ctx: &Framework) { } #[phd_testcase] -async fn long_line_serial_test(ctx: &Framework) { +async fn long_line_serial_test(ctx: &TestCtx) { let os = ctx.default_guest_os_kind().await?; if matches!( os, diff --git a/phd-tests/tests/src/hw.rs b/phd-tests/tests/src/hw.rs index b322d63d8..db2b2cbd3 100644 --- a/phd-tests/tests/src/hw.rs +++ b/phd-tests/tests/src/hw.rs @@ -6,7 +6,7 @@ use phd_framework::lifecycle::Action; use phd_testcase::*; #[phd_testcase] -async fn lspci_lifecycle_test(ctx: &Framework) { +async fn lspci_lifecycle_test(ctx: &TestCtx) { const LSPCI: &str = "sudo lspci -vvx"; const LSHW: &str = "sudo lshw -notime"; diff --git a/phd-tests/tests/src/hyperv.rs b/phd-tests/tests/src/hyperv.rs index 353d86c76..8ab9df6ba 100644 --- a/phd-tests/tests/src/hyperv.rs +++ b/phd-tests/tests/src/hyperv.rs @@ -51,7 +51,7 @@ async fn guest_detect_hyperv(vm: &TestVm) -> anyhow::Result<()> { } #[phd_testcase] -async fn hyperv_smoke_test(ctx: &Framework) { +async fn hyperv_smoke_test(ctx: &TestCtx) { let mut cfg = ctx.vm_config_builder("hyperv_smoke_test"); cfg.guest_hv_interface(GuestHypervisorInterface::HyperV { features: Default::default(), @@ -64,7 +64,7 @@ async fn hyperv_smoke_test(ctx: &Framework) { } #[phd_testcase] -async fn hyperv_lifecycle_test(ctx: &Framework) { +async fn hyperv_lifecycle_test(ctx: &TestCtx) { let mut cfg = ctx.vm_config_builder("hyperv_lifecycle_test"); cfg.guest_hv_interface(GuestHypervisorInterface::HyperV { features: Default::default(), @@ -89,7 +89,7 @@ async fn hyperv_lifecycle_test(ctx: &Framework) { } #[phd_testcase] -async fn hyperv_reference_tsc_clocksource_test(ctx: &Framework) { +async fn hyperv_reference_tsc_clocksource_test(ctx: &TestCtx) { let mut cfg = ctx.vm_config_builder("hyperv_reference_tsc_test"); cfg.guest_hv_interface(GuestHypervisorInterface::HyperV { features: [HyperVFeatureFlag::ReferenceTsc].into_iter().collect(), @@ -149,7 +149,7 @@ async fn hyperv_reference_tsc_clocksource_test(ctx: &Framework) { } #[phd_testcase] -async fn hyperv_reference_tsc_elapsed_time_test(ctx: &Framework) { +async fn hyperv_reference_tsc_elapsed_time_test(ctx: &TestCtx) { #[derive(Debug)] struct Reading { taken_at: Instant, diff --git a/phd-tests/tests/src/migrate.rs b/phd-tests/tests/src/migrate.rs index e75978887..e7875acb1 100644 --- a/phd-tests/tests/src/migrate.rs +++ b/phd-tests/tests/src/migrate.rs @@ -13,12 +13,12 @@ use tracing::info; use uuid::Uuid; #[phd_testcase] -async fn smoke_test(ctx: &Framework) { +async fn smoke_test(ctx: &TestCtx) { run_smoke_test(ctx, ctx.spawn_default_vm("migration_smoke").await?).await?; } #[phd_testcase] -async fn serial_history(ctx: &Framework) { +async fn serial_history(ctx: &TestCtx) { run_serial_history_test( ctx, ctx.spawn_default_vm("migration_serial_history").await?, @@ -32,13 +32,13 @@ mod from_base { use super::*; #[phd_testcase] - async fn can_migrate_from_base(ctx: &Framework) { + async fn can_migrate_from_base(ctx: &TestCtx) { run_smoke_test(ctx, spawn_base_vm(ctx, "migration_from_base").await?) .await?; } #[phd_testcase] - async fn serial_history(ctx: &Framework) { + async fn serial_history(ctx: &TestCtx) { run_serial_history_test( ctx, spawn_base_vm(ctx, "migration_serial_history_base").await?, @@ -50,7 +50,7 @@ mod from_base { // version under test, back to "base", and back to the version under // test. #[phd_testcase] - async fn migration_from_base_and_back(ctx: &Framework) { + async fn migration_from_base_and_back(ctx: &TestCtx) { let mut source = spawn_base_vm(ctx, "migration_from_base_and_back").await?; source.launch().await?; @@ -83,7 +83,7 @@ mod from_base { .await?; } - async fn spawn_base_vm(ctx: &Framework, name: &str) -> Result { + async fn spawn_base_vm(ctx: &TestCtx, name: &str) -> Result { if !ctx.migration_base_enabled() { phd_skip!("No 'migration base' Propolis revision available"); } @@ -109,7 +109,7 @@ mod running_process { use super::*; #[phd_testcase] - async fn migrate_running_process(ctx: &Framework) { + async fn migrate_running_process(ctx: &TestCtx) { let mut source = ctx.spawn_default_vm("migrate_running_process_source").await?; let mut target = ctx @@ -129,7 +129,7 @@ mod running_process { } #[phd_testcase] - async fn import_failure(ctx: &Framework) { + async fn import_failure(ctx: &TestCtx) { let mut cfg = ctx.vm_config_builder( "migrate_running_process::import_failure_source", ); @@ -199,7 +199,7 @@ mod running_process { } #[phd_testcase] - async fn export_failure(ctx: &Framework) { + async fn export_failure(ctx: &TestCtx) { let mut source = { let mut cfg = ctx.vm_config_builder( "migrate_running_process::export_failure_source", @@ -293,7 +293,7 @@ mod running_process { } #[phd_testcase] -async fn multiple_migrations(ctx: &Framework) { +async fn multiple_migrations(ctx: &TestCtx) { let mut vm0 = ctx.spawn_default_vm("multiple_migrations_0").await?; let mut vm1 = ctx.spawn_successor_vm("multiple_migrations_1", &vm0, None).await?; @@ -311,7 +311,7 @@ async fn multiple_migrations(ctx: &Framework) { ); } -async fn run_smoke_test(ctx: &Framework, mut source: TestVm) -> Result<()> { +async fn run_smoke_test(ctx: &TestCtx, mut source: TestVm) -> Result<()> { source.launch().await?; source.wait_to_boot().await?; let lsout = source.run_shell_command("ls foo.bar 2> /dev/null").await?; @@ -339,7 +339,7 @@ async fn run_smoke_test(ctx: &Framework, mut source: TestVm) -> Result<()> { } async fn run_serial_history_test( - ctx: &Framework, + ctx: &TestCtx, mut source: TestVm, ) -> Result<()> { source.launch().await?; @@ -377,7 +377,7 @@ async fn run_serial_history_test( } #[phd_testcase] -async fn migration_ensures_instance_metadata(ctx: &Framework) { +async fn migration_ensures_instance_metadata(ctx: &TestCtx) { // Create a source instance, and fetch the instance metadata its metrics are // generated with. let mut source = ctx @@ -422,7 +422,7 @@ async fn migration_ensures_instance_metadata(ctx: &Framework) { } #[phd_testcase] -async fn vm_reaches_destroyed_after_migration_out(ctx: &Framework) { +async fn vm_reaches_destroyed_after_migration_out(ctx: &TestCtx) { let mut source = ctx .spawn_default_vm("vm_reaches_destroyed_after_migration_out_source") .await?; diff --git a/phd-tests/tests/src/server_state_machine.rs b/phd-tests/tests/src/server_state_machine.rs index 352019751..9b9421610 100644 --- a/phd-tests/tests/src/server_state_machine.rs +++ b/phd-tests/tests/src/server_state_machine.rs @@ -14,7 +14,7 @@ use phd_testcase::*; use propolis_client::types::InstanceState; #[phd_testcase] -async fn instance_start_stop_test(ctx: &Framework) { +async fn instance_start_stop_test(ctx: &TestCtx) { let mut vm = ctx.spawn_default_vm("instance_ensure_running_test").await?; vm.instance_ensure().await?; @@ -30,7 +30,7 @@ async fn instance_start_stop_test(ctx: &Framework) { } #[phd_testcase] -async fn instance_stop_unstarted_test(ctx: &Framework) { +async fn instance_stop_unstarted_test(ctx: &TestCtx) { let mut vm = ctx.spawn_default_vm("instance_stop_unstarted_test").await?; vm.instance_ensure().await?; @@ -46,7 +46,7 @@ async fn instance_stop_unstarted_test(ctx: &Framework) { } #[phd_testcase] -async fn instance_stop_causes_destroy_test(ctx: &Framework) { +async fn instance_stop_causes_destroy_test(ctx: &TestCtx) { let mut vm = ctx.spawn_default_vm("instance_stop_causes_destroy_test").await?; @@ -70,7 +70,7 @@ async fn instance_stop_causes_destroy_test(ctx: &Framework) { } #[phd_testcase] -async fn instance_reset_test(ctx: &Framework) { +async fn instance_reset_test(ctx: &TestCtx) { let mut vm = ctx.spawn_default_vm("instance_reset_returns_to_running_test").await?; @@ -101,7 +101,7 @@ async fn instance_reset_test(ctx: &Framework) { } #[phd_testcase] -async fn instance_reset_requires_running_test(ctx: &Framework) { +async fn instance_reset_requires_running_test(ctx: &TestCtx) { let mut vm = ctx.spawn_default_vm("instance_reset_requires_running_test").await?; @@ -111,7 +111,7 @@ async fn instance_reset_requires_running_test(ctx: &Framework) { } #[phd_testcase] -async fn stop_while_blocked_on_start_test(ctx: &Framework) { +async fn stop_while_blocked_on_start_test(ctx: &TestCtx) { // This test uses a Crucible disk backend to cause VM startup to block. if !ctx.crucible_enabled() { phd_skip!("test requires Crucible support"); diff --git a/phd-tests/tests/src/smoke.rs b/phd-tests/tests/src/smoke.rs index 560e57ccd..6c12583d5 100644 --- a/phd-tests/tests/src/smoke.rs +++ b/phd-tests/tests/src/smoke.rs @@ -6,7 +6,7 @@ use phd_testcase::*; use propolis_client::instance_spec::InstanceSpecStatus; #[phd_testcase] -async fn nproc_test(ctx: &Framework) { +async fn nproc_test(ctx: &TestCtx) { let mut vm = ctx.spawn_vm(ctx.vm_config_builder("nproc_test").cpus(6), None).await?; vm.launch().await?; @@ -17,7 +17,7 @@ async fn nproc_test(ctx: &Framework) { } #[phd_testcase] -async fn api_reboot_test(ctx: &Framework) { +async fn api_reboot_test(ctx: &TestCtx) { let mut vm = ctx.spawn_default_vm("api_reboot_test").await?; vm.launch().await?; vm.wait_to_boot().await?; @@ -26,7 +26,7 @@ async fn api_reboot_test(ctx: &Framework) { } #[phd_testcase] -async fn guest_reboot_test(ctx: &Framework) { +async fn guest_reboot_test(ctx: &TestCtx) { let mut vm = ctx.spawn_default_vm("guest_reboot_test").await?; vm.launch().await?; vm.wait_to_boot().await?; @@ -35,7 +35,7 @@ async fn guest_reboot_test(ctx: &Framework) { } #[phd_testcase] -async fn instance_spec_get_test(ctx: &Framework) { +async fn instance_spec_get_test(ctx: &TestCtx) { let mut vm = ctx .spawn_vm( ctx.vm_config_builder("instance_spec_test") diff --git a/phd-tests/tests/src/stats.rs b/phd-tests/tests/src/stats.rs index 8ab39e050..5372dedcc 100644 --- a/phd-tests/tests/src/stats.rs +++ b/phd-tests/tests/src/stats.rs @@ -196,7 +196,7 @@ impl VirtualMachineMetrics { } #[phd_testcase] -async fn instance_vcpu_stats(ctx: &Framework) { +async fn instance_vcpu_stats(ctx: &TestCtx) { /// Allow as much as 20% measurement error for time comparisons in this /// test. When measuring active guest time, some guests (looking at you /// Windows) may have services that continue running in the time period From 4f21c10bb3c3dcafb57d0fa5c3ed4428dea6ab06 Mon Sep 17 00:00:00 2001 From: Eliza Weisman Date: Fri, 6 Feb 2026 11:21:51 -0800 Subject: [PATCH 2/6] make log file create errors useful --- phd-tests/framework/src/log_config.rs | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/phd-tests/framework/src/log_config.rs b/phd-tests/framework/src/log_config.rs index f813717a4..a2f7a1123 100644 --- a/phd-tests/framework/src/log_config.rs +++ b/phd-tests/framework/src/log_config.rs @@ -7,6 +7,7 @@ use std::{path::Path, process::Stdio, str::FromStr}; +use anyhow::Context; use tracing::info; /// Specifies how a test's logging should be managed. @@ -69,10 +70,9 @@ impl OutputMode { stderr_path.push(format!("{file_prefix}.stderr.log")); info!(?stdout_path, ?stderr_path, "Opening server log files"); - Ok(( - std::fs::File::create(stdout_path)?.into(), - std::fs::File::create(stderr_path)?.into(), - )) + let stdout = create_file(&stdout_path)?.into(); + let stderr = create_file(&stderr_path)?.into(); + Ok((stdout, stderr)) } OutputMode::Stdio => Ok((Stdio::inherit(), Stdio::inherit())), OutputMode::Null => Ok((Stdio::null(), Stdio::null())), @@ -90,3 +90,9 @@ pub enum LogFormat { /// as in CI). Bunyan, } + +fn create_file(path: &impl AsRef) -> anyhow::Result { + let path = path.as_ref(); + std::fs::File::create(path) + .with_context(|| format!("failed to create file {}", path.display())) +} From f95192a19b13ed64fcc48ccf075cdb67b3834b09 Mon Sep 17 00:00:00 2001 From: Eliza Weisman Date: Fri, 6 Feb 2026 11:25:48 -0800 Subject: [PATCH 3/6] make sure output dir is there before creating logfiles --- phd-tests/framework/src/log_config.rs | 25 +++++++++++++++++++------ 1 file changed, 19 insertions(+), 6 deletions(-) diff --git a/phd-tests/framework/src/log_config.rs b/phd-tests/framework/src/log_config.rs index a2f7a1123..cec223541 100644 --- a/phd-tests/framework/src/log_config.rs +++ b/phd-tests/framework/src/log_config.rs @@ -52,9 +52,12 @@ impl OutputMode { /// /// # Parameters /// - /// - directory: The directory in which to store any files written under + /// - `directory`: The directory in which to store any files written under /// the selected discipline. - /// - file_prefix: The prefix to add to the names of any files written + /// + /// If this directory does not already exist, it (and any parents) will + /// be created. + /// - `file_prefix`: The prefix to add to the names of any files written /// under the selected discipline. pub(crate) fn get_handles( &self, @@ -63,15 +66,25 @@ impl OutputMode { ) -> anyhow::Result<(Stdio, Stdio)> { match self { OutputMode::TmpFile => { - let mut stdout_path = directory.as_ref().to_path_buf(); - stdout_path.push(format!("{file_prefix}.stdout.log")); + let directory = directory.as_ref(); - let mut stderr_path = directory.as_ref().to_path_buf(); - stderr_path.push(format!("{file_prefix}.stderr.log")); + // Make sure the output dir actually exists before creating log + // files. + std::fs::create_dir_all(&directory).with_context(|| { + format!( + "failed to create log file directory {}", + directory.display() + ) + })?; + let stdout_path = + directory.join(format!("{file_prefix}.stdout.log")); + let stderr_path = + directory.join(format!("{file_prefix}.stderr.log")); info!(?stdout_path, ?stderr_path, "Opening server log files"); let stdout = create_file(&stdout_path)?.into(); let stderr = create_file(&stderr_path)?.into(); + Ok((stdout, stderr)) } OutputMode::Stdio => Ok((Stdio::inherit(), Stdio::inherit())), From ac3abb2b7305930b2553c1e0622d0f3ab47399db Mon Sep 17 00:00:00 2001 From: Eliza Weisman Date: Fri, 6 Feb 2026 12:04:53 -0800 Subject: [PATCH 4/6] make bmat script actually tar the correct thing --- .github/buildomat/phd-run-with-args.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/buildomat/phd-run-with-args.sh b/.github/buildomat/phd-run-with-args.sh index 6c505749c..9f82c1c61 100755 --- a/.github/buildomat/phd-run-with-args.sh +++ b/.github/buildomat/phd-run-with-args.sh @@ -81,7 +81,7 @@ failcount=$? set -e tar -czvf /tmp/phd-tmp-files.tar.gz \ - -C /tmp/propolis-phd /tmp/propolis-phd/*.log + -C "$tmpdir" . exitcode=0 if [ $failcount -eq 0 ]; then From 61a1d0b76c9b0c4b0bb2c07b319e11792f5b6984 Mon Sep 17 00:00:00 2001 From: Eliza Weisman Date: Fri, 6 Feb 2026 12:25:13 -0800 Subject: [PATCH 5/6] clippy --- phd-tests/framework/src/log_config.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/phd-tests/framework/src/log_config.rs b/phd-tests/framework/src/log_config.rs index cec223541..4ada1ad81 100644 --- a/phd-tests/framework/src/log_config.rs +++ b/phd-tests/framework/src/log_config.rs @@ -56,7 +56,7 @@ impl OutputMode { /// the selected discipline. /// /// If this directory does not already exist, it (and any parents) will - /// be created. + /// be created. /// - `file_prefix`: The prefix to add to the names of any files written /// under the selected discipline. pub(crate) fn get_handles( @@ -70,7 +70,7 @@ impl OutputMode { // Make sure the output dir actually exists before creating log // files. - std::fs::create_dir_all(&directory).with_context(|| { + std::fs::create_dir_all(directory).with_context(|| { format!( "failed to create log file directory {}", directory.display() From 328963da37261e33114cf51edc168b8a4a0028a5 Mon Sep 17 00:00:00 2001 From: Eliza Weisman Date: Fri, 6 Feb 2026 16:18:56 -0800 Subject: [PATCH 6/6] Update server.rs Co-authored-by: iximeow --- phd-tests/framework/src/test_vm/server.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/phd-tests/framework/src/test_vm/server.rs b/phd-tests/framework/src/test_vm/server.rs index 38ad53e7a..be88a665b 100644 --- a/phd-tests/framework/src/test_vm/server.rs +++ b/phd-tests/framework/src/test_vm/server.rs @@ -24,7 +24,7 @@ pub struct ServerProcessParameters<'a> { /// The path to the server binary to launch. pub server_path: Utf8PathBuf, - /// The directory in which toplace files that are written by this server + /// The directory in which to place files that are written by this server /// process. pub output_dir: &'a Utf8Path,