From 45ee6234f2ef1296ea38799a7619572aedfdf25a Mon Sep 17 00:00:00 2001 From: "Sherif A. Nada" Date: Tue, 27 Jul 2021 08:33:51 -0700 Subject: [PATCH] Only run pytest if test files are found in the test directories (#5012) --- .../bases/base-python/build.gradle | 2 - .../bases/base-singer/build.gradle | 2 - .../bases/source-acceptance-test/build.gradle | 2 - .../source-appstore-singer/build.gradle | 3 +- .../connectors/source-mongodb/build.gradle | 1 + .../src/main/groovy/airbyte-python.gradle | 75 +++++++++++++------ .../airbyte-source-acceptance-test.gradle | 5 -- .../airbyte-standard-source-test-file.gradle | 2 +- 8 files changed, 54 insertions(+), 38 deletions(-) diff --git a/airbyte-integrations/bases/base-python/build.gradle b/airbyte-integrations/bases/base-python/build.gradle index 7126ed32af3e0..9d562e5ef0ff1 100644 --- a/airbyte-integrations/bases/base-python/build.gradle +++ b/airbyte-integrations/bases/base-python/build.gradle @@ -14,5 +14,3 @@ dependencies { installReqs.dependsOn(":airbyte-integrations:bases:airbyte-protocol:installReqs") -// no-op integration test task to allow build script to succeed. TODO fix build script to not require a task with this name. -task("integrationTest") diff --git a/airbyte-integrations/bases/base-singer/build.gradle b/airbyte-integrations/bases/base-singer/build.gradle index d99770a983cd0..0077a70adc07c 100644 --- a/airbyte-integrations/bases/base-singer/build.gradle +++ b/airbyte-integrations/bases/base-singer/build.gradle @@ -14,5 +14,3 @@ dependencies { installReqs.dependsOn(":airbyte-integrations:bases:airbyte-protocol:installReqs", ":airbyte-integrations:bases:base-python:installReqs") -// no-op integration test task to allow build script to succeed. TODO fix build script to not require a task with this name. -task("integrationTest") diff --git a/airbyte-integrations/bases/source-acceptance-test/build.gradle b/airbyte-integrations/bases/source-acceptance-test/build.gradle index 4418811e4f960..a3594ea235e4f 100644 --- a/airbyte-integrations/bases/source-acceptance-test/build.gradle +++ b/airbyte-integrations/bases/source-acceptance-test/build.gradle @@ -7,5 +7,3 @@ airbytePython { moduleDirectory 'source_acceptance_test' } -// no-op integration test task to allow build script to succeed. TODO fix build script to not require a task with this name. -task("integrationTest") diff --git a/airbyte-integrations/connectors/source-appstore-singer/build.gradle b/airbyte-integrations/connectors/source-appstore-singer/build.gradle index 6c93bc8d89fe0..933cb102e9bcc 100644 --- a/airbyte-integrations/connectors/source-appstore-singer/build.gradle +++ b/airbyte-integrations/connectors/source-appstore-singer/build.gradle @@ -8,6 +8,7 @@ airbytePython { moduleDirectory 'source_appstore_singer' } +// https://github.com/airbytehq/airbyte/issues/1651 //airbyteStandardSourceTestFile { // // For more information on standard source tests, see https://docs.airbyte.io/contributing-to-airbyte/building-new-connector/testing-connectors // specPath = "source_appstore_singer/spec.json" @@ -15,8 +16,6 @@ airbytePython { // configuredCatalogPath = "sample_files/configured_catalog.json" //} -task("integrationTest") // https://github.com/airbytehq/airbyte/issues/1651 - dependencies { implementation files(project(':airbyte-integrations:bases:base-standard-source-test-file').airbyteDocker.outputs) implementation files(project(':airbyte-integrations:bases:base-singer').airbyteDocker.outputs) diff --git a/airbyte-integrations/connectors/source-mongodb/build.gradle b/airbyte-integrations/connectors/source-mongodb/build.gradle index 2f9122c19aacc..e07be5a8efd6a 100644 --- a/airbyte-integrations/connectors/source-mongodb/build.gradle +++ b/airbyte-integrations/connectors/source-mongodb/build.gradle @@ -36,3 +36,4 @@ airbyteStandardSourceTestFile { dependencies { implementation files(project(':airbyte-integrations:bases:base-standard-source-test-file').airbyteDocker.outputs) } + diff --git a/buildSrc/src/main/groovy/airbyte-python.gradle b/buildSrc/src/main/groovy/airbyte-python.gradle index c67046a21d054..18da4afc0c315 100644 --- a/buildSrc/src/main/groovy/airbyte-python.gradle +++ b/buildSrc/src/main/groovy/airbyte-python.gradle @@ -1,3 +1,5 @@ +import groovy.io.FileType +import groovy.io.FileVisitResult import org.gradle.api.GradleException import org.gradle.api.Plugin import org.gradle.api.Project @@ -8,6 +10,47 @@ class AirbytePythonConfiguration { String moduleDirectory } +class Helpers { + static addTestTaskIfTestFilesFound(Project project, String testFilesDirectory, String taskName, taskDependencies) { + """ + This method verifies if there are test files in a directory before adding the pytest task to run tests on that directory. This is needed + because if there are no tests in that dir and we run pytest on it, it exits with exit code 5 which gradle takes to mean that the process + failed, since it's non-zero. This means that if a module doesn't need a unit or integration test, it still needs to add a dummy test file + like: + + ``` + def make_ci_pass_test(): + assert True + ``` + + So we use this method to leverage pytest's test discovery rules (https://docs.pytest.org/en/6.2.x/goodpractices.html#conventions-for-python-test-discovery) + to selectively run pytest based on whether there seem to be test files in that directory. + Namely, if the directory contains a file whose name is test_*.py or *_test.py then it's a test. + + See https://github.com/airbytehq/airbyte/issues/4979 for original context + """ + if (project.file(testFilesDirectory).exists()) { + + project.projectDir.toPath().resolve(testFilesDirectory).traverse(type: FileType.FILES, nameFilter: ~/(^test_.*|.*_test)\.py$/) { file -> + project.task(taskName, type: PythonTask, dependsOn: taskDependencies) { + module = "pytest" + command = "-s ${testFilesDirectory}" + } + // If a file is found, terminate the traversal, thus causing this task to be declared at most once + return FileVisitResult.TERMINATE + } + } + + // If the task doesn't exist then we didn't find a matching file. So add an empty task since other tasks will + // probably rely on this one existing. + if (!project.hasProperty(taskName)) { + project.task(taskName) { + logger.info "Skipping task ${taskName} because ${testFilesDirectory} doesn't exist." + } + } + } +} + class AirbytePythonPlugin implements Plugin { void apply(Project project) { @@ -60,7 +103,7 @@ class AirbytePythonPlugin implements Plugin { dependsOn project.rootProject.getTasksByName("airbytePythonApply", true).findAll { it.project.getPath().startsWith(":airbyte-integrations:bases") } } } - } else if(project.file('setup.py').exists()) { + } else if (project.file('setup.py').exists()) { // If requirements.txt does not exists, install from setup.py instead, assume a dev or "tests" profile exists. // In this case, there is no need to depend on the base python modules since everything should be contained in the setup.py. project.task('installLocalReqs', type: PythonTask) { @@ -85,28 +128,12 @@ class AirbytePythonPlugin implements Plugin { outputs.file('build/installedtestreqs.txt') } - if (project.file('unit_tests').exists()) { - project.task('unitTest', type: PythonTask, dependsOn: project.installTestReqs) { - module = "pytest" - command = "-s unit_tests" - } - } else { - project.task('unitTest') { - logger.info "Skipping Python unit tests because unit_tests directory doesn't exist." - } - } - - if (project.file('integration_tests').exists()){ - project.task('customIntegrationTests', type: PythonTask, dependsOn: project.installTestReqs) { - module = "pytest" - command = "-s integration_tests" - } - if (!project.hasProperty('integrationTest')) { - project.task('integrationTest') - } - - project.integrationTest.dependsOn(project.customIntegrationTests) + Helpers.addTestTaskIfTestFilesFound(project, 'unit_tests', 'unitTest', project.installTestReqs) + Helpers.addTestTaskIfTestFilesFound(project, 'integration_tests', 'customIntegrationTests', project.installTestReqs) + if (!project.tasks.findByName('integrationTest')) { + project.task('integrationTest') } + project.integrationTest.dependsOn(project.customIntegrationTests) if (extension.moduleDirectory) { project.task('mypyCheck', type: PythonTask) { @@ -136,13 +163,13 @@ class AirbytePythonPlugin implements Plugin { } // Add a task that allows cleaning up venvs to every python project - project.task('cleanPythonVenv', type: Exec){ + project.task('cleanPythonVenv', type: Exec) { commandLine 'rm' args '-rf', "$project.projectDir.absolutePath/$venvDirectoryName" } // Add a task which can be run at the root project level to delete all python venvs - if (!project.rootProject.hasProperty('cleanPythonVenvs')){ + if (!project.rootProject.hasProperty('cleanPythonVenvs')) { project.rootProject.task('cleanPythonVenvs') } project.rootProject.cleanPythonVenvs.dependsOn(project.cleanPythonVenv) diff --git a/buildSrc/src/main/groovy/airbyte-source-acceptance-test.gradle b/buildSrc/src/main/groovy/airbyte-source-acceptance-test.gradle index 0d7d9eb543726..4195d570557b0 100644 --- a/buildSrc/src/main/groovy/airbyte-source-acceptance-test.gradle +++ b/buildSrc/src/main/groovy/airbyte-source-acceptance-test.gradle @@ -35,11 +35,6 @@ class AirbyteSourceAcceptanceTestPlugin implements Plugin { project.sourceAcceptanceTest.dependsOn(project.airbyteDockerTest) } - // make sure we create the integrationTest task once in case a java integration test was already initialized - if (!project.hasProperty('integrationTest')) { - project.task('integrationTest') - } - project.integrationTest.dependsOn(project.sourceAcceptanceTest) } } diff --git a/buildSrc/src/main/groovy/airbyte-standard-source-test-file.gradle b/buildSrc/src/main/groovy/airbyte-standard-source-test-file.gradle index 8e0b1bb77ebff..3eede9b6321ba 100644 --- a/buildSrc/src/main/groovy/airbyte-standard-source-test-file.gradle +++ b/buildSrc/src/main/groovy/airbyte-standard-source-test-file.gradle @@ -73,7 +73,7 @@ class AirbyteStandardSourceTestFilePlugin implements Plugin { } // make sure we create the integrationTest task once in case a java integration test was already initialized - if (!project.hasProperty('integrationTest')) { + if (!project.tasks.findByName('integrationTest')) { project.task('integrationTest') }