From 12d17faa5ff4ca01e67b65a1bfc764be203725c1 Mon Sep 17 00:00:00 2001 From: Nicolas Filotto Date: Thu, 17 Oct 2024 12:46:37 +0200 Subject: [PATCH] Integration Test Framework: Improve failure management (#529) 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 ## 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 | 2 +- .../camel/itests/AbstractCamelRouteITest.java | 124 +++++++++++++--- .../camel/itests/CamelKarafTestHint.java | 15 ++ .../karaf/camel/itests/DumpFileOnError.java | 55 ++++++++ .../TestContainerFactoryFailureAware.java | 132 ++++++++++++++++++ .../org/apache/karaf/camel/itests/Utils.java | 24 ++++ tests/features/pom.xml | 2 + 7 files changed, 335 insertions(+), 19 deletions(-) create mode 100644 tests/camel-integration-test/src/main/java/org/apache/karaf/camel/itests/DumpFileOnError.java create mode 100644 tests/camel-integration-test/src/main/java/org/apache/karaf/camel/itests/TestContainerFactoryFailureAware.java 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..bd3cd8380 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,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 contexts = new ConcurrentHashMap<>(); private final Map templates = new ConcurrentHashMap<>(); + @Rule + public final DumpFileOnError dumpFileOnError; private List 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,6 +224,9 @@ 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() @@ -204,33 +234,85 @@ private static Option[] getExternalResourceOptions() { .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 getCamelKarafTestHint() { + return getCamelKarafTestHint(getClass()); + } + /** + * @return the {@link CamelKarafTestHint} annotation of the given test class + */ + private static Optional 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 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 getAllRequiredFeatures() { - CamelKarafTestHint hint = getClass().getAnnotation(CamelKarafTestHint.class); - if (hint == null || hint.additionalRequiredFeatures().length == 0) { - return getRequiredFeatures(); + if (hasAdditionalRequiredFeatures()) { + List requiredFeatures = new ArrayList<>(getRequiredFeatures()); + requiredFeatures.addAll(List.of(getCamelKarafTestHint().orElseThrow().additionalRequiredFeatures())); + return requiredFeatures; } - List 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 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")); } 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 @@ @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; } 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..fde04c61d --- /dev/null +++ b/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 + *

+ * http://www.apache.org/licenses/LICENSE-2.0 + *

+ * 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; + } + } + }; + } +} diff --git a/tests/camel-integration-test/src/main/java/org/apache/karaf/camel/itests/TestContainerFactoryFailureAware.java b/tests/camel-integration-test/src/main/java/org/apache/karaf/camel/itests/TestContainerFactoryFailureAware.java new file mode 100644 index 000000000..c926fb47b --- /dev/null +++ b/tests/camel-integration-test/src/main/java/org/apache/karaf/camel/itests/TestContainerFactoryFailureAware.java @@ -0,0 +1,132 @@ +/** + * 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 + *

+ * http://www.apache.org/licenses/LICENSE-2.0 + *

+ * 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.InputStream; +import java.util.LinkedList; +import java.util.List; +import java.util.Objects; +import java.util.Queue; + +import org.ops4j.pax.exam.ExamSystem; +import org.ops4j.pax.exam.TestAddress; +import org.ops4j.pax.exam.TestContainer; +import org.ops4j.pax.exam.TestContainerFactory; +import org.ops4j.pax.exam.spi.PaxExamRuntime; +import org.ops4j.pax.exam.util.PathUtils; + +import static org.apache.karaf.camel.itests.AbstractCamelRouteITest.CAMEL_KARAF_INTEGRATION_TEST_DUMP_LOGS_PROPERTY; + +public class TestContainerFactoryFailureAware implements TestContainerFactory { + + private final boolean enabled; + private final TestContainerFactory delegate; + + public TestContainerFactoryFailureAware() { + this.enabled = Boolean.getBoolean(CAMEL_KARAF_INTEGRATION_TEST_DUMP_LOGS_PROPERTY); + this.delegate = PaxExamRuntime.getTestContainerFactory(); + } + + @Override + public TestContainer[] create(ExamSystem system) { + TestContainer[] containers = delegate.create(system); + if (enabled) { + for (int i = 0; i < containers.length; i++) { + containers[i] = new TestContainerFailureAware(containers[i]); + } + } + return containers; + } + + private static class TestContainerFailureAware implements TestContainer { + + private final TestContainer delegate; + + private TestContainerFailureAware(TestContainer delegate) { + this.delegate = delegate; + } + + @Override + public TestContainer start() { + try { + return delegate.start(); + } catch (RuntimeException e) { + Utils.dumpFile(getKarafLogFile(), System.err); + throw e; + } + } + + @Override + public long install(InputStream stream) { + return delegate.install(stream); + } + + @Override + public long install(String location, InputStream stream) { + return delegate.install(location, stream); + } + + @Override + public long installProbe(InputStream stream) { + return delegate.installProbe(stream); + } + + @Override + public void uninstallProbe() { + delegate.uninstallProbe(); + } + + @Override + public void call(TestAddress address) { + delegate.call(address); + } + + @Override + public TestContainer stop() { + return delegate.stop(); + } + + private static File getKarafUnpackDirectory() { + return new File(PathUtils.getBaseDir(), "target/exam"); + } + + private static File getKarafLogFile() { + return new File(searchKarafBase(getKarafUnpackDirectory()), "data/log/karaf.log"); + } + + /** + * Since we might get quite deep use a simple breath first search algorithm + */ + private static File searchKarafBase(File root) { + Queue searchNext = new LinkedList<>(); + searchNext.add(root); + while (!searchNext.isEmpty()) { + File head = searchNext.poll(); + if (!head.isDirectory()) { + continue; + } + if (new File(head, "system").isDirectory() && new File(head, "etc").isDirectory()) { + return head; + } + searchNext.addAll(List.of(Objects.requireNonNull(head.listFiles()))); + } + return null; + } + } +} diff --git a/tests/camel-integration-test/src/main/java/org/apache/karaf/camel/itests/Utils.java b/tests/camel-integration-test/src/main/java/org/apache/karaf/camel/itests/Utils.java index 9d06ffe51..0c3594b22 100644 --- a/tests/camel-integration-test/src/main/java/org/apache/karaf/camel/itests/Utils.java +++ b/tests/camel-integration-test/src/main/java/org/apache/karaf/camel/itests/Utils.java @@ -16,8 +16,12 @@ */ package org.apache.karaf.camel.itests; +import java.io.File; +import java.io.IOException; import java.io.InputStream; import java.io.OutputStream; +import java.io.PrintStream; +import java.io.UncheckedIOException; import java.net.InetAddress; import java.net.InetSocketAddress; import java.net.ServerSocket; @@ -25,6 +29,7 @@ import java.nio.file.Path; import java.nio.file.Paths; import java.util.function.IntPredicate; +import java.util.stream.Stream; import org.jetbrains.annotations.NotNull; import org.ops4j.pax.exam.MavenUtils; @@ -119,4 +124,23 @@ private static String loadUsersFile(Path location) { throw new IllegalStateException("Can't find the users.properties file, please provide it using the system " + "property users.file.location"); } + + /** + * Dump the given file into the stream. + */ + static void dumpFile(File file, PrintStream out) { + if (file.exists()) { + out.println(">>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>"); + out.printf(">>>>> START Dumping file %s%n", file.getName()); + out.println(">>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>"); + try (Stream 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 @@ Apache Camel :: Karaf :: Tests :: Features + false ${project.basedir}/../../camel-integration-test/src/main/resources/etc/users.properties @@ -296,6 +297,7 @@ **/*Test.java + ${dump.logs.on.failure} ${project.version} ${project.version} ${project.build.directory}