From da14627282d656ccc0df792baee3b171555bc74c Mon Sep 17 00:00:00 2001 From: Viktor Somogyi-Vass Date: Tue, 28 Jan 2020 13:44:49 +0100 Subject: [PATCH 1/8] KAFKA-8164: Improve test passing rate by rerunning flaky tests --- build.gradle | 7 +++++++ gradle/dependencies.gradle | 1 + 2 files changed, 8 insertions(+) diff --git a/build.gradle b/build.gradle index 1ed38fe3342ff..fb86f2c11bce1 100644 --- a/build.gradle +++ b/build.gradle @@ -37,6 +37,7 @@ buildscript { classpath "org.owasp:dependency-check-gradle:$versions.owaspDepCheckPlugin" classpath "com.diffplug.spotless:spotless-plugin-gradle:$versions.spotlessPlugin" classpath "com.github.spotbugs:spotbugs-gradle-plugin:$versions.spotbugsPlugin" + classpath "org.gradle:test-retry-gradle-plugin:$versions.testRetryPlugin" } } @@ -163,6 +164,7 @@ subprojects { apply plugin: 'signing' apply plugin: 'checkstyle' apply plugin: "com.github.spotbugs" + apply plugin: 'org.gradle.test-retry' sourceCompatibility = minJavaVersion targetCompatibility = minJavaVersion @@ -289,6 +291,11 @@ subprojects { exceptionFormat = testExceptionFormat } logTestStdout.rehydrate(delegate, owner, this)() + + retry { + maxRetries = 3 + maxFailures = 50 + } } task integrationTest(type: Test, dependsOn: compileJava) { diff --git a/gradle/dependencies.gradle b/gradle/dependencies.gradle index f5673de4c8de6..5a6e220c14507 100644 --- a/gradle/dependencies.gradle +++ b/gradle/dependencies.gradle @@ -112,6 +112,7 @@ versions += [ spotbugs: "3.1.12", spotbugsPlugin: "3.0.0", spotlessPlugin: "3.27.1", + testRetryPlugin: "1.0.2", zookeeper: "3.5.6", zstd: "1.4.4-7" ] From 96baf78c59534c3a5f1ae11a69b9bd234e7f7944 Mon Sep 17 00:00:00 2001 From: Viktor Somogyi-Vass Date: Fri, 31 Jan 2020 15:36:52 +0100 Subject: [PATCH 2/8] Lower defaults, add configurable properties --- build.gradle | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/build.gradle b/build.gradle index fb86f2c11bce1..2d70611eba8fd 100644 --- a/build.gradle +++ b/build.gradle @@ -98,6 +98,9 @@ ext { userMaxForks = project.hasProperty('maxParallelForks') ? maxParallelForks.toInteger() : null + userFlakyRetries = project.hasProperty('flakyRetries') ? flakyRetries.toInteger() : 1 + userMaxTestFailures = project.hasProperty('maxTestFailures') ? maxTestFailures.toInteger() : 5 + skipSigning = project.hasProperty('skipSigning') && skipSigning.toBoolean() shouldSign = !skipSigning && !version.endsWith("SNAPSHOT") && project.gradle.startParameter.taskNames.any { it.contains("upload") } @@ -293,8 +296,8 @@ subprojects { logTestStdout.rehydrate(delegate, owner, this)() retry { - maxRetries = 3 - maxFailures = 50 + maxRetries = userFlakyRetries + maxFailures = userMaxTestFailures } } From 9891b23e0b2bc788852397fc1bdf29d718af3f0b Mon Sep 17 00:00:00 2001 From: Viktor Somogyi-Vass Date: Mon, 3 Feb 2020 17:23:04 +0100 Subject: [PATCH 3/8] Update readme, correct namings --- README.md | 9 +++++++++ build.gradle | 8 ++++---- 2 files changed, 13 insertions(+), 4 deletions(-) diff --git a/README.md b/README.md index 07153fb202669..70865f94c8fa4 100644 --- a/README.md +++ b/README.md @@ -48,6 +48,15 @@ Change the log4j setting in either `clients/src/test/resources/log4j.properties` ./gradlew clients:test --tests RequestResponseTest +### Specifying test retries ### +By default every failed test will be retried once but it is possible to change this value with the `maxTestRetries` +parameter. Tests will be retried at the end of a test task. By specifying `maxTestRetryFailures` one can set a limit +on maximum number of test failures that are allowed before retrying is disabled. For instance if this is set to 5 and +4 test fails initially and 3 again on retry then this will not be considered too many and retries would continue but if +more than 5 failed initially then retries would be disabled. + + ./gradlew test -PmaxTestRetries=1 -PmaxTestRetryFailures=5 + ### Generating test coverage reports ### Generate coverage reports for the whole project: diff --git a/build.gradle b/build.gradle index 2d70611eba8fd..e31f9642e6a20 100644 --- a/build.gradle +++ b/build.gradle @@ -98,8 +98,8 @@ ext { userMaxForks = project.hasProperty('maxParallelForks') ? maxParallelForks.toInteger() : null - userFlakyRetries = project.hasProperty('flakyRetries') ? flakyRetries.toInteger() : 1 - userMaxTestFailures = project.hasProperty('maxTestFailures') ? maxTestFailures.toInteger() : 5 + userMaxTestRetries = project.hasProperty('maxTestRetries') ? maxTestRetries.toInteger() : 1 + userMaxTestRetryFailures = project.hasProperty('maxTestRetryFailures') ? maxTestRetryFailures.toInteger() : 5 skipSigning = project.hasProperty('skipSigning') && skipSigning.toBoolean() shouldSign = !skipSigning && !version.endsWith("SNAPSHOT") && project.gradle.startParameter.taskNames.any { it.contains("upload") } @@ -296,8 +296,8 @@ subprojects { logTestStdout.rehydrate(delegate, owner, this)() retry { - maxRetries = userFlakyRetries - maxFailures = userMaxTestFailures + maxRetries = userMaxTestRetries + maxFailures = userMaxTestRetryFailures } } From 6d9841bf781de9992354cd24ed93d10668143a89 Mon Sep 17 00:00:00 2001 From: Viktor Somogyi-Vass Date: Wed, 5 Feb 2020 10:03:08 +0100 Subject: [PATCH 4/8] Add new options to common build options --- README.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/README.md b/README.md index 70865f94c8fa4..3d9dce6c0762c 100644 --- a/README.md +++ b/README.md @@ -200,6 +200,8 @@ The following options should be set with a `-P` switch, for example `./gradlew - * `skipSigning`: skips signing of artifacts. * `testLoggingEvents`: unit test events to be logged, separated by comma. For example `./gradlew -PtestLoggingEvents=started,passed,skipped,failed test`. * `xmlSpotBugsReport`: enable XML reports for spotBugs. This also disables HTML reports as only one can be enabled at a time. +* `maxTestRetries`: the maximum number of retries for a failing test case +* `maxTestRetryFailures`: maximum number of test failures that are allowed before retrying is disabled ### Dependency Analysis ### From 0b3ac638569cf77d5249ed38684055a1660acac5 Mon Sep 17 00:00:00 2001 From: Ismael Juma Date: Wed, 5 Feb 2020 07:30:04 -0800 Subject: [PATCH 5/8] Update testRetry to 1.1.0 --- gradle/dependencies.gradle | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gradle/dependencies.gradle b/gradle/dependencies.gradle index 5a6e220c14507..0c2116cab7c9c 100644 --- a/gradle/dependencies.gradle +++ b/gradle/dependencies.gradle @@ -112,7 +112,7 @@ versions += [ spotbugs: "3.1.12", spotbugsPlugin: "3.0.0", spotlessPlugin: "3.27.1", - testRetryPlugin: "1.0.2", + testRetryPlugin: "1.1.0", zookeeper: "3.5.6", zstd: "1.4.4-7" ] From e2db38bed42871c42fc897bb2a6d249f6faf05d0 Mon Sep 17 00:00:00 2001 From: Ismael Juma Date: Wed, 5 Feb 2020 07:41:27 -0800 Subject: [PATCH 6/8] Adjust readme --- README.md | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/README.md b/README.md index 3d9dce6c0762c..baaf12ca4a9b7 100644 --- a/README.md +++ b/README.md @@ -49,13 +49,11 @@ Change the log4j setting in either `clients/src/test/resources/log4j.properties` ./gradlew clients:test --tests RequestResponseTest ### Specifying test retries ### -By default every failed test will be retried once but it is possible to change this value with the `maxTestRetries` -parameter. Tests will be retried at the end of a test task. By specifying `maxTestRetryFailures` one can set a limit -on maximum number of test failures that are allowed before retrying is disabled. For instance if this is set to 5 and -4 test fails initially and 3 again on retry then this will not be considered too many and retries would continue but if -more than 5 failed initially then retries would be disabled. +By default, each failed test is retried once up to a maximum of five retries per test run. Tests are retried at the end of the test task. Adjust these parameters in the following way: ./gradlew test -PmaxTestRetries=1 -PmaxTestRetryFailures=5 + +See [Test Retry Gradle Plugin](https://github.com/gradle/test-retry-gradle-plugin) for more details. ### Generating test coverage reports ### Generate coverage reports for the whole project: @@ -200,8 +198,8 @@ The following options should be set with a `-P` switch, for example `./gradlew - * `skipSigning`: skips signing of artifacts. * `testLoggingEvents`: unit test events to be logged, separated by comma. For example `./gradlew -PtestLoggingEvents=started,passed,skipped,failed test`. * `xmlSpotBugsReport`: enable XML reports for spotBugs. This also disables HTML reports as only one can be enabled at a time. -* `maxTestRetries`: the maximum number of retries for a failing test case -* `maxTestRetryFailures`: maximum number of test failures that are allowed before retrying is disabled +* `maxTestRetries`: the maximum number of retries for a failing test case. +* `maxTestRetryFailures`: maximum number of test failures before retrying is disabled for subsequent tests. ### Dependency Analysis ### From 0e47aa0940e1004e09301f7e799b4bcd9a1361ab Mon Sep 17 00:00:00 2001 From: Ismael Juma Date: Wed, 5 Feb 2020 07:44:18 -0800 Subject: [PATCH 7/8] Mention that Readme should be updated if defaults are adjusted --- build.gradle | 1 + 1 file changed, 1 insertion(+) diff --git a/build.gradle b/build.gradle index e31f9642e6a20..3fb3ecc93dae5 100644 --- a/build.gradle +++ b/build.gradle @@ -98,6 +98,7 @@ ext { userMaxForks = project.hasProperty('maxParallelForks') ? maxParallelForks.toInteger() : null + // Update Readme if defaults are adjusted userMaxTestRetries = project.hasProperty('maxTestRetries') ? maxTestRetries.toInteger() : 1 userMaxTestRetryFailures = project.hasProperty('maxTestRetryFailures') ? maxTestRetryFailures.toInteger() : 5 From 078bc6947a430d009447dee80ce80a0b6c147fb5 Mon Sep 17 00:00:00 2001 From: Ismael Juma Date: Wed, 5 Feb 2020 10:57:27 -0800 Subject: [PATCH 8/8] Change the defaults to 0. --- build.gradle | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/build.gradle b/build.gradle index 3fb3ecc93dae5..658821d1154b7 100644 --- a/build.gradle +++ b/build.gradle @@ -98,9 +98,8 @@ ext { userMaxForks = project.hasProperty('maxParallelForks') ? maxParallelForks.toInteger() : null - // Update Readme if defaults are adjusted - userMaxTestRetries = project.hasProperty('maxTestRetries') ? maxTestRetries.toInteger() : 1 - userMaxTestRetryFailures = project.hasProperty('maxTestRetryFailures') ? maxTestRetryFailures.toInteger() : 5 + userMaxTestRetries = project.hasProperty('maxTestRetries') ? maxTestRetries.toInteger() : 0 + userMaxTestRetryFailures = project.hasProperty('maxTestRetryFailures') ? maxTestRetryFailures.toInteger() : 0 skipSigning = project.hasProperty('skipSigning') && skipSigning.toBoolean() shouldSign = !skipSigning && !version.endsWith("SNAPSHOT") && project.gradle.startParameter.taskNames.any { it.contains("upload") }