You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@camel.apache.org by "anton-goncharov (via GitHub)" <gi...@apache.org> on 2023/11/07 18:19:52 UTC

[PR] CAMEL-19835: camel-core: replace temporary dir logic in test code [camel]

anton-goncharov opened a new pull request, #11934:
URL: https://github.com/apache/camel/pull/11934

   # Description
   
   Making use of junit5 `@TempDir` in TestSupport to control lifecycle of a temporary directory when it's used in camel-core tests. @TempDir uses a system-specific path dedicated for temporary data.
   
   - Removed `deleteDirectory()` methods and their usage 
   - Changed `testDirectory()` and `fileUri()` and their overloads to use the temporary directory
   - Removed `deleteDirectory()` from SpringTestSupport
   
   Adapted tests where needed in `camel-core` and `camel-spring-xml`, which contains tests depending on `camel-core`
   
   In some corner cases Camel context is initialized on class level (e.g. Spring tests with `@ContextConfiguration`), they're addressed in a way of supplying custom initializer that passes temp directory path through a system variable.
   
   # Target
   
   - [x] I checked that the commit is targeting the correct branch (note that Camel 3 uses `camel-3.x`, whereas Camel 4 uses the `main` branch)
   
   # Tracking
   - [x] If this is a large change, bug fix, or code improvement, I checked there is a [JIRA issue](https://issues.apache.org/jira/browse/CAMEL) filed for the change (usually before you start working on it).
   
   <!--
   # *Note*: trivial changes like, typos, minor documentation fixes and other small items do not require a JIRA issue. In this case your pull request should address just this issue, without pulling in other changes.
   -->
   
   # Apache Camel coding standards and style
   
   - [x] I checked that each commit in the pull request has a meaningful subject line and body.
   
   <!--
   If you're unsure, you can format the pull request title like `[CAMEL-XXX] Fixes bug in camel-file component`, where you replace `CAMEL-XXX` with the appropriate JIRA issue.
   -->
   
   - [x] I have run `mvn clean install -DskipTests` locally and I have committed all auto-generated changes
   
   <!--
   You can run the aforementioned command in your module so that the build auto-formats your code. This will also be verified as part of the checks and your PR may be rejected if if there are uncommited changes after running `mvn clean install -DskipTests`.
   
   You can learn more about the contribution guidelines at https://github.com/apache/camel/blob/main/CONTRIBUTING.md
   -->
   
   


-- 
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: commits-unsubscribe@camel.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] CAMEL-19835: camel-core: replace temporary dir logic in test code [camel]

Posted by "anton-goncharov (via GitHub)" <gi...@apache.org>.
anton-goncharov commented on code in PR #11934:
URL: https://github.com/apache/camel/pull/11934#discussion_r1389210442


##########
core/camel-core/src/test/java/org/apache/camel/component/file/FileConsumerFailureHandledTest.java:
##########
@@ -57,7 +57,7 @@ public void testLondon() throws Exception {
         mock.expectedBodiesReceived("London");
 
         template.sendBodyAndHeader(fileUri(), "London", Exchange.FILE_NAME, "london.txt");
-        mock.assertIsSatisfied(1000);
+        mock.assertIsSatisfied(3000);

Review Comment:
   Thanks for the explanation, I will revert the timeouts then and set them back to 1000



-- 
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: commits-unsubscribe@camel.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] CAMEL-19835: camel-core: replace temporary dir logic in test code [camel]

Posted by "anton-goncharov (via GitHub)" <gi...@apache.org>.
anton-goncharov commented on PR #11934:
URL: https://github.com/apache/camel/pull/11934#issuecomment-1814464794

   thanks @orpiske , you're a mind reader, I was about to write to ask if we can proceed with this one.
   I'll be checking on ASF CI pipeline results


-- 
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: commits-unsubscribe@camel.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] CAMEL-19835: camel-core: replace temporary dir logic in test code [camel]

Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #11934:
URL: https://github.com/apache/camel/pull/11934#issuecomment-1800160425

   :robot: The Apache Camel test robot will run the tests for you :+1:


-- 
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: commits-unsubscribe@camel.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] CAMEL-19835: camel-core: replace temporary dir logic in test code [camel]

