davsclaus commented on code in PR #27903:
URL: https://github.com/apache/camel/pull/27903#discussion_r4291696341
##########
components/camel-ftp/pom.xml:
##########
@@ -57,6 +57,11 @@
<artifactId>camel-test-junit6</artifactId>
<scope>test</scope>
</dependency>
+ <dependency>
+ <groupId>org.assertj</groupId>
+ <artifactId>assertj-core</artifactId>
+ <scope>test</scope>
+ </dependency>
Review Comment:
camel-ftp has no AssertJ usage (its tests use JUnit `Assertions`), so this
adds a dependency for one check. With the suggestion in
`FtpConsumerDirectoriesNotMatchedIT`, this dependency can go.
##########
components/camel-ftp/src/test/java/org/apache/camel/component/file/remote/integration/FtpConsumerRelativeFileNameIT.java:
##########
@@ -30,13 +30,17 @@ public class FtpConsumerRelativeFileNameIT extends
FtpServerTestSupport {
private String getFtpUrl() {
return "ftp://admin@localhost:{{ftp.server.port}}";
- +
"/out/filename-consumer?password=admin&recursive=true&sortBy=file:name";
+ +
"/out/filename-consumer?password=admin&recursive=true&sortBy=file:name&noop=true";
+ }
+
+ private String getFtpProducerUrl() {
+ return
"ftp://admin@localhost:{{ftp.server.port}}/out/filename-consumer?password=admin";;
}
@Override
public void doPostSetup() throws Exception {
- sendFile(getFtpUrl(), "Hello World",
"out/filename-consumer-hello.txt");
- sendFile(getFtpUrl(), "Bye World", "out/filename-consumer-bye.txt");
+ sendFile(getFtpProducerUrl(), "Hello World",
"out/filename-consumer-hello.txt");
+ sendFile(getFtpProducerUrl(), "Bye World",
"out/filename-consumer-bye.txt");
}
Review Comment:
The sorter test has the same race: hello is uploaded before bye, but
`sortBy=file:name` should deliver bye first. A first poll that only sees hello
still fails, even with `noop=true`. Same fix:
`from(getFtpUrl()).routeId("foo").autoStartup(false).to("mock:result");` on
line 51, and
```java
@Override
public void doPostSetup() throws Exception {
sendFile(getFtpProducerUrl(), "Hello World",
"out/filename-consumer-hello.txt");
sendFile(getFtpProducerUrl(), "Bye World",
"out/filename-consumer-bye.txt");
context.getRouteController().startRoute("foo");
}
```
##########
components/camel-ftp/src/test/java/org/apache/camel/component/file/remote/integration/FtpConsumerDirectoriesNotMatchedIT.java:
##########
@@ -39,18 +49,24 @@ public void testSkipDirectories() throws Exception {
MockEndpoint mock = getMockEndpoint("mock:result");
mock.expectedMessageCount(3);
mock.assertIsSatisfied();
+
+ List<String> receivedFileNames = mock.getReceivedExchanges().stream()
+ .map(e -> e.getIn().getHeader(Exchange.FILE_NAME,
String.class))
+ .toList();
+ assertThat(receivedFileNames)
+ .as("Expected only .txt files to be consumed, but got: %s",
receivedFileNames)
+ .containsExactlyInAnyOrder("readme.txt",
"2007/report2007.txt", "2008/report2008.txt");
Review Comment:
`MockEndpoint` can check this itself, and when it fails it lists the file
names it did not receive. It also sets the expected count to 3. The `List` and
AssertJ imports, and the pom dependency, can then go.
```suggestion
MockEndpoint mock = getMockEndpoint("mock:result");
mock.expectedHeaderValuesReceivedInAnyOrder(Exchange.FILE_NAME,
"readme.txt", "2007/report2007.txt",
"2008/report2008.txt");
mock.assertIsSatisfied();
```
##########
components/camel-ftp/src/test/java/org/apache/camel/component/file/remote/integration/FromFtpRemoteFileSorterIT.java:
##########
@@ -33,7 +33,11 @@ public class FromFtpRemoteFileSorterIT extends
FtpServerTestSupport {
private final MyRemoteFileSorter sorter = new MyRemoteFileSorter();
private String getFtpUrl() {
- return
"ftp://admin@localhost:{{ftp.server.port}}/sorter?password=admin&sorter=#mySorter";;
+ return
"ftp://admin@localhost:{{ftp.server.port}}/sorter?password=admin&sorter=#mySorter&noop=true";;
Review Comment:
`noop=true` stops repeat reads, but the route still starts before
`doPostSetup()` uploads the files. If the first poll runs after `paris.txt` and
`london.txt` but before `copenhagen.txt`, the order is wrong and
`expectedBodiesReceived` fails. Since the upload order is the reverse of the
sort order, any partial poll breaks it. Starting the route after the upload
fixes this, on line 69:
```java
from(getFtpUrl()).routeId("foo").autoStartup(false).to("mock:result");
```
and in `doPostSetup()`, after `prepareFtpServer();`:
```java
context.getRouteController().startRoute("foo");
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
