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 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 diff --git a/tempto-core/pom.xml b/tempto-core/pom.xml index a08f90c6..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 @@ -128,6 +140,11 @@ freemarker + + org.junit.jupiter + junit-jupiter-api + + org.slf4j slf4j-api @@ -194,12 +211,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 new file mode 100644 index 00000000..947938d2 --- /dev/null +++ b/tempto-core/src/main/java/io/trino/tempto/internal/initialization/TestInitializationListenerJUnit.java @@ -0,0 +1,128 @@ +/* + * 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.Module; +import io.trino.tempto.TemptoPlugin; +import io.trino.tempto.configuration.Configuration; +import io.trino.tempto.context.TestContext; +import io.trino.tempto.fulfillment.table.TableManager; +import io.trino.tempto.fulfillment.table.TableManagerDispatcher; +import io.trino.tempto.initialization.SuiteModuleProvider; +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.BeforeAllCallback; +import org.junit.jupiter.api.extension.ExtensionContext; +import org.slf4j.Logger; + +import java.util.List; +import java.util.ServiceLoader; + +import static com.google.common.collect.ImmutableList.toImmutableList; +import static com.google.inject.util.Modules.combine; +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.internal.configuration.TestConfigurationFactory.testConfiguration; +import static org.slf4j.LoggerFactory.getLogger; + +public class TestInitializationListenerJUnit + implements + BeforeAllCallback, + AfterAllCallback +{ + private static final Logger LOGGER = getLogger(TestInitializationListenerJUnit.class); + + private final List suiteModuleProviders; + + private final Configuration configuration; + + 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()), + testConfiguration()); + } + + TestInitializationListenerJUnit( + List suiteModuleProviders, + Configuration configuration) + { + this.suiteModuleProviders = suiteModuleProviders; + this.configuration = configuration; + } + + @Override + public void beforeAll(ExtensionContext context) + { + displayConfigurationToUser(); + + Module suiteModule = combine(getSuiteModules()); + GuiceTestContext initSuiteTestContext = new GuiceTestContext(suiteModule); + TestContextStack suiteTextContextStack = new TestContextStack<>(); + suiteTextContextStack.push(initSuiteTestContext); + assertTestContextNotSet(); + pushAllTestContexts(suiteTextContextStack); + TestContext topTestContext = suiteTextContextStack.peek(); + topTestContext.injectMembers(context.getRequiredTestInstance()); + } + + 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 afterAll(ExtensionContext context) + { + if (testContextIfSet().isEmpty()) { + throw new IllegalStateException("Test context at this point may not be initialized only because of exception during initialization"); + } + + TestContextStack testContextStack = popAllTestContexts(); + // 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()); + } +} 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..ef991188 --- /dev/null +++ b/tempto-core/src/main/java/io/trino/tempto/internal/listeners/ProgressLoggingListenerJUnit.java @@ -0,0 +1,114 @@ +/* + * 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 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; +import org.junit.jupiter.api.extension.BeforeEachCallback; +import org.junit.jupiter.api.extension.ExtensionContext; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import java.math.BigDecimal; +import java.math.RoundingMode; + +import static com.google.common.base.Preconditions.checkState; +import static java.lang.System.currentTimeMillis; + +public class ProgressLoggingListenerJUnit + implements BeforeAllCallback, BeforeEachCallback, AfterEachCallback, AfterAllCallback +{ + private final static Logger LOGGER = LoggerFactory.getLogger(ProgressLoggingListenerJUnit.class); + + static { + Logging.initialize(); + } + + private int started; + private int succeeded; + private int failed; + private long startTime; + private long testStartTime; + + @Override + public void beforeAll(ExtensionContext context) + { + startTime = currentTimeMillis(); + LOGGER.info("Starting tests"); + } + + @Override + public void beforeEach(ExtensionContext context) + { + testStartTime = currentTimeMillis(); + started++; + LOGGER.info("[{}] Starting test: {}", started, formatTestName(context)); + } + + @Override + public void afterEach(ExtensionContext context) + { + long executionTime = currentTimeMillis() - testStartTime; + 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 afterAll(ExtensionContext context) + { + checkState(succeeded + failed > 0, "No tests executed"); + LOGGER.info(""); + LOGGER.info("Completed {} tests", started); + LOGGER.info("{} SUCCEEDED / {} FAILED", succeeded, failed); + LOGGER.info("Tests execution took {}", formatDuration(currentTimeMillis() - startTime)); + } + + 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); + return "%s#%s".formatted(context.getRequiredTestClass().getName(), context.getDisplayName()); + } + + 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); + } +} 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; } 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()