From 865848e350484ae64e4b75853fa07fd15689f590 Mon Sep 17 00:00:00 2001 From: Guillaume Nodet Date: Sat, 11 Jul 2026 16:58:44 +0000 Subject: [PATCH] CAMEL-23921: Replace Thread.sleep with Awaitility in SFTP tests - MinaSftpAdvancedFileOperationsIT: replace 3x Thread.sleep(500) after mock assertions with Awaitility to wait for filesystem move operations - MinaSftpProtocolIT: replace 2x Thread.sleep(1000) after mock assertions with Awaitility for file delete/move; replace Thread.sleep(2000) timestamp gap with explicit setLastModified() - MinaSftpConcurrencyIT: remove 2x unnecessary pacing delays (Thread.sleep(50/100)) in concurrent read/write test Co-Authored-By: Claude Opus 4.6 --- .../MinaSftpAdvancedFileOperationsIT.java | 28 +++++++++------ .../integration/MinaSftpConcurrencyIT.java | 2 -- .../mina/integration/MinaSftpProtocolIT.java | 35 +++++++++++-------- 3 files changed, 37 insertions(+), 28 deletions(-) diff --git a/components/camel-mina-sftp/src/test/java/org/apache/camel/component/file/remote/mina/integration/MinaSftpAdvancedFileOperationsIT.java b/components/camel-mina-sftp/src/test/java/org/apache/camel/component/file/remote/mina/integration/MinaSftpAdvancedFileOperationsIT.java index 71bfa1b913fc7..a4b05d6091700 100644 --- a/components/camel-mina-sftp/src/test/java/org/apache/camel/component/file/remote/mina/integration/MinaSftpAdvancedFileOperationsIT.java +++ b/components/camel-mina-sftp/src/test/java/org/apache/camel/component/file/remote/mina/integration/MinaSftpAdvancedFileOperationsIT.java @@ -20,6 +20,7 @@ import java.io.IOException; import java.nio.file.Files; import java.nio.file.Path; +import java.util.concurrent.TimeUnit; import org.apache.camel.CamelExecutionException; import org.apache.camel.Exchange; @@ -35,6 +36,7 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import static org.awaitility.Awaitility.await; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertThrows; @@ -333,10 +335,11 @@ public void configure() { context.getRouteController().startRoute("renameTest"); mock.assertIsSatisfied(); - // Verify source file is gone and target file exists - Thread.sleep(500); - assertTrue(!sourceFile.exists(), "Source file should be moved"); - assertTrue(ftpFile("done/rename-source.txt").toFile().exists(), "Moved file should exist in done folder"); + // Wait for the post-processing file move to complete on the filesystem + await().atMost(10, TimeUnit.SECONDS).untilAsserted(() -> { + assertTrue(!sourceFile.exists(), "Source file should be moved"); + assertTrue(ftpFile("done/rename-source.txt").toFile().exists(), "Moved file should exist in done folder"); + }); // Verify content is intact (no download/upload) assertEquals("Rename test content", @@ -378,9 +381,10 @@ public void configure() { context.getRouteController().startRoute("renameSubTest"); mock.assertIsSatisfied(); - Thread.sleep(500); - assertTrue(!ftpFile("subdir/rename-sub.txt").toFile().exists(), "Source should be moved"); - assertTrue(ftpFile("subdir/processed/rename-sub.txt").toFile().exists(), "Target should exist"); + await().atMost(10, TimeUnit.SECONDS).untilAsserted(() -> { + assertTrue(!ftpFile("subdir/rename-sub.txt").toFile().exists(), "Source should be moved"); + assertTrue(ftpFile("subdir/processed/rename-sub.txt").toFile().exists(), "Target should exist"); + }); context.getRouteController().stopRoute("renameSubTest"); } @@ -437,10 +441,12 @@ public void configure() { context.getRouteController().startRoute("sizePreserveTest"); mock.assertIsSatisfied(); - Thread.sleep(500); - // Verify moved file has same size - long movedSize = ftpFile("moved/size-preserve.txt").toFile().length(); - assertEquals(originalSize, movedSize, "Moved file should have same size (server-side rename)"); + // Wait for the post-processing file move to complete on the filesystem + await().atMost(10, TimeUnit.SECONDS).untilAsserted(() -> { + assertTrue(ftpFile("moved/size-preserve.txt").toFile().exists(), "Moved file should exist"); + long movedSize = ftpFile("moved/size-preserve.txt").toFile().length(); + assertEquals(originalSize, movedSize, "Moved file should have same size (server-side rename)"); + }); context.getRouteController().stopRoute("sizePreserveTest"); } diff --git a/components/camel-mina-sftp/src/test/java/org/apache/camel/component/file/remote/mina/integration/MinaSftpConcurrencyIT.java b/components/camel-mina-sftp/src/test/java/org/apache/camel/component/file/remote/mina/integration/MinaSftpConcurrencyIT.java index 5e93704084e48..c538050aa810a 100644 --- a/components/camel-mina-sftp/src/test/java/org/apache/camel/component/file/remote/mina/integration/MinaSftpConcurrencyIT.java +++ b/components/camel-mina-sftp/src/test/java/org/apache/camel/component/file/remote/mina/integration/MinaSftpConcurrencyIT.java @@ -269,7 +269,6 @@ public void testConcurrentReadAndWrite() throws Exception { template.sendBodyAndHeader(baseUri(), "Write content " + i, Exchange.FILE_NAME, "write-" + i + ".txt"); writeSuccess.incrementAndGet(); - Thread.sleep(50); // Small delay between writes } } catch (Exception e) { errorCount.incrementAndGet(); @@ -291,7 +290,6 @@ public void testConcurrentReadAndWrite() throws Exception { readSuccess.incrementAndGet(); } } - Thread.sleep(100); // Small delay between reads } } catch (Exception e) { errorCount.incrementAndGet(); diff --git a/components/camel-mina-sftp/src/test/java/org/apache/camel/component/file/remote/mina/integration/MinaSftpProtocolIT.java b/components/camel-mina-sftp/src/test/java/org/apache/camel/component/file/remote/mina/integration/MinaSftpProtocolIT.java index 50b7d512ff453..6027316de752a 100644 --- a/components/camel-mina-sftp/src/test/java/org/apache/camel/component/file/remote/mina/integration/MinaSftpProtocolIT.java +++ b/components/camel-mina-sftp/src/test/java/org/apache/camel/component/file/remote/mina/integration/MinaSftpProtocolIT.java @@ -21,6 +21,7 @@ import java.nio.charset.StandardCharsets; import java.nio.file.Files; import java.security.MessageDigest; +import java.util.concurrent.TimeUnit; import org.apache.camel.Exchange; import org.apache.camel.builder.RouteBuilder; @@ -31,6 +32,7 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import static org.awaitility.Awaitility.await; import static org.junit.jupiter.api.Assertions.assertArrayEquals; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertNotNull; @@ -296,9 +298,10 @@ public void configure() { context.getRouteController().startRoute("zeroDelete"); mock.assertIsSatisfied(); - // Verify the file was deleted - Thread.sleep(1000); // Give time for delete - assertTrue(!ftpFile("zero-delete.txt").toFile().exists(), "Zero-byte file should be deleted after consumption"); + // Wait for the post-processing file delete to complete on the filesystem + await().atMost(10, TimeUnit.SECONDS) + .untilAsserted(() -> assertTrue(!ftpFile("zero-delete.txt").toFile().exists(), + "Zero-byte file should be deleted after consumption")); context.getRouteController().stopRoute("zeroDelete"); } @@ -329,10 +332,11 @@ public void configure() { context.getRouteController().startRoute("zeroMove"); mock.assertIsSatisfied(); - // Verify the file was moved - Thread.sleep(1000); - assertTrue(!ftpFile("zero-move.txt").toFile().exists(), "Original zero-byte file should not exist"); - assertTrue(ftpFile("done/zero-move.txt").toFile().exists(), "Zero-byte file should be in done folder"); + // Wait for the post-processing file move to complete on the filesystem + await().atMost(10, TimeUnit.SECONDS).untilAsserted(() -> { + assertTrue(!ftpFile("zero-move.txt").toFile().exists(), "Original zero-byte file should not exist"); + assertTrue(ftpFile("done/zero-move.txt").toFile().exists(), "Zero-byte file should be in done folder"); + }); context.getRouteController().stopRoute("zeroMove"); } @@ -442,14 +446,15 @@ public void testFilePermissionsMetadata() throws Exception { @Test @Timeout(60) public void testPreserveTimestampOnDownload() throws Exception { - // Create a file + // Create a file and set its modification time to a known value in the past. + // This avoids Thread.sleep() — instead of waiting for clock drift, we explicitly + // set a past timestamp and verify the consumer reports it correctly. template.sendBodyAndHeader(baseUri(), "Timestamp test", Exchange.FILE_NAME, "timestamp-test.txt"); - // Record original timestamp - long originalModified = ftpFile("timestamp-test.txt").toFile().lastModified(); - - // Wait a bit to ensure time difference would be noticeable - Thread.sleep(2000); + File timestampFile = ftpFile("timestamp-test.txt").toFile(); + long pastTimestamp = System.currentTimeMillis() - 60_000; // 1 minute ago + timestampFile.setLastModified(pastTimestamp); + long originalModified = timestampFile.lastModified(); MockEndpoint mock = getMockEndpoint("mock:timestamp"); mock.expectedMessageCount(1); @@ -471,8 +476,8 @@ public void configure() { Long headerModified = exchange.getIn().getHeader(Exchange.FILE_LAST_MODIFIED, Long.class); assertNotNull(headerModified, "FILE_LAST_MODIFIED header should be present"); - // The header should reflect the file's modification time from the server - // Allow for some timestamp granularity differences (within 2 seconds) + // The header should reflect the file's modification time from the server. + // Allow for some timestamp granularity differences (within 2 seconds). assertTrue(Math.abs(headerModified - originalModified) < 2000, "Timestamp in header should be close to original file timestamp");