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 2018/05/07 15:02:19 UTC
[kudu-CR] [java] Fix TestUtils on Java 9+
Grant Henke has uploaded this change for review. ( http://gerrit.cloudera.org:8080/10324
Change subject: [java] Fix TestUtils on Java 9+
......................................................................
[java] Fix TestUtils on Java 9+
Java 9 made some improvments to the
process API causing expected classnames to
change and break TestUtils.pidOfProcess.
This change looks for both cases of classnames
that could be returned in order to support Java 9+.
Change-Id: Ied4ffdca10dc5a0cdc0516964bf6202fd3871f30
---
M java/kudu-client/src/test/java/org/apache/kudu/client/TestUtils.java
1 file changed, 8 insertions(+), 3 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/24/10324/1
--
To view, visit http://gerrit.cloudera.org:8080/10324
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ied4ffdca10dc5a0cdc0516964bf6202fd3871f30
Gerrit-Change-Number: 10324
Gerrit-PatchSet: 1
Gerrit-Owner: Grant Henke <gr...@apache.org>
[kudu-CR] [java] Fix TestUtils on Java 9+
Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/10324 )
Change subject: [java] Fix TestUtils on Java 9+
......................................................................
Patch Set 2: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/10324
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ied4ffdca10dc5a0cdc0516964bf6202fd3871f30
Gerrit-Change-Number: 10324
Gerrit-PatchSet: 2
Gerrit-Owner: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Mon, 07 May 2018 19:55:28 +0000
Gerrit-HasComments: No
[kudu-CR] [java] Fix TestUtils on Java 9+
Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins, Adar Dembo,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/10324
to look at the new patch set (#2).
Change subject: [java] Fix TestUtils on Java 9+
......................................................................
[java] Fix TestUtils on Java 9+
Java 9 made some improvments to the
process API causing expected classnames to
change and break TestUtils.pidOfProcess.
This change optimistically tries to access the pid field
of any Process passed to TestUtils.pidOfProcess and
throws an exception if the field could not be found
or accessed.
Change-Id: Ied4ffdca10dc5a0cdc0516964bf6202fd3871f30
---
M java/kudu-client/src/test/java/org/apache/kudu/client/TestUtils.java
1 file changed, 9 insertions(+), 8 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/24/10324/2
--
To view, visit http://gerrit.cloudera.org:8080/10324
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ied4ffdca10dc5a0cdc0516964bf6202fd3871f30
Gerrit-Change-Number: 10324
Gerrit-PatchSet: 2
Gerrit-Owner: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
[kudu-CR] [java] Fix TestUtils on Java 9+
Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/10324 )
Change subject: [java] Fix TestUtils on Java 9+
......................................................................
Patch Set 1:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/10324/1/java/kudu-client/src/test/java/org/apache/kudu/client/TestUtils.java
File java/kudu-client/src/test/java/org/apache/kudu/client/TestUtils.java:
http://gerrit.cloudera.org:8080/#/c/10324/1/java/kudu-client/src/test/java/org/apache/kudu/client/TestUtils.java@131
PS1, Line 131: if (!(procCls.getName().equals(UNIX_PROCESS_CLS_NAME) ||
: procCls.getName().equals(PROCESS_IMPL_CLS_NAME))) {
: throw new IllegalArgumentException("stopProcess() expects objects of class " +
: UNIX_PROCESS_CLS_NAME + " or " + PROCESS_IMPL_CLS_NAME + ", but " + procCls.getName() +
: " was passed in instead!");
: }
> Maybe just drop this and let the reflection below throw if the class is all
Yeah, I am onboard with that simplification.
--
To view, visit http://gerrit.cloudera.org:8080/10324
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ied4ffdca10dc5a0cdc0516964bf6202fd3871f30
Gerrit-Change-Number: 10324
Gerrit-PatchSet: 1
Gerrit-Owner: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Mon, 07 May 2018 16:07:07 +0000
Gerrit-HasComments: Yes
[kudu-CR] [java] Fix TestUtils on Java 9+
Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Grant Henke has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/10324 )
Change subject: [java] Fix TestUtils on Java 9+
......................................................................
[java] Fix TestUtils on Java 9+
Java 9 made some improvments to the
process API causing expected classnames to
change and break TestUtils.pidOfProcess.
This change optimistically tries to access the pid field
of any Process passed to TestUtils.pidOfProcess and
throws an exception if the field could not be found
or accessed.
Change-Id: Ied4ffdca10dc5a0cdc0516964bf6202fd3871f30
Reviewed-on: http://gerrit.cloudera.org:8080/10324
Tested-by: Kudu Jenkins
Reviewed-by: Adar Dembo <ad...@cloudera.com>
---
M java/kudu-client/src/test/java/org/apache/kudu/client/TestUtils.java
1 file changed, 9 insertions(+), 8 deletions(-)
Approvals:
Kudu Jenkins: Verified
Adar Dembo: Looks good to me, approved
--
To view, visit http://gerrit.cloudera.org:8080/10324
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: Ied4ffdca10dc5a0cdc0516964bf6202fd3871f30
Gerrit-Change-Number: 10324
Gerrit-PatchSet: 3
Gerrit-Owner: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
[kudu-CR] [java] Fix TestUtils on Java 9+
Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/10324 )
Change subject: [java] Fix TestUtils on Java 9+
......................................................................
Patch Set 1:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/10324/1/java/kudu-client/src/test/java/org/apache/kudu/client/TestUtils.java
File java/kudu-client/src/test/java/org/apache/kudu/client/TestUtils.java:
http://gerrit.cloudera.org:8080/#/c/10324/1/java/kudu-client/src/test/java/org/apache/kudu/client/TestUtils.java@131
PS1, Line 131: if (!(procCls.getName().equals(UNIX_PROCESS_CLS_NAME) ||
: procCls.getName().equals(PROCESS_IMPL_CLS_NAME))) {
: throw new IllegalArgumentException("stopProcess() expects objects of class " +
: UNIX_PROCESS_CLS_NAME + " or " + PROCESS_IMPL_CLS_NAME + ", but " + procCls.getName() +
: " was passed in instead!");
: }
Maybe just drop this and let the reflection below throw if the class is all wrong?
--
To view, visit http://gerrit.cloudera.org:8080/10324
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ied4ffdca10dc5a0cdc0516964bf6202fd3871f30
Gerrit-Change-Number: 10324
Gerrit-PatchSet: 1
Gerrit-Owner: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Mon, 07 May 2018 15:49:04 +0000
Gerrit-HasComments: Yes