fix: remove flakiness on download retries

Author: sfauvel

mithril-client/src/file_downloader/retry.rs

@@ -97,7 +97,7 @@ impl FileDownloader for RetryDownloader {
 #[cfg(test)]
 mod tests {
 
-    use std::time::Instant;
+    use std::{sync::Mutex, time::Instant};
 
     use mithril_common::entities::FileUri;
 
@@ -227,22 +227,59 @@ mod tests {
 
     #[tokio::test]
     async fn should_delay_between_retries() {
-        let mock_file_downloader = MockFileDownloaderBuilder::default()
-            .with_file_uri("http://whatever/00001.tar.gz")
-            .with_compression(None)
-            .with_failure()
-            .with_times(4)
-            .build();
-        let delay = Duration::from_millis(50);
+        struct FileDownloaderAssertDelay {
+            expected_delay_greater_than_or_equal: Duration,
+            expected_delay_less_than: Duration,
+            last_attempt_start_time: Mutex<Option<Instant>>,
+        }
+
+        #[async_trait]
+        impl FileDownloader for FileDownloaderAssertDelay {
+            async fn download_unpack(
+                &self,
+                _location: &FileDownloaderUri,
+                _file_size: u64,
+                _target_dir: &Path,
+                _compression_algorithm: Option<CompressionAlgorithm>,
+                _download_event_type: DownloadEvent,
+            ) -> StdResult<()> {
+                let mut last_attempt_start_time = self.last_attempt_start_time.lock().unwrap();
+                if let Some(last_start_attempt) = *last_attempt_start_time {
+                    let duration = last_start_attempt.elapsed();
+                    assert!(
+                        duration >= self.expected_delay_greater_than_or_equal,
+                        "duration should be greater than or equal to {}ms but was {}ms",
+                        self.expected_delay_greater_than_or_equal.as_millis(),
+                        duration.as_millis()
+                    );
+                    assert!(
+                        duration < self.expected_delay_less_than,
+                        "duration should be less than {}ms but was {}ms",
+                        self.expected_delay_less_than.as_millis(),
+                        duration.as_millis()
+                    );
+                }
+                *last_attempt_start_time = Some(Instant::now());
+
+                Err(anyhow::anyhow!("Download failed"))
+            }
+        }
+
+        let delay_ms = 50;
+        let mock_file_downloader = Arc::new(FileDownloaderAssertDelay {
+            expected_delay_greater_than_or_equal: Duration::from_millis(delay_ms),
+            expected_delay_less_than: Duration::from_millis(2 * delay_ms),
+            last_attempt_start_time: Mutex::new(None),
+        });
+
         let retry_downloader = RetryDownloader::new(
-            Arc::new(mock_file_downloader),
+            mock_file_downloader.clone(),
             FileDownloadRetryPolicy {
                 attempts: 4,
-                delay_between_attempts: delay,
+                delay_between_attempts: Duration::from_millis(delay_ms),
             },
         );
 
-        let start = Instant::now();
         retry_downloader
             .download_unpack(
                 &FileDownloaderUri::FileUri(FileUri("http://whatever/00001.tar.gz".to_string())),
@@ -256,19 +293,5 @@ mod tests {
             )
             .await
             .expect_err("An error should be returned when all download attempts fail");
-        let duration = start.elapsed();
-
-        assert!(
-            duration >= delay * 3,
-            "Duration should be at least 3 times the delay ({}ms) but was {}ms",
-            delay.as_millis() * 3,
-            duration.as_millis()
-        );
-        assert!(
-            duration < delay * 4,
-            "Duration should be less than 4 times the delay ({}ms) but was {}ms",
-            delay.as_millis() * 4,
-            duration.as_millis()
-        );
     }
 }