Merge pull request #2302 from cgwalters/tests-inst-clippy

tests/inst: Fix lots of `cargo clippy` warnings

tests/inst/src/destructive.rs

@@ -33,10 +33,10 @@ use strum_macros::EnumIter;
 
 use crate::test::*;
 
-const ORIGREF: &'static str = "orig-booted";
-const TESTREF: &'static str = "testcontent";
-const TDATAPATH: &'static str = "/var/tmp/ostree-test-transaction-data.json";
-const SRVREPO: &'static str = "/var/tmp/ostree-test-srv";
+const ORIGREF: &str = "orig-booted";
+const TESTREF: &str = "testcontent";
+const TDATAPATH: &str = "/var/tmp/ostree-test-transaction-data.json";
+const SRVREPO: &str = "/var/tmp/ostree-test-srv";
 // Percentage of ELF files to change per update
 const TREEGEN_PERCENTAGE: u32 = 15;
 /// Total number of reboots
@@ -111,21 +111,18 @@ impl RebootMark {
             InterruptStrategy::Polite(t) => self
                 .polite
                 .entry(t.clone())
-                .or_insert_with(|| BTreeMap::new()),
+                .or_insert_with(BTreeMap::new),
             InterruptStrategy::Force(t) => self
                 .force
                 .entry(t.clone())
-                .or_insert_with(|| BTreeMap::new()),
+                .or_insert_with(BTreeMap::new),
         }
     }
 }
 
 impl InterruptStrategy {
     pub(crate) fn is_noop(&self) -> bool {
-        match self {
-            InterruptStrategy::Polite(PoliteInterruptStrategy::None) => true,
-            _ => false,
-        }
+        matches!(self, InterruptStrategy::Polite(PoliteInterruptStrategy::None))
     }
 }
 
@@ -324,18 +321,16 @@ fn validate_live_interrupted_upgrade(commitstates: &CommitStates) -> Result<Upda
         assert!(!firstdeploy.booted);
         validate_pending_commit(pending_commit, &commitstates)?;
         UpdateResult::Staged
+    } else if pending_commit == commitstates.booted {
+        UpdateResult::NotCompleted
+    } else if pending_commit == commitstates.target {
+        UpdateResult::Completed
     } else {
-        if pending_commit == commitstates.booted {
-            UpdateResult::NotCompleted
-        } else if pending_commit == commitstates.target {
-            UpdateResult::Completed
-        } else {
-            anyhow::bail!(
-                "Unexpected pending commit: {} ({:?})",
-                pending_commit,
-                commitstates.describe(pending_commit)
-            );
-        }
+        anyhow::bail!(
+            "Unexpected pending commit: {} ({:?})",
+            pending_commit,
+            commitstates.describe(pending_commit)
+        );
     };
     Ok(res)
 }
@@ -504,7 +499,7 @@ fn impl_transaction_test<M: AsRef<str>>(
                     live_strategy = Some(strategy);
                 }
                 InterruptStrategy::Force(ForceInterruptStrategy::Reboot) => {
-                    mark.reboot_strategy = Some(strategy.clone());
+                    mark.reboot_strategy = Some(strategy);
                     prepare_reboot(serde_json::to_string(&mark)?)?;
                     // This is a forced reboot - no syncing of the filesystem.
                     bash!("reboot -ff")?;
@@ -516,8 +511,8 @@ fn impl_transaction_test<M: AsRef<str>>(
                     anyhow::bail!("Failed to wait for uninterrupted upgrade");
                 }
                 InterruptStrategy::Polite(PoliteInterruptStrategy::Reboot) => {
-                    mark.reboot_strategy = Some(strategy.clone());
-                    Err(reboot(serde_json::to_string(&mark)?))?;
+                    mark.reboot_strategy = Some(strategy);
+                    return Err(reboot(serde_json::to_string(&mark)?).into());
                     // We either rebooted, or failed to reboot
                 }
                 InterruptStrategy::Polite(PoliteInterruptStrategy::Stop) => {
@@ -545,7 +540,7 @@ fn transactionality() -> Result<()> {
     // We need this static across reboots
     let srvrepo = Path::new(SRVREPO);
     let firstrun = !srvrepo.exists();
-    if let Some(_) = mark.as_ref() {
+    if mark.as_ref().is_some() {
         if firstrun {
             anyhow::bail!("Missing {:?}", srvrepo);
         }
@@ -604,7 +599,7 @@ fn transactionality() -> Result<()> {
             let end = time::Instant::now();
             let cycle_time = end.duration_since(start);
             let tdata = TransactionalTestInfo {
-                cycle_time: cycle_time,
+                cycle_time,
             };
             let mut f = std::io::BufWriter::new(std::fs::File::create(&TDATAPATH)?);
             serde_json::to_writer(&mut f, &tdata)?;

tests/inst/src/insttestmain.rs

@@ -8,10 +8,11 @@ mod test;
 mod treegen;
 
 // Written by Ignition
-const DESTRUCTIVE_TEST_STAMP: &'static str = "/etc/ostree-destructive-test-ok";
+const DESTRUCTIVE_TEST_STAMP: &str = "/etc/ostree-destructive-test-ok";
 
 #[derive(Debug, StructOpt)]
 #[structopt(rename_all = "kebab-case")]
+#[allow(clippy::enum_variant_names)]
 /// Main options struct
 enum Opt {
     /// List the destructive tests
@@ -74,13 +75,11 @@ fn main() -> Result<()> {
             for t in test::DESTRUCTIVE_TESTS.iter() {
                 println!("{}", t.name);
             }
-            return Ok(());
+            Ok(())
         }
         Opt::NonDestructive(subopt) => {
             // FIXME add method to parse subargs
-            let iter = match subopt {
-                NonDestructiveOpts::Args(subargs) => subargs,
-            };
+            let NonDestructiveOpts::Args(iter) = subopt;
             let libtestargs = libtest_mimic::Arguments::from_iter(iter);
             let tests: Vec<_> = test::NONDESTRUCTIVE_TESTS
                 .iter()

tests/inst/src/test.rs

@@ -41,7 +41,7 @@ pub(crate) fn cmd_fails_with<C: BorrowMut<Command>>(mut c: C, pat: &str) -> Resu
     if o.status.success() {
         bail!("Command {:?} unexpectedly succeeded", c);
     }
-    if !twoway::find_bytes(&o.stderr, pat.as_bytes()).is_some() {
+    if twoway::find_bytes(&o.stderr, pat.as_bytes()).is_none() {
         dbg!(String::from_utf8_lossy(&o.stdout));
         dbg!(String::from_utf8_lossy(&o.stderr));
         bail!("Command {:?} stderr did not match: {}", c, pat);
@@ -56,7 +56,7 @@ pub(crate) fn cmd_has_output<C: BorrowMut<Command>>(mut c: C, pat: &str) -> Resu
     if !o.status.success() {
         bail!("Command {:?} failed", c);
     }
-    if !twoway::find_bytes(&o.stdout, pat.as_bytes()).is_some() {
+    if twoway::find_bytes(&o.stdout, pat.as_bytes()).is_none() {
         dbg!(String::from_utf8_lossy(&o.stdout));
         bail!("Command {:?} stdout did not match: {}", c, pat);
     }
@@ -77,12 +77,12 @@ pub(crate) struct TestHttpServerOpts {
     pub(crate) random_delay: Option<time::Duration>,
 }
 
-pub(crate) const TEST_HTTP_BASIC_AUTH: &'static str = "foouser:barpw";
+pub(crate) const TEST_HTTP_BASIC_AUTH: &str = "foouser:barpw";
 
 fn validate_authz(value: &[u8]) -> Result<bool> {
     let buf = std::str::from_utf8(&value)?;
     if let Some(o) = buf.find("Basic ") {
-        let (_, buf) = buf.split_at(o + "Basic ".len());
+    let (_, buf) = buf.split_at(o + "Basic ".len());
         let buf = base64::decode(buf).context("decoding")?;
         let buf = std::str::from_utf8(&buf)?;
         Ok(buf == TEST_HTTP_BASIC_AUTH)
@@ -136,7 +136,6 @@ pub(crate) async fn http_server<P: AsRef<Path>>(
 
     let make_service = make_service_fn(move |_| {
         let sv = sv.clone();
-        let opts = opts.clone();
         future::ok::<_, hyper::Error>(service_fn(move |req| handle_request(req, sv.clone(), opts)))
     });
     let server: hyper::Server<_, _, _> = hyper::Server::bind(&addr).serve(make_service);
@@ -161,7 +160,7 @@ where
     let path = path.as_ref();
     let mut rt = Runtime::new()?;
     rt.block_on(async move {
-        let addr = http_server(path, opts.clone()).await?;
+        let addr = http_server(path, *opts).await?;
         tokio::task::spawn_blocking(move || f(&addr)).await?
     })?;
     Ok(())