[5/n] [test-utils] turn redactor into the builder pattern

dev-tools/omdb/tests/test_all_output.rs

@@ -16,9 +16,8 @@ use nexus_types::deployment::Blueprint;
 use nexus_types::deployment::SledFilter;
 use nexus_types::deployment::UnstableReconfiguratorState;
 use omicron_test_utils::dev::test_cmds::path_to_executable;
-use omicron_test_utils::dev::test_cmds::redact_extra;
 use omicron_test_utils::dev::test_cmds::run_command;
-use omicron_test_utils::dev::test_cmds::ExtraRedactions;
+use omicron_test_utils::dev::test_cmds::Redactor;
 use slog_error_chain::InlineErrorChain;
 use std::fmt::Write;
 use std::net::IpAddr;
@@ -203,19 +202,21 @@ async fn test_omdb_success_cases(cptestctx: &ControlPlaneTestContext) {
         // ControlPlaneTestContext.
     ];
 
-    let mut redactions = ExtraRedactions::new();
-    redactions
-        .variable_length("tmp_path", tmppath.as_str())
-        .fixed_length("blueprint_id", &initial_blueprint_id)
-        .variable_length(
+    let mut redactor = Redactor::default();
+    redactor
+        .extra_variable_length("tmp_path", tmppath.as_str())
+        .extra_fixed_length("blueprint_id", &initial_blueprint_id)
+        .extra_variable_length(
             "cockroachdb_fingerprint",
             &initial_blueprint.cockroachdb_fingerprint,
         );
+
     let crdb_version =
         initial_blueprint.cockroachdb_setting_preserve_downgrade.to_string();
     if initial_blueprint.cockroachdb_setting_preserve_downgrade.is_set() {
-        redactions.variable_length("cockroachdb_version", &crdb_version);
+        redactor.extra_variable_length("cockroachdb_version", &crdb_version);
     }
+
     for args in invocations {
         println!("running commands with args: {:?}", args);
         let p = postgres_url.to_string();
@@ -234,7 +235,7 @@ async fn test_omdb_success_cases(cptestctx: &ControlPlaneTestContext) {
             },
             &cmd_path,
             args,
-            Some(&redactions),
+            &redactor,
         )
         .await;
     }
