From 3c2684071de5e33162cba0b9c4a030d22f6b25c6 Mon Sep 17 00:00:00 2001 From: Sasha Sheikin Date: Sun, 10 Nov 2024 23:55:07 +0100 Subject: [PATCH 1/9] Bump airbase --- pom.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pom.xml b/pom.xml index 077d37d5..43c01ec7 100644 --- a/pom.xml +++ b/pom.xml @@ -5,7 +5,7 @@ io.airlift airbase - 157 + 187 io.trino.tempto From 0118fdec80847a716f39ac2d6437d36669e682c2 Mon Sep 17 00:00:00 2001 From: Sasha Sheikin Date: Sun, 10 Nov 2024 22:20:58 +0100 Subject: [PATCH 2/9] Copy from TestNG --- .../TestInitializationListenerJUnit.java | 423 ++++++++++++++++++ .../ProgressLoggingListenerJUnit.java | 147 ++++++ 2 files changed, 570 insertions(+) create mode 100644 tempto-core/src/main/java/io/trino/tempto/internal/initialization/TestInitializationListenerJUnit.java create mode 100644 tempto-core/src/main/java/io/trino/tempto/internal/listeners/ProgressLoggingListenerJUnit.java diff --git a/tempto-core/src/main/java/io/trino/tempto/internal/initialization/TestInitializationListenerJUnit.java b/tempto-core/src/main/java/io/trino/tempto/internal/initialization/TestInitializationListenerJUnit.java new file mode 100644 index 00000000..decce3a8 --- /dev/null +++ b/tempto-core/src/main/java/io/trino/tempto/internal/initialization/TestInitializationListenerJUnit.java @@ -0,0 +1,423 @@ +/* + * Licensed 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 io.trino.tempto.internal.initialization; + +import com.google.common.collect.ImmutableList; +import com.google.common.collect.Ordering; +import com.google.inject.Binder; +import com.google.inject.Module; +import com.google.inject.Singleton; +import io.trino.tempto.AfterMethodWithContext; +import io.trino.tempto.BeforeMethodWithContext; +import io.trino.tempto.Requirement; +import io.trino.tempto.TemptoPlugin; +import io.trino.tempto.configuration.Configuration; +import io.trino.tempto.context.TestContext; +import io.trino.tempto.fulfillment.RequirementFulfiller; +import io.trino.tempto.fulfillment.RequirementFulfiller.SuiteLevelFulfiller; +import io.trino.tempto.fulfillment.RequirementFulfiller.TestLevelFulfiller; +import io.trino.tempto.fulfillment.TestStatus; +import io.trino.tempto.fulfillment.table.TableManager; +import io.trino.tempto.fulfillment.table.TableManagerDispatcher; +import io.trino.tempto.initialization.SuiteModuleProvider; +import io.trino.tempto.initialization.TestMethodModuleProvider; +import io.trino.tempto.internal.ReflectionHelper; +import io.trino.tempto.internal.ReflectionInjectorHelper; +import io.trino.tempto.internal.TestSpecificRequirementsResolver; +import io.trino.tempto.internal.context.GuiceTestContext; +import io.trino.tempto.internal.context.TestContextStack; +import org.slf4j.Logger; +import org.testng.ITestContext; +import org.testng.ITestListener; +import org.testng.ITestNGMethod; +import org.testng.ITestResult; + +import java.lang.annotation.Annotation; +import java.lang.reflect.InvocationTargetException; +import java.lang.reflect.Method; +import java.util.ArrayList; +import java.util.Comparator; +import java.util.HashSet; +import java.util.List; +import java.util.Optional; +import java.util.ServiceLoader; +import java.util.Set; +import java.util.concurrent.atomic.AtomicLong; +import java.util.function.Function; +import java.util.stream.Stream; + +import static com.google.common.base.Preconditions.checkState; +import static com.google.common.collect.ImmutableList.toImmutableList; +import static com.google.common.collect.Lists.reverse; +import static com.google.inject.util.Modules.combine; +import static io.trino.tempto.context.TestContextDsl.runWithTestContext; +import static io.trino.tempto.context.ThreadLocalTestContextHolder.assertTestContextNotSet; +import static io.trino.tempto.context.ThreadLocalTestContextHolder.popAllTestContexts; +import static io.trino.tempto.context.ThreadLocalTestContextHolder.pushAllTestContexts; +import static io.trino.tempto.context.ThreadLocalTestContextHolder.testContext; +import static io.trino.tempto.context.ThreadLocalTestContextHolder.testContextIfSet; +import static io.trino.tempto.fulfillment.TestStatus.FAILURE; +import static io.trino.tempto.fulfillment.TestStatus.SUCCESS; +import static io.trino.tempto.internal.configuration.TestConfigurationFactory.testConfiguration; +import static io.trino.tempto.internal.logging.LoggingMdcHelper.cleanLoggingMdc; +import static io.trino.tempto.internal.logging.LoggingMdcHelper.setupLoggingMdcForTest; +import static java.util.Collections.emptyList; +import static java.util.Comparator.comparing; +import static org.slf4j.LoggerFactory.getLogger; + +public class TestInitializationListenerJUnit + implements ITestListener +{ + private static final Logger LOGGER = getLogger(TestInitializationListenerJUnit.class); + + private final List suiteModuleProviders; + private final List testMethodModuleProviders; + private final List> suiteLevelFulfillers; + private final List> testMethodLevelFulfillers; + private final ReflectionInjectorHelper reflectionInjectorHelper = new ReflectionInjectorHelper(); + + private final Configuration configuration; + private Optional> suiteTestContextStack = Optional.empty(); + + public TestInitializationListenerJUnit() + { + this(ImmutableList.copyOf(ServiceLoader.load(TemptoPlugin.class).iterator())); + } + + private TestInitializationListenerJUnit(List plugins) + { + this( + plugins.stream() + .flatMap(plugin -> plugin.getSuiteModules().stream()) + .map(ReflectionHelper::instantiate) + .collect(toImmutableList()), + plugins.stream() + .flatMap(plugin -> plugin.getTestModules().stream()) + .map(ReflectionHelper::instantiate) + .collect(toImmutableList()), + plugins.stream() + .flatMap(plugin -> plugin.getFulfillers().stream()) + .filter(clazz -> clazz.isAnnotationPresent(SuiteLevelFulfiller.class)) + .collect(toImmutableList()), + plugins.stream() + .flatMap(plugin -> plugin.getFulfillers().stream()) + .filter(clazz -> clazz.isAnnotationPresent(TestLevelFulfiller.class)) + .collect(toImmutableList()), + testConfiguration()); + } + + TestInitializationListenerJUnit( + List suiteModuleProviders, + List testMethodModuleProviders, + List> suiteLevelFulfillers, + List> testMethodLevelFulfillers, + Configuration configuration) + { + this.suiteModuleProviders = suiteModuleProviders; + this.testMethodModuleProviders = testMethodModuleProviders; + this.suiteLevelFulfillers = suiteLevelFulfillers; + this.testMethodLevelFulfillers = testMethodLevelFulfillers; + this.configuration = configuration; + } + + @Override + public void onStart(ITestContext context) + { + displayConfigurationToUser(); + + Module suiteModule = combine(combine(getSuiteModules()), bind(suiteLevelFulfillers), bind(testMethodLevelFulfillers)); + GuiceTestContext initSuiteTestContext = new GuiceTestContext(suiteModule); + TestContextStack suiteTextContextStack = new TestContextStack<>(); + suiteTextContextStack.push(initSuiteTestContext); + + try { + Set allTestsRequirements = resolveAllTestsRequirements(context); + doFulfillment(suiteTextContextStack, suiteLevelFulfillers, allTestsRequirements); + } + catch (RuntimeException e) { + LOGGER.error("cannot initialize test suite", e); + throw e; + } + + setSuiteTestContextStack(suiteTextContextStack); + } + + private void displayConfigurationToUser() + { + LOGGER.info("Configuration:"); + List configurationKeys = Ordering.natural() + .sortedCopy(configuration.listKeys()); + for (String key : configurationKeys) { + LOGGER.info(String.format("%s -> %s", key, configuration.getString(key).orElse(""))); + } + } + + @Override + public void onFinish(ITestContext context) + { + if (!suiteTestContextStack.isPresent()) { + return; + } + + TestStatus testStatus = context.getFailedTests().size() > 0 ? FAILURE : SUCCESS; + doCleanup(suiteTestContextStack.get(), suiteLevelFulfillers, testStatus); + } + + @Override + public void onTestStart(ITestResult testResult) + { + setupLoggingMdcForTest(testResult); + if (!suiteTestContextStack.isPresent()) { + throw new SuiteInitializationException("test suite not initialized"); + } + GuiceTestContext initTestContext = ((GuiceTestContext) suiteTestContextStack.get().peek()).createChildContext(emptyList(), getTestModules(testResult)); + TestContextStack testContextStack = new TestContextStack<>(); + testContextStack.push(initTestContext); + + try { + Set testSpecificRequirements = getTestSpecificRequirements(testResult.getMethod()); + doFulfillment(testContextStack, testMethodLevelFulfillers, testSpecificRequirements); + } + catch (RuntimeException e) { + LOGGER.debug("error within test initialization", e); + throw e; + } + + assertTestContextNotSet(); + pushAllTestContexts(testContextStack); + TestContext topTestContext = testContextStack.peek(); + topTestContext.injectMembers(testResult.getInstance()); + + runBeforeWithContextMethods(testResult, topTestContext); + } + + @Override + public void onTestSuccess(ITestResult result) + { + onTestFinished(result, SUCCESS); + } + + @Override + public void onTestFailure(ITestResult result) + { + LOGGER.debug("test failure", result.getThrowable()); + onTestFinished(result, FAILURE); + } + + @Override + public void onTestSkipped(ITestResult result) + { + onTestFinished(result, SUCCESS); + } + + private void onTestFinished(ITestResult testResult, TestStatus testStatus) + { + if (!testContextIfSet().isPresent()) { + return; + } + + boolean runAfterSucceeded = false; + try { + runAfterWithContextMethods(testResult, testContext()); + runAfterSucceeded = true; + } + finally { + TestContextStack testContextStack = popAllTestContexts(); + doCleanup(testContextStack, testMethodLevelFulfillers, runAfterSucceeded ? testStatus : FAILURE); + cleanLoggingMdc(); + } + } + + private void runBeforeWithContextMethods(ITestResult testResult, TestContext testContext) + { + try { + invokeMethodsAnnotatedWith(BeforeMethodWithContext.class, testResult, testContext, ClassOrdering.SUPER_FIRST); + } + catch (RuntimeException e) { + TestContextStack testContextStack = popAllTestContexts(); + doCleanup(testContextStack, testMethodLevelFulfillers, FAILURE); + throw e; + } + } + + private void runAfterWithContextMethods(ITestResult testResult, TestContext testContext) + { + invokeMethodsAnnotatedWith(AfterMethodWithContext.class, testResult, testContext, ClassOrdering.SUPER_LAST); + } + + private void invokeMethodsAnnotatedWith(Class annotationClass, ITestResult testCase, TestContext testContext, ClassOrdering ordering) + { + Comparator> depthComparator = comparing(clazz -> { + int depth = 0; + while (clazz.getSuperclass() != null) { + clazz = clazz.getSuperclass(); + depth++; + } + return depth; + }); + Comparator> classComparator; + switch (ordering) { + case SUPER_FIRST: + classComparator = depthComparator; + break; + case SUPER_LAST: + classComparator = depthComparator.reversed(); + break; + default: + throw new UnsupportedOperationException("Unsupported ordering: " + ordering); + } + + Stream.of(testCase.getTestClass().getRealClass().getMethods()) + .filter(declaredMethod -> declaredMethod.isAnnotationPresent(annotationClass)) + .sorted(comparing(Method::getDeclaringClass, classComparator)) + .forEachOrdered(declaredMethod -> { + try { + declaredMethod.invoke(testCase.getInstance(), reflectionInjectorHelper.getMethodArguments(testContext, declaredMethod)); + } + catch (IllegalAccessException | InvocationTargetException e) { + throw new RuntimeException("error invoking method annotated with " + annotationClass.getName(), e); + } + }); + } + + private void doFulfillment(TestContextStack testContextStack, + List> fulfillerClasses, + Set requirements) + { + List> successfulFulfillerClasses = new ArrayList<>(); + + try { + for (Class fulfillerClass : fulfillerClasses) { + LOGGER.debug("Fulfilling using {}", fulfillerClass); + TestContext testContext = testContextStack.peek(); + runWithTestContext(testContext, () -> { + RequirementFulfiller fulfiller = testContext.getDependency(fulfillerClass); + TestContext testContextWithNewStates = testContext.createChildContext(fulfiller.fulfill(requirements)); + successfulFulfillerClasses.add(fulfillerClass); + testContextStack.push(testContextWithNewStates); + }); + } + } + catch (RuntimeException e) { + LOGGER.debug("error during fulfillment", e); + try { + doCleanup(testContextStack, successfulFulfillerClasses, FAILURE); + } + catch (RuntimeException cleanupException) { + e.addSuppressed(cleanupException); + } + throw e; + } + } + + private void doCleanup(TestContextStack testContextStack, List> fulfillerClasses, TestStatus testStatus) + { + // one base test context plus one test context for each fulfiller + checkState(testContextStack.size() == fulfillerClasses.size() + 1); + + for (Class fulfillerClass : reverse(fulfillerClasses)) { + LOGGER.debug("Cleaning for fulfiller {}", fulfillerClass); + TestContext testContext = testContextStack.pop(); + testContext.close(); + runWithTestContext(testContext, () -> testContextStack.peek().getDependency(fulfillerClass).cleanup(testStatus)); + } + + if (testContextStack.size() == 1) { + // we are going to close last context, so we need to close TableManager's first + testContextStack.peek().getOptionalDependency(TableManagerDispatcher.class) + .ifPresent(dispatcher -> dispatcher.getAllTableManagers().forEach(TableManager::close)); + } + + // remove close init test context too + testContextStack.peek().close(); + } + + private List getSuiteModules() + { + return suiteModuleProviders + .stream() + .map(provider -> provider.getModule(configuration)) + .collect(toImmutableList()); + } + + private List getTestModules(ITestResult testResult) + { + return testMethodModuleProviders + .stream() + .map(provider -> provider.getModule(configuration, testResult)) + .collect(toImmutableList()); + } + + private Module bind(List> classes) + { + Function, Module> bindToModule = clazz -> (Binder binder) -> binder.bind(clazz).in(Singleton.class); + List modules = classes.stream() + .map(bindToModule) + .collect(toImmutableList()); + return combine(modules); + } + + private Set resolveAllTestsRequirements(ITestContext context) + { + // we cannot assume that context contains RequirementsAwareTestNGMethod instances here + // as interceptor is for some reason called after onStart() which uses this method. + Set allTestsRequirements = new HashSet<>(); + for (ITestNGMethod iTestNGMethod : context.getAllTestMethods()) { + Set> requirementsSets = new TestSpecificRequirementsResolver(configuration).resolve(iTestNGMethod); + for (Set requirementsSet : requirementsSets) { + allTestsRequirements.addAll(requirementsSet); + } + } + return allTestsRequirements; + } + + private Set getTestSpecificRequirements(ITestNGMethod testMethod) + { + return ((RequirementsAwareTestNGMethod) testMethod).getRequirements(); + } + + private void setSuiteTestContextStack(TestContextStack suiteTestContextStack) + { + checkState(!this.suiteTestContextStack.isPresent(), "suite fulfillment result already set"); + this.suiteTestContextStack = Optional.of(suiteTestContextStack); + } + + @Override + public void onTestFailedButWithinSuccessPercentage(ITestResult result) + { + } + + private static class SuiteInitializationException + extends RuntimeException + { + private static final AtomicLong instanceCount = new AtomicLong(); + + SuiteInitializationException(String message) + { + super( + message, + null, + true, + // Suppress stacktrace for all but first 10 exceptions. It is not useful when printed for every test. + instanceCount.getAndIncrement() < 10); + } + } + + private enum ClassOrdering + { + SUPER_FIRST, + SUPER_LAST, + } +} diff --git a/tempto-core/src/main/java/io/trino/tempto/internal/listeners/ProgressLoggingListenerJUnit.java b/tempto-core/src/main/java/io/trino/tempto/internal/listeners/ProgressLoggingListenerJUnit.java new file mode 100644 index 00000000..96824d6c --- /dev/null +++ b/tempto-core/src/main/java/io/trino/tempto/internal/listeners/ProgressLoggingListenerJUnit.java @@ -0,0 +1,147 @@ +/* + * Licensed 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 io.trino.tempto.internal.listeners; + +import com.google.common.base.Joiner; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; +import org.testng.ITestContext; +import org.testng.ITestListener; +import org.testng.ITestResult; + +import java.math.BigDecimal; +import java.math.RoundingMode; + +import static com.google.common.base.Preconditions.checkState; +import static io.trino.tempto.internal.initialization.RequirementsExpanderInterceptor.getMethodsCountFromContext; +import static java.lang.String.format; +import static java.lang.System.currentTimeMillis; + +public class ProgressLoggingListenerJUnit + implements ITestListener +{ + private final static Logger LOGGER = LoggerFactory.getLogger(ProgressLoggingListenerJUnit.class); + + private int started; + private int succeeded; + private int skipped; + private int failed; + private long startTime; + private long testStartTime; + + private final TestMetadataReader testMetadataReader; + + public ProgressLoggingListenerJUnit() + { + this.testMetadataReader = new TestMetadataReader(); + } + + @Override + public void onStart(ITestContext context) + { + startTime = currentTimeMillis(); + LOGGER.info("Starting tests"); + } + + @Override + public void onTestStart(ITestResult testCase) + { + testStartTime = currentTimeMillis(); + started++; + int total = getMethodsCountFromContext(testCase.getTestContext()); + LOGGER.info("[{} of {}] {}", started, total, formatTestName(testCase)); + } + + @Override + public void onTestSuccess(ITestResult testCase) + { + succeeded++; + logTestEnd(testCase, "SUCCESS"); + } + + @Override + public void onTestFailure(ITestResult testCase) + { + failed++; + logTestEnd(testCase, "FAILURE"); + if (testCase.getThrowable() != null) { + LOGGER.error("Failure cause:", testCase.getThrowable()); + } + } + + @Override + public void onTestSkipped(ITestResult testCase) + { + skipped++; + LOGGER.info("SKIPPED"); + } + + private void logTestEnd(ITestResult testCase, String outcome) + { + long executionTime = currentTimeMillis() - testStartTime; + LOGGER.info("{} / {} took {}", outcome, formatTestName(testCase), formatDuration(executionTime)); + } + + @Override + public void onTestFailedButWithinSuccessPercentage(ITestResult testCase) + { + } + + @Override + public void onFinish(ITestContext context) + { + checkState(succeeded + failed + skipped > 0, "No tests executed"); + LOGGER.info(""); + LOGGER.info("Completed {} tests", started); + LOGGER.info("{} SUCCEEDED / {} FAILED / {} SKIPPED", succeeded, failed, skipped); + LOGGER.info("Tests execution took {}", formatDuration(currentTimeMillis() - startTime)); + } + + private String formatTestName(ITestResult testCase) + { + TestMetadata testMetadata = testMetadataReader.readTestMetadata(testCase); + String testGroups = Joiner.on(", ").join(testMetadata.testGroups); + String testParameters = formatTestParameters(testMetadata.testParameters); + + return format("%s%s (Groups: %s)", testMetadata.testName, testParameters, testGroups); + } + + private String formatTestParameters(Object[] testParameters) + { + if (testParameters.length == 0) { + return ""; + } + + return format(" [%s]", Joiner.on(", ").useForNull("null").join(testParameters)); + } + + private static String formatDuration(long durationInMillis) + { + BigDecimal durationSeconds = durationInSeconds(durationInMillis); + if (durationSeconds.longValue() > 60) { + long minutes = durationSeconds.longValue() / 60; + long restSeconds = durationSeconds.longValue() % 60; + return String.format("%d minutes and %d seconds", minutes, restSeconds); + } + else { + return String.format("%s seconds", durationSeconds); + } + } + + private static BigDecimal durationInSeconds(long millis) + { + return new BigDecimal(millis).divide(new BigDecimal(1000), 1, RoundingMode.HALF_UP); + } +} From cea570fe8d3503977e20bf6d9a214f9807b7498e Mon Sep 17 00:00:00 2001 From: Sasha Sheikin Date: Sun, 10 Nov 2024 22:29:10 +0100 Subject: [PATCH 3/9] Run JUnit from intellij Requiremets and method requirements are disabled for all tests for now. --- tempto-core/pom.xml | 11 +- .../java/io/trino/tempto/ContextAware.java | 39 ++++ .../TestInitializationListenerJUnit.java | 209 +++++++----------- .../ProgressLoggingListenerJUnit.java | 96 +++----- 4 files changed, 148 insertions(+), 207 deletions(-) create mode 100644 tempto-core/src/main/java/io/trino/tempto/ContextAware.java diff --git a/tempto-core/pom.xml b/tempto-core/pom.xml index a08f90c6..94d40abf 100644 --- a/tempto-core/pom.xml +++ b/tempto-core/pom.xml @@ -128,6 +128,11 @@ freemarker + + org.junit.jupiter + junit-jupiter-api + + org.slf4j slf4j-api @@ -194,12 +199,6 @@ test - - org.junit.jupiter - junit-jupiter-api - test - - org.junit.jupiter junit-jupiter-engine diff --git a/tempto-core/src/main/java/io/trino/tempto/ContextAware.java b/tempto-core/src/main/java/io/trino/tempto/ContextAware.java new file mode 100644 index 00000000..8c8379a4 --- /dev/null +++ b/tempto-core/src/main/java/io/trino/tempto/ContextAware.java @@ -0,0 +1,39 @@ +/* + * Licensed 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 io.trino.tempto; + +import io.trino.tempto.internal.initialization.TestInitializationListenerJUnit; +import io.trino.tempto.internal.listeners.ProgressLoggingListenerJUnit; +import org.junit.jupiter.api.TestInstance; +import org.junit.jupiter.api.extension.ExtendWith; +import org.junit.jupiter.api.parallel.Execution; + +import java.lang.annotation.ElementType; +import java.lang.annotation.Retention; +import java.lang.annotation.RetentionPolicy; +import java.lang.annotation.Target; + +import static org.junit.jupiter.api.TestInstance.Lifecycle.PER_CLASS; +import static org.junit.jupiter.api.parallel.ExecutionMode.CONCURRENT; + +@Retention(RetentionPolicy.RUNTIME) +@Target({ElementType.TYPE}) +@ExtendWith({ + TestInitializationListenerJUnit.class, + ProgressLoggingListenerJUnit.class +}) +@TestInstance(PER_CLASS) +@Execution(CONCURRENT) +public @interface ContextAware {} diff --git a/tempto-core/src/main/java/io/trino/tempto/internal/initialization/TestInitializationListenerJUnit.java b/tempto-core/src/main/java/io/trino/tempto/internal/initialization/TestInitializationListenerJUnit.java index decce3a8..34a51ee1 100644 --- a/tempto-core/src/main/java/io/trino/tempto/internal/initialization/TestInitializationListenerJUnit.java +++ b/tempto-core/src/main/java/io/trino/tempto/internal/initialization/TestInitializationListenerJUnit.java @@ -15,12 +15,11 @@ package io.trino.tempto.internal.initialization; import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableSet; import com.google.common.collect.Ordering; import com.google.inject.Binder; import com.google.inject.Module; import com.google.inject.Singleton; -import io.trino.tempto.AfterMethodWithContext; -import io.trino.tempto.BeforeMethodWithContext; import io.trino.tempto.Requirement; import io.trino.tempto.TemptoPlugin; import io.trino.tempto.configuration.Configuration; @@ -34,29 +33,24 @@ import io.trino.tempto.initialization.SuiteModuleProvider; import io.trino.tempto.initialization.TestMethodModuleProvider; import io.trino.tempto.internal.ReflectionHelper; -import io.trino.tempto.internal.ReflectionInjectorHelper; -import io.trino.tempto.internal.TestSpecificRequirementsResolver; import io.trino.tempto.internal.context.GuiceTestContext; import io.trino.tempto.internal.context.TestContextStack; +import org.junit.jupiter.api.extension.AfterAllCallback; +import org.junit.jupiter.api.extension.AfterTestExecutionCallback; +import org.junit.jupiter.api.extension.BeforeAllCallback; +import org.junit.jupiter.api.extension.BeforeEachCallback; +import org.junit.jupiter.api.extension.ExtensionContext; +import org.junit.jupiter.api.extension.TestExecutionExceptionHandler; import org.slf4j.Logger; -import org.testng.ITestContext; -import org.testng.ITestListener; -import org.testng.ITestNGMethod; import org.testng.ITestResult; -import java.lang.annotation.Annotation; -import java.lang.reflect.InvocationTargetException; -import java.lang.reflect.Method; import java.util.ArrayList; -import java.util.Comparator; -import java.util.HashSet; import java.util.List; import java.util.Optional; import java.util.ServiceLoader; import java.util.Set; import java.util.concurrent.atomic.AtomicLong; import java.util.function.Function; -import java.util.stream.Stream; import static com.google.common.base.Preconditions.checkState; import static com.google.common.collect.ImmutableList.toImmutableList; @@ -66,19 +60,21 @@ import static io.trino.tempto.context.ThreadLocalTestContextHolder.assertTestContextNotSet; import static io.trino.tempto.context.ThreadLocalTestContextHolder.popAllTestContexts; import static io.trino.tempto.context.ThreadLocalTestContextHolder.pushAllTestContexts; -import static io.trino.tempto.context.ThreadLocalTestContextHolder.testContext; import static io.trino.tempto.context.ThreadLocalTestContextHolder.testContextIfSet; import static io.trino.tempto.fulfillment.TestStatus.FAILURE; import static io.trino.tempto.fulfillment.TestStatus.SUCCESS; import static io.trino.tempto.internal.configuration.TestConfigurationFactory.testConfiguration; import static io.trino.tempto.internal.logging.LoggingMdcHelper.cleanLoggingMdc; -import static io.trino.tempto.internal.logging.LoggingMdcHelper.setupLoggingMdcForTest; import static java.util.Collections.emptyList; -import static java.util.Comparator.comparing; import static org.slf4j.LoggerFactory.getLogger; public class TestInitializationListenerJUnit - implements ITestListener + implements + BeforeAllCallback, + BeforeEachCallback, + AfterTestExecutionCallback, + AfterAllCallback, + TestExecutionExceptionHandler { private static final Logger LOGGER = getLogger(TestInitializationListenerJUnit.class); @@ -86,7 +82,6 @@ public class TestInitializationListenerJUnit private final List testMethodModuleProviders; private final List> suiteLevelFulfillers; private final List> testMethodLevelFulfillers; - private final ReflectionInjectorHelper reflectionInjectorHelper = new ReflectionInjectorHelper(); private final Configuration configuration; private Optional> suiteTestContextStack = Optional.empty(); @@ -132,8 +127,9 @@ private TestInitializationListenerJUnit(List plugins) this.configuration = configuration; } + @Override - public void onStart(ITestContext context) + public void beforeAll(ExtensionContext context) { displayConfigurationToUser(); @@ -143,8 +139,8 @@ public void onStart(ITestContext context) suiteTextContextStack.push(initSuiteTestContext); try { - Set allTestsRequirements = resolveAllTestsRequirements(context); - doFulfillment(suiteTextContextStack, suiteLevelFulfillers, allTestsRequirements); +// Set allTestsRequirements = resolveAllTestsRequirements(testInstance, context); + doFulfillment(suiteTextContextStack, suiteLevelFulfillers, ImmutableSet.of()); } catch (RuntimeException e) { LOGGER.error("cannot initialize test suite", e); @@ -164,31 +160,36 @@ private void displayConfigurationToUser() } } + @Override - public void onFinish(ITestContext context) + public void afterAll(ExtensionContext context) { if (!suiteTestContextStack.isPresent()) { return; } - TestStatus testStatus = context.getFailedTests().size() > 0 ? FAILURE : SUCCESS; + TestStatus testStatus = context.getExecutionException().isPresent() ? FAILURE : SUCCESS; doCleanup(suiteTestContextStack.get(), suiteLevelFulfillers, testStatus); } @Override - public void onTestStart(ITestResult testResult) + public void beforeEach(ExtensionContext context) + throws Exception { - setupLoggingMdcForTest(testResult); +// setupLoggingMdcForTest(testResult); if (!suiteTestContextStack.isPresent()) { throw new SuiteInitializationException("test suite not initialized"); } - GuiceTestContext initTestContext = ((GuiceTestContext) suiteTestContextStack.get().peek()).createChildContext(emptyList(), getTestModules(testResult)); + GuiceTestContext initTestContext = ((GuiceTestContext) suiteTestContextStack.get().peek()).createChildContext(emptyList(), +// getTestModules(testResult) + emptyList() + ); TestContextStack testContextStack = new TestContextStack<>(); testContextStack.push(initTestContext); try { - Set testSpecificRequirements = getTestSpecificRequirements(testResult.getMethod()); - doFulfillment(testContextStack, testMethodLevelFulfillers, testSpecificRequirements); +// Set testSpecificRequirements = getTestSpecificRequirements(context.getRequiredTestInstance()); + doFulfillment(testContextStack, testMethodLevelFulfillers, ImmutableSet.of()); } catch (RuntimeException e) { LOGGER.debug("error within test initialization", e); @@ -198,98 +199,45 @@ public void onTestStart(ITestResult testResult) assertTestContextNotSet(); pushAllTestContexts(testContextStack); TestContext topTestContext = testContextStack.peek(); - topTestContext.injectMembers(testResult.getInstance()); - - runBeforeWithContextMethods(testResult, topTestContext); + topTestContext.injectMembers(context.getRequiredTestInstance()); } @Override - public void onTestSuccess(ITestResult result) + public void afterTestExecution(ExtensionContext context) { - onTestFinished(result, SUCCESS); - } - - @Override - public void onTestFailure(ITestResult result) - { - LOGGER.debug("test failure", result.getThrowable()); - onTestFinished(result, FAILURE); + if (context.getExecutionException().isEmpty()) { + onTestFinished(SUCCESS); + } + else { + onTestFinished(FAILURE); + } } @Override - public void onTestSkipped(ITestResult result) + public void handleTestExecutionException(ExtensionContext context, Throwable throwable) + throws Throwable { - onTestFinished(result, SUCCESS); + LOGGER.debug("test failure", throwable); + onTestFinished(FAILURE); + throw throwable; } - private void onTestFinished(ITestResult testResult, TestStatus testStatus) + private void onTestFinished(TestStatus testStatus) { if (!testContextIfSet().isPresent()) { return; } - boolean runAfterSucceeded = false; - try { - runAfterWithContextMethods(testResult, testContext()); - runAfterSucceeded = true; - } - finally { - TestContextStack testContextStack = popAllTestContexts(); - doCleanup(testContextStack, testMethodLevelFulfillers, runAfterSucceeded ? testStatus : FAILURE); - cleanLoggingMdc(); - } - } - - private void runBeforeWithContextMethods(ITestResult testResult, TestContext testContext) - { - try { - invokeMethodsAnnotatedWith(BeforeMethodWithContext.class, testResult, testContext, ClassOrdering.SUPER_FIRST); - } - catch (RuntimeException e) { - TestContextStack testContextStack = popAllTestContexts(); - doCleanup(testContextStack, testMethodLevelFulfillers, FAILURE); - throw e; - } - } - - private void runAfterWithContextMethods(ITestResult testResult, TestContext testContext) - { - invokeMethodsAnnotatedWith(AfterMethodWithContext.class, testResult, testContext, ClassOrdering.SUPER_LAST); - } - - private void invokeMethodsAnnotatedWith(Class annotationClass, ITestResult testCase, TestContext testContext, ClassOrdering ordering) - { - Comparator> depthComparator = comparing(clazz -> { - int depth = 0; - while (clazz.getSuperclass() != null) { - clazz = clazz.getSuperclass(); - depth++; - } - return depth; - }); - Comparator> classComparator; - switch (ordering) { - case SUPER_FIRST: - classComparator = depthComparator; - break; - case SUPER_LAST: - classComparator = depthComparator.reversed(); - break; - default: - throw new UnsupportedOperationException("Unsupported ordering: " + ordering); - } - - Stream.of(testCase.getTestClass().getRealClass().getMethods()) - .filter(declaredMethod -> declaredMethod.isAnnotationPresent(annotationClass)) - .sorted(comparing(Method::getDeclaringClass, classComparator)) - .forEachOrdered(declaredMethod -> { - try { - declaredMethod.invoke(testCase.getInstance(), reflectionInjectorHelper.getMethodArguments(testContext, declaredMethod)); - } - catch (IllegalAccessException | InvocationTargetException e) { - throw new RuntimeException("error invoking method annotated with " + annotationClass.getName(), e); - } - }); +// boolean runAfterSucceeded = false; +// try { +// runAfterWithContextMethods(testResult, testContext()); +// runAfterSucceeded = true; +// } +// finally { + TestContextStack testContextStack = popAllTestContexts(); + doCleanup(testContextStack, testMethodLevelFulfillers, testStatus); + cleanLoggingMdc(); +// } } private void doFulfillment(TestContextStack testContextStack, @@ -369,24 +317,28 @@ private Module bind(List> classes) return combine(modules); } - private Set resolveAllTestsRequirements(ITestContext context) - { - // we cannot assume that context contains RequirementsAwareTestNGMethod instances here - // as interceptor is for some reason called after onStart() which uses this method. - Set allTestsRequirements = new HashSet<>(); - for (ITestNGMethod iTestNGMethod : context.getAllTestMethods()) { - Set> requirementsSets = new TestSpecificRequirementsResolver(configuration).resolve(iTestNGMethod); - for (Set requirementsSet : requirementsSets) { - allTestsRequirements.addAll(requirementsSet); - } - } - return allTestsRequirements; - } - - private Set getTestSpecificRequirements(ITestNGMethod testMethod) - { - return ((RequirementsAwareTestNGMethod) testMethod).getRequirements(); - } +// private Set resolveAllTestsRequirements(Object testInstance, ExtensionContext context) +// { +// // we cannot assume that context contains RequirementsAwareTestNGMethod instances here +// // as interceptor is for some reason called after onStart() which uses this method. +// Set allTestsRequirements = new HashSet<>(); +// List methods = Arrays.stream(context.getTestClass().get().getDeclaredMethods()) +// .filter(method -> method.isAnnotationPresent(Test.class)) +// .toList(); +// for (Method method : methods) { +// ITestNGMethod iTestNGMethod = null; +// Set> requirementsSets = new TestSpecificRequirementsResolver(configuration).resolve(method, testInstance); +// for (Set requirementsSet : requirementsSets) { +// allTestsRequirements.addAll(requirementsSet); +// } +// } +// return allTestsRequirements; +// } +// +// private Set getTestSpecificRequirements(ITestNGMethod testMethod) +// { +// return ((RequirementsAwareTestNGMethod) testMethod).getRequirements(); +// } private void setSuiteTestContextStack(TestContextStack suiteTestContextStack) { @@ -394,11 +346,6 @@ private void setSuiteTestContextStack(TestContextStack suiteTestCon this.suiteTestContextStack = Optional.of(suiteTestContextStack); } - @Override - public void onTestFailedButWithinSuccessPercentage(ITestResult result) - { - } - private static class SuiteInitializationException extends RuntimeException { @@ -414,10 +361,4 @@ private static class SuiteInitializationException instanceCount.getAndIncrement() < 10); } } - - private enum ClassOrdering - { - SUPER_FIRST, - SUPER_LAST, - } } diff --git a/tempto-core/src/main/java/io/trino/tempto/internal/listeners/ProgressLoggingListenerJUnit.java b/tempto-core/src/main/java/io/trino/tempto/internal/listeners/ProgressLoggingListenerJUnit.java index 96824d6c..3685cfc2 100644 --- a/tempto-core/src/main/java/io/trino/tempto/internal/listeners/ProgressLoggingListenerJUnit.java +++ b/tempto-core/src/main/java/io/trino/tempto/internal/listeners/ProgressLoggingListenerJUnit.java @@ -14,117 +14,79 @@ package io.trino.tempto.internal.listeners; -import com.google.common.base.Joiner; +import org.junit.jupiter.api.extension.AfterAllCallback; +import org.junit.jupiter.api.extension.AfterEachCallback; +import org.junit.jupiter.api.extension.BeforeAllCallback; +import org.junit.jupiter.api.extension.BeforeEachCallback; +import org.junit.jupiter.api.extension.ExtensionContext; import org.slf4j.Logger; import org.slf4j.LoggerFactory; -import org.testng.ITestContext; -import org.testng.ITestListener; -import org.testng.ITestResult; import java.math.BigDecimal; import java.math.RoundingMode; import static com.google.common.base.Preconditions.checkState; -import static io.trino.tempto.internal.initialization.RequirementsExpanderInterceptor.getMethodsCountFromContext; -import static java.lang.String.format; import static java.lang.System.currentTimeMillis; public class ProgressLoggingListenerJUnit - implements ITestListener + implements BeforeAllCallback, BeforeEachCallback, AfterEachCallback, AfterAllCallback { private final static Logger LOGGER = LoggerFactory.getLogger(ProgressLoggingListenerJUnit.class); private int started; private int succeeded; - private int skipped; private int failed; private long startTime; private long testStartTime; - private final TestMetadataReader testMetadataReader; - - public ProgressLoggingListenerJUnit() - { - this.testMetadataReader = new TestMetadataReader(); - } - @Override - public void onStart(ITestContext context) + public void beforeAll(ExtensionContext context) { startTime = currentTimeMillis(); LOGGER.info("Starting tests"); } @Override - public void onTestStart(ITestResult testCase) + public void beforeEach(ExtensionContext context) { testStartTime = currentTimeMillis(); started++; - int total = getMethodsCountFromContext(testCase.getTestContext()); - LOGGER.info("[{} of {}] {}", started, total, formatTestName(testCase)); - } - - @Override - public void onTestSuccess(ITestResult testCase) - { - succeeded++; - logTestEnd(testCase, "SUCCESS"); + LOGGER.info("[{}] Starting test: {}", started, formatTestName(context)); } @Override - public void onTestFailure(ITestResult testCase) - { - failed++; - logTestEnd(testCase, "FAILURE"); - if (testCase.getThrowable() != null) { - LOGGER.error("Failure cause:", testCase.getThrowable()); - } - } - - @Override - public void onTestSkipped(ITestResult testCase) - { - skipped++; - LOGGER.info("SKIPPED"); - } - - private void logTestEnd(ITestResult testCase, String outcome) + public void afterEach(ExtensionContext context) { long executionTime = currentTimeMillis() - testStartTime; - LOGGER.info("{} / {} took {}", outcome, formatTestName(testCase), formatDuration(executionTime)); - } - - @Override - public void onTestFailedButWithinSuccessPercentage(ITestResult testCase) - { + if (context.getExecutionException().isPresent()) { + failed++; + LOGGER.info("FAILURE: {} took {}", formatTestName(context), formatDuration(executionTime)); + LOGGER.error("Failure cause:", context.getExecutionException().get()); + } + else { + succeeded++; + LOGGER.info("SUCCESS: {} took {}", formatTestName(context), formatDuration(executionTime)); + } } @Override - public void onFinish(ITestContext context) + public void afterAll(ExtensionContext context) { - checkState(succeeded + failed + skipped > 0, "No tests executed"); + checkState(succeeded + failed > 0, "No tests executed"); LOGGER.info(""); LOGGER.info("Completed {} tests", started); - LOGGER.info("{} SUCCEEDED / {} FAILED / {} SKIPPED", succeeded, failed, skipped); + LOGGER.info("{} SUCCEEDED / {} FAILED", succeeded, failed); LOGGER.info("Tests execution took {}", formatDuration(currentTimeMillis() - startTime)); } - private String formatTestName(ITestResult testCase) + private String formatTestName(ExtensionContext context) { - TestMetadata testMetadata = testMetadataReader.readTestMetadata(testCase); - String testGroups = Joiner.on(", ").join(testMetadata.testGroups); - String testParameters = formatTestParameters(testMetadata.testParameters); - - return format("%s%s (Groups: %s)", testMetadata.testName, testParameters, testGroups); - } - - private String formatTestParameters(Object[] testParameters) - { - if (testParameters.length == 0) { - return ""; - } - - return format(" [%s]", Joiner.on(", ").useForNull("null").join(testParameters)); +// TestMetadata testMetadata = testMetadataReader.readTestMetadata(testCase); +// String testGroups = Joiner.on(", ").join(testMetadata.testGroups); +// String testParameters = formatTestParameters(testMetadata.testParameters); +// +// return format("%s%s (Groups: %s)", testMetadata.testName, testParameters, testGroups); + return "%s#%s".formatted(context.getRequiredTestClass().getName(), context.getDisplayName()); } private static String formatDuration(long durationInMillis) From e94866740adc47a260fcb9b5191d5f58d018bd7f Mon Sep 17 00:00:00 2001 From: Sasha Sheikin Date: Sun, 10 Nov 2024 20:49:21 +0100 Subject: [PATCH 4/9] Run JUnit with TemptoRunner --- tempto-runner/pom.xml | 10 ++++ .../io/trino/tempto/runner/TemptoRunner.java | 48 +++++++++++++++++-- 2 files changed, 55 insertions(+), 3 deletions(-) diff --git a/tempto-runner/pom.xml b/tempto-runner/pom.xml index 91af176d..fcd2ff7b 100644 --- a/tempto-runner/pom.xml +++ b/tempto-runner/pom.xml @@ -38,6 +38,16 @@ commons-lang3 + + org.junit.platform + junit-platform-engine + + + + org.junit.platform + junit-platform-launcher + + org.slf4j slf4j-api diff --git a/tempto-runner/src/main/java/io/trino/tempto/runner/TemptoRunner.java b/tempto-runner/src/main/java/io/trino/tempto/runner/TemptoRunner.java index 28a30c05..d4ab122d 100644 --- a/tempto-runner/src/main/java/io/trino/tempto/runner/TemptoRunner.java +++ b/tempto-runner/src/main/java/io/trino/tempto/runner/TemptoRunner.java @@ -15,7 +15,15 @@ package io.trino.tempto.runner; import com.google.common.base.Joiner; +import com.google.common.collect.ImmutableList; import io.trino.tempto.internal.listeners.TestNameGroupNameMethodSelector; +import org.junit.platform.engine.discovery.ClassNameFilter; +import org.junit.platform.engine.discovery.DiscoverySelectors; +import org.junit.platform.launcher.LauncherDiscoveryRequest; +import org.junit.platform.launcher.TagFilter; +import org.junit.platform.launcher.core.LauncherDiscoveryRequestBuilder; +import org.junit.platform.launcher.core.LauncherFactory; +import org.junit.platform.launcher.listeners.SummaryGeneratingListener; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.testng.TestNG; @@ -36,6 +44,7 @@ import static io.trino.tempto.internal.listeners.TestNameGroupNameMethodSelector.TEST_NAMES_TO_EXCLUDE_PROPERTY; import static io.trino.tempto.internal.listeners.TestNameGroupNameMethodSelector.TEST_NAMES_TO_RUN_PROPERTY; import static java.util.Collections.singletonList; +import static org.junit.platform.launcher.TagFilter.includeTags; import static org.testng.xml.XmlSuite.ParallelMode.getValidParallel; public class TemptoRunner @@ -78,10 +87,17 @@ private void run() parser.printHelpMessage(); return; } + setupTestsConfiguration(); + if (!testWithJUnit() || !testWithTestNg()) { + // tempto-runner is a CLI tool. It has to fail when there are test failures. That way CI step will be marked as failed. + System.exit(1); + } + } + private boolean testWithTestNg() + { XmlSuite testSuite = getXmlSuite(); testSuite.setThreadCount(options.getThreadCount()); - setupTestsConfiguration(); System.setProperty(CONVENTION_TESTS_DIR_KEY, options.getConventionTestsDirectory()); TestNG testNG = new TestNG(); testNG.setXmlSuites(singletonList(testSuite)); @@ -91,9 +107,35 @@ private void run() options.getConventionResultsDumpPath() .ifPresent(path -> System.setProperty(CONVENTION_TESTS_RESULTS_DUMP_PATH_KEY, path)); testNG.run(); - if (testNG.hasFailure()) { - System.exit(1); + return !testNG.hasFailure(); + } + + private boolean testWithJUnit() + { + // TODO how set setOutputDirectory in Junit 5? +// testNG.setOutputDirectory(options.getReportDir()); + LauncherDiscoveryRequestBuilder requestBuilder = LauncherDiscoveryRequestBuilder.request(); + options.getTestsPackage().stream() + .map(x -> x.replace(".*", "")) // TODO remove. Temporary to comply with TestNG: package pattern + .map(DiscoverySelectors::selectPackage).forEach(requestBuilder::selectors); + options.getTests().stream().map(DiscoverySelectors::selectClass).forEach(requestBuilder::selectors); + options.getExcludedTests().stream().map(ClassNameFilter::excludeClassNamePatterns).forEach(requestBuilder::filters); + if (!options.getTestGroups().isEmpty()) { + requestBuilder.filters(includeTags(ImmutableList.copyOf(options.getTestGroups()))); } + options.getExcludeGroups().stream().map(TagFilter::excludeTags).forEach(requestBuilder::filters); + + LauncherDiscoveryRequest request = requestBuilder +// TODO ConventionBasedTestFactory is still testng +// .selectors(selectClass("io.trino.tempto.internal.convention.ConventionBasedTestFactory")) + .configurationParameter("junit.jupiter.execution.parallel.enabled", "true") + .configurationParameter("junit.jupiter.execution.parallel.config.fixed.parallelism", Integer.toString(options.getThreadCount())) + .build(); + + // Configure the Launcher and listener + SummaryGeneratingListener listener = new SummaryGeneratingListener(); + LauncherFactory.create().execute(request, listener); + return listener.getSummary().getTestsFailedCount() == 0; } private void setupTestsConfiguration() From 93e6f9eb770ee77c0881415b6353b09ae1339eb8 Mon Sep 17 00:00:00 2001 From: Sasha Sheikin Date: Mon, 11 Nov 2024 12:59:11 +0100 Subject: [PATCH 5/9] Remove testMethodLevelFulfillers --- .../TestInitializationListenerJUnit.java | 15 +++------------ 1 file changed, 3 insertions(+), 12 deletions(-) diff --git a/tempto-core/src/main/java/io/trino/tempto/internal/initialization/TestInitializationListenerJUnit.java b/tempto-core/src/main/java/io/trino/tempto/internal/initialization/TestInitializationListenerJUnit.java index 34a51ee1..9d5c70df 100644 --- a/tempto-core/src/main/java/io/trino/tempto/internal/initialization/TestInitializationListenerJUnit.java +++ b/tempto-core/src/main/java/io/trino/tempto/internal/initialization/TestInitializationListenerJUnit.java @@ -26,7 +26,6 @@ import io.trino.tempto.context.TestContext; import io.trino.tempto.fulfillment.RequirementFulfiller; import io.trino.tempto.fulfillment.RequirementFulfiller.SuiteLevelFulfiller; -import io.trino.tempto.fulfillment.RequirementFulfiller.TestLevelFulfiller; import io.trino.tempto.fulfillment.TestStatus; import io.trino.tempto.fulfillment.table.TableManager; import io.trino.tempto.fulfillment.table.TableManagerDispatcher; @@ -81,7 +80,6 @@ public class TestInitializationListenerJUnit private final List suiteModuleProviders; private final List testMethodModuleProviders; private final List> suiteLevelFulfillers; - private final List> testMethodLevelFulfillers; private final Configuration configuration; private Optional> suiteTestContextStack = Optional.empty(); @@ -106,10 +104,6 @@ private TestInitializationListenerJUnit(List plugins) .flatMap(plugin -> plugin.getFulfillers().stream()) .filter(clazz -> clazz.isAnnotationPresent(SuiteLevelFulfiller.class)) .collect(toImmutableList()), - plugins.stream() - .flatMap(plugin -> plugin.getFulfillers().stream()) - .filter(clazz -> clazz.isAnnotationPresent(TestLevelFulfiller.class)) - .collect(toImmutableList()), testConfiguration()); } @@ -117,13 +111,11 @@ private TestInitializationListenerJUnit(List plugins) List suiteModuleProviders, List testMethodModuleProviders, List> suiteLevelFulfillers, - List> testMethodLevelFulfillers, Configuration configuration) { this.suiteModuleProviders = suiteModuleProviders; this.testMethodModuleProviders = testMethodModuleProviders; this.suiteLevelFulfillers = suiteLevelFulfillers; - this.testMethodLevelFulfillers = testMethodLevelFulfillers; this.configuration = configuration; } @@ -133,7 +125,7 @@ public void beforeAll(ExtensionContext context) { displayConfigurationToUser(); - Module suiteModule = combine(combine(getSuiteModules()), bind(suiteLevelFulfillers), bind(testMethodLevelFulfillers)); + Module suiteModule = combine(combine(getSuiteModules()), bind(suiteLevelFulfillers)); GuiceTestContext initSuiteTestContext = new GuiceTestContext(suiteModule); TestContextStack suiteTextContextStack = new TestContextStack<>(); suiteTextContextStack.push(initSuiteTestContext); @@ -174,7 +166,6 @@ public void afterAll(ExtensionContext context) @Override public void beforeEach(ExtensionContext context) - throws Exception { // setupLoggingMdcForTest(testResult); if (!suiteTestContextStack.isPresent()) { @@ -189,7 +180,7 @@ public void beforeEach(ExtensionContext context) try { // Set testSpecificRequirements = getTestSpecificRequirements(context.getRequiredTestInstance()); - doFulfillment(testContextStack, testMethodLevelFulfillers, ImmutableSet.of()); + doFulfillment(testContextStack, ImmutableList.of(), ImmutableSet.of()); } catch (RuntimeException e) { LOGGER.debug("error within test initialization", e); @@ -235,7 +226,7 @@ private void onTestFinished(TestStatus testStatus) // } // finally { TestContextStack testContextStack = popAllTestContexts(); - doCleanup(testContextStack, testMethodLevelFulfillers, testStatus); + doCleanup(testContextStack, ImmutableList.of(), testStatus); cleanLoggingMdc(); // } } From 0c6e9ac71f98a0392522461ff0b55e554a3e6e1c Mon Sep 17 00:00:00 2001 From: Sasha Sheikin Date: Mon, 11 Nov 2024 17:57:50 +0100 Subject: [PATCH 6/9] Setup context for test only once Assume requirements are not needed for the test. Requirement CommandRequirement - is not used now TableRequirement - most probably can be substituted by @Before.., @After.. LdapObjectRequirement - to find out how to deal with it. As time shows, method level requirements are not used in the new code, and most probably, were implemented for `Convention based SQL query tests` --- .../TestInitializationListenerJUnit.java | 249 +----------------- 1 file changed, 11 insertions(+), 238 deletions(-) diff --git a/tempto-core/src/main/java/io/trino/tempto/internal/initialization/TestInitializationListenerJUnit.java b/tempto-core/src/main/java/io/trino/tempto/internal/initialization/TestInitializationListenerJUnit.java index 9d5c70df..947938d2 100644 --- a/tempto-core/src/main/java/io/trino/tempto/internal/initialization/TestInitializationListenerJUnit.java +++ b/tempto-core/src/main/java/io/trino/tempto/internal/initialization/TestInitializationListenerJUnit.java @@ -15,74 +15,44 @@ package io.trino.tempto.internal.initialization; import com.google.common.collect.ImmutableList; -import com.google.common.collect.ImmutableSet; import com.google.common.collect.Ordering; -import com.google.inject.Binder; import com.google.inject.Module; -import com.google.inject.Singleton; -import io.trino.tempto.Requirement; import io.trino.tempto.TemptoPlugin; import io.trino.tempto.configuration.Configuration; import io.trino.tempto.context.TestContext; -import io.trino.tempto.fulfillment.RequirementFulfiller; -import io.trino.tempto.fulfillment.RequirementFulfiller.SuiteLevelFulfiller; -import io.trino.tempto.fulfillment.TestStatus; import io.trino.tempto.fulfillment.table.TableManager; import io.trino.tempto.fulfillment.table.TableManagerDispatcher; import io.trino.tempto.initialization.SuiteModuleProvider; -import io.trino.tempto.initialization.TestMethodModuleProvider; import io.trino.tempto.internal.ReflectionHelper; import io.trino.tempto.internal.context.GuiceTestContext; import io.trino.tempto.internal.context.TestContextStack; import org.junit.jupiter.api.extension.AfterAllCallback; -import org.junit.jupiter.api.extension.AfterTestExecutionCallback; import org.junit.jupiter.api.extension.BeforeAllCallback; -import org.junit.jupiter.api.extension.BeforeEachCallback; import org.junit.jupiter.api.extension.ExtensionContext; -import org.junit.jupiter.api.extension.TestExecutionExceptionHandler; import org.slf4j.Logger; -import org.testng.ITestResult; -import java.util.ArrayList; import java.util.List; -import java.util.Optional; import java.util.ServiceLoader; -import java.util.Set; -import java.util.concurrent.atomic.AtomicLong; -import java.util.function.Function; -import static com.google.common.base.Preconditions.checkState; import static com.google.common.collect.ImmutableList.toImmutableList; -import static com.google.common.collect.Lists.reverse; import static com.google.inject.util.Modules.combine; -import static io.trino.tempto.context.TestContextDsl.runWithTestContext; import static io.trino.tempto.context.ThreadLocalTestContextHolder.assertTestContextNotSet; import static io.trino.tempto.context.ThreadLocalTestContextHolder.popAllTestContexts; import static io.trino.tempto.context.ThreadLocalTestContextHolder.pushAllTestContexts; import static io.trino.tempto.context.ThreadLocalTestContextHolder.testContextIfSet; -import static io.trino.tempto.fulfillment.TestStatus.FAILURE; -import static io.trino.tempto.fulfillment.TestStatus.SUCCESS; import static io.trino.tempto.internal.configuration.TestConfigurationFactory.testConfiguration; -import static io.trino.tempto.internal.logging.LoggingMdcHelper.cleanLoggingMdc; -import static java.util.Collections.emptyList; import static org.slf4j.LoggerFactory.getLogger; public class TestInitializationListenerJUnit implements BeforeAllCallback, - BeforeEachCallback, - AfterTestExecutionCallback, - AfterAllCallback, - TestExecutionExceptionHandler + AfterAllCallback { private static final Logger LOGGER = getLogger(TestInitializationListenerJUnit.class); private final List suiteModuleProviders; - private final List testMethodModuleProviders; - private final List> suiteLevelFulfillers; private final Configuration configuration; - private Optional> suiteTestContextStack = Optional.empty(); public TestInitializationListenerJUnit() { @@ -96,50 +66,30 @@ private TestInitializationListenerJUnit(List plugins) .flatMap(plugin -> plugin.getSuiteModules().stream()) .map(ReflectionHelper::instantiate) .collect(toImmutableList()), - plugins.stream() - .flatMap(plugin -> plugin.getTestModules().stream()) - .map(ReflectionHelper::instantiate) - .collect(toImmutableList()), - plugins.stream() - .flatMap(plugin -> plugin.getFulfillers().stream()) - .filter(clazz -> clazz.isAnnotationPresent(SuiteLevelFulfiller.class)) - .collect(toImmutableList()), testConfiguration()); } TestInitializationListenerJUnit( List suiteModuleProviders, - List testMethodModuleProviders, - List> suiteLevelFulfillers, Configuration configuration) { this.suiteModuleProviders = suiteModuleProviders; - this.testMethodModuleProviders = testMethodModuleProviders; - this.suiteLevelFulfillers = suiteLevelFulfillers; this.configuration = configuration; } - @Override public void beforeAll(ExtensionContext context) { displayConfigurationToUser(); - Module suiteModule = combine(combine(getSuiteModules()), bind(suiteLevelFulfillers)); + Module suiteModule = combine(getSuiteModules()); GuiceTestContext initSuiteTestContext = new GuiceTestContext(suiteModule); TestContextStack suiteTextContextStack = new TestContextStack<>(); suiteTextContextStack.push(initSuiteTestContext); - - try { -// Set allTestsRequirements = resolveAllTestsRequirements(testInstance, context); - doFulfillment(suiteTextContextStack, suiteLevelFulfillers, ImmutableSet.of()); - } - catch (RuntimeException e) { - LOGGER.error("cannot initialize test suite", e); - throw e; - } - - setSuiteTestContextStack(suiteTextContextStack); + assertTestContextNotSet(); + pushAllTestContexts(suiteTextContextStack); + TestContext topTestContext = suiteTextContextStack.peek(); + topTestContext.injectMembers(context.getRequiredTestInstance()); } private void displayConfigurationToUser() @@ -152,132 +102,17 @@ private void displayConfigurationToUser() } } - @Override public void afterAll(ExtensionContext context) { - if (!suiteTestContextStack.isPresent()) { - return; - } - - TestStatus testStatus = context.getExecutionException().isPresent() ? FAILURE : SUCCESS; - doCleanup(suiteTestContextStack.get(), suiteLevelFulfillers, testStatus); - } - - @Override - public void beforeEach(ExtensionContext context) - { -// setupLoggingMdcForTest(testResult); - if (!suiteTestContextStack.isPresent()) { - throw new SuiteInitializationException("test suite not initialized"); - } - GuiceTestContext initTestContext = ((GuiceTestContext) suiteTestContextStack.get().peek()).createChildContext(emptyList(), -// getTestModules(testResult) - emptyList() - ); - TestContextStack testContextStack = new TestContextStack<>(); - testContextStack.push(initTestContext); - - try { -// Set testSpecificRequirements = getTestSpecificRequirements(context.getRequiredTestInstance()); - doFulfillment(testContextStack, ImmutableList.of(), ImmutableSet.of()); - } - catch (RuntimeException e) { - LOGGER.debug("error within test initialization", e); - throw e; - } - - assertTestContextNotSet(); - pushAllTestContexts(testContextStack); - TestContext topTestContext = testContextStack.peek(); - topTestContext.injectMembers(context.getRequiredTestInstance()); - } - - @Override - public void afterTestExecution(ExtensionContext context) - { - if (context.getExecutionException().isEmpty()) { - onTestFinished(SUCCESS); - } - else { - onTestFinished(FAILURE); - } - } - - @Override - public void handleTestExecutionException(ExtensionContext context, Throwable throwable) - throws Throwable - { - LOGGER.debug("test failure", throwable); - onTestFinished(FAILURE); - throw throwable; - } - - private void onTestFinished(TestStatus testStatus) - { - if (!testContextIfSet().isPresent()) { - return; + if (testContextIfSet().isEmpty()) { + throw new IllegalStateException("Test context at this point may not be initialized only because of exception during initialization"); } -// boolean runAfterSucceeded = false; -// try { -// runAfterWithContextMethods(testResult, testContext()); -// runAfterSucceeded = true; -// } -// finally { TestContextStack testContextStack = popAllTestContexts(); - doCleanup(testContextStack, ImmutableList.of(), testStatus); - cleanLoggingMdc(); -// } - } - - private void doFulfillment(TestContextStack testContextStack, - List> fulfillerClasses, - Set requirements) - { - List> successfulFulfillerClasses = new ArrayList<>(); - - try { - for (Class fulfillerClass : fulfillerClasses) { - LOGGER.debug("Fulfilling using {}", fulfillerClass); - TestContext testContext = testContextStack.peek(); - runWithTestContext(testContext, () -> { - RequirementFulfiller fulfiller = testContext.getDependency(fulfillerClass); - TestContext testContextWithNewStates = testContext.createChildContext(fulfiller.fulfill(requirements)); - successfulFulfillerClasses.add(fulfillerClass); - testContextStack.push(testContextWithNewStates); - }); - } - } - catch (RuntimeException e) { - LOGGER.debug("error during fulfillment", e); - try { - doCleanup(testContextStack, successfulFulfillerClasses, FAILURE); - } - catch (RuntimeException cleanupException) { - e.addSuppressed(cleanupException); - } - throw e; - } - } - - private void doCleanup(TestContextStack testContextStack, List> fulfillerClasses, TestStatus testStatus) - { - // one base test context plus one test context for each fulfiller - checkState(testContextStack.size() == fulfillerClasses.size() + 1); - - for (Class fulfillerClass : reverse(fulfillerClasses)) { - LOGGER.debug("Cleaning for fulfiller {}", fulfillerClass); - TestContext testContext = testContextStack.pop(); - testContext.close(); - runWithTestContext(testContext, () -> testContextStack.peek().getDependency(fulfillerClass).cleanup(testStatus)); - } - - if (testContextStack.size() == 1) { - // we are going to close last context, so we need to close TableManager's first - testContextStack.peek().getOptionalDependency(TableManagerDispatcher.class) - .ifPresent(dispatcher -> dispatcher.getAllTableManagers().forEach(TableManager::close)); - } + // we are going to close last context, so we need to close TableManager's first + testContextStack.peek().getOptionalDependency(TableManagerDispatcher.class) + .ifPresent(dispatcher -> dispatcher.getAllTableManagers().forEach(TableManager::close)); // remove close init test context too testContextStack.peek().close(); @@ -290,66 +125,4 @@ private List getSuiteModules() .map(provider -> provider.getModule(configuration)) .collect(toImmutableList()); } - - private List getTestModules(ITestResult testResult) - { - return testMethodModuleProviders - .stream() - .map(provider -> provider.getModule(configuration, testResult)) - .collect(toImmutableList()); - } - - private Module bind(List> classes) - { - Function, Module> bindToModule = clazz -> (Binder binder) -> binder.bind(clazz).in(Singleton.class); - List modules = classes.stream() - .map(bindToModule) - .collect(toImmutableList()); - return combine(modules); - } - -// private Set resolveAllTestsRequirements(Object testInstance, ExtensionContext context) -// { -// // we cannot assume that context contains RequirementsAwareTestNGMethod instances here -// // as interceptor is for some reason called after onStart() which uses this method. -// Set allTestsRequirements = new HashSet<>(); -// List methods = Arrays.stream(context.getTestClass().get().getDeclaredMethods()) -// .filter(method -> method.isAnnotationPresent(Test.class)) -// .toList(); -// for (Method method : methods) { -// ITestNGMethod iTestNGMethod = null; -// Set> requirementsSets = new TestSpecificRequirementsResolver(configuration).resolve(method, testInstance); -// for (Set requirementsSet : requirementsSets) { -// allTestsRequirements.addAll(requirementsSet); -// } -// } -// return allTestsRequirements; -// } -// -// private Set getTestSpecificRequirements(ITestNGMethod testMethod) -// { -// return ((RequirementsAwareTestNGMethod) testMethod).getRequirements(); -// } - - private void setSuiteTestContextStack(TestContextStack suiteTestContextStack) - { - checkState(!this.suiteTestContextStack.isPresent(), "suite fulfillment result already set"); - this.suiteTestContextStack = Optional.of(suiteTestContextStack); - } - - private static class SuiteInitializationException - extends RuntimeException - { - private static final AtomicLong instanceCount = new AtomicLong(); - - SuiteInitializationException(String message) - { - super( - message, - null, - true, - // Suppress stacktrace for all but first 10 exceptions. It is not useful when printed for every test. - instanceCount.getAndIncrement() < 10); - } - } } From 13c270ed27880f340a640b101bf2f576f887f5df Mon Sep 17 00:00:00 2001 From: Sasha Sheikin Date: Tue, 12 Nov 2024 11:00:05 +0100 Subject: [PATCH 7/9] Bump java version --- .github/workflows/ci.yml | 2 +- .java-version | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 73e9e6d9..04f6cdf3 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -12,7 +12,7 @@ jobs: strategy: fail-fast: false matrix: - java: ['21', '22'] + java: ['23'] steps: - uses: actions/checkout@v2 - uses: actions/setup-java@v2 diff --git a/.java-version b/.java-version index 98d9bcb7..40994076 100644 --- a/.java-version +++ b/.java-version @@ -1 +1 @@ -17 +23 From 6e924769996ae3f9da4db8cf761e275117c3e93f Mon Sep 17 00:00:00 2001 From: Sasha Sheikin Date: Wed, 13 Nov 2024 12:19:42 +0100 Subject: [PATCH 8/9] Return reopened connection if closed --- .../io/trino/tempto/query/JdbcQueryExecutor.java | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/tempto-core/src/main/java/io/trino/tempto/query/JdbcQueryExecutor.java b/tempto-core/src/main/java/io/trino/tempto/query/JdbcQueryExecutor.java index 655de318..62bc2626 100644 --- a/tempto-core/src/main/java/io/trino/tempto/query/JdbcQueryExecutor.java +++ b/tempto-core/src/main/java/io/trino/tempto/query/JdbcQueryExecutor.java @@ -14,16 +14,14 @@ package io.trino.tempto.query; +import com.google.inject.Inject; import io.trino.tempto.context.TestContext; import org.slf4j.Logger; -import com.google.inject.Inject; - import java.sql.Connection; import java.sql.PreparedStatement; import java.sql.SQLException; import java.sql.Statement; -import java.util.Arrays; import static io.trino.tempto.query.QueryResult.forSingleIntegerValue; import static io.trino.tempto.query.QueryResult.toSqlIndex; @@ -87,8 +85,13 @@ public QueryResult executeQuery(String sql, QueryParam... params) @Override public Connection getConnection() { - if (connection == null) { - openConnection(); + try { + if (connection == null || connection.isClosed()) { + openConnection(); + } + } + catch (SQLException e) { + throw new RuntimeException(e); } return connection; } From 08b9a5fed9b0eba3d27ef50dd9b85e2f5d75a7a1 Mon Sep 17 00:00:00 2001 From: Sasha Sheikin Date: Fri, 15 Nov 2024 17:49:03 +0100 Subject: [PATCH 9/9] Initialize logs for Product Tests Revert? --- tempto-core/pom.xml | 12 ++++++++++++ .../listeners/ProgressLoggingListenerJUnit.java | 5 +++++ 2 files changed, 17 insertions(+) diff --git a/tempto-core/pom.xml b/tempto-core/pom.xml index 94d40abf..797d89a8 100644 --- a/tempto-core/pom.xml +++ b/tempto-core/pom.xml @@ -67,6 +67,18 @@ commons-io + + io.airlift + log-manager + 275 + + + org.slf4j + log4j-over-slf4j + + + + io.trino.hive hive-apache diff --git a/tempto-core/src/main/java/io/trino/tempto/internal/listeners/ProgressLoggingListenerJUnit.java b/tempto-core/src/main/java/io/trino/tempto/internal/listeners/ProgressLoggingListenerJUnit.java index 3685cfc2..ef991188 100644 --- a/tempto-core/src/main/java/io/trino/tempto/internal/listeners/ProgressLoggingListenerJUnit.java +++ b/tempto-core/src/main/java/io/trino/tempto/internal/listeners/ProgressLoggingListenerJUnit.java @@ -14,6 +14,7 @@ package io.trino.tempto.internal.listeners; +import io.airlift.log.Logging; import org.junit.jupiter.api.extension.AfterAllCallback; import org.junit.jupiter.api.extension.AfterEachCallback; import org.junit.jupiter.api.extension.BeforeAllCallback; @@ -33,6 +34,10 @@ public class ProgressLoggingListenerJUnit { private final static Logger LOGGER = LoggerFactory.getLogger(ProgressLoggingListenerJUnit.class); + static { + Logging.initialize(); + } + private int started; private int succeeded; private int failed;