You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "Grant Henke (Code Review)" <ge...@cloudera.org> on 2021/04/12 18:19:46 UTC
[kudu-CR] [test] Fix handling of assume staments in Java RetryRule
Grant Henke has uploaded this change for review. ( http://gerrit.cloudera.org:8080/17307
Change subject: [test] Fix handling of assume staments in Java RetryRule
......................................................................
[test] Fix handling of assume staments in Java RetryRule
The java RetryRule did not treat the AssumptionViolatedException
thrown by the various assume statments in Junit correctly. This
can result in test failures which should actually be skipped tests
and in the case of the flaky test reporting can result in reporting
failed tests which were actually skipped.
This patch adjusts the handling of the AssumptionViolatedException
to skip the test correctly and mark the test as a success when
reporting given there is no “skipped” result we can report to dist-test.
I used the JUnit TestWatcher rule which I used for reference:
https://github.com/junit-team/junit4/blob/main/src/main/java/org/junit/rules/TestWatcher.java#L63-L65
Change-Id: I638f56c8e21ed7c57ee576cbc23332e676dba7cf
---
M java/kudu-test-utils/src/main/java/org/apache/kudu/test/junit/RetryRule.java
M java/kudu-test-utils/src/test/java/org/apache/kudu/test/junit/TestRetryRule.java
2 files changed, 21 insertions(+), 0 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/07/17307/1
--
To view, visit http://gerrit.cloudera.org:8080/17307
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I638f56c8e21ed7c57ee576cbc23332e676dba7cf
Gerrit-Change-Number: 17307
Gerrit-PatchSet: 1
Gerrit-Owner: Grant Henke <gr...@apache.org>
[kudu-CR] [test] Fix handling of assume statements in Java RetryRule
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/17307 )
Change subject: [test] Fix handling of assume statements in Java RetryRule
......................................................................
Patch Set 3: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/17307
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I638f56c8e21ed7c57ee576cbc23332e676dba7cf
Gerrit-Change-Number: 17307
Gerrit-PatchSet: 3
Gerrit-Owner: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Mon, 12 Apr 2021 20:10:28 +0000
Gerrit-HasComments: No
[kudu-CR] [test] Fix handling of assume statements in Java RetryRule
Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Hello Alexey Serbin, Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/17307
to look at the new patch set (#3).
Change subject: [test] Fix handling of assume statements in Java RetryRule
......................................................................
[test] Fix handling of assume statements in Java RetryRule
The java RetryRule did not treat the AssumptionViolatedException
thrown by the various assume statements in Junit correctly. This
can result in test failures which should actually be skipped tests
and in the case of the flaky test reporting can result in reporting
failed tests which were actually skipped.
This patch adjusts the handling of the AssumptionViolatedException
to skip the test correctly and mark the test as a success when
reporting given there is no “skipped” result we can report to dist-test.
I used the JUnit TestWatcher rule which I used for reference:
https://github.com/junit-team/junit4/blob/main/src/main/java/org/junit/rules/TestWatcher.java#L63-L65
Change-Id: I638f56c8e21ed7c57ee576cbc23332e676dba7cf
---
M java/kudu-test-utils/src/main/java/org/apache/kudu/test/junit/RetryRule.java
M java/kudu-test-utils/src/test/java/org/apache/kudu/test/junit/TestRetryRule.java
2 files changed, 21 insertions(+), 0 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/07/17307/3
--
To view, visit http://gerrit.cloudera.org:8080/17307
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I638f56c8e21ed7c57ee576cbc23332e676dba7cf
Gerrit-Change-Number: 17307
Gerrit-PatchSet: 3
Gerrit-Owner: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] [test] Fix handling of assume statements in Java RetryRule
Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/17307 )
Change subject: [test] Fix handling of assume statements in Java RetryRule
......................................................................
Patch Set 2:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/17307/2//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/17307/2//COMMIT_MSG@10
PS2, Line 10: statments
> typo: statements
Done
http://gerrit.cloudera.org:8080/#/c/17307/2/java/kudu-test-utils/src/main/java/org/apache/kudu/test/junit/RetryRule.java
File java/kudu-test-utils/src/main/java/org/apache/kudu/test/junit/RetryRule.java:
http://gerrit.cloudera.org:8080/#/c/17307/2/java/kudu-test-utils/src/main/java/org/apache/kudu/test/junit/RetryRule.java@193
PS2, Line 193:
> nit: an extra space
Done
--
To view, visit http://gerrit.cloudera.org:8080/17307
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I638f56c8e21ed7c57ee576cbc23332e676dba7cf
Gerrit-Change-Number: 17307
Gerrit-PatchSet: 2
Gerrit-Owner: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Mon, 12 Apr 2021 19:50:23 +0000
Gerrit-HasComments: Yes
[kudu-CR] [test] Fix handling of assume statements in Java RetryRule
Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/17307 )
Change subject: [test] Fix handling of assume statements in Java RetryRule
......................................................................
Patch Set 3: Verified+1
--
To view, visit http://gerrit.cloudera.org:8080/17307
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I638f56c8e21ed7c57ee576cbc23332e676dba7cf
Gerrit-Change-Number: 17307
Gerrit-PatchSet: 3
Gerrit-Owner: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Mon, 12 Apr 2021 21:20:36 +0000
Gerrit-HasComments: No
[kudu-CR] [test] Fix handling of assume statements in Java RetryRule
Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/17307
to look at the new patch set (#2).
Change subject: [test] Fix handling of assume statements in Java RetryRule
......................................................................
[test] Fix handling of assume statements in Java RetryRule
The java RetryRule did not treat the AssumptionViolatedException
thrown by the various assume statments in Junit correctly. This
can result in test failures which should actually be skipped tests
and in the case of the flaky test reporting can result in reporting
failed tests which were actually skipped.
This patch adjusts the handling of the AssumptionViolatedException
to skip the test correctly and mark the test as a success when
reporting given there is no “skipped” result we can report to dist-test.
I used the JUnit TestWatcher rule which I used for reference:
https://github.com/junit-team/junit4/blob/main/src/main/java/org/junit/rules/TestWatcher.java#L63-L65
Change-Id: I638f56c8e21ed7c57ee576cbc23332e676dba7cf
---
M java/kudu-test-utils/src/main/java/org/apache/kudu/test/junit/RetryRule.java
M java/kudu-test-utils/src/test/java/org/apache/kudu/test/junit/TestRetryRule.java
2 files changed, 21 insertions(+), 0 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/07/17307/2
--
To view, visit http://gerrit.cloudera.org:8080/17307
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I638f56c8e21ed7c57ee576cbc23332e676dba7cf
Gerrit-Change-Number: 17307
Gerrit-PatchSet: 2
Gerrit-Owner: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] [test] Fix handling of assume statements in Java RetryRule
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/17307 )
Change subject: [test] Fix handling of assume statements in Java RetryRule
......................................................................
Patch Set 2: Code-Review+2
(2 comments)
Thank you for the fix!
http://gerrit.cloudera.org:8080/#/c/17307/2//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/17307/2//COMMIT_MSG@10
PS2, Line 10: statments
typo: statements
http://gerrit.cloudera.org:8080/#/c/17307/2/java/kudu-test-utils/src/main/java/org/apache/kudu/test/junit/RetryRule.java
File java/kudu-test-utils/src/main/java/org/apache/kudu/test/junit/RetryRule.java:
http://gerrit.cloudera.org:8080/#/c/17307/2/java/kudu-test-utils/src/main/java/org/apache/kudu/test/junit/RetryRule.java@193
PS2, Line 193:
nit: an extra space
--
To view, visit http://gerrit.cloudera.org:8080/17307
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I638f56c8e21ed7c57ee576cbc23332e676dba7cf
Gerrit-Change-Number: 17307
Gerrit-PatchSet: 2
Gerrit-Owner: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Mon, 12 Apr 2021 19:09:26 +0000
Gerrit-HasComments: Yes
[kudu-CR] [test] Fix handling of assume statements in Java RetryRule
Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Grant Henke has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/17307 )
Change subject: [test] Fix handling of assume statements in Java RetryRule
......................................................................
[test] Fix handling of assume statements in Java RetryRule
The java RetryRule did not treat the AssumptionViolatedException
thrown by the various assume statements in Junit correctly. This
can result in test failures which should actually be skipped tests
and in the case of the flaky test reporting can result in reporting
failed tests which were actually skipped.
This patch adjusts the handling of the AssumptionViolatedException
to skip the test correctly and mark the test as a success when
reporting given there is no “skipped” result we can report to dist-test.
I used the JUnit TestWatcher rule which I used for reference:
https://github.com/junit-team/junit4/blob/main/src/main/java/org/junit/rules/TestWatcher.java#L63-L65
Change-Id: I638f56c8e21ed7c57ee576cbc23332e676dba7cf
Reviewed-on: http://gerrit.cloudera.org:8080/17307
Reviewed-by: Alexey Serbin <as...@cloudera.com>
Tested-by: Grant Henke <gr...@apache.org>
---
M java/kudu-test-utils/src/main/java/org/apache/kudu/test/junit/RetryRule.java
M java/kudu-test-utils/src/test/java/org/apache/kudu/test/junit/TestRetryRule.java
2 files changed, 21 insertions(+), 0 deletions(-)
Approvals:
Alexey Serbin: Looks good to me, approved
Grant Henke: Verified
--
To view, visit http://gerrit.cloudera.org:8080/17307
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I638f56c8e21ed7c57ee576cbc23332e676dba7cf
Gerrit-Change-Number: 17307
Gerrit-PatchSet: 4
Gerrit-Owner: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] [test] Fix handling of assume statements in Java RetryRule
Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Grant Henke has removed a vote on this change.
Change subject: [test] Fix handling of assume statements in Java RetryRule
......................................................................
Removed Verified-1 by Kudu Jenkins (120)
--
To view, visit http://gerrit.cloudera.org:8080/17307
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: deleteVote
Gerrit-Change-Id: I638f56c8e21ed7c57ee576cbc23332e676dba7cf
Gerrit-Change-Number: 17307
Gerrit-PatchSet: 3
Gerrit-Owner: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)