@@ -444,14 +445,7 @@ async fn do_run<F>(
 ) where
     F: FnOnce(Exec) -> Exec + Send + 'static,
 {
-    do_run_extra(
-        output,
-        modexec,
-        cmd_path,
-        args,
-        Some(&ExtraRedactions::new()),
-    )
-    .await;
+    do_run_extra(output, modexec, cmd_path, args, &Redactor::default()).await;
 }
 
 async fn do_run_no_redactions<F>(
@@ -462,30 +456,23 @@ async fn do_run_no_redactions<F>(
 ) where
     F: FnOnce(Exec) -> Exec + Send + 'static,
 {
-    do_run_extra(output, modexec, cmd_path, args, None).await;
+    do_run_extra(output, modexec, cmd_path, args, &Redactor::noop()).await;
 }
 
 async fn do_run_extra<F>(
     output: &mut String,
     modexec: F,
     cmd_path: &Path,
     args: &[&str],
-    extra_redactions: Option<&ExtraRedactions<'_>>,
+    redactor: &Redactor<'_>,
 ) where
     F: FnOnce(Exec) -> Exec + Send + 'static,
 {
     write!(
         output,
         "EXECUTING COMMAND: {} {:?}\n",
         cmd_path.file_name().expect("missing command").to_string_lossy(),
-        args.iter()
-            .map(|r| {
-                extra_redactions.map_or_else(
-                    || r.to_string(),
-                    |redactions| redact_extra(r, redactions),
-                )
-            })
-            .collect::<Vec<_>>()
+        args.iter().map(|r| redactor.do_redact(r)).collect::<Vec<_>>()
     )
     .unwrap();
 
@@ -521,21 +508,11 @@ async fn do_run_extra<F>(
     write!(output, "termination: {:?}\n", exit_status).unwrap();
     write!(output, "---------------------------------------------\n").unwrap();
     write!(output, "stdout:\n").unwrap();
-
-    if let Some(extra_redactions) = extra_redactions {
-        output.push_str(&redact_extra(&stdout_text, extra_redactions));
-    } else {
-        output.push_str(&stdout_text);
-    }
+    output.push_str(&redactor.do_redact(&stdout_text));
 
     write!(output, "---------------------------------------------\n").unwrap();
     write!(output, "stderr:\n").unwrap();
-
-    if let Some(extra_redactions) = extra_redactions {
-        output.push_str(&redact_extra(&stderr_text, extra_redactions));
-    } else {
-        output.push_str(&stderr_text);
-    }
+    output.push_str(&redactor.do_redact(&stderr_text));
 
     write!(output, "=============================================\n").unwrap();
 }

dev-tools/reconfigurator-cli/tests/test_basic.rs

@@ -19,8 +19,8 @@ use omicron_test_utils::dev::poll::wait_for_condition;
 use omicron_test_utils::dev::poll::CondCheckError;
 use omicron_test_utils::dev::test_cmds::assert_exit_code;
 use omicron_test_utils::dev::test_cmds::path_to_executable;
-use omicron_test_utils::dev::test_cmds::redact_variable;
 use omicron_test_utils::dev::test_cmds::run_command;
+use omicron_test_utils::dev::test_cmds::Redactor;
 use omicron_test_utils::dev::test_cmds::EXIT_SUCCESS;
 use omicron_uuid_kinds::SledUuid;
 use slog::debug;
@@ -43,7 +43,7 @@ fn test_basic() {
     let exec = Exec::cmd(path_to_cli()).arg("tests/input/cmds.txt");
     let (exit_status, stdout_text, stderr_text) = run_command(exec);
     assert_exit_code(exit_status, EXIT_SUCCESS, &stderr_text);
-    let stdout_text = redact_variable(&stdout_text);
+    let stdout_text = Redactor::default().do_redact(&stdout_text);
     assert_contents("tests/output/cmd-stdout", &stdout_text);
     assert_contents("tests/output/cmd-stderr", &stderr_text);
 }

test-utils/src/dev/test_cmds.rs

@@ -125,7 +125,83 @@ pub fn error_for_enoent() -> String {
 /// invocation to invocation (e.g., assigned TCP port numbers, timestamps)
 ///
 /// This allows use to use expectorate to verify the shape of the CLI output.
-pub fn redact_variable(input: &str) -> String {
+#[derive(Clone, Debug)]
+pub struct Redactor<'a> {
+    basic: bool,
+    uuids: bool,
+    extra: Vec<(&'a str, String)>,
+}
+
+impl Default for Redactor<'_> {
+    fn default() -> Self {
+        Self { basic: true, uuids: true, extra: Vec::new() }
+    }
+}
+
+impl<'a> Redactor<'a> {
+    /// Create a new redactor that does not do any redactions.
+    pub fn noop() -> Self {
+        Self { basic: false, uuids: false, extra: Vec::new() }
+    }
+
+    pub fn basic(&mut self, basic: bool) -> &mut Self {
+        self.basic = basic;
+        self
+    }
+
+    pub fn uuids(&mut self, uuids: bool) -> &mut Self {
+        self.uuids = uuids;
+        self
+    }
+
+    pub fn extra_fixed_length(
+        &mut self,
+        name: &str,
+        text_to_redact: &'a str,
+    ) -> &mut Self {
+        // Use the same number of chars as the number of bytes in
+        // text_to_redact. We're almost entirely in ASCII-land so they're the
+        // same, and getting the length right is nice but doesn't matter for
+        // correctness.
+        //
+        // A technically more correct impl would use unicode-width, but ehhh.
+        let replacement = fill_redaction_text(name, text_to_redact.len());
+        self.extra.push((text_to_redact, replacement));
+        self
+    }
+
+    pub fn extra_variable_length(
+        &mut self,
+        name: &str,
+        text_to_redact: &'a str,
+    ) -> &mut Self {
+        let replacement = format!("<{}_REDACTED>", name.to_uppercase());
+        self.extra.push((text_to_redact, replacement));
+        self
+    }
+
+    pub fn do_redact(&self, input: &str) -> String {
+        // Perform extra redactions at the beginning, not the end. This is because
+        // some of the built-in redactions in redact_variable might match a
+        // substring of something that should be handled by extra_redactions (e.g.
+        // a temporary path).
+        let mut s = input.to_owned();
+        for (name, replacement) in &self.extra {
+            s = s.replace(name, replacement);
+        }
+
+        if self.basic {
+            s = redact_basic(&s);
+        }
+        if self.uuids {
+            s = redact_uuids(&s);
+        }
+
+        s
+    }
+}
+
+fn redact_basic(input: &str) -> String {
     // Replace TCP port numbers. We include the localhost
     // characters to avoid catching any random sequence of numbers.
     let s = regex::Regex::new(r"\[::1\]:\d{4,5}")
@@ -141,19 +217,6 @@ pub fn redact_variable(input: &str) -> String {
         .replace_all(&s, "127.0.0.1:REDACTED_PORT")
         .to_string();
 
-    // Replace uuids.
-    //
-    // The length of a UUID is 32 nibbles for the hex encoding of a u128 + 4
-    // dashes = 36.
-    const UUID_LEN: usize = 36;
-    let s = regex::Regex::new(
-        "[a-zA-Z0-9]{8}-[a-zA-Z0-9]{4}-[a-zA-Z0-9]{4}-\
-        [a-zA-Z0-9]{4}-[a-zA-Z0-9]{12}",
-    )
-    .unwrap()
-    .replace_all(&s, fill_redaction_text("uuid", UUID_LEN))
-    .to_string();
-
     // Replace timestamps.
     //
     // Format: RFC 3339 (ISO 8601)
@@ -213,63 +276,14 @@ pub fn redact_variable(input: &str) -> String {
     s
 }
 
-/// Redact text from a string, allowing for extra redactions to be specified.
-pub fn redact_extra(
-    input: &str,
-    extra_redactions: &ExtraRedactions<'_>,
-) -> String {
-    // Perform extra redactions at the beginning, not the end. This is because
-    // some of the built-in redactions in redact_variable might match a
-    // substring of something that should be handled by extra_redactions (e.g.
-    // a temporary path).
-    let mut s = input.to_owned();
-    for (name, replacement) in &extra_redactions.redactions {
-        s = s.replace(name, replacement);
-    }
-    redact_variable(&s)
-}
-
-/// Represents a list of extra redactions for [`redact_variable`].
-///
-/// Extra redactions are applied in-order, before any builtin redactions.
-#[derive(Clone, Debug, Default)]
-pub struct ExtraRedactions<'a> {
-    // A pair of redaction and replacement strings.
-    redactions: Vec<(&'a str, String)>,
-}
-
-impl<'a> ExtraRedactions<'a> {
-    pub fn new() -> Self {
-        Self { redactions: Vec::new() }
-    }
-
-    pub fn fixed_length(
-        &mut self,
-        name: &str,
-        text_to_redact: &'a str,
-    ) -> &mut Self {
-        // Use the same number of chars as the number of bytes in
-        // text_to_redact. We're almost entirely in ASCII-land so they're the
-        // same, and getting the length right is nice but doesn't matter for
-        // correctness.
-        //
-        // A technically more correct impl would use unicode-width, but ehhh.
-        let replacement = fill_redaction_text(name, text_to_redact.len());
-        self.redactions.push((text_to_redact, replacement));
-        self
-    }
-
-    pub fn variable_length(
-        &mut self,
-        name: &str,
-        text_to_redact: &'a str,
-    ) -> &mut Self {
-        let gen = format!("<{}_REDACTED>", name.to_uppercase());
-        let replacement = gen.to_string();
-
-        self.redactions.push((text_to_redact, replacement));
-        self
-    }
+fn redact_uuids(input: &str) -> String {
+    // The length of a UUID is 32 nibbles for the hex encoding of a u128 + 4
+    // dashes = 36.
+    const UUID_LEN: usize = 36;
+    regex::Regex::new(r"[0-9a-fA-F]{8}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}-[0-9a-fA-F]{12}")
+        .unwrap()
+        .replace_all(&input, fill_redaction_text("uuid", UUID_LEN))
+        .to_string()
 }
 
 fn fill_redaction_text(name: &str, text_to_redact_len: usize) -> String {
@@ -309,13 +323,11 @@ mod tests {
         let input = "time: 123ms, path: /var/tmp/tmp.456ms123s, \
             path2: /short, \
             path3: /variable-length/path";
-        let actual = redact_extra(
-            input,
-            ExtraRedactions::new()
-                .fixed_length("tp", "/var/tmp/tmp.456ms123s")
-                .fixed_length("short_redact", "/short")
-                .variable_length("variable", "/variable-length/path"),
-        );
+        let actual = Redactor::default()
+            .extra_fixed_length("tp", "/var/tmp/tmp.456ms123s")
+            .extra_fixed_length("short_redact", "/short")
+            .extra_variable_length("variable", "/variable-length/path")
+            .do_redact(input);
         assert_eq!(
             actual,
             "time: <REDACTED DURATION>ms, path: ....<REDACTED_TP>....., \
@@ -347,7 +359,7 @@ mod tests {
         for time in times {
             let input = format!("{:?}", time);
             assert_eq!(
-                redact_variable(&input),
+                Redactor::default().do_redact(&input),
                 "<REDACTED_TIMESTAMP>",
                 "Failed to redact {:?}",
                 time