[kudu-CR] [Java] Retry tests that don’t inherit from BaseKuduTest
Grant Henke has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/11037 ) Change subject: [Java] Retry tests that don’t inherit from BaseKuduTest .. [Java] Retry tests that don’t inherit from BaseKuduTest The Scala tests and a couple special case tests don’t inherit from BaseKuduTest but may still be flaky. To fix this I moved the retry configuration logic into the RetryRule and added the rule to the a few tests directly: - TestSecurity - TestKuduMetastorePlugin Additionally I updated the Scala TestContext, now called KuduTestSuite, to use the JUnitSuite and include the RetryRule so that all of the Scala tests would be retried. This required a few syntax changes but has the added benefit of not needing `@RunWith(classOf[JUnitRunner])` at the top of each test class. It also follows similar syntax to the rest of our tests. Change-Id: I596fde91a13f7e644fd1b1814e9a672f96fa0e4b Reviewed-on: http://gerrit.cloudera.org:8080/11037 Reviewed-by: Adar Dembo Tested-by: Kudu Jenkins --- M java/kudu-backup/src/test/scala/org/apache/kudu/backup/TestKuduBackup.scala M java/kudu-client/src/test/java/org/apache/kudu/client/BaseKuduTest.java M java/kudu-client/src/test/java/org/apache/kudu/client/TestSecurity.java M java/kudu-client/src/test/java/org/apache/kudu/junit/RetryRule.java M java/kudu-hive/build.gradle M java/kudu-hive/pom.xml M java/kudu-hive/src/test/java/org/apache/kudu/hive/metastore/TestKuduMetastorePlugin.java M java/kudu-spark-tools/src/test/scala/org/apache/kudu/spark/tools/ITBigLinkedListTest.scala M java/kudu-spark-tools/src/test/scala/org/apache/kudu/spark/tools/TestImportExportFiles.scala M java/kudu-spark/src/test/scala/org/apache/kudu/spark/kudu/DefaultSourceTest.scala M java/kudu-spark/src/test/scala/org/apache/kudu/spark/kudu/KuduContextTest.scala M java/kudu-spark/src/test/scala/org/apache/kudu/spark/kudu/KuduRDDTest.scala R java/kudu-spark/src/test/scala/org/apache/kudu/spark/kudu/KuduTestSuite.scala 13 files changed, 196 insertions(+), 125 deletions(-) Approvals: Adar Dembo: Looks good to me, approved Kudu Jenkins: Verified -- To view, visit http://gerrit.cloudera.org:8080/11037 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: I596fde91a13f7e644fd1b1814e9a672f96fa0e4b Gerrit-Change-Number: 11037 Gerrit-PatchSet: 4 Gerrit-Owner: Grant Henke Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins
[kudu-CR] [Java] Retry tests that don’t inherit from BaseKuduTest
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/11037 ) Change subject: [Java] Retry tests that don’t inherit from BaseKuduTest .. Patch Set 3: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/11037 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I596fde91a13f7e644fd1b1814e9a672f96fa0e4b Gerrit-Change-Number: 11037 Gerrit-PatchSet: 3 Gerrit-Owner: Grant Henke Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins Gerrit-Comment-Date: Tue, 24 Jul 2018 22:58:01 + Gerrit-HasComments: No
[kudu-CR] [Java] Retry tests that don’t inherit from BaseKuduTest
Hello Kudu Jenkins, Adar Dembo, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/11037 to look at the new patch set (#3). Change subject: [Java] Retry tests that don’t inherit from BaseKuduTest .. [Java] Retry tests that don’t inherit from BaseKuduTest The Scala tests and a couple special case tests don’t inherit from BaseKuduTest but may still be flaky. To fix this I moved the retry configuration logic into the RetryRule and added the rule to the a few tests directly: - TestSecurity - TestKuduMetastorePlugin Additionally I updated the Scala TestContext, now called KuduTestSuite, to use the JUnitSuite and include the RetryRule so that all of the Scala tests would be retried. This required a few syntax changes but has the added benefit of not needing `@RunWith(classOf[JUnitRunner])` at the top of each test class. It also follows similar syntax to the rest of our tests. Change-Id: I596fde91a13f7e644fd1b1814e9a672f96fa0e4b --- M java/kudu-backup/src/test/scala/org/apache/kudu/backup/TestKuduBackup.scala M java/kudu-client/src/test/java/org/apache/kudu/client/BaseKuduTest.java M java/kudu-client/src/test/java/org/apache/kudu/client/TestSecurity.java M java/kudu-client/src/test/java/org/apache/kudu/junit/RetryRule.java M java/kudu-hive/build.gradle M java/kudu-hive/pom.xml M java/kudu-hive/src/test/java/org/apache/kudu/hive/metastore/TestKuduMetastorePlugin.java M java/kudu-spark-tools/src/test/scala/org/apache/kudu/spark/tools/ITBigLinkedListTest.scala M java/kudu-spark-tools/src/test/scala/org/apache/kudu/spark/tools/TestImportExportFiles.scala M java/kudu-spark/src/test/scala/org/apache/kudu/spark/kudu/DefaultSourceTest.scala M java/kudu-spark/src/test/scala/org/apache/kudu/spark/kudu/KuduContextTest.scala M java/kudu-spark/src/test/scala/org/apache/kudu/spark/kudu/KuduRDDTest.scala R java/kudu-spark/src/test/scala/org/apache/kudu/spark/kudu/KuduTestSuite.scala 13 files changed, 196 insertions(+), 125 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/37/11037/3 -- To view, visit http://gerrit.cloudera.org:8080/11037 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I596fde91a13f7e644fd1b1814e9a672f96fa0e4b Gerrit-Change-Number: 11037 Gerrit-PatchSet: 3 Gerrit-Owner: Grant Henke Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins
[kudu-CR] [Java] Retry tests that don’t inherit from BaseKuduTest
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/11037 ) Change subject: [Java] Retry tests that don’t inherit from BaseKuduTest .. Patch Set 2: (2 comments) http://gerrit.cloudera.org:8080/#/c/11037/1/java/kudu-client/src/test/java/org/apache/kudu/client/TestSecurity.java File java/kudu-client/src/test/java/org/apache/kudu/client/TestSecurity.java: http://gerrit.cloudera.org:8080/#/c/11037/1/java/kudu-client/src/test/java/org/apache/kudu/client/TestSecurity.java@95 PS1, Line 95: public RetryRule retryRule = new RetryRule(); > Each test adjust the MiniKuduCluster options a bit. It's likely we could wo Okay, different patch sounds fine. http://gerrit.cloudera.org:8080/#/c/11037/2/java/kudu-client/src/test/java/org/apache/kudu/junit/RetryRule.java File java/kudu-client/src/test/java/org/apache/kudu/junit/RetryRule.java: http://gerrit.cloudera.org:8080/#/c/11037/2/java/kudu-client/src/test/java/org/apache/kudu/junit/RetryRule.java@31 PS2, Line 31: ,0 Nit: space between comma and 0 -- To view, visit http://gerrit.cloudera.org:8080/11037 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I596fde91a13f7e644fd1b1814e9a672f96fa0e4b Gerrit-Change-Number: 11037 Gerrit-PatchSet: 2 Gerrit-Owner: Grant Henke Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins Gerrit-Comment-Date: Tue, 24 Jul 2018 22:15:57 + Gerrit-HasComments: Yes
[kudu-CR] [Java] Retry tests that don’t inherit from BaseKuduTest
Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/11037 ) Change subject: [Java] Retry tests that don’t inherit from BaseKuduTest .. Patch Set 1: (4 comments) http://gerrit.cloudera.org:8080/#/c/11037/1/java/kudu-client/src/test/java/org/apache/kudu/client/TestSecurity.java File java/kudu-client/src/test/java/org/apache/kudu/client/TestSecurity.java: http://gerrit.cloudera.org:8080/#/c/11037/1/java/kudu-client/src/test/java/org/apache/kudu/client/TestSecurity.java@95 PS1, Line 95: public RetryRule retryRule = new RetryRule(); > Why can't TestSecurity extend BaseKuduTest? Can its use of MiniKuduCluster Each test adjust the MiniKuduCluster options a bit. It's likely we could work it in to use it. I would prefer to keep it a different patch though. http://gerrit.cloudera.org:8080/#/c/11037/1/java/kudu-client/src/test/java/org/apache/kudu/junit/RetryRule.java File java/kudu-client/src/test/java/org/apache/kudu/junit/RetryRule.java: http://gerrit.cloudera.org:8080/#/c/11037/1/java/kudu-client/src/test/java/org/apache/kudu/junit/RetryRule.java@30 PS1, Line 30: private static final String RETRY_PROPERTY_NAME = "rerunFailingTestsCount"; : private static final int DEFAULT_RETRY_COUNT = 0; : private static final String retryPropVal = System.getProperty(RETRY_PROPERTY_NAME); : private static final int retryCount = : retryPropVal != null ? Integer.parseInt(retryPropVal) : DEFAULT_RETRY_COUNT; > I think the entire block would be more readable as: definitely. http://gerrit.cloudera.org:8080/#/c/11037/1/java/kudu-spark-tools/src/test/scala/org/apache/kudu/spark/tools/TestImportExportFiles.scala File java/kudu-spark-tools/src/test/scala/org/apache/kudu/spark/tools/TestImportExportFiles.scala: http://gerrit.cloudera.org:8080/#/c/11037/1/java/kudu-spark-tools/src/test/scala/org/apache/kudu/spark/tools/TestImportExportFiles.scala@32 PS1, Line 32: > Nit: there's an extra space here. Done http://gerrit.cloudera.org:8080/#/c/11037/1/java/kudu-spark/src/test/scala/org/apache/kudu/spark/kudu/KuduRDDTest.scala File java/kudu-spark/src/test/scala/org/apache/kudu/spark/kudu/KuduRDDTest.scala: http://gerrit.cloudera.org:8080/#/c/11037/1/java/kudu-spark/src/test/scala/org/apache/kudu/spark/kudu/KuduRDDTest.scala@29 PS1, Line 29: fail() > Did you mean to add this? No, missed one of the places I was testing retries. -- To view, visit http://gerrit.cloudera.org:8080/11037 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I596fde91a13f7e644fd1b1814e9a672f96fa0e4b Gerrit-Change-Number: 11037 Gerrit-PatchSet: 1 Gerrit-Owner: Grant Henke Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins Gerrit-Comment-Date: Tue, 24 Jul 2018 21:03:00 + Gerrit-HasComments: Yes
[kudu-CR] [Java] Retry tests that don’t inherit from BaseKuduTest
Hello Kudu Jenkins, Adar Dembo, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/11037 to look at the new patch set (#2). Change subject: [Java] Retry tests that don’t inherit from BaseKuduTest .. [Java] Retry tests that don’t inherit from BaseKuduTest The Scala tests and a couple special case tests don’t inherit from BaseKuduTest but may still be flaky. To fix this I moved the retry configuration logic into the RetryRule and added the rule to the a few tests directly: - TestSecurity - TestKuduMetastorePlugin Additionally I updated the Scala TestContext, now called KuduTestSuite, to use the JUnitSuite and include the RetryRule so that all of the Scala tests would be retried. This required a few syntax changes but has the added benefit of not needing `@RunWith(classOf[JUnitRunner])` at the top of each test class. It also follows similar syntax to the rest of our tests. Change-Id: I596fde91a13f7e644fd1b1814e9a672f96fa0e4b --- M java/kudu-backup/src/test/scala/org/apache/kudu/backup/TestKuduBackup.scala M java/kudu-client/src/test/java/org/apache/kudu/client/BaseKuduTest.java M java/kudu-client/src/test/java/org/apache/kudu/client/TestSecurity.java M java/kudu-client/src/test/java/org/apache/kudu/junit/RetryRule.java M java/kudu-hive/build.gradle M java/kudu-hive/pom.xml M java/kudu-hive/src/test/java/org/apache/kudu/hive/metastore/TestKuduMetastorePlugin.java M java/kudu-spark-tools/src/test/scala/org/apache/kudu/spark/tools/ITBigLinkedListTest.scala M java/kudu-spark-tools/src/test/scala/org/apache/kudu/spark/tools/TestImportExportFiles.scala M java/kudu-spark/src/test/scala/org/apache/kudu/spark/kudu/DefaultSourceTest.scala M java/kudu-spark/src/test/scala/org/apache/kudu/spark/kudu/KuduContextTest.scala M java/kudu-spark/src/test/scala/org/apache/kudu/spark/kudu/KuduRDDTest.scala R java/kudu-spark/src/test/scala/org/apache/kudu/spark/kudu/KuduTestSuite.scala 13 files changed, 196 insertions(+), 125 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/37/11037/2 -- To view, visit http://gerrit.cloudera.org:8080/11037 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I596fde91a13f7e644fd1b1814e9a672f96fa0e4b Gerrit-Change-Number: 11037 Gerrit-PatchSet: 2 Gerrit-Owner: Grant Henke Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins
[kudu-CR] [Java] Retry tests that don’t inherit from BaseKuduTest
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/11037 ) Change subject: [Java] Retry tests that don’t inherit from BaseKuduTest .. Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/11037/1/java/kudu-client/src/test/java/org/apache/kudu/client/TestSecurity.java File java/kudu-client/src/test/java/org/apache/kudu/client/TestSecurity.java: http://gerrit.cloudera.org:8080/#/c/11037/1/java/kudu-client/src/test/java/org/apache/kudu/client/TestSecurity.java@95 PS1, Line 95: public RetryRule retryRule = new RetryRule(); Why can't TestSecurity extend BaseKuduTest? Can its use of MiniKuduCluster be massaged slightly to fit into BaseKuduTest and avoid this? -- To view, visit http://gerrit.cloudera.org:8080/11037 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I596fde91a13f7e644fd1b1814e9a672f96fa0e4b Gerrit-Change-Number: 11037 Gerrit-PatchSet: 1 Gerrit-Owner: Grant Henke Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Kudu Jenkins Gerrit-Comment-Date: Tue, 24 Jul 2018 20:45:26 + Gerrit-HasComments: Yes
[kudu-CR] [Java] Retry tests that don’t inherit from BaseKuduTest
Adar Dembo has posted comments on this change. (
http://gerrit.cloudera.org:8080/11037 )
Change subject: [Java] Retry tests that don’t inherit from BaseKuduTest
..
Patch Set 1:
(3 comments)
http://gerrit.cloudera.org:8080/#/c/11037/1/java/kudu-client/src/test/java/org/apache/kudu/junit/RetryRule.java
File java/kudu-client/src/test/java/org/apache/kudu/junit/RetryRule.java:
http://gerrit.cloudera.org:8080/#/c/11037/1/java/kudu-client/src/test/java/org/apache/kudu/junit/RetryRule.java@30
PS1, Line 30: private static final String RETRY_PROPERTY_NAME =
"rerunFailingTestsCount";
: private static final int DEFAULT_RETRY_COUNT = 0;
: private static final String retryPropVal =
System.getProperty(RETRY_PROPERTY_NAME);
: private static final int retryCount =
: retryPropVal != null ? Integer.parseInt(retryPropVal) :
DEFAULT_RETRY_COUNT;
I think the entire block would be more readable as:
private static final int RETRY_COUNT =
Integer.getInteger("rerunFailingTestsCount", 0);
http://gerrit.cloudera.org:8080/#/c/11037/1/java/kudu-spark-tools/src/test/scala/org/apache/kudu/spark/tools/TestImportExportFiles.scala
File
java/kudu-spark-tools/src/test/scala/org/apache/kudu/spark/tools/TestImportExportFiles.scala:
http://gerrit.cloudera.org:8080/#/c/11037/1/java/kudu-spark-tools/src/test/scala/org/apache/kudu/spark/tools/TestImportExportFiles.scala@32
PS1, Line 32:
Nit: there's an extra space here.
http://gerrit.cloudera.org:8080/#/c/11037/1/java/kudu-spark/src/test/scala/org/apache/kudu/spark/kudu/KuduRDDTest.scala
File
java/kudu-spark/src/test/scala/org/apache/kudu/spark/kudu/KuduRDDTest.scala:
http://gerrit.cloudera.org:8080/#/c/11037/1/java/kudu-spark/src/test/scala/org/apache/kudu/spark/kudu/KuduRDDTest.scala@29
PS1, Line 29: fail()
Did you mean to add this?
--
To view, visit http://gerrit.cloudera.org:8080/11037
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I596fde91a13f7e644fd1b1814e9a672f96fa0e4b
Gerrit-Change-Number: 11037
Gerrit-PatchSet: 1
Gerrit-Owner: Grant Henke
Gerrit-Reviewer: Adar Dembo
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Tue, 24 Jul 2018 20:43:58 +
Gerrit-HasComments: Yes
[kudu-CR] [Java] Retry tests that don’t inherit from BaseKuduTest
Grant Henke has uploaded this change for review. ( http://gerrit.cloudera.org:8080/11037 Change subject: [Java] Retry tests that don’t inherit from BaseKuduTest .. [Java] Retry tests that don’t inherit from BaseKuduTest The Scala tests and a couple special case tests don’t inherit from BaseKuduTest but may still be flaky. To fix this I moved the retry configuration logic into the RetryRule and added the rule to the a few tests directly: - TestSecurity - TestKuduMetastorePlugin Additionally I updated the Scala TestContext, now called KuduTestSuite, to use the JUnitSuite and include the RetryRule so that all of the Scala tests would be retried. This required a few syntax changes but has the added benefit of not needing `@RunWith(classOf[JUnitRunner])` at the top of each test class. It also follows similar syntax to the rest of our tests. Change-Id: I596fde91a13f7e644fd1b1814e9a672f96fa0e4b --- M java/kudu-backup/src/test/scala/org/apache/kudu/backup/TestKuduBackup.scala M java/kudu-client/src/test/java/org/apache/kudu/client/BaseKuduTest.java M java/kudu-client/src/test/java/org/apache/kudu/client/TestSecurity.java M java/kudu-client/src/test/java/org/apache/kudu/junit/RetryRule.java M java/kudu-hive/build.gradle M java/kudu-hive/pom.xml M java/kudu-hive/src/test/java/org/apache/kudu/hive/metastore/TestKuduMetastorePlugin.java M java/kudu-spark-tools/src/test/scala/org/apache/kudu/spark/tools/ITBigLinkedListTest.scala M java/kudu-spark-tools/src/test/scala/org/apache/kudu/spark/tools/TestImportExportFiles.scala M java/kudu-spark/src/test/scala/org/apache/kudu/spark/kudu/DefaultSourceTest.scala M java/kudu-spark/src/test/scala/org/apache/kudu/spark/kudu/KuduContextTest.scala M java/kudu-spark/src/test/scala/org/apache/kudu/spark/kudu/KuduRDDTest.scala R java/kudu-spark/src/test/scala/org/apache/kudu/spark/kudu/KuduTestSuite.scala 13 files changed, 199 insertions(+), 124 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/37/11037/1 -- To view, visit http://gerrit.cloudera.org:8080/11037 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I596fde91a13f7e644fd1b1814e9a672f96fa0e4b Gerrit-Change-Number: 11037 Gerrit-PatchSet: 1 Gerrit-Owner: Grant Henke
