diff --git a/phd-tests/framework/src/guest_os/alpine.rs b/phd-tests/framework/src/guest_os/alpine.rs index 00b226288..69dc34697 100644 --- a/phd-tests/framework/src/guest_os/alpine.rs +++ b/phd-tests/framework/src/guest_os/alpine.rs @@ -11,9 +11,9 @@ pub(super) struct Alpine; impl GuestOs for Alpine { fn get_login_sequence(&self) -> CommandSequence { CommandSequence(vec![ - CommandSequenceEntry::WaitFor("localhost login: "), - CommandSequenceEntry::WriteStr("root"), - CommandSequenceEntry::WaitFor(self.get_shell_prompt()), + CommandSequenceEntry::wait_for("localhost login: "), + CommandSequenceEntry::write_str("root"), + CommandSequenceEntry::wait_for(self.get_shell_prompt()), ]) } @@ -24,4 +24,11 @@ impl GuestOs for Alpine { fn read_only_fs(&self) -> bool { true } + + fn shell_command_sequence<'a>(&self, cmd: &'a str) -> CommandSequence<'a> { + super::shell_commands::shell_command_sequence( + std::borrow::Cow::Borrowed(cmd), + crate::serial::BufferKind::Raw, + ) + } } diff --git a/phd-tests/framework/src/guest_os/debian11_nocloud.rs b/phd-tests/framework/src/guest_os/debian11_nocloud.rs index dceca4d42..dec331470 100644 --- a/phd-tests/framework/src/guest_os/debian11_nocloud.rs +++ b/phd-tests/framework/src/guest_os/debian11_nocloud.rs @@ -11,9 +11,9 @@ pub(super) struct Debian11NoCloud; impl GuestOs for Debian11NoCloud { fn get_login_sequence(&self) -> CommandSequence { CommandSequence(vec![ - CommandSequenceEntry::WaitFor("debian login: "), - CommandSequenceEntry::WriteStr("root"), - CommandSequenceEntry::WaitFor(self.get_shell_prompt()), + CommandSequenceEntry::wait_for("debian login: "), + CommandSequenceEntry::write_str("root"), + CommandSequenceEntry::wait_for(self.get_shell_prompt()), ]) } diff --git a/phd-tests/framework/src/guest_os/mod.rs b/phd-tests/framework/src/guest_os/mod.rs index 15cd22639..e77d70008 100644 --- a/phd-tests/framework/src/guest_os/mod.rs +++ b/phd-tests/framework/src/guest_os/mod.rs @@ -11,30 +11,45 @@ use serde::{Deserialize, Serialize}; mod alpine; mod debian11_nocloud; +mod shell_commands; mod ubuntu22_04; mod windows; +mod windows_server_2016; mod windows_server_2019; mod windows_server_2022; /// An entry in a sequence of interactions with the guest's command prompt. #[derive(Debug)] -pub(super) enum CommandSequenceEntry { +pub(super) enum CommandSequenceEntry<'a> { /// Wait for the supplied string to appear on the guest serial console. - WaitFor(&'static str), + WaitFor(Cow<'a, str>), /// Write the specified string as a command to the guest serial console. - WriteStr(&'static str), + WriteStr(Cow<'a, str>), + + /// Tell the serial console task to clear its buffer. + ClearBuffer, /// Change the serial console buffering discipline to the supplied /// discipline. ChangeSerialConsoleBuffer(crate::serial::BufferKind), - /// Set a delay between writing individual bytes to the guest serial console + /// Set a delay between writing identical bytes to the guest serial console /// to avoid keyboard debouncing logic in guests. - SetSerialByteWriteDelay(std::time::Duration), + SetRepeatedCharacterDebounce(std::time::Duration), } -pub(super) struct CommandSequence(pub Vec); +impl<'a> CommandSequenceEntry<'a> { + fn write_str(s: impl Into>) -> Self { + Self::WriteStr(s.into()) + } + + fn wait_for(s: impl Into>) -> Self { + Self::WaitFor(s.into()) + } +} + +pub(super) struct CommandSequence<'a>(pub Vec>); pub(super) trait GuestOs: Send + Sync { /// Retrieves the command sequence used to wait for the OS to boot and log @@ -47,13 +62,13 @@ pub(super) trait GuestOs: Send + Sync { /// Indicates whether the guest has a read-only filesystem. fn read_only_fs(&self) -> bool; - /// Some guests need to amend incoming shell commands from tests in order to - /// get output to display on the serial console in a way those guests can - /// accept (e.g. by clearing the screen immediately before running each - /// command). This function amends an incoming command according to the - /// guest adapter's instructions. - fn amend_shell_command<'a>(&self, cmd: &'a str) -> Cow<'a, str> { - Cow::Borrowed(cmd) + /// Returns the sequence of serial console operations a test VM should issue + /// in order to execute `cmd` in the guest's shell. + fn shell_command_sequence<'a>(&self, cmd: &'a str) -> CommandSequence<'a> { + shell_commands::shell_command_sequence( + Cow::Borrowed(cmd), + crate::serial::BufferKind::Raw, + ) } } @@ -64,6 +79,7 @@ pub enum GuestOsKind { Alpine, Debian11NoCloud, Ubuntu2204, + WindowsServer2016, WindowsServer2019, WindowsServer2022, } @@ -76,6 +92,8 @@ impl FromStr for GuestOsKind { "alpine" => Ok(Self::Alpine), "debian11nocloud" => Ok(Self::Debian11NoCloud), "ubuntu2204" => Ok(Self::Ubuntu2204), + "windowsserver2016" => Ok(Self::WindowsServer2016), + "windowsserver2019" => Ok(Self::WindowsServer2019), "windowsserver2022" => Ok(Self::WindowsServer2022), _ => Err(std::io::Error::new( std::io::ErrorKind::InvalidInput, @@ -92,6 +110,9 @@ pub(super) fn get_guest_os_adapter(kind: GuestOsKind) -> Box { Box::new(debian11_nocloud::Debian11NoCloud) } GuestOsKind::Ubuntu2204 => Box::new(ubuntu22_04::Ubuntu2204), + GuestOsKind::WindowsServer2016 => { + Box::new(windows_server_2016::WindowsServer2016) + } GuestOsKind::WindowsServer2019 => { Box::new(windows_server_2019::WindowsServer2019) } diff --git a/phd-tests/framework/src/guest_os/shell_commands.rs b/phd-tests/framework/src/guest_os/shell_commands.rs new file mode 100644 index 000000000..97d926b70 --- /dev/null +++ b/phd-tests/framework/src/guest_os/shell_commands.rs @@ -0,0 +1,61 @@ +// This Source Code Form is subject to the terms of the Mozilla Public +// License, v. 2.0. If a copy of the MPL was not distributed with this +// file, You can obtain one at https://mozilla.org/MPL/2.0/. + +//! Common helper functions for issuing shell commands to guests and handling +//! their outputs. + +use std::borrow::Cow; + +use super::{CommandSequence, CommandSequenceEntry}; + +/// Produces the shell command sequence necessary to execute `cmd` in a guest's +/// shell, given that the guest is using the supplied serial console buffering +/// discipline. +/// +/// This routine assumes that multi-line commands will be echoed with `> ` at +/// the start of each line in the command. This is technically shell-dependent +/// but is true for all the shell types in PHD's currently-supported guests. +pub(super) fn shell_command_sequence( + cmd: Cow<'_, str>, + buffer_kind: crate::serial::BufferKind, +) -> CommandSequence { + let echo = cmd.trim_end().replace('\n', "\n> "); + match buffer_kind { + crate::serial::BufferKind::Raw => CommandSequence(vec![ + CommandSequenceEntry::write_str(cmd), + CommandSequenceEntry::wait_for(echo), + CommandSequenceEntry::ClearBuffer, + CommandSequenceEntry::write_str("\n"), + ]), + + crate::serial::BufferKind::Vt80x24 => { + // In 80x24 mode, it's simplest to issue multi-line operations one + // line at a time and wait for each line to be echoed before + // starting the next. For very long commands (more than 24 lines), + // this avoids having to deal with lines scrolling off the buffer + // before they can be waited for. + let cmd_lines = cmd.trim_end().lines(); + let echo_lines = echo.lines(); + let mut seq = vec![]; + + let mut iter = cmd_lines.zip(echo_lines).peekable(); + while let Some((cmd, echo)) = iter.next() { + seq.push(CommandSequenceEntry::write_str(cmd.to_owned())); + seq.push(CommandSequenceEntry::wait_for(echo.to_owned())); + + if iter.peek().is_some() { + seq.push(CommandSequenceEntry::write_str("\n")); + } + } + + // Before issuing the command, clear any stale echoed characters + // from the serial console buffer. This ensures that the next prompt + // is preceded in the buffer only by the output of the issued + // command. + seq.push(CommandSequenceEntry::ClearBuffer); + seq.push(CommandSequenceEntry::write_str("\n")); + CommandSequence(seq) + } + } +} diff --git a/phd-tests/framework/src/guest_os/ubuntu22_04.rs b/phd-tests/framework/src/guest_os/ubuntu22_04.rs index 1d3cc7143..a888f4551 100644 --- a/phd-tests/framework/src/guest_os/ubuntu22_04.rs +++ b/phd-tests/framework/src/guest_os/ubuntu22_04.rs @@ -12,11 +12,11 @@ pub(super) struct Ubuntu2204; impl GuestOs for Ubuntu2204 { fn get_login_sequence(&self) -> CommandSequence { CommandSequence(vec![ - CommandSequenceEntry::WaitFor("ubuntu login: "), - CommandSequenceEntry::WriteStr("ubuntu"), - CommandSequenceEntry::WaitFor("Password: "), - CommandSequenceEntry::WriteStr("1!Passw0rd"), - CommandSequenceEntry::WaitFor(self.get_shell_prompt()), + CommandSequenceEntry::wait_for("ubuntu login: "), + CommandSequenceEntry::write_str("ubuntu"), + CommandSequenceEntry::wait_for("Password: "), + CommandSequenceEntry::write_str("1!Passw0rd"), + CommandSequenceEntry::wait_for(self.get_shell_prompt()), ]) } diff --git a/phd-tests/framework/src/guest_os/windows.rs b/phd-tests/framework/src/guest_os/windows.rs index d7eefc18d..7a06cf338 100644 --- a/phd-tests/framework/src/guest_os/windows.rs +++ b/phd-tests/framework/src/guest_os/windows.rs @@ -14,52 +14,60 @@ use super::{CommandSequence, CommandSequenceEntry, GuestOsKind}; /// - Cygwin is installed to C:\cygwin and can be launched by invoking /// C:\cygwin\cygwin.bat. /// - The local administrator account is enabled with password `0xide#1Fan`. -pub(super) fn get_login_sequence_for(guest: GuestOsKind) -> CommandSequence { +pub(super) fn get_login_sequence_for<'a>( + guest: GuestOsKind, +) -> CommandSequence<'a> { assert!(matches!( guest, - GuestOsKind::WindowsServer2019 | GuestOsKind::WindowsServer2022 + GuestOsKind::WindowsServer2016 + | GuestOsKind::WindowsServer2019 + | GuestOsKind::WindowsServer2022 )); let mut commands = vec![ - CommandSequenceEntry::WaitFor( + CommandSequenceEntry::wait_for( "Computer is booting, SAC started and initialized.", ), - CommandSequenceEntry::WaitFor( + CommandSequenceEntry::wait_for( "EVENT: The CMD command is now available.", ), - CommandSequenceEntry::WaitFor("SAC>"), - CommandSequenceEntry::WriteStr("cmd"), - CommandSequenceEntry::WaitFor("Channel: Cmd0001"), - CommandSequenceEntry::WaitFor("SAC>"), - CommandSequenceEntry::WriteStr("ch -sn Cmd0001"), - CommandSequenceEntry::WaitFor( + CommandSequenceEntry::wait_for("SAC>"), + CommandSequenceEntry::write_str("cmd"), + CommandSequenceEntry::wait_for("Channel: Cmd0001"), + CommandSequenceEntry::wait_for("SAC>"), + CommandSequenceEntry::write_str("ch -sn Cmd0001"), + CommandSequenceEntry::wait_for( "Use any other key to view this channel.", ), - CommandSequenceEntry::WriteStr(""), - CommandSequenceEntry::WaitFor("Username:"), - CommandSequenceEntry::WriteStr("Administrator"), - CommandSequenceEntry::WaitFor("Domain :"), - CommandSequenceEntry::WriteStr(""), - CommandSequenceEntry::WaitFor("Password:"), - CommandSequenceEntry::WriteStr("0xide#1Fan"), + CommandSequenceEntry::write_str(""), + CommandSequenceEntry::wait_for("Username:"), + CommandSequenceEntry::write_str("Administrator"), + CommandSequenceEntry::wait_for("Domain :"), + CommandSequenceEntry::write_str(""), + CommandSequenceEntry::wait_for("Password:"), + CommandSequenceEntry::write_str("0xide#1Fan"), ]; - // Windows Server 2019's serial console-based command prompts default to - // trying to drive a VT100 terminal themselves instead of emitting - // characters and letting the recipient display them in whatever style it - // likes. This only happens once the command prompt has been activated, so - // only switch buffering modes after entering credentials. - if let GuestOsKind::WindowsServer2019 = guest { + // Earlier Windows Server versions' serial console-based command prompts + // default to trying to drive a VT100 terminal themselves instead of + // emitting characters and letting the recipient display them in whatever + // style it likes. This only happens once the command prompt has been + // activated, so only switch buffering modes after entering credentials. + if matches!( + guest, + GuestOsKind::WindowsServer2016 | GuestOsKind::WindowsServer2019 + ) { commands.extend([ CommandSequenceEntry::ChangeSerialConsoleBuffer( crate::serial::BufferKind::Vt80x24, ), - // Server 2019 also likes to debounce keystrokes, so set a small - // delay between characters to try to avoid this. (This value was - // chosen by experimentation; there doesn't seem to be a guest - // setting that controls this interval.) - CommandSequenceEntry::SetSerialByteWriteDelay( - std::time::Duration::from_millis(125), + // These versions also like to debounce keystrokes, so set a delay + // between repeated characters to try to avoid this. This is a very + // conservative delay to try to avoid test flakiness; fortunately, + // it only applies when typing the same character multiple times in + // a row. + CommandSequenceEntry::SetRepeatedCharacterDebounce( + std::time::Duration::from_secs(1), ), ]); } @@ -70,14 +78,14 @@ pub(super) fn get_login_sequence_for(guest: GuestOsKind) -> CommandSequence { // eat the command and just process the newline). It also appears to // prefer carriage returns to linefeeds. Accommodate this behavior // until Cygwin is launched. - CommandSequenceEntry::WaitFor("C:\\Windows\\system32>"), - CommandSequenceEntry::WriteStr("cls\r"), - CommandSequenceEntry::WaitFor("C:\\Windows\\system32>"), - CommandSequenceEntry::WriteStr("C:\\cygwin\\cygwin.bat\r"), - CommandSequenceEntry::WaitFor("$ "), + CommandSequenceEntry::wait_for("C:\\Windows\\system32>"), + CommandSequenceEntry::write_str("cls\r"), + CommandSequenceEntry::wait_for("C:\\Windows\\system32>"), + CommandSequenceEntry::write_str("C:\\cygwin\\cygwin.bat\r"), + CommandSequenceEntry::wait_for("$ "), // Tweak the command prompt so that it appears on a single line with // no leading newlines. - CommandSequenceEntry::WriteStr("PS1='\\u@\\h:$ '"), + CommandSequenceEntry::write_str("PS1='\\u@\\h:$ '"), ]); CommandSequence(commands) diff --git a/phd-tests/framework/src/guest_os/windows_server_2016.rs b/phd-tests/framework/src/guest_os/windows_server_2016.rs new file mode 100644 index 000000000..3b3a141aa --- /dev/null +++ b/phd-tests/framework/src/guest_os/windows_server_2016.rs @@ -0,0 +1,45 @@ +// This Source Code Form is subject to the terms of the Mozilla Public +// License, v. 2.0. If a copy of the MPL was not distributed with this +// file, You can obtain one at https://mozilla.org/MPL/2.0/. + +// This Source Code Form is subject to the terms of the Mozilla Public +// License, v. 2.0. If a copy of the MPL was not distributed with this +// file, You can obtain one at https://mozilla.org/MPL/2.0/. + +//! Guest OS adaptations for Windows Server 2016 images. See [the general +//! Windows module](mod@super::windows) documentation for more information. + +use std::borrow::Cow; + +use super::{CommandSequence, GuestOs, GuestOsKind}; + +/// The guest adapter for Windows Server 2016 images. See [the general +/// Windows module](mod@super::windows) documentation for more information about +/// the configuration this adapter requires. +pub(super) struct WindowsServer2016; + +impl GuestOs for WindowsServer2016 { + fn get_login_sequence(&self) -> CommandSequence { + super::windows::get_login_sequence_for(GuestOsKind::WindowsServer2016) + } + + fn get_shell_prompt(&self) -> &'static str { + "Administrator@PHD-WINDOWS:$ " + } + + fn read_only_fs(&self) -> bool { + false + } + + fn shell_command_sequence<'a>(&self, cmd: &'a str) -> CommandSequence<'a> { + // `reset` the command prompt before issuing the command to try to force + // Windows to redraw the subsequent command prompt. Without this, + // Windows may not draw the prompt if the post-command state happens to + // place a prompt at a location that already had one pre-command. + let cmd = format!("reset && {cmd}"); + super::shell_commands::shell_command_sequence( + Cow::Owned(cmd), + crate::serial::BufferKind::Vt80x24, + ) + } +} diff --git a/phd-tests/framework/src/guest_os/windows_server_2019.rs b/phd-tests/framework/src/guest_os/windows_server_2019.rs index 6a8e4dcd7..39dddc3c3 100644 --- a/phd-tests/framework/src/guest_os/windows_server_2019.rs +++ b/phd-tests/framework/src/guest_os/windows_server_2019.rs @@ -27,19 +27,15 @@ impl GuestOs for WindowsServer2019 { false } - fn amend_shell_command<'a>(&self, cmd: &'a str) -> Cow<'a, str> { - // The simplest way to ensure that the 80x24 terminal buffer contains - // just the output of the most recent command and the subsequent prompt - // is to ask Windows to clear the screen and run the command in a single - // statement. - // - // Use Cygwin bash's `reset` instead of `clear` or `cls` to try to force - // Windows to clear and redraw the entire terminal before displaying any - // command output. Without this, Windows sometimes reprints a new - // command prompt to its internal screen buffer before re-rendering - // anything to the terminal; when this happens, it doesn't re-send the - // new command prompt, since it's "already there" on the output terminal - // (even though it may have been cleared from the match buffer). - Cow::from(format!("reset; {}", cmd)) + fn shell_command_sequence<'a>(&self, cmd: &'a str) -> CommandSequence<'a> { + // `reset` the command prompt before issuing the command to try to force + // Windows to redraw the subsequent command prompt. Without this, + // Windows may not draw the prompt if the post-command state happens to + // place a prompt at a location that already had one pre-command. + let cmd = format!("reset && {cmd}"); + super::shell_commands::shell_command_sequence( + Cow::Owned(cmd), + crate::serial::BufferKind::Vt80x24, + ) } } diff --git a/phd-tests/framework/src/serial/mod.rs b/phd-tests/framework/src/serial/mod.rs index bdad7e2e1..fbff7f50c 100644 --- a/phd-tests/framework/src/serial/mod.rs +++ b/phd-tests/framework/src/serial/mod.rs @@ -35,6 +35,9 @@ trait Buffer: Send { /// Processes the supplied `bytes` as input to the buffer. fn process_bytes(&mut self, bytes: &[u8]); + /// Clears the unprocessed contents of the buffer. + fn clear(&mut self); + /// Registers a new request to wait for a string to appear in the buffer. fn register_wait_for_output(&mut self, waiter: OutputWaiter); @@ -59,7 +62,11 @@ pub enum BufferKind { /// The set of commands that the serial console can send to its processing task. enum TaskCommand { /// Send the supplied bytes to the VM. - SendBytes(Vec), + SendBytes { bytes: Vec, done: oneshot::Sender<()> }, + + /// Clears the contents of the task's console buffer. This does not cancel + /// the active wait, if there is one. + Clear, /// Register to be notified if and when a supplied string appears in the /// serial console's buffer. @@ -107,18 +114,32 @@ impl SerialConsole { } /// Directs the console worker thread to send the supplied `bytes` to the - /// guest. - pub fn send_bytes(&self, bytes: Vec) -> anyhow::Result<()> { - self.cmd_tx.send(TaskCommand::SendBytes(bytes))?; + /// guest. Returns a `oneshot::Receiver` that the console worker thread + /// signals once all the bytes have been set. + pub fn send_bytes( + &self, + bytes: Vec, + ) -> anyhow::Result> { + let (done, done_rx) = oneshot::channel(); + self.cmd_tx.send(TaskCommand::SendBytes { bytes, done })?; + Ok(done_rx) + } + + /// Directs the console worker thread to clear the serial console buffer. + pub fn clear(&self) -> anyhow::Result<()> { + self.cmd_tx.send(TaskCommand::Clear)?; Ok(()) } /// Registers with the current buffer a request to wait for `wanted` to /// appear in the console buffer. When a match is found, the buffer sends - /// all buffered characters preceding the match to `preceding_tx`, consuming - /// those characters and the matched string. If the buffer already contains - /// one or more matches at the time the waiter is registered, the last match - /// is used to satisfy the wait immediately. + /// all buffered characters preceding the match to `preceding_tx`. If the + /// buffer already contains one or more matches at the time the waiter is + /// registered, the last match is used to satisfy the wait immediately. + /// + /// Note that this function *does not* clear any characters from the buffer. + /// Callers who want to retire previously-echoed characters in the buffer + /// must explicitly call `clear`. pub fn register_wait_for_string( &self, wanted: String, @@ -144,7 +165,7 @@ impl SerialConsole { } /// Sets the delay to insert between sending individual bytes to the guest. - pub fn set_guest_write_delay( + pub fn set_repeated_character_debounce( &self, delay: std::time::Duration, ) -> Result<()> { @@ -193,7 +214,7 @@ async fn serial_task( break; }; match cmd { - TaskCommand::SendBytes(bytes) => { + TaskCommand::SendBytes { bytes, done } => { if debounce.is_zero() { if let Err(e) = stream.send(Message::Binary(bytes)).await { error!( @@ -202,18 +223,26 @@ async fn serial_task( ); } } else { - for b in bytes { - if let Err(e) = stream.send(Message::Binary(vec![b])).await { + let mut bytes = bytes.iter().peekable(); + while let Some(b) = bytes.next() { + if let Err(e) = stream.send(Message::Binary(vec![*b])).await { error!( ?e, "failed to send input to serial console websocket" ); } - tokio::time::sleep(debounce).await; + if let Some(next) = bytes.peek() { + if *next == b { + tokio::time::sleep(debounce).await; + } + } } } + + let _ = done.send(()); } + TaskCommand::Clear => buffer.clear(), TaskCommand::RegisterWait(waiter) => { buffer.register_wait_for_output(waiter); } diff --git a/phd-tests/framework/src/serial/raw_buffer.rs b/phd-tests/framework/src/serial/raw_buffer.rs index 843506944..3e411a1b9 100644 --- a/phd-tests/framework/src/serial/raw_buffer.rs +++ b/phd-tests/framework/src/serial/raw_buffer.rs @@ -87,19 +87,14 @@ impl RawBuffer { fn satisfy_or_set_wait(&mut self, waiter: OutputWaiter) { assert!(self.waiter.is_none()); if let Some(idx) = self.wait_buffer.rfind(&waiter.wanted) { - // Send all of the data in the buffer prior to the target string - // out the waiter's channel. - // + let out = self.wait_buffer[..idx].to_owned(); + // Because incoming bytes from Propolis may be processed on a // separate task than the task that registered the wait, this // can race such that the wait is satisfied just as the waiter // times out and closes its half of the channel. There's nothing // to be done about this, so just ignore any errors here. - let out = self.wait_buffer.drain(..idx).collect(); let _ = waiter.preceding_tx.send(out); - - // Clear the matched string out of the wait buffer. - self.wait_buffer = self.wait_buffer.split_off(waiter.wanted.len()); } else { self.waiter = Some(waiter); } @@ -126,6 +121,10 @@ impl Buffer for RawBuffer { } } + fn clear(&mut self) { + self.wait_buffer.clear(); + } + fn register_wait_for_output(&mut self, waiter: OutputWaiter) { self.satisfy_or_set_wait(waiter); } diff --git a/phd-tests/framework/src/serial/vt80x24.rs b/phd-tests/framework/src/serial/vt80x24.rs index 5df55b753..3b1b3daff 100644 --- a/phd-tests/framework/src/serial/vt80x24.rs +++ b/phd-tests/framework/src/serial/vt80x24.rs @@ -110,47 +110,6 @@ impl Vt80x24 { let mut contents = self.surface.screen_chars_to_string(); trace!(?contents, "termwiz contents"); if let Some(idx) = contents.rfind(&waiter.wanted) { - // Callers who set waits assume that matched strings are consumed - // from the buffer and won't match again unless the string is - // rendered to the buffer again. In the raw buffering case, this is - // straightforward: the buffer is already a String, so it suffices - // just to split the string just after the match. In this case, - // however, life is harder, because the buffer is not a string but a - // collection of cells containing `char`s. - // - // To "consume" the buffer, overwrite everything up through the - // match string with spaces. Start by truncating the current buffer - // contents down to the substring that ends in the match. - let last_byte = idx + waiter.wanted.len(); - contents.truncate(last_byte); - - // Then move the cursor to the top left and "type" as many blank - // spaces as there were characters in the buffer. Note that termwiz - // inserts extra '\n' characters at the end of every line that need - // to be ignored here. (It's assumed that none of the actual - // terminal cells contain a '\n' control character--if one of those - // is printed the cursor moves instead.) - let char_count = contents.chars().filter(|c| *c != '\n').count(); - - // Before typing anything, remember the old cursor position so that - // it can be restored after typing the spaces. - // - // It's insufficient to assume that the last character of the match - // was actually "typed" such that the cursor advanced past it. For - // example, if a match string ends with "$ ", and the guest moves to - // the start of an empty line and types a single "$", the cursor is - // in column 1 (just past the "$"), but the match is satisfied by - // the "pre-existing" space in that column. - let (old_col, old_row) = self.surface.cursor_position(); - self.surface.add_change(make_absolute_cursor_position(0, 0)); - self.surface.add_change(Change::Text(" ".repeat(char_count))); - let seq = self - .surface - .add_change(make_absolute_cursor_position(old_col, old_row)); - self.surface.flush_changes_older_than(seq); - - // Remove the match string from the match contents and push - // everything else back to the listener. contents.truncate(idx); let _ = waiter.preceding_tx.send(contents); } else { @@ -165,6 +124,13 @@ impl Buffer for Vt80x24 { self.apply_actions(&actions); } + fn clear(&mut self) { + let seq = self + .surface + .add_change(Change::ClearScreen(ColorAttribute::Default)); + self.surface.flush_changes_older_than(seq); + } + fn register_wait_for_output(&mut self, waiter: OutputWaiter) { self.satisfy_or_set_wait(waiter); } diff --git a/phd-tests/framework/src/test_vm/mod.rs b/phd-tests/framework/src/test_vm/mod.rs index f0db51151..9298fa043 100644 --- a/phd-tests/framework/src/test_vm/mod.rs +++ b/phd-tests/framework/src/test_vm/mod.rs @@ -650,20 +650,25 @@ impl TestVm { match step { CommandSequenceEntry::WaitFor(s) => { self.wait_for_serial_output( - s, + s.as_ref(), SerialOutputTimeout::CallerTimeout, ) .await?; } CommandSequenceEntry::WriteStr(s) => { - self.send_serial_str(s).await?; + self.send_serial_str(s.as_ref()).await?; self.send_serial_str("\n").await?; } + CommandSequenceEntry::ClearBuffer => { + self.clear_serial_buffer()? + } CommandSequenceEntry::ChangeSerialConsoleBuffer(kind) => { self.change_serial_buffer_kind(kind)?; } - CommandSequenceEntry::SetSerialByteWriteDelay(duration) => { - self.set_serial_byte_write_delay(duration)?; + CommandSequenceEntry::SetRepeatedCharacterDebounce( + duration, + ) => { + self.set_serial_repeated_character_debounce(duration)?; } } } @@ -684,8 +689,8 @@ impl TestVm { } /// Waits for up to `timeout_duration` for `line` to appear on the guest - /// serial console, then returns the unconsumed portion of the serial - /// console buffer that preceded the requested string. + /// serial console, then returns the contents of the console buffer that + /// preceded the requested string. #[instrument(skip_all, fields(vm = self.spec.vm_name, vm_id = %self.id))] pub async fn wait_for_serial_output( &self, @@ -731,26 +736,40 @@ impl TestVm { /// [`Self::wait_for_serial_output`] and returns any text that was buffered /// to the serial console after the command was sent. pub async fn run_shell_command(&self, cmd: &str) -> Result { - // Send the command out the serial port, including any amendments - // required by the guest. Do not send the final '\n' keystroke that - // actually issues the command. - let to_send = self.guest_os.amend_shell_command(cmd); - self.send_serial_str(&to_send).await?; - - // Wait for the command to be echoed back. This ensures that the echoed - // command is consumed from the buffer such that it won't be returned - // as output when waiting for the post-command shell prompt to appear. - // - // Tests may send multi-line commands. Assume these won't be echoed - // literally and that each line will instead be preceded by `> `. - let echo = to_send.trim_end().replace('\n', "\n> "); - self.wait_for_serial_output(&echo, Duration::from_secs(15)).await?; - self.send_serial_str("\n").await?; - - // Once the command has run, the guest should display another prompt. - // Treat the unconsumed buffered text before this point as the command - // output. (Note again that the command itself was already consumed by - // the wait above.) + // Allow the guest OS to transform the input command into a + // guest-specific command sequence. This accounts for the guest's shell + // type (which affects e.g. affects how it displays multi-line commands) + // and serial console buffering discipline. + let command_sequence = self.guest_os.shell_command_sequence(cmd); + for step in command_sequence.0 { + match step { + CommandSequenceEntry::WaitFor(s) => { + self.wait_for_serial_output( + s.as_ref(), + std::time::Duration::from_secs(15), + ) + .await?; + } + CommandSequenceEntry::WriteStr(s) => { + self.send_serial_str(s.as_ref()).await?; + } + CommandSequenceEntry::ClearBuffer => { + self.clear_serial_buffer()? + } + _ => { + anyhow::bail!( + "Unexpected command sequence entry {step:?} while \ + running shell command" + ); + } + } + } + + // `shell_command_sequence` promises that the generated command sequence + // clears buffer of everything up to and including the input command + // before actually issuing the final '\n' that issues the command. + // This ensures that the buffer contents returned by this call contain + // only the command's output. let out = self .wait_for_serial_output( self.guest_os.get_shell_prompt(), @@ -758,42 +777,48 @@ impl TestVm { ) .await?; - // Trim both ends of the output to get rid of any echoed newlines and/or - // whitespace that were inserted when sending '\n' to start processing - // the command. + // Trim any leading newlines inserted when the command was issued and + // any trailing whitespace that isn't actually part of the command + // output. Any other embedded whitespace is the caller's problem. Ok(out.trim().to_string()) } - async fn send_serial_str(&self, string: &str) -> Result<()> { + /// Sends `string` to the guest's serial console worker, then waits for the + /// entire string to be sent to the guest before returning. + pub(crate) async fn send_serial_str(&self, string: &str) -> Result<()> { if !string.is_empty() { - self.send_serial_bytes_async(Vec::from(string.as_bytes())).await - } else { - Ok(()) + self.send_serial_bytes(Vec::from(string.as_bytes()))?.await?; } + Ok(()) } - async fn send_serial_bytes_async(&self, bytes: Vec) -> Result<()> { + fn serial_console(&self) -> Result<&SerialConsole> { match &self.state { - VmState::Ensured { serial } => serial.send_bytes(bytes), + VmState::Ensured { serial } => Ok(serial), VmState::New => Err(VmStateError::InstanceNotEnsured.into()), } } + fn send_serial_bytes( + &self, + bytes: Vec, + ) -> Result> { + self.serial_console()?.send_bytes(bytes) + } + + fn clear_serial_buffer(&self) -> Result<()> { + self.serial_console()?.clear() + } + fn change_serial_buffer_kind(&self, kind: BufferKind) -> Result<()> { - match &self.state { - VmState::Ensured { serial } => serial.change_buffer_kind(kind), - VmState::New => Err(VmStateError::InstanceNotEnsured.into()), - } + self.serial_console()?.change_buffer_kind(kind) } - fn set_serial_byte_write_delay( + fn set_serial_repeated_character_debounce( &self, delay: std::time::Duration, ) -> Result<()> { - match &self.state { - VmState::Ensured { serial } => serial.set_guest_write_delay(delay), - VmState::New => Err(VmStateError::InstanceNotEnsured.into()), - } + self.serial_console()?.set_repeated_character_debounce(delay) } /// Indicates whether this VM's guest OS has a read-only filesystem.