From 752d51b93c4cd57db80e6526be77c629fa18ef81 Mon Sep 17 00:00:00 2001 From: Marius Posta Date: Thu, 2 Nov 2023 12:06:27 -0400 Subject: [PATCH 1/5] gradle: test task configuration changes --- .../resources/{log4j2.xml => log4j2-test.xml} | 0 .../config-models-oss/build.gradle | 27 ------- .../java/airbyte-cdk/db-sources/build.gradle | 7 -- .../gradle.properties | 2 +- .../destination-mssql/gradle.properties | 2 +- .../gradle.properties | 2 +- .../destination-postgres/gradle.properties | 2 +- .../destination-snowflake/gradle.properties | 3 +- build.gradle | 70 +++++++++++-------- .../airbyte-integration-test-java.gradle | 36 +++------- .../src/main/groovy/airbyte-python.gradle | 8 +++ 11 files changed, 62 insertions(+), 97 deletions(-) rename airbyte-cdk/java/airbyte-cdk/airbyte-commons/src/main/resources/{log4j2.xml => log4j2-test.xml} (100%) diff --git a/airbyte-cdk/java/airbyte-cdk/airbyte-commons/src/main/resources/log4j2.xml b/airbyte-cdk/java/airbyte-cdk/airbyte-commons/src/main/resources/log4j2-test.xml similarity index 100% rename from airbyte-cdk/java/airbyte-cdk/airbyte-commons/src/main/resources/log4j2.xml rename to airbyte-cdk/java/airbyte-cdk/airbyte-commons/src/main/resources/log4j2-test.xml diff --git a/airbyte-cdk/java/airbyte-cdk/config-models-oss/build.gradle b/airbyte-cdk/java/airbyte-cdk/config-models-oss/build.gradle index 060480262c98..a50cc2645608 100644 --- a/airbyte-cdk/java/airbyte-cdk/config-models-oss/build.gradle +++ b/airbyte-cdk/java/airbyte-cdk/config-models-oss/build.gradle @@ -31,30 +31,3 @@ jsonSchema2Pojo { tasks.register('generate').configure { dependsOn tasks.named('generateJsonSchema2Pojo') } - -test { - useJUnitPlatform { - excludeTags 'log4j2-config', 'logger-client' - } - testLogging { - events "passed", "skipped", "failed" - } -} - -tasks.register('log4j2IntegrationTest', Test) { - useJUnitPlatform { - includeTags 'log4j2-config' - } - testLogging { - events "passed", "skipped", "failed" - } -} - -tasks.register('logClientsIntegrationTest', Test) { - useJUnitPlatform { - includeTags 'logger-client' - } - testLogging { - events "passed", "skipped", "failed" - } -} diff --git a/airbyte-cdk/java/airbyte-cdk/db-sources/build.gradle b/airbyte-cdk/java/airbyte-cdk/db-sources/build.gradle index 7dd4ed80299e..cfff03fe153c 100644 --- a/airbyte-cdk/java/airbyte-cdk/db-sources/build.gradle +++ b/airbyte-cdk/java/airbyte-cdk/db-sources/build.gradle @@ -17,13 +17,6 @@ plugins { id "java-test-fixtures" // https://docs.gradle.org/current/userguide/java_testing.html#sec:java_test_fixtures } -test { - testLogging { - // TODO: Remove this after debugging - showStandardStreams = true - } -} - project.configurations { // From `base-debezium`: testFixturesImplementation.extendsFrom implementation diff --git a/airbyte-integrations/connectors/destination-mssql-strict-encrypt/gradle.properties b/airbyte-integrations/connectors/destination-mssql-strict-encrypt/gradle.properties index 5d1adb3b55c1..2b147dcf7175 100644 --- a/airbyte-integrations/connectors/destination-mssql-strict-encrypt/gradle.properties +++ b/airbyte-integrations/connectors/destination-mssql-strict-encrypt/gradle.properties @@ -1,3 +1,3 @@ # currently limit the number of parallel threads until further investigation into the issues \ # where integration tests run into race conditions -numberThreads=1 +testExecutionConcurrency=1 diff --git a/airbyte-integrations/connectors/destination-mssql/gradle.properties b/airbyte-integrations/connectors/destination-mssql/gradle.properties index 5d1adb3b55c1..2b147dcf7175 100644 --- a/airbyte-integrations/connectors/destination-mssql/gradle.properties +++ b/airbyte-integrations/connectors/destination-mssql/gradle.properties @@ -1,3 +1,3 @@ # currently limit the number of parallel threads until further investigation into the issues \ # where integration tests run into race conditions -numberThreads=1 +testExecutionConcurrency=1 diff --git a/airbyte-integrations/connectors/destination-postgres-strict-encrypt/gradle.properties b/airbyte-integrations/connectors/destination-postgres-strict-encrypt/gradle.properties index 5d1adb3b55c1..2b147dcf7175 100644 --- a/airbyte-integrations/connectors/destination-postgres-strict-encrypt/gradle.properties +++ b/airbyte-integrations/connectors/destination-postgres-strict-encrypt/gradle.properties @@ -1,3 +1,3 @@ # currently limit the number of parallel threads until further investigation into the issues \ # where integration tests run into race conditions -numberThreads=1 +testExecutionConcurrency=1 diff --git a/airbyte-integrations/connectors/destination-postgres/gradle.properties b/airbyte-integrations/connectors/destination-postgres/gradle.properties index 5d1adb3b55c1..2b147dcf7175 100644 --- a/airbyte-integrations/connectors/destination-postgres/gradle.properties +++ b/airbyte-integrations/connectors/destination-postgres/gradle.properties @@ -1,3 +1,3 @@ # currently limit the number of parallel threads until further investigation into the issues \ # where integration tests run into race conditions -numberThreads=1 +testExecutionConcurrency=1 diff --git a/airbyte-integrations/connectors/destination-snowflake/gradle.properties b/airbyte-integrations/connectors/destination-snowflake/gradle.properties index b677e2e43411..3ce49dd31e29 100644 --- a/airbyte-integrations/connectors/destination-snowflake/gradle.properties +++ b/airbyte-integrations/connectors/destination-snowflake/gradle.properties @@ -1,4 +1,3 @@ # currently limit the number of parallel threads until further investigation into the issues \ # where Snowflake will fail to login using config credentials -numberThreads=4 -parallelExecutionsPerThread=6 +testExecutionConcurrency=4 diff --git a/build.gradle b/build.gradle index 119ec338cd1e..9ed90e1124ba 100644 --- a/build.gradle +++ b/build.gradle @@ -505,27 +505,50 @@ subprojects { subproj -> } test { - maxHeapSize = '3g' - maxParallelForks = Runtime.runtime.availableProcessors() - // This allows to set up a `gradle.properties` file inside the connector folder to reduce parallelization. - // This is especially useful for connectors that share resources, like Redshift or Snowflake. - // This limits the number of test classes that execute in parallel. - // See also usages of parallelExecutionsPerThread. - if (subproj.hasProperty('numberThreads')) { - int numberThreads = 0 - String numberThreadsString = subproj.property('numberThreads').toString() - if (numberThreadsString.isInteger()) { - numberThreads = numberThreadsString as int - } - if (numberThreads > 0 && numberThreads < maxParallelForks) { - maxParallelForks = numberThreads - } + useJUnitPlatform() + testLogging() { + events 'skipped', 'started', 'passed', 'failed' + exceptionFormat 'full' + showStandardStreams = true } + // Set the timezone to UTC instead of picking up the host machine's timezone, + // which on a developer's laptop is more likely to be PST. systemProperty 'user.timezone', 'UTC' + // Enable parallel test execution in JUnit by default. + // This is to support @Execution(ExecutionMode.CONCURRENT) annotations + systemProperty 'junit.jupiter.execution.parallel.enabled', 'true' + // Concurrency takes place at the class level. + systemProperty 'junit.jupiter.execution.parallel.mode.classes.default', 'concurrent' + // Within a class, the test methods are still run serially on the same thread. + systemProperty 'junit.jupiter.execution.parallel.mode.default', 'same_thread' + // Effectively disable JUnit concurrency by running tests in only one thread by default. + systemProperty 'junit.jupiter.execution.parallel.config.strategy', 'fixed' + systemProperty 'junit.jupiter.execution.parallel.config.fixed.parallelism', 1 + + if (!subproj.hasProperty('testExecutionConcurrency')) { + // By default, let gradle spawn as many independent workers as it wants. + maxParallelForks = Runtime.runtime.availableProcessors() + maxHeapSize = '3G' + } else { + // Otherwise, run tests within the same JVM. + // Let gradle spawn only one worker. + maxParallelForks = 1 + maxHeapSize = '8G' + // Manage test execution concurrency in JUnit. + String concurrency = subproj.property('testExecutionConcurrency').toString() + if (concurrency.isInteger() && (concurrency as int) > 0) { + // Define a fixed number of threads when the property is set to a positive integer. + systemProperty 'junit.jupiter.execution.parallel.config.fixed.parallelism', concurrency + } else { + // Otherwise let JUnit manage the concurrency dynamically. + systemProperty 'junit.jupiter.execution.parallel.config.strategy', 'dynamic' + } + } + + // Exclude all connector unit tests upon request. if (rootProject.ext.skipSlowTests) { - // Exclude all connector unit tests exclude '**/io/airbyte/integrations/source/**' exclude '**/io/airbyte/integrations/destination/**' } @@ -534,15 +557,6 @@ subprojects { subproj -> enabled = !rootProject.ext.skipSlowTests excludes = ['**/*Test*', '**/generated*'] } - useJUnitPlatform { - excludeTags('cloud-storage') - } - testLogging() { - events "passed", "skipped", "failed" - exceptionFormat 'full' - // uncomment to get the full log output - // showStandardStreams = true - } finalizedBy jacocoTestReportTask } @@ -587,8 +601,6 @@ subprojects { subproj -> implementation libs.airbyte.protocol } - - tasks.withType(SpotBugsTask).configureEach { // Reports can be found under each subproject in build/spotbugs/ reports { @@ -598,9 +610,6 @@ subprojects { subproj -> } javadoc.options.addStringOption('Xdoclint:none', '-quiet') - tasks.named('check').configure { - dependsOn tasks.named('jacocoTestCoverageVerification') - } } // integration and performance test tasks per project @@ -610,7 +619,6 @@ allprojects { [ 'integrationTestJava', 'integrationTestPython', - 'standardSourceTestFile', ].contains(it.name) } } diff --git a/buildSrc/src/main/groovy/airbyte-integration-test-java.gradle b/buildSrc/src/main/groovy/airbyte-integration-test-java.gradle index 0490abfb36e7..75d6d6c43d89 100644 --- a/buildSrc/src/main/groovy/airbyte-integration-test-java.gradle +++ b/buildSrc/src/main/groovy/airbyte-integration-test-java.gradle @@ -28,41 +28,25 @@ class AirbyteIntegrationTestJavaPlugin implements Plugin { testClassesDirs = project.sourceSets.integrationTestJava.output.classesDirs classpath += project.sourceSets.integrationTestJava.runtimeClasspath - useJUnitPlatform { - // todo (cgardens) - figure out how to de-dupe this exclusion with the one in build.gradle. - excludeTags 'log4j2-config', 'logger-client', 'cloud-storage' - } - testLogging() { - events "passed", "failed", "started" - exceptionFormat "full" - // uncomment to get the full log output - // showStandardStreams = true + events 'skipped', 'started', 'passed', 'failed' + exceptionFormat 'full' + showStandardStreams = true } - outputs.upToDateWhen { false } - + systemProperties = project.test.systemProperties maxParallelForks = project.test.maxParallelForks - maxHeapSize = project.test.maxHeapSize + maxHeapSize = project.test.maxParallelForks - systemProperties = [ - // Allow tests to set @Execution(ExecutionMode.CONCURRENT) - 'junit.jupiter.execution.parallel.enabled': 'true', - ] - - // Limit the number of concurrent tests within a single test class. - // See also usages of numberThreads. - if (project.hasProperty('parallelExecutionsPerThread')) { - int parallelExecutionsPerThread = project.property('parallelExecutionsPerThread').toString() as int - systemProperties = systemProperties + [ - 'junit.jupiter.execution.parallel.config.strategy': 'fixed', - 'junit.jupiter.execution.parallel.config.fixed.parallelism': parallelExecutionsPerThread, - ] - } + // Always re-run integration tests no matter what. + outputs.upToDateWhen { false } } integrationTestJava.configure { mustRunAfter project.tasks.named('check') dependsOn project.tasks.matching { it.name == 'assemble' } } + project.tasks.named('build').configure { + dependsOn integrationTestJava + } } } diff --git a/buildSrc/src/main/groovy/airbyte-python.gradle b/buildSrc/src/main/groovy/airbyte-python.gradle index 2511a27ba4af..e7913ca0a16e 100644 --- a/buildSrc/src/main/groovy/airbyte-python.gradle +++ b/buildSrc/src/main/groovy/airbyte-python.gradle @@ -173,6 +173,14 @@ class AirbytePythonPlugin implements Plugin { } Helpers.addTestTaskIfTestFilesFound(project, 'integration_tests', 'integrationTestPython', installTestReqs) + def integrationTestTasks = project.tasks.matching { it.name == 'integrationTestPython' } + integrationTestTasks.configureEach { + dependsOn project.tasks.named('assemble') + mustRunAfter project.tasks.named('check') + } + project.tasks.named('build').configure { + dependsOn integrationTestTasks + } } } From fdeaca71a58717e6eaa7657c661adf5ae8de41d2 Mon Sep 17 00:00:00 2001 From: Marius Posta Date: Thu, 2 Nov 2023 13:15:17 -0400 Subject: [PATCH 2/5] fix integration test task definition --- buildSrc/src/main/groovy/airbyte-integration-test-java.gradle | 1 + 1 file changed, 1 insertion(+) diff --git a/buildSrc/src/main/groovy/airbyte-integration-test-java.gradle b/buildSrc/src/main/groovy/airbyte-integration-test-java.gradle index 75d6d6c43d89..13417b653462 100644 --- a/buildSrc/src/main/groovy/airbyte-integration-test-java.gradle +++ b/buildSrc/src/main/groovy/airbyte-integration-test-java.gradle @@ -28,6 +28,7 @@ class AirbyteIntegrationTestJavaPlugin implements Plugin { testClassesDirs = project.sourceSets.integrationTestJava.output.classesDirs classpath += project.sourceSets.integrationTestJava.runtimeClasspath + useJUnitPlatform() testLogging() { events 'skipped', 'started', 'passed', 'failed' exceptionFormat 'full' From 3594d828ca883b9c31f74d4fb28e84d0fe81d325 Mon Sep 17 00:00:00 2001 From: Marius Posta Date: Thu, 2 Nov 2023 14:04:52 -0400 Subject: [PATCH 3/5] add comment --- build.gradle | 1 + 1 file changed, 1 insertion(+) diff --git a/build.gradle b/build.gradle index 9ed90e1124ba..24ef9ea9b219 100644 --- a/build.gradle +++ b/build.gradle @@ -518,6 +518,7 @@ subprojects { subproj -> // Enable parallel test execution in JUnit by default. // This is to support @Execution(ExecutionMode.CONCURRENT) annotations + // See https://junit.org/junit5/docs/current/user-guide/#writing-tests-parallel-execution for details. systemProperty 'junit.jupiter.execution.parallel.enabled', 'true' // Concurrency takes place at the class level. systemProperty 'junit.jupiter.execution.parallel.mode.classes.default', 'concurrent' From 894cb7045bc4e93f681ab28331455d476ec9192c Mon Sep 17 00:00:00 2001 From: Marius Posta Date: Thu, 2 Nov 2023 14:56:53 -0400 Subject: [PATCH 4/5] fix broken tests --- .../cdk/integrations/base/AirbyteLogMessageTemplateTest.java | 5 ++++- .../src/main/groovy/airbyte-integration-test-java.gradle | 2 +- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/airbyte-cdk/java/airbyte-cdk/core/src/test/java/io/airbyte/cdk/integrations/base/AirbyteLogMessageTemplateTest.java b/airbyte-cdk/java/airbyte-cdk/core/src/test/java/io/airbyte/cdk/integrations/base/AirbyteLogMessageTemplateTest.java index 8cdb8b907f15..6910b0a014be 100644 --- a/airbyte-cdk/java/airbyte-cdk/core/src/test/java/io/airbyte/cdk/integrations/base/AirbyteLogMessageTemplateTest.java +++ b/airbyte-cdk/java/airbyte-cdk/core/src/test/java/io/airbyte/cdk/integrations/base/AirbyteLogMessageTemplateTest.java @@ -24,6 +24,7 @@ import org.apache.logging.log4j.core.LoggerContext; import org.apache.logging.log4j.core.appender.OutputStreamAppender; import org.apache.logging.log4j.core.config.Configuration; +import org.apache.logging.log4j.core.config.Configurator; import org.apache.logging.log4j.core.config.LoggerConfig; import org.junit.jupiter.api.AfterAll; import org.junit.jupiter.api.BeforeAll; @@ -43,6 +44,7 @@ public class AirbyteLogMessageTemplateTest { public static final String CONSOLE_JSON_APPENDER = "ConsoleJSONAppender"; private static OutputStreamAppender outputStreamAppender; private static LoggerConfig rootLoggerConfig; + private static LoggerContext loggerContext; @BeforeAll static void init() { @@ -50,7 +52,7 @@ static void init() { // as the console json appender defined in this project's log4j2.xml file. // We then attach this log appender with the LOGGER instance so that we can validate the logs // produced by code and assert that it matches the expected format. - final LoggerContext loggerContext = (LoggerContext) LogManager.getContext(false); + loggerContext = Configurator.initialize(null, "log4j2.xml"); final Configuration configuration = loggerContext.getConfiguration(); rootLoggerConfig = configuration.getLoggerConfig(""); @@ -71,6 +73,7 @@ void setup() { static void cleanUp() { outputStreamAppender.stop(); rootLoggerConfig.removeAppender(OUTPUT_STREAM_APPENDER); + loggerContext.close(); } @Test diff --git a/buildSrc/src/main/groovy/airbyte-integration-test-java.gradle b/buildSrc/src/main/groovy/airbyte-integration-test-java.gradle index 13417b653462..3c824bfb853f 100644 --- a/buildSrc/src/main/groovy/airbyte-integration-test-java.gradle +++ b/buildSrc/src/main/groovy/airbyte-integration-test-java.gradle @@ -37,7 +37,7 @@ class AirbyteIntegrationTestJavaPlugin implements Plugin { systemProperties = project.test.systemProperties maxParallelForks = project.test.maxParallelForks - maxHeapSize = project.test.maxParallelForks + maxHeapSize = project.test.maxHeapSize // Always re-run integration tests no matter what. outputs.upToDateWhen { false } From 1ae5667ec37bfc5cf656f9e49e4d915526d92c07 Mon Sep 17 00:00:00 2001 From: postamar Date: Thu, 2 Nov 2023 19:13:02 +0000 Subject: [PATCH 5/5] Automated Commit - Formatting Changes --- .../cdk/integrations/base/AirbyteLogMessageTemplateTest.java | 1 - 1 file changed, 1 deletion(-) diff --git a/airbyte-cdk/java/airbyte-cdk/core/src/test/java/io/airbyte/cdk/integrations/base/AirbyteLogMessageTemplateTest.java b/airbyte-cdk/java/airbyte-cdk/core/src/test/java/io/airbyte/cdk/integrations/base/AirbyteLogMessageTemplateTest.java index 6910b0a014be..39795319dbf7 100644 --- a/airbyte-cdk/java/airbyte-cdk/core/src/test/java/io/airbyte/cdk/integrations/base/AirbyteLogMessageTemplateTest.java +++ b/airbyte-cdk/java/airbyte-cdk/core/src/test/java/io/airbyte/cdk/integrations/base/AirbyteLogMessageTemplateTest.java @@ -20,7 +20,6 @@ import java.util.regex.Matcher; import java.util.regex.Pattern; import org.apache.logging.log4j.Level; -import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.core.LoggerContext; import org.apache.logging.log4j.core.appender.OutputStreamAppender; import org.apache.logging.log4j.core.config.Configuration;