Posted by "orpiske (via GitHub)" <gi...@apache.org>.
orpiske merged PR #11934:
URL: https://github.com/apache/camel/pull/11934


-- 
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: commits-unsubscribe@camel.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] CAMEL-19835: camel-core: replace temporary dir logic in test code [camel]

Posted by "orpiske (via GitHub)" <gi...@apache.org>.
orpiske commented on code in PR #11934:
URL: https://github.com/apache/camel/pull/11934#discussion_r1386117008


##########
core/camel-core/src/test/java/org/apache/camel/component/file/FileConsumerFailureHandledTest.java:
##########
@@ -57,7 +57,7 @@ public void testLondon() throws Exception {
         mock.expectedBodiesReceived("London");
 
         template.sendBodyAndHeader(fileUri(), "London", Exchange.FILE_NAME, "london.txt");
-        mock.assertIsSatisfied(1000);
+        mock.assertIsSatisfied(3000);

Review Comment:
   Why the timeout increase?



##########
core/camel-core/src/test/java/org/apache/camel/component/file/FileConsumerFailureHandledTest.java:
##########
@@ -87,7 +87,7 @@ public void testMadrid() throws Exception {
         mock.expectedBodiesReceived("Madrid");
 
         template.sendBodyAndHeader(fileUri(), "Madrid", Exchange.FILE_NAME, "madrid.txt");
-        mock.assertIsSatisfied(1000);
+        mock.assertIsSatisfied(3000);

Review Comment:
   Same note about the timeout increase.



##########
core/camel-core/src/test/java/org/apache/camel/component/file/FileConsumerFailureHandledTest.java:
##########
@@ -72,7 +72,7 @@ public void testDublin() throws Exception {
         mock.expectedBodiesReceived("Dublin");
 
         template.sendBodyAndHeader(fileUri(), "Dublin", Exchange.FILE_NAME, "dublin.txt");
-        mock.assertIsSatisfied(1000);
+        mock.assertIsSatisfied(3000);

Review Comment:
   Same note about the timeout increase.



##########
core/camel-core/src/test/java/org/apache/camel/TestSupport.java:
##########
@@ -445,43 +422,6 @@ public static Channel unwrapChannel(Processor processor) {
         }
     }
 
-    /**
-     * Recursively delete a directory, useful to zapping test data
-     *
-     * @param file the directory to be deleted
-     */
-    public static void deleteDirectory(String file) {
-        deleteDirectory(new File(file));
-    }
-
-    /**
-     * Recursively delete a directory, useful to zapping test data
-     *
-     * @param file the directory to be deleted
-     */
-    public static void deleteDirectory(File file) {
-        if (file.isDirectory()) {
-            File[] files = file.listFiles();
-            if (files != null) {
-                for (File child : files) {
-                    deleteDirectory(child);
-                }
-            }
-        }
-
-        file.delete();
-    }
-
-    /**
-     * create the directory
-     *
-     * @param file the directory to be created
-     */
-    public static void createDirectory(String file) {
-        File dir = new File(file);
-        dir.mkdirs();
-    }
-

Review Comment:
   We probably need to document the removal of these methods on the upgrade documentation. They may be used elsewhere / by users.



-- 
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: commits-unsubscribe@camel.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] CAMEL-19835: camel-core: replace temporary dir logic in test code [camel]

Posted by "oscerd (via GitHub)" <gi...@apache.org>.
oscerd commented on PR #11934:
URL: https://github.com/apache/camel/pull/11934#issuecomment-1800255388

   It's something reserved to PMC and committer, so each time you push something we need to approve and run the pipeline again.


-- 
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: commits-unsubscribe@camel.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] CAMEL-19835: camel-core: replace temporary dir logic in test code [camel]

Posted by "orpiske (via GitHub)" <gi...@apache.org>.
orpiske commented on code in PR #11934:
URL: https://github.com/apache/camel/pull/11934#discussion_r1386122476


##########
core/camel-core/src/test/java/org/apache/camel/TestSupport.java:
##########
@@ -445,43 +422,6 @@ public static Channel unwrapChannel(Processor processor) {
         }
     }
 
