Skip to content

Commit

Permalink
Only run pytest if test files are found in the test directories (#5012)
Browse files Browse the repository at this point in the history
  • Loading branch information
sherifnada committed Jul 27, 2021
1 parent f499026 commit 45ee623
Show file tree
Hide file tree
Showing 8 changed files with 54 additions and 38 deletions.
2 changes: 0 additions & 2 deletions airbyte-integrations/bases/base-python/build.gradle
Expand Up @@ -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")
2 changes: 0 additions & 2 deletions airbyte-integrations/bases/base-singer/build.gradle
Expand Up @@ -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")
Expand Up @@ -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")
Expand Up @@ -8,15 +8,14 @@ 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"
// configPath = "secrets/config.json"
// 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)
Expand Down
Expand Up @@ -36,3 +36,4 @@ airbyteStandardSourceTestFile {
dependencies {
implementation files(project(':airbyte-integrations:bases:base-standard-source-test-file').airbyteDocker.outputs)
}

75 changes: 51 additions & 24 deletions 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
Expand All @@ -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<Project> {

void apply(Project project) {
Expand Down Expand Up @@ -60,7 +103,7 @@ class AirbytePythonPlugin implements Plugin<Project> {
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) {
Expand All @@ -85,28 +128,12 @@ class AirbytePythonPlugin implements Plugin<Project> {
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) {
Expand Down Expand Up @@ -136,13 +163,13 @@ class AirbytePythonPlugin implements Plugin<Project> {
}

// 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)
Expand Down
Expand Up @@ -35,11 +35,6 @@ class AirbyteSourceAcceptanceTestPlugin implements Plugin<Project> {
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)
}
}
Expand Down
Expand Up @@ -73,7 +73,7 @@ class AirbyteStandardSourceTestFilePlugin implements Plugin<Project> {
}

// 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')
}

Expand Down

0 comments on commit 45ee623

Please sign in to comment.