Integration Test Framework: Improve failure management (apache#529)

Ref apache#526: Integration Test Framework: Allow to set a timeout
Ref apache#527: Integration Test Framework: Avoid retry by default
Ref apache#528: Integration Test Framework: Dump log file on failure

## Modifications:

* Added an element named `timeout` to be able to set the timeout of the integration test knowing that the default value is 5 minutes instead of the legacy value which was 1 hour
* Added an element named `retryOnFailure` to enable the auto-retry mechanism like before, knowing that the default value is now `false`. It should only be enabled for known flaky tests.
* Added a system property `camel.karaf.itest.dump.logs` to indicate whether the Karaf log file should be dumped on failure. By default, it is disabled locally and can be enabled by overriding the Maven property `dump.logs.on.failure`.

.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

tests/camel-integration-test/src/main/java/org/apache/karaf/camel/itests/AbstractCamelRouteITest.java

@@ -20,7 +20,9 @@
 import java.util.List;
 import java.util.Map;
 import java.util.Objects;
+import java.util.Optional;
 import java.util.concurrent.ConcurrentHashMap;
+import java.util.concurrent.TimeUnit;
 
 import org.apache.camel.CamelContext;
 import org.apache.camel.ProducerTemplate;
@@ -31,9 +33,12 @@
 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.ExamFactory;
 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,8 +49,10 @@
 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;
 
+@ExamFactory(TestContainerFactoryFailureAware.class)
 public abstract class AbstractCamelRouteITest extends KarafTestSupport implements CamelContextProvider {
 
     public static final int CAMEL_KARAF_INTEGRATION_TEST_DEBUG_DEFAULT_PORT = 8889;
@@ -56,12 +63,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(
+            getKarafLogFile(), System.err, Boolean.getBoolean(CAMEL_KARAF_INTEGRATION_TEST_DUMP_LOGS_PROPERTY)
+        );
+    }
+
     public String getCamelKarafVersion() {
         String version = System.getProperty("camel.karaf.version");
         if (version == null) {
@@ -105,6 +122,9 @@ public Option[] config() {
                     "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, "false")),
             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 +144,13 @@ public Option[] config() {
         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 +186,7 @@ private static int getContextFinderRetry() {
      * 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,40 +224,95 @@ private static Option[] updatePorts(Option[] options) {
         return options;
     }
 
+    /**
+     * @return the options provided by the external resources.
+     */
     @NotNull
     private static Option[] getExternalResourceOptions() {
         return PaxExamWithExternalResource.systemProperties().entrySet().stream()
                 .map(e -> CoreOptions.systemProperty(e.getKey()).value(e.getValue()))
                 .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 TimeUnit.SECONDS.toMillis(getCamelKarafTestHint().map(CamelKarafTestHint::timeout).orElse(DEFAULT_TIMEOUT));
+    }
+
+    /**
+     * 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 +334,10 @@ protected List<String> getRequiredFeaturesRepositories() {
         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 +354,18 @@ private void installRequiredFeaturesRepositories() throws Exception {
      * 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 +390,7 @@ protected List<String> installRequiredBundles() throws Exception {
      * @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 +469,7 @@ protected void assertBundleInstalledAndRunning(String name) {
         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"));
     }

tests/camel-integration-test/src/main/java/org/apache/karaf/camel/itests/CamelKarafTestHint.java

@@ -15,6 +15,11 @@
 @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 @@
      * 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;
 }

tests/camel-integration-test/src/main/java/org/apache/karaf/camel/itests/DumpFileOnError.java

@@ -0,0 +1,55 @@
+/**
+ * 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.PrintStream;
+
+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 enabled;
+
+    public DumpFileOnError(File file, PrintStream out, boolean enabled) {
+        this.file = file;
+        this.out = out;
+        this.enabled = enabled;
+    }
+
+    @Override
+    public Statement apply(Statement statement, Description description) {
+        return new Statement() {
+            @Override
+            public void evaluate() throws Throwable {
+                try {
+                    statement.evaluate();
+                } catch (Throwable t) {
+                    if (enabled) {
+                        Utils.dumpFile(file, out);
+                    }
+                    throw t;
+                }
+            }
+        };
+    }
+}