-    /**
-     * Recursively delete a directory, useful to zapping test data
-     *
-     * @param file the directory to be deleted
-     */
-    public static void deleteDirectory(String file) {
-        deleteDirectory(new File(file));
-    }
-
-    /**
-     * Recursively delete a directory, useful to zapping test data
-     *
-     * @param file the directory to be deleted
-     */
-    public static void deleteDirectory(File file) {
-        if (file.isDirectory()) {
-            File[] files = file.listFiles();
-            if (files != null) {
-                for (File child : files) {
-                    deleteDirectory(child);
-                }
-            }
-        }
-
-        file.delete();
-    }
-
-    /**
-     * create the directory
-     *
-     * @param file the directory to be created
-     */
-    public static void createDirectory(String file) {
-        File dir = new File(file);
-        dir.mkdirs();
-    }
-

Review Comment:
   Alternatively: wrapping them around the new `tempDir` stuff and marking them as deprecated would be acceptable as well (and, also, adding a note about their deprecation on the upgrade doc).



-- 
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: commits-unsubscribe@camel.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] CAMEL-19835: camel-core: replace temporary dir logic in test code [camel]

Posted by "anton-goncharov (via GitHub)" <gi...@apache.org>.
anton-goncharov commented on code in PR #11934:
URL: https://github.com/apache/camel/pull/11934#discussion_r1394980990


##########
core/camel-core/src/test/java/org/apache/camel/TestSupport.java:
##########
@@ -445,43 +422,6 @@ public static Channel unwrapChannel(Processor processor) {
         }
     }
 
-    /**
-     * Recursively delete a directory, useful to zapping test data
-     *
-     * @param file the directory to be deleted
-     */
-    public static void deleteDirectory(String file) {
-        deleteDirectory(new File(file));
-    }
-
-    /**
-     * Recursively delete a directory, useful to zapping test data
-     *
-     * @param file the directory to be deleted
-     */
-    public static void deleteDirectory(File file) {
-        if (file.isDirectory()) {
-            File[] files = file.listFiles();
-            if (files != null) {
-                for (File child : files) {
-                    deleteDirectory(child);
-                }
-            }
-        }
-
-        file.delete();
-    }
-
-    /**
-     * create the directory
-     *
-     * @param file the directory to be created
-     */
-    public static void createDirectory(String file) {
-        File dir = new File(file);
-        dir.mkdirs();
-    }
-

Review Comment:
   I added the deleted methods back and annotated as `@Deprecated`. About the doc, please let me know if I'm incorrect but this class `org.apache.camel.TestSupport` is from the `test` directory and shouldn't be visible to the end users of `camel-core`. Should I update 4.2→4.3 upgrade doc anyway?



##########
core/camel-core/src/test/java/org/apache/camel/TestSupport.java:
##########
@@ -445,43 +422,6 @@ public static Channel unwrapChannel(Processor processor) {
         }
     }
 
-    /**
-     * Recursively delete a directory, useful to zapping test data
-     *
-     * @param file the directory to be deleted
-     */
-    public static void deleteDirectory(String file) {
-        deleteDirectory(new File(file));
-    }
-
-    /**
-     * Recursively delete a directory, useful to zapping test data
-     *
-     * @param file the directory to be deleted
-     */
-    public static void deleteDirectory(File file) {
-        if (file.isDirectory()) {
-            File[] files = file.listFiles();
-            if (files != null) {
-                for (File child : files) {
-                    deleteDirectory(child);
-                }
-            }
-        }
-
-        file.delete();
-    }
-
-    /**
-     * create the directory
-     *
-     * @param file the directory to be created
-     */
-    public static void createDirectory(String file) {
-        File dir = new File(file);
-        dir.mkdirs();
-    }
-

Review Comment:
   I added the deleted methods back and annotated as `@Deprecated`. About the doc, please let me know if I'm incorrect but this class `org.apache.camel.TestSupport` is from the `test` directory and shouldn't be visible to the end users of `camel-core`. Should I update 4.2→4.3 upgrade doc anyway?



-- 
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: commits-unsubscribe@camel.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] CAMEL-19835: camel-core: replace temporary dir logic in test code [camel]

Posted by "anton-goncharov (via GitHub)" <gi...@apache.org>.
anton-goncharov commented on code in PR #11934:
URL: https://github.com/apache/camel/pull/11934#discussion_r1394979333


##########
core/camel-core/src/test/java/org/apache/camel/component/file/FileConsumerFailureHandledTest.java:
##########
@@ -72,7 +72,7 @@ public void testDublin() throws Exception {
         mock.expectedBodiesReceived("Dublin");
 
         template.sendBodyAndHeader(fileUri(), "Dublin", Exchange.FILE_NAME, "dublin.txt");
-        mock.assertIsSatisfied(1000);
+        mock.assertIsSatisfied(3000);

Review Comment:
   Reverted back to 1000



##########
core/camel-core/src/test/java/org/apache/camel/component/file/FileConsumerFailureHandledTest.java:
##########
@@ -87,7 +87,7 @@ public void testMadrid() throws Exception {
         mock.expectedBodiesReceived("Madrid");
 
         template.sendBodyAndHeader(fileUri(), "Madrid", Exchange.FILE_NAME, "madrid.txt");
-        mock.assertIsSatisfied(1000);
+        mock.assertIsSatisfied(3000);

Review Comment:
   Reverted back to 1000



##########
core/camel-core/src/test/java/org/apache/camel/component/file/FileConsumerFailureHandledTest.java:
##########
@@ -57,7 +57,7 @@ public void testLondon() throws Exception {
         mock.expectedBodiesReceived("London");
 
         template.sendBodyAndHeader(fileUri(), "London", Exchange.FILE_NAME, "london.txt");
-        mock.assertIsSatisfied(1000);
+        mock.assertIsSatisfied(3000);

Review Comment:
   Reverted back to 1000



-- 
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: commits-unsubscribe@camel.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] CAMEL-19835: camel-core: replace temporary dir logic in test code [camel]

Posted by "orpiske (via GitHub)" <gi...@apache.org>.
orpiske commented on PR #11934:
URL: https://github.com/apache/camel/pull/11934#issuecomment-1814484997

   Thanks for your contribution @anton-goncharov !


-- 
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: commits-unsubscribe@camel.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] CAMEL-19835: camel-core: replace temporary dir logic in test code [camel]

Posted by "anton-goncharov (via GitHub)" <gi...@apache.org>.
anton-goncharov commented on PR #11934:
URL: https://github.com/apache/camel/pull/11934#issuecomment-1800159885

   /component-test camel-core camel-spring-xml


-- 
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: commits-unsubscribe@camel.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] CAMEL-19835: camel-core: replace temporary dir logic in test code [camel]

Posted by "anton-goncharov (via GitHub)" <gi...@apache.org>.
anton-goncharov commented on PR #11934:
URL: https://github.com/apache/camel/pull/11934#issuecomment-1800227570

   @oscerd I guess since it's my first contribution, it needs workflow approval each time. Can you please approve again?


-- 
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: commits-unsubscribe@camel.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] CAMEL-19835: camel-core: replace temporary dir logic in test code [camel]

Posted by "anton-goncharov (via GitHub)" <gi...@apache.org>.
anton-goncharov commented on code in PR #11934:
URL: https://github.com/apache/camel/pull/11934#discussion_r1386693104


##########
core/camel-core/src/test/java/org/apache/camel/TestSupport.java:
##########
@@ -445,43 +422,6 @@ public static Channel unwrapChannel(Processor processor) {
         }
     }
 
-    /**
-     * Recursively delete a directory, useful to zapping test data
-     *
-     * @param file the directory to be deleted
-     */
-    public static void deleteDirectory(String file) {
-        deleteDirectory(new File(file));
-    }
-
-    /**
-     * Recursively delete a directory, useful to zapping test data
-     *
-     * @param file the directory to be deleted
-     */
-    public static void deleteDirectory(File file) {
-        if (file.isDirectory()) {
-            File[] files = file.listFiles();
-            if (files != null) {
-                for (File child : files) {
-                    deleteDirectory(child);
-                }
-            }
-        }
-
-        file.delete();
-    }
-
-    /**
-     * create the directory
-     *
-     * @param file the directory to be created
-     */
-    public static void createDirectory(String file) {
-        File dir = new File(file);
-        dir.mkdirs();
-    }
-

Review Comment:
   That's a good point about not removing them and deprecating instead. I'll take this approach, and will also update the upgrade doc. Thanks!



-- 
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: commits-unsubscribe@camel.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] CAMEL-19835: camel-core: replace temporary dir logic in test code [camel]

Posted by "orpiske (via GitHub)" <gi...@apache.org>.
orpiske commented on code in PR #11934:
URL: https://github.com/apache/camel/pull/11934#discussion_r1395281329


##########
core/camel-core/src/test/java/org/apache/camel/TestSupport.java:
##########
@@ -445,43 +422,6 @@ public static Channel unwrapChannel(Processor processor) {
         }
     }
 
-    /**
-     * Recursively delete a directory, useful to zapping test data
-     *
-     * @param file the directory to be deleted
-     */
-    public static void deleteDirectory(String file) {
-        deleteDirectory(new File(file));
-    }
-
-    /**
-     * Recursively delete a directory, useful to zapping test data
-     *
-     * @param file the directory to be deleted
-     */
-    public static void deleteDirectory(File file) {
-        if (file.isDirectory()) {
-            File[] files = file.listFiles();
-            if (files != null) {
-                for (File child : files) {
-                    deleteDirectory(child);
-                }
-            }
-        }
-
-        file.delete();
-    }
-
-    /**
-     * create the directory
-     *
-     * @param file the directory to be created
-     */
-    public static void createDirectory(String file) {
-        File dir = new File(file);
-        dir.mkdirs();
-    }
-

Review Comment:
   Ahh, good point. In this case we don't have to update the docs.



-- 
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: commits-unsubscribe@camel.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] CAMEL-19835: camel-core: replace temporary dir logic in test code [camel]

Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #11934:
URL: https://github.com/apache/camel/pull/11934#issuecomment-1799400324

   :star2: Thank you for your contribution to the Apache Camel project! :star2: 
   
   :robot: CI automation will test this PR automatically.
   
   :camel: Apache Camel Committers, please review the following items:
   
   * First-time contributors **require MANUAL approval** for the GitHub Actions to run
   
   * You can use the command `/component-test (camel-)component-name1 (camel-)component-name2..` to request a test from the test bot.
   
   * You can label PRs using `build-all`, `build-dependents`, `skip-tests` and `test-dependents` to fine-tune the checks executed by this PR.
   
   * Build and test logs are available in the Summary page. **Only** [Apache Camel committers](https://camel.apache.org/community/team/#committers) have access to the summary. 
   
   * :warning: Be careful when sharing logs. Review their contents before sharing them publicly.


-- 
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: commits-unsubscribe@camel.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] CAMEL-19835: camel-core: replace temporary dir logic in test code [camel]

Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #11934:
URL: https://github.com/apache/camel/pull/11934#issuecomment-1799811958

   :robot: The Apache Camel test robot will run the tests for you :+1:


-- 
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: commits-unsubscribe@camel.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] CAMEL-19835: camel-core: replace temporary dir logic in test code [camel]

Posted by "anton-goncharov (via GitHub)" <gi...@apache.org>.
anton-goncharov commented on code in PR #11934:
URL: https://github.com/apache/camel/pull/11934#discussion_r1386766506


