This is an automated email from the ASF dual-hosted git repository.

nfilotto pushed a commit to branch essobedo/error-management-improvements
in repository https://gitbox.apache.org/repos/asf/camel-karaf.git

commit 4282fdb90af4e61b4b2a820cd43f1c0f406ceecf
Author: Nicolas Filotto <[email protected]>
AuthorDate: Tue Oct 15 19:24:55 2024 +0200

    Ref #526: Integration Test Framework: Allow to set a timeout
    Ref #527: Integration Test Framework: Avoid retry by default
    Ref #528: Integration Test Framework: Dump log file on failure
---
 .github/workflows/main.yml                         |   2 +-
 .../camel/itests/AbstractCamelRouteITest.java      | 121 ++++++++++++++++++---
 .../karaf/camel/itests/CamelKarafTestHint.java     |  15 +++
 .../apache/karaf/camel/itests/DumpFileOnError.java |  79 ++++++++++++++
 tests/features/pom.xml                             |   2 +
 5 files changed, 200 insertions(+), 19 deletions(-)

diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml
index b924127f1..f6da17ca9 100644
--- a/.github/workflows/main.yml
+++ b/.github/workflows/main.yml
@@ -38,4 +38,4 @@ jobs:
         with:
           java-version: 17
           distribution: zulu
-      - run: ./mvnw -V --no-transfer-progress clean install
+      - run: ./mvnw -V --no-transfer-progress clean install 
-Ddump.logs.on.failure=true
diff --git 
a/tests/camel-integration-test/src/main/java/org/apache/karaf/camel/itests/AbstractCamelRouteITest.java
 
b/tests/camel-integration-test/src/main/java/org/apache/karaf/camel/itests/AbstractCamelRouteITest.java
index 72a6c69c3..f66307810 100644
--- 
a/tests/camel-integration-test/src/main/java/org/apache/karaf/camel/itests/AbstractCamelRouteITest.java
+++ 
b/tests/camel-integration-test/src/main/java/org/apache/karaf/camel/itests/AbstractCamelRouteITest.java
@@ -20,6 +20,7 @@ import java.util.ArrayList;
 import java.util.List;
 import java.util.Map;
 import java.util.Objects;
+import java.util.Optional;
 import java.util.concurrent.ConcurrentHashMap;
 
 import org.apache.camel.CamelContext;
@@ -31,9 +32,11 @@ import org.jetbrains.annotations.NotNull;
 import org.junit.After;
 import org.junit.Assert;
 import org.junit.Before;
+import org.junit.Rule;
 import org.ops4j.pax.exam.Configuration;
 import org.ops4j.pax.exam.CoreOptions;
 import org.ops4j.pax.exam.Option;
+import org.ops4j.pax.exam.container.remote.RBCRemoteTargetOptions;
 import 
org.ops4j.pax.exam.karaf.options.KarafDistributionConfigurationFilePutOption;
 import org.ops4j.pax.exam.karaf.options.KarafDistributionOption;
 import org.osgi.framework.Bundle;
@@ -44,6 +47,7 @@ import org.osgi.framework.ServiceReference;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
+import static org.apache.karaf.camel.itests.CamelKarafTestHint.DEFAULT_TIMEOUT;
 import static org.ops4j.pax.exam.OptionUtils.combine;
 
 public abstract class AbstractCamelRouteITest extends KarafTestSupport 
