Merge pull request #1008 from bgilbert/clippy

Fix clippy lints and deprecated chrono methods in tests
coreos · Jun 2, 2023 · 292fba9 · 292fba9
2 parents 9a424f3 + 855bdf2
commit 292fba9
Show file tree

Hide file tree

Showing 11 changed files with 83 additions and 81 deletions.
diff --git a/Cargo.toml b/Cargo.toml
@@ -14,7 +14,7 @@ rust-version = "1.64.0"
 actix = "0.13"
 anyhow = "1.0"
 cfg-if = "1.0"
-chrono = { version = "0.4", features = ["serde"] }
+chrono = { version = "0.4.23", features = ["serde"] }
 clap = { version = "4.1", features = ["cargo", "derive"] }
 coreos-stream-metadata = "0.1.0"
 env_logger = "0.10"

diff --git a/src/cincinnati/mock_tests.rs b/src/cincinnati/mock_tests.rs
@@ -9,7 +9,7 @@ fn test_empty_graph() {
     let empty_graph = r#"{ "nodes": [], "edges": [] }"#;
     let m_graph = mockito::mock("GET", Matcher::Regex(r"^/v1/graph?.+$".to_string()))
         .match_header("accept", Matcher::Regex("application/json".to_string()))
-        .with_body(&empty_graph)
+        .with_body(empty_graph)
         .with_status(200)
         .create();
 

diff --git a/src/cli/deadend.rs b/src/cli/deadend.rs
@@ -115,30 +115,20 @@ mod tests {
             assert!(cli.is_err());
         }
         {
-            let mut is_ok = false;
             let empty_reason = vec!["zincati", "deadend-motd", "set", "--reason", ""];
             let cli = CliOptions::try_parse_from(empty_reason).unwrap();
-            if let CliCommand::DeadendMotd(cmd) = &cli.cmd {
-                if let Cmd::Set { reason } = cmd {
-                    assert_eq!(reason, "");
-                    is_ok = true;
-                }
-            }
-            if !is_ok {
+            if let CliCommand::DeadendMotd(Cmd::Set { reason }) = &cli.cmd {
+                assert_eq!(reason, "");
+            } else {
                 panic!("unexpected result: {:?}", cli);
             }
         }
         {
-            let mut is_ok = false;
             let reason_message = vec!["zincati", "deadend-motd", "set", "--reason", "foo"];
             let cli = CliOptions::try_parse_from(reason_message).unwrap();
-            if let CliCommand::DeadendMotd(cmd) = &cli.cmd {
-                if let Cmd::Set { reason } = cmd {
-                    assert_eq!(reason, "foo");
-                    is_ok = true;
-                }
-            }
-            if !is_ok {
+            if let CliCommand::DeadendMotd(Cmd::Set { reason }) = &cli.cmd {
+                assert_eq!(reason, "foo");
+            } else {
                 panic!("unexpected result: {:?}", cli);
             }
         }
@@ -152,15 +142,9 @@ mod tests {
             assert!(cli.is_err());
         }
         {
-            let mut is_ok = false;
             let unset = vec!["zincati", "deadend-motd", "unset"];
             let cli = CliOptions::try_parse_from(unset).unwrap();
-            if let CliCommand::DeadendMotd(cmd) = &cli.cmd {
-                if let Cmd::Unset = cmd {
-                    is_ok = true;
-                }
-            }
-            if !is_ok {
+            if !matches!(&cli.cmd, CliCommand::DeadendMotd(Cmd::Unset)) {
                 panic!("unexpected result: {:?}", cli);
             }
         }

diff --git a/src/fleet_lock/mock_tests.rs b/src/fleet_lock/mock_tests.rs
@@ -28,7 +28,7 @@ fn test_pre_reboot_lock() {
     m_pre_reboot.assert();
 
     let lock = res.unwrap();
-    assert_eq!(lock, true);
+    assert!(lock);
 }
 
 #[test]
@@ -81,7 +81,7 @@ fn test_steady_state_lock() {
     m_steady_state.assert();
 
     let unlock = res.unwrap();
-    assert_eq!(unlock, true);
+    assert!(unlock);
 }
 
 #[test]

diff --git a/src/rpm_ostree/mock_tests.rs b/src/rpm_ostree/mock_tests.rs
@@ -37,7 +37,7 @@ fn test_simple_graph() {
 
     let m_graph = mockito::mock("GET", Matcher::Regex(r"^/v1/graph?.+$".to_string()))
         .match_header("accept", Matcher::Regex("application/json".to_string()))
-        .with_body(&simple_graph)
+        .with_body(simple_graph)
         .with_status(200)
         .create();
 
@@ -86,7 +86,7 @@ fn test_downgrade() {
 
     let m_graph = mockito::mock("GET", Matcher::Regex(r"^/v1/graph?.+$".to_string()))
         .match_header("accept", Matcher::Regex("application/json".to_string()))
-        .with_body(&simple_graph)
+        .with_body(simple_graph)
         .with_status(200)
         .expect(2)
         .create();

diff --git a/src/rpm_ostree/mod.rs b/src/rpm_ostree/mod.rs
@@ -148,6 +148,7 @@ mod tests {
     }
 
     #[test]
+    #[allow(clippy::nonminimal_bool)]
     fn release_cmp() {
         {
             let n0 = Release {
@@ -160,10 +161,10 @@ mod tests {
                 checksum: "p1".to_string(),
                 age_index: Some(1),
             };
-            assert_eq!(n0 < n1, true);
-            assert_eq!(n0 == n0, true);
-            assert_eq!(n0 < n0, false);
-            assert_eq!(n0 > n0, false);
+            assert!(n0 < n1);
+            assert!(n0 == n0);
+            assert!(!(n0 < n0));
+            assert!(!(n0 > n0));
         }
         {
             let n0 = Release {
@@ -176,9 +177,9 @@ mod tests {
                 checksum: "p1".to_string(),
                 age_index: Some(0),
             };
-            assert_eq!(n0 < n1, true);
-            assert_eq!(n0 < n0, false);
-            assert_eq!(n0 > n0, false);
+            assert!(n0 < n1);
+            assert!(!(n0 < n0));
+            assert!(!(n0 > n0));
         }
         {
             let n0 = Release {
@@ -191,9 +192,9 @@ mod tests {
                 checksum: "p1".to_string(),
                 age_index: Some(0),
             };
-            assert_eq!(n0 < n1, true);
-            assert_eq!(n0 < n0, false);
-            assert_eq!(n0 > n0, false);
+            assert!(n0 < n1);
+            assert!(!(n0 < n0));
+            assert!(!(n0 > n0));
         }
     }
 }
diff --git a/src/strategy/immediate.rs b/src/strategy/immediate.rs
@@ -41,14 +41,14 @@ mod tests {
         let default = StrategyImmediate::default();
         let runtime = rt::Runtime::new().unwrap();
         let steady = runtime.block_on(default.report_steady()).unwrap();
-        assert_eq!(steady, true);
+        assert!(steady);
     }
 
     #[test]
     fn can_finalize() {
         let default = StrategyImmediate::default();
         let runtime = rt::Runtime::new().unwrap();
         let can_finalize = runtime.block_on(default.can_finalize()).unwrap();
-        assert_eq!(can_finalize, true);
+        assert!(can_finalize);
     }
 }
diff --git a/src/strategy/periodic.rs b/src/strategy/periodic.rs
@@ -203,15 +203,15 @@ mod tests {
         let default = StrategyPeriodic::default();
         let runtime = rt::Runtime::new().unwrap();
         let steady = runtime.block_on(default.can_finalize()).unwrap();
-        assert_eq!(steady, false);
+        assert!(!steady);
     }
 
     #[test]
     fn test_report_steady() {
         let default = StrategyPeriodic::default();
         let runtime = rt::Runtime::new().unwrap();
         let steady = runtime.block_on(default.report_steady()).unwrap();
-        assert_eq!(steady, true);
+        assert!(steady);
     }
 
     #[test]
@@ -230,15 +230,15 @@ mod tests {
         let non_utc_time_cfg = parse_config_input("tests/fixtures/30-periodic-sample-non-utc.toml");
         // Create current datetime with non UTC time.
         let naive_utc_dt = Utc::now().naive_utc();
-        let tz = Tz::named(&non_utc_time_cfg.updates.periodic.time_zone.clone()).unwrap();
+        let tz = Tz::named(&non_utc_time_cfg.updates.periodic.time_zone).unwrap();
         let dt = (&tz).from_utc_datetime(&naive_utc_dt);
         let weekday = dt.weekday();
         let time = format!("{}:{}", dt.hour(), dt.minute());
         // Modify time windows to only allow naive time in non-UTC time zone's current and following minute.
         let mut non_utc_time_update_input: inputs::UpdateInput = non_utc_time_cfg.updates;
         non_utc_time_update_input.periodic.intervals = vec![inputs::PeriodicIntervalInput {
             start_day: weekday.to_string(),
-            start_time: time.to_string(),
+            start_time: time,
             length_minutes: 2,
         }];
 
@@ -257,14 +257,14 @@ mod tests {
             Tz::named("America/Toronto").unwrap()
         );
         // Check that strategy allows reboot now.
-        assert_eq!(steady, true);
+        assert!(steady);
 
         let utc_strategy = StrategyPeriodic::new(utc_update_input).unwrap();
         let runtime = rt::Runtime::new().unwrap();
         let steady = runtime.block_on(utc_strategy.can_finalize()).unwrap();
         assert_eq!(utc_strategy.time_zone, Tz::named("UTC").unwrap());
         // Check that reboot is NOT allowed for UTC strategy.
-        assert_eq!(steady, false);
+        assert!(!steady);
     }
 
     #[test]
@@ -274,20 +274,18 @@ mod tests {
         let local_time_path = Path::new("/etc/localtime");
         let expected_tz;
         // If symlink `/etc/localtime` doesn't exist, we expect to default to UTC.
-        if let Err(_) = read_link(local_time_path) {
+        if read_link(local_time_path).is_err() {
             expected_tz = Some(Tz::named("UTC").unwrap());
+        } else if let Ok(tz_path) = local_time_path.canonicalize() {
+            let tz_str = tz_path
+                .strip_prefix(Path::new("/usr/share/zoneinfo"))
+                .unwrap()
+                .to_str()
+                .unwrap();
+            expected_tz = Some(Tz::named(tz_str).unwrap());
         } else {
-            if let Ok(tz_path) = local_time_path.canonicalize() {
-                let tz_str = tz_path
-                    .strip_prefix(Path::new("/usr/share/zoneinfo"))
-                    .unwrap()
-                    .to_str()
-                    .unwrap();
-                expected_tz = Some(Tz::named(tz_str).unwrap());
-            } else {
-                // `/etc/localtime` exists but points to an invalid time zone.
-                expected_tz = None;
-            }
+            // `/etc/localtime` exists but points to an invalid time zone.
+            expected_tz = None;
         }
         let config = parse_config_input("tests/fixtures/31-periodic-sample-non-utc.toml");
         let strategy = StrategyPeriodic::new(config.updates);

diff --git a/src/update_agent/actor.rs b/src/update_agent/actor.rs
@@ -635,7 +635,7 @@ mod tests {
         let prev_state =
             UpdateAgentMachineState::UpdateStaged((update.clone(), MAX_FINALIZE_POSTPONEMENTS));
         let cur_state = UpdateAgentMachineState::UpdateStaged((
-            update.clone(),
+            update,
             MAX_FINALIZE_POSTPONEMENTS.saturating_sub(1),
         ));
         assert!(!UpdateAgent::should_tick_immediately(

diff --git a/src/update_agent/mod.rs b/src/update_agent/mod.rs
@@ -610,7 +610,7 @@ mod tests {
         );
 
         let (persistent_err, _) = machine.record_failed_deploy();
-        assert_eq!(persistent_err, false);
+        assert!(!persistent_err);
         assert_eq!(
             machine,
             UpdateAgentMachineState::UpdateAvailable((update.clone(), 1))
@@ -626,10 +626,7 @@ mod tests {
         );
 
         machine.update_finalized(update.clone());
-        assert_eq!(
-            machine,
-            UpdateAgentMachineState::UpdateFinalized(update.clone())
-        );
+        assert_eq!(machine, UpdateAgentMachineState::UpdateFinalized(update));
 
         machine.end();
         assert_eq!(machine, UpdateAgentMachineState::EndState);
@@ -653,16 +650,16 @@ mod tests {
         // MAX-1 temporary failures.
         for attempt in 1..MAX_DEPLOY_ATTEMPTS {
             let (persistent_err, _) = machine.record_failed_deploy();
-            assert_eq!(persistent_err, false);
+            assert!(!persistent_err);
             assert_eq!(
                 machine,
-                UpdateAgentMachineState::UpdateAvailable((update.clone(), attempt as u8))
+                UpdateAgentMachineState::UpdateAvailable((update.clone(), attempt))
             )
         }
 
         // Persistent error threshold reached.
         let (persistent_err, _) = machine.record_failed_deploy();
-        assert_eq!(persistent_err, true);
+        assert!(persistent_err);
         assert_eq!(machine, UpdateAgentMachineState::NoNewUpdate);
     }
 
@@ -734,19 +731,16 @@ mod tests {
         // Reached 0 remaining postponements.
         let can_finalize = machine.handle_interactive_sessions(&interactive_sessions_present);
         assert!(can_finalize);
-        assert_eq!(
-            machine,
-            UpdateAgentMachineState::UpdateStaged((update.clone(), 0))
-        );
+        assert_eq!(machine, UpdateAgentMachineState::UpdateStaged((update, 0)));
     }
 
     #[test]
     fn test_format_seconds() {
         assert_eq!("1 second", format_seconds(1));
         assert_eq!("2 seconds", format_seconds(2));
         assert_eq!("1 minute", format_seconds(60));
-        assert_eq!("1 minute and 1 second", format_seconds(1 * 60 + 1));
-        assert_eq!("1 minute and 30 seconds", format_seconds(1 * 60 + 30));
+        assert_eq!("1 minute and 1 second", format_seconds(60 + 1));
+        assert_eq!("1 minute and 30 seconds", format_seconds(60 + 30));
         assert_eq!("2 minutes", format_seconds(2 * 60));
         assert_eq!("42 minutes and 23 seconds", format_seconds(42 * 60 + 23));
     }

diff --git a/src/weekly/mod.rs b/src/weekly/mod.rs
@@ -461,10 +461,10 @@ mod tests {
         let calendar = WeeklyCalendar::new(windows);
         assert_eq!(calendar.windows.iter().count(), 1);
 
-        let datetime = Utc.ymd(2019, 6, 25).and_hms(21, 10, 0);
+        let datetime = Utc.with_ymd_and_hms(2019, 6, 25, 21, 10, 0).unwrap();
         assert!(calendar.contains_datetime(&datetime));
         // Sanity check that `WeeklyCalendar` is `TimeZone`-agnostic.
-        let datetime = Local.ymd(2019, 6, 25).and_hms(21, 10, 0);
+        let datetime = Local.with_ymd_and_hms(2019, 6, 25, 21, 10, 0).unwrap();
         assert!(calendar.contains_datetime(&datetime));
     }
 
@@ -591,11 +591,36 @@ mod tests {
         let calendar = WeeklyCalendar::new(w1.clone());
 
         let tz = Tz::named("UTC").unwrap();
-        let dt0 = (&tz).from_utc_datetime(&NaiveDate::from_ymd(2021, 4, 12).and_hms(0, 0, 0));
-        let dt1 = (&tz).from_utc_datetime(&NaiveDate::from_ymd(2021, 4, 12).and_hms(1, 5, 0));
-        let dt2 = (&tz).from_utc_datetime(&NaiveDate::from_ymd(2021, 4, 12).and_hms(2, 16, 0));
-        let dt3 = (&tz).from_utc_datetime(&NaiveDate::from_ymd(2021, 4, 16).and_hms(15, 14, 56));
-        let dt4 = (&tz).from_utc_datetime(&NaiveDate::from_ymd(2021, 4, 18).and_hms(23, 35, 00));
+        let dt0 = (&tz).from_utc_datetime(
+            &NaiveDate::from_ymd_opt(2021, 4, 12)
+                .unwrap()
+                .and_hms_opt(0, 0, 0)
+                .unwrap(),
+        );
+        let dt1 = (&tz).from_utc_datetime(
+            &NaiveDate::from_ymd_opt(2021, 4, 12)
+                .unwrap()
+                .and_hms_opt(1, 5, 0)
+                .unwrap(),
+        );
+        let dt2 = (&tz).from_utc_datetime(
+            &NaiveDate::from_ymd_opt(2021, 4, 12)
+                .unwrap()
+                .and_hms_opt(2, 16, 0)
+                .unwrap(),
+        );
+        let dt3 = (&tz).from_utc_datetime(
+            &NaiveDate::from_ymd_opt(2021, 4, 16)
+                .unwrap()
+                .and_hms_opt(15, 14, 56)
+                .unwrap(),
+        );
+        let dt4 = (&tz).from_utc_datetime(
+            &NaiveDate::from_ymd_opt(2021, 4, 18)
+                .unwrap()
+                .and_hms_opt(23, 35, 00)
+                .unwrap(),
+        );
 
         let cases = vec![
             (