##########
core/camel-core/src/test/java/org/apache/camel/component/file/FileConsumerFailureHandledTest.java:
##########
@@ -57,7 +57,7 @@ public void testLondon() throws Exception {
         mock.expectedBodiesReceived("London");
 
         template.sendBodyAndHeader(fileUri(), "London", Exchange.FILE_NAME, "london.txt");
-        mock.assertIsSatisfied(1000);
+        mock.assertIsSatisfied(3000);

Review Comment:
   I couldn't find another explanation why this test has failed on a previous PR check run, as locally it passes when I run all camel-core tests. The same I see now but with other tests. After this change of timeout, this `FileConsumerFailureHandledTest` passes but two others fail: 
   - `org.apache.camel.processor.MulticastParallelTimeoutStreamCachingTest.testCreateOutputStreamCacheBeforeTimeoutButWriteToOutputStreamCacheAfterTimeout`
   - `org.apache.camel.processor.MulticastParallelStreamingTest.testMulticastParallel`
   
   Both don't use temp directory anyhow and shouldn't be affected by my changes, so I'm wondering how stable such tests are. I'm very new to it so I'm trying to understand if it's something that can happen ocassionally due to slower test execution, or if it's related to my PR changes. I apologize for my confusion as someone new, but really appreciate your guidance and open to suggestions 🙂



##########
core/camel-core/src/test/java/org/apache/camel/component/file/FileConsumerFailureHandledTest.java:
##########
@@ -57,7 +57,7 @@ public void testLondon() throws Exception {
         mock.expectedBodiesReceived("London");
 
         template.sendBodyAndHeader(fileUri(), "London", Exchange.FILE_NAME, "london.txt");
-        mock.assertIsSatisfied(1000);
+        mock.assertIsSatisfied(3000);

Review Comment:
   I couldn't find another explanation why this test has failed on a previous PR check run, as locally it passes when I run all camel-core tests. The same I see now but with other tests. After this change of timeout, this `FileConsumerFailureHandledTest` passes but two others fail: 
   - `org.apache.camel.processor.MulticastParallelTimeoutStreamCachingTest.testCreateOutputStreamCacheBeforeTimeoutButWriteToOutputStreamCacheAfterTimeout`
   - `org.apache.camel.processor.MulticastParallelStreamingTest.testMulticastParallel`
   
   Both don't use temp directory anyhow and shouldn't be affected by my changes, so I'm wondering how stable such tests are. I'm very new to it so I'm trying to understand if it's something that can happen ocassionally due to slower test execution, or if it's related to my PR changes. I apologize for my confusion as someone new, but really appreciate your guidance and open to suggestions 🙂



-- 
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: commits-unsubscribe@camel.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] CAMEL-19835: camel-core: replace temporary dir logic in test code [camel]

Posted by "anton-goncharov (via GitHub)" <gi...@apache.org>.
anton-goncharov commented on PR #11934:
URL: https://github.com/apache/camel/pull/11934#issuecomment-1799811362

   /component-test camel-core camel-spring-xml


-- 
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: commits-unsubscribe@camel.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] CAMEL-19835: camel-core: replace temporary dir logic in test code [camel]

Posted by "orpiske (via GitHub)" <gi...@apache.org>.
orpiske commented on code in PR #11934:
URL: https://github.com/apache/camel/pull/11934#discussion_r1387733384


##########
core/camel-core/src/test/java/org/apache/camel/component/file/FileConsumerFailureHandledTest.java:
##########
@@ -57,7 +57,7 @@ public void testLondon() throws Exception {
         mock.expectedBodiesReceived("London");
 
         template.sendBodyAndHeader(fileUri(), "London", Exchange.FILE_NAME, "london.txt");
-        mock.assertIsSatisfied(1000);
+        mock.assertIsSatisfied(3000);

Review Comment:
   So, the challenge here is that we have to balance two things: 
   
   1. Quick test execution 
   2. Stability 
   
   
   In some of the systems where these tests run, such as GitHub, the systems running them could be overloaded at the time of execution. Some tests use fixed sleeps and/or timeouts to wait for "something" to happen, but this process is not stable at all (hence, why we have been converting most of the sleeps with Awaitility). 
   
   I suspect that's why you see these new failures: maybe if you run them again, you'll see a different set of failures. 
   
   Normally, what we do is that we use the [ASF CI](https://ci-builds.apache.org/job/Camel/job/Camel%20JDK17/) as the baseline: as long as tests run fine there, we are OK with it (and we are a bit lenient with [most of] the core check failures on GitHub).
   
   So, in this case, I'd leave it as is. 



-- 
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: commits-unsubscribe@camel.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] CAMEL-19835: camel-core: replace temporary dir logic in test code [camel]

Posted by "orpiske (via GitHub)" <gi...@apache.org>.
orpiske commented on PR #11934:
URL: https://github.com/apache/camel/pull/11934#issuecomment-1805390826

   I think we won't have time to get this on in 4.2, but it's a good change. We might be able to add it to 4.3. 


-- 
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: commits-unsubscribe@camel.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org