implements CamelContextProvider {
@@ -56,12 +60,22 @@ public abstract class AbstractCamelRouteITest extends 
KarafTestSupport implement
     static final String 
CAMEL_KARAF_INTEGRATION_TEST_CONTEXT_FINDER_RETRY_INTERVAL_PROPERTY = 
"camel.karaf.itest.context.finder.retry.interval";
     static final String CAMEL_KARAF_INTEGRATION_TEST_ROUTE_SUPPLIERS_PROPERTY 
= "camel.karaf.itest.route.suppliers";
     static final String 
CAMEL_KARAF_INTEGRATION_TEST_IGNORE_ROUTE_SUPPLIERS_PROPERTY = 
"camel.karaf.itest.ignore.route.suppliers";
+    static final String CAMEL_KARAF_INTEGRATION_TEST_DUMP_LOGS_PROPERTY = 
"camel.karaf.itest.dump.logs";
 
     private static final Logger LOG = 
LoggerFactory.getLogger(AbstractCamelRouteITest.class);
     private final Map<CamelContextKey, CamelContext> contexts = new 
ConcurrentHashMap<>();
     private final Map<CamelContextKey, ProducerTemplate> templates = new 
ConcurrentHashMap<>();
+    @Rule
+    public final DumpFileOnError dumpFileOnError;
     private List<String> requiredBundles;
 
+    protected AbstractCamelRouteITest() {
+        this.retry = new Retry(retryOnFailure());
+        this.dumpFileOnError = new DumpFileOnError(
+            System.err, getKarafLogFile(), 
Boolean.getBoolean(CAMEL_KARAF_INTEGRATION_TEST_DUMP_LOGS_PROPERTY)
+        );
+    }
+
     public String getCamelKarafVersion() {
         String version = System.getProperty("camel.karaf.version");
         if (version == null) {
@@ -105,6 +119,9 @@ public abstract class AbstractCamelRouteITest extends 
KarafTestSupport implement
                     "getCamelKarafVersion must be overridden to provide the 
version of Camel Karaf to use");
         }
         Option[] options = new Option[]{
+            CoreOptions.systemTimeout(timeoutInMillis()),
+            RBCRemoteTargetOptions.waitForRBCFor((int) timeoutInMillis()),
+            
CoreOptions.systemProperty(CAMEL_KARAF_INTEGRATION_TEST_DUMP_LOGS_PROPERTY).value(System.getProperty(CAMEL_KARAF_INTEGRATION_TEST_DUMP_LOGS_PROPERTY)),
             CoreOptions.systemProperty("project.target").value(getBaseDir()),
             
KarafDistributionOption.features("mvn:org.apache.camel.karaf/apache-camel/%s/xml/features".formatted(camelKarafVersion),
 "scr", getMode().getFeatureName()),
             
CoreOptions.mavenBundle().groupId("org.apache.camel.karaf").artifactId("camel-integration-test").version(camelKarafVersion)
@@ -124,6 +141,13 @@ public abstract class AbstractCamelRouteITest extends 
KarafTestSupport implement
         return combine(combine, getAdditionalOptions());
     }
 
+    /**
+     * @return the location of the Karaf log file.
+     */
+    private static @NotNull File getKarafLogFile() {
+        return new File(System.getProperty("karaf.log"), "karaf.log");
+    }
+
     /**
      * Indicates whether the debug mode is enabled or not. The debug mode is 
enabled when the system property
      * {@link #CAMEL_KARAF_INTEGRATION_TEST_DEBUG_PROPERTY} is set.
@@ -159,7 +183,7 @@ public abstract class AbstractCamelRouteITest extends 
KarafTestSupport implement
      * Returns the interval in seconds between each retry when trying to find 
a Camel context, corresponding to the value of the
      * system property {@link 
#CAMEL_KARAF_INTEGRATION_TEST_CONTEXT_FINDER_RETRY_INTERVAL_PROPERTY}. The 
default value is
      * {@link 
#CAMEL_KARAF_INTEGRATION_TEST_CONTEXT_FINDER_RETRY_INTERVAL_DEFAULT}.
-     * @return
+     * @return the interval in seconds between each retry when trying to find 
a Camel context
      */
     private static int getContextFinderRetryInterval() {
         return Integer.getInteger(
@@ -197,6 +221,9 @@ public abstract class AbstractCamelRouteITest extends 
KarafTestSupport implement
         return options;
     }
 
+    /**
+     * @return the options provided by the external resources.
+     */
     @NotNull
     private static Option[] getExternalResourceOptions() {
         return 
PaxExamWithExternalResource.systemProperties().entrySet().stream()
@@ -204,33 +231,85 @@ public abstract class AbstractCamelRouteITest extends 
KarafTestSupport implement
                 .toArray(Option[]::new);
     }
 
+    /**
+     * Returns the timeout in milliseconds for the test, corresponding to the 
value of the
+     * {@link CamelKarafTestHint#timeout()} annotation multiplied by one 
thousand.
+     * @return the timeout in milliseconds for the test
+     */
+    private long timeoutInMillis() {
+        return 
getCamelKarafTestHint().map(CamelKarafTestHint::timeout).orElse(DEFAULT_TIMEOUT)
 * 1_000L;
+    }
+
+    /**
+     * Indicates whether the test should be retried on failure. By default, no 
retry is performed.
+     * @return {@code true} if the test should be retried on failure, {@code 
false} otherwise
+     */
+    private boolean retryOnFailure() {
+        return 
getCamelKarafTestHint().filter(CamelKarafTestHint::retryOnFailure).isPresent();
+    }
+
+    /**
+     * Indicates whether the test requires external resources or not. By 
default, no external resources are required.
+     * @return {@code true} if the test requires external resources, {@code 
false} otherwise
+     */
     private boolean hasExternalResources() {
-        CamelKarafTestHint hint = 
getClass().getAnnotation(CamelKarafTestHint.class);
-        return hint != null && hint.externalResourceProvider() != Object.class;
+        return getCamelKarafTestHint().filter(hint -> 
hint.externalResourceProvider() != Object.class).isPresent();
     }
 
+    /**
+     * @return the {@link CamelKarafTestHint} annotation of the test class
+     */
+    private Optional<CamelKarafTestHint> getCamelKarafTestHint() {
+        return getCamelKarafTestHint(getClass());
+    }
 
+    /**
+     * @return the {@link CamelKarafTestHint} annotation of the given test 
class
+     */
+    private static Optional<CamelKarafTestHint> getCamelKarafTestHint(Class<?> 
clazz) {
+        return 
Optional.ofNullable(clazz.getAnnotation(CamelKarafTestHint.class));
+    }
+
+    /**
+     * @return the option to enable only the Camel route suppliers provided in 
the {@link CamelKarafTestHint} annotation.
+     */
     private Option getCamelRouteSupplierFilter() {
         return 
CoreOptions.systemProperty(CAMEL_KARAF_INTEGRATION_TEST_ROUTE_SUPPLIERS_PROPERTY)
-                .value(String.join(",", 
getClass().getAnnotation(CamelKarafTestHint.class).camelRouteSuppliers()));
+                .value(String.join(",", 
getCamelKarafTestHint().orElseThrow().camelRouteSuppliers()));
     }
 
+    /**
+     * Indicates whether specific Camel route suppliers have been provided in 
the {@link CamelKarafTestHint} annotation.
+     * @return {@code true} if specific Camel route suppliers have been 
provided, {@code false} otherwise
+     */
     private boolean hasCamelRouteSupplierFilter() {
-        CamelKarafTestHint hint = 
getClass().getAnnotation(CamelKarafTestHint.class);
-        return hint != null && hint.camelRouteSuppliers().length > 0;
+        return getCamelKarafTestHint().filter(hint -> 
hint.camelRouteSuppliers().length > 0).isPresent();
     }
 
+    /**
+     * Indicates whether all Camel route suppliers should be ignored or not. 
By default, all Camel route suppliers are used.
+     * @return {@code true} if all Camel route suppliers should be ignored, 
{@code false} otherwise
+     */
     private boolean ignoreCamelRouteSuppliers() {
-        CamelKarafTestHint hint = 
getClass().getAnnotation(CamelKarafTestHint.class);
-        return hint != null && hint.ignoreRouteSuppliers();
+        return 
getCamelKarafTestHint().filter(CamelKarafTestHint::ignoreRouteSuppliers).isPresent();
     }
 
+    /**
+     * Indicates whether additional required features have been provided in 
the {@link CamelKarafTestHint} annotation.
+     * @return {@code true} if additional required features have been 
provided, {@code false} otherwise
+     */
+    private boolean hasAdditionalRequiredFeatures() {
+        return getCamelKarafTestHint().filter(hint -> 
hint.additionalRequiredFeatures().length > 0).isPresent();
+    }
+
+    /**
+     * @return the option to ignore all Camel route suppliers.
+     */
     private Option getIgnoreCamelRouteSupplier() {
         return 
CoreOptions.systemProperty(CAMEL_KARAF_INTEGRATION_TEST_IGNORE_ROUTE_SUPPLIERS_PROPERTY)
                 .value(Boolean.toString(Boolean.TRUE));
     }
 
-
     /**
      * Returns the list of additional options to add to the configuration.
      */
@@ -252,6 +331,10 @@ public abstract class AbstractCamelRouteITest extends 
KarafTestSupport implement
         return List.of();
     }
 
+    /**
+     * Install all the required features repositories.
+     * @throws Exception if an error occurs while installing a features 
repository
+     */
     private void installRequiredFeaturesRepositories() throws Exception {
         for (String featuresRepository : getRequiredFeaturesRepositories()) {
             addFeaturesRepository(featuresRepository);
@@ -268,15 +351,18 @@ public abstract class AbstractCamelRouteITest extends 
KarafTestSupport implement
      * the {@link CamelKarafTestHint#additionalRequiredFeatures()}.
      */
     private List<String> getAllRequiredFeatures() {
-        CamelKarafTestHint hint = 
getClass().getAnnotation(CamelKarafTestHint.class);
-        if (hint == null || hint.additionalRequiredFeatures().length == 0) {
-            return getRequiredFeatures();
+        if (hasAdditionalRequiredFeatures()) {
+            List<String> requiredFeatures = new 
ArrayList<>(getRequiredFeatures());
+            
requiredFeatures.addAll(List.of(getCamelKarafTestHint().orElseThrow().additionalRequiredFeatures()));
+            return requiredFeatures;
         }
-        List<String> requiredFeatures = new ArrayList<>(getRequiredFeatures());
-        requiredFeatures.addAll(List.of(hint.additionalRequiredFeatures()));
-        return requiredFeatures;
+        return getRequiredFeatures();
     }
 
+    /**
+     * Installs the required features for the test.
+     * @throws Exception if an error occurs while installing a feature
+     */
     private void installRequiredFeatures() throws Exception {
         for (String featureName : getAllRequiredFeatures()) {
             if (featureService.getFeature(featureName) == null) {
@@ -301,8 +387,7 @@ public abstract class AbstractCamelRouteITest extends 
KarafTestSupport implement
      * @return {@code true} if the test is a blueprint test, {@code false} 
otherwise
      */
     private static boolean isBlueprintTest(Class<?> clazz) {
-        CamelKarafTestHint hint = 
clazz.getAnnotation(CamelKarafTestHint.class);
-        return hint != null && hint.isBlueprintTest();
+        return 
getCamelKarafTestHint(clazz).filter(CamelKarafTestHint::isBlueprintTest).isPresent();
     }
 
     private static Mode getMode(Class<?> clazz) {
@@ -381,7 +466,7 @@ public abstract class AbstractCamelRouteITest extends 
KarafTestSupport implement
         Assert.assertEquals(Bundle.ACTIVE, bundle.getState());
         //need to check with the command because the status may be Active 
while it's displayed as Waiting in the console
         //because of an exception for instance
-        String bundles = executeCommand("bundle:list -s -t 0 | grep 
%s".formatted(name));
+        String bundles = executeCommand("bundle:list -s -t 0 | grep 
%s".formatted(name), timeoutInMillis(), false);
         Assert.assertTrue("bundle %s is in state %d 
/%s".formatted(bundle.getSymbolicName(), bundle.getState(), bundles),
                 bundles.contains("Active"));
     }
diff --git 
a/tests/camel-integration-test/src/main/java/org/apache/karaf/camel/itests/CamelKarafTestHint.java
 
b/tests/camel-integration-test/src/main/java/org/apache/karaf/camel/itests/CamelKarafTestHint.java
index b7c79a595..858146974 100644
--- 
a/tests/camel-integration-test/src/main/java/org/apache/karaf/camel/itests/CamelKarafTestHint.java
+++ 
b/tests/camel-integration-test/src/main/java/org/apache/karaf/camel/itests/CamelKarafTestHint.java
@@ -15,6 +15,11 @@ import java.util.List;
 @Inherited
 public @interface CamelKarafTestHint {
 
+    /**
+     * The default timeout in seconds for the test.
+     */
+    int DEFAULT_TIMEOUT = 300;
+
     /**
      * Specify the class that provides the methods to create all the external 
resources required by the test.
      * In the provider class, each public static method that returns an 
instance of a subtype of {@link ExternalResource}
@@ -52,4 +57,14 @@ public @interface CamelKarafTestHint {
      * Forces to ignore all Camel route suppliers within the context of the 
tests. False by default.
      */
     boolean ignoreRouteSuppliers() default false;
+
+    /**
+     * Specify whether the test should be retried on failure. By default, no 
retry is performed.
+     */
+    boolean retryOnFailure() default false;
+
+    /**
+     * Specify the timeout in seconds for the test. By default, the timeout is 
300 seconds.
+     */
+    int timeout() default DEFAULT_TIMEOUT;
 }
diff --git 
a/tests/camel-integration-test/src/main/java/org/apache/karaf/camel/itests/DumpFileOnError.java
 
b/tests/camel-integration-test/src/main/java/org/apache/karaf/camel/itests/DumpFileOnError.java
new file mode 100644
index 000000000..ffa7c1050
--- /dev/null
+++ 
b/tests/camel-integration-test/src/main/java/org/apache/karaf/camel/itests/DumpFileOnError.java
@@ -0,0 +1,79 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ * <p>
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * <p>
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.karaf.camel.itests;
+
+import java.io.File;
+import java.io.IOException;
+import java.io.PrintStream;
+import java.io.UncheckedIOException;
+import java.nio.file.Files;
+import java.util.function.Supplier;
+import java.util.stream.Stream;
+
+import org.junit.rules.TestRule;
+import org.junit.runner.Description;
+import org.junit.runners.model.Statement;
+
+public class DumpFileOnError implements TestRule {
+
+    private final PrintStream out;
+    private final File file;
+    private final boolean dump;
+
+    public DumpFileOnError(PrintStream out, File file, boolean dump) {
+        this.out = out;
+        this.file = file;
+        this.dump = dump;
+    }
+
+    @Override
+    public Statement apply(Statement statement, Description description) {
+        return new Statement() {
+            @Override
+            public void evaluate() throws Throwable {
+                try {
+                    statement.evaluate();
+                } catch (Throwable t) {
+                    if (dump) {
+                        dumpFile();
+                    }
+                    throw t;
+                }
+            }
+        };
+    }
+
+    /**
+     * Dump the file.
+     */
+    private void dumpFile() {
+        if (file.exists()) {
+            out.println(">>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>");
+            out.printf(">>>>> START Dumping file %s%n", file.getName());
+            out.println(">>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>");
+            try (Stream<String> lines = Files.lines(file.toPath())) {
+                lines.forEach(out::println);
+            } catch (IOException e) {
+                throw new UncheckedIOException(e);
+            }
+            out.println("<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<");
+            out.printf("<<<<< END Dumping file %s%n", file.getName());
+            out.println("<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<");
+        }
+    }
+}
diff --git a/tests/features/pom.xml b/tests/features/pom.xml
index 307832aee..f00f671a5 100644
--- a/tests/features/pom.xml
+++ b/tests/features/pom.xml
@@ -33,6 +33,7 @@
     <name>Apache Camel :: Karaf :: Tests :: Features</name>
 
     <properties>
+        <dump.logs.on.failure>false</dump.logs.on.failure>
         
<users.file.location>${project.basedir}/../../camel-integration-test/src/main/resources/etc/users.properties</users.file.location>
     </properties>
 
@@ -296,6 +297,7 @@
                                 <include>**/*Test.java</include>
                             </includes>
                             <systemPropertyVariables>
+                                
<camel.karaf.itest.dump.logs>${dump.logs.on.failure}</camel.karaf.itest.dump.logs>
                                 
<camel.karaf.version>${project.version}</camel.karaf.version>
                                 
<project.version>${project.version}</project.version>
                                 
<project.target>${project.build.directory}</project.target>

Reply via email to