You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@hbase.apache.org by GitBox <gi...@apache.org> on 2020/06/29 20:17:51 UTC
[GitHub] [hbase] d-c-manning opened a new pull request #1999: HBASE-24657 fix JsonBean.java from HBASE-23015 HBASE-20571
d-c-manning opened a new pull request #1999:
URL: https://github.com/apache/hbase/pull/1999
[HBASE-24657](https://issues.apache.org/jira/browse/HBASE-24657)
Fixing merge bug from [HBASE-23015](https://issues.apache.org/jira/browse/HBASE-23015) and [HBASE-20571](https://issues.apache.org/jira/browse/HBASE-20571)
To keep the ordering the same between branch-1 and branch-2 for any future refactoring, I just negated the boolean construct.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] Apache-HBase commented on pull request #1999: HBASE-24657 fix JsonBean.java from HBASE-23015 HBASE-20571
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #1999:
URL: https://github.com/apache/hbase/pull/1999#issuecomment-652181794
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 36s | Docker mode activated. |
||| _ Prechecks _ |
| +1 :green_heart: | dupname | 0m 0s | No case conflicting files found. |
| +1 :green_heart: | hbaseanti | 0m 0s | Patch does not have any anti-patterns. |
| +1 :green_heart: | @author | 0m 0s | The patch does not contain any @author tags. |
| +1 :green_heart: | test4tests | 0m 0s | The patch appears to include 1 new or modified test files. |
||| _ branch-1 Compile Tests _ |
| +1 :green_heart: | mvninstall | 9m 52s | branch-1 passed |
| +1 :green_heart: | compile | 0m 39s | branch-1 passed with JDK v1.8.0_252 |
| +1 :green_heart: | compile | 0m 44s | branch-1 passed with JDK v1.7.0_262 |
| +1 :green_heart: | checkstyle | 1m 40s | branch-1 passed |
| +1 :green_heart: | shadedjars | 3m 0s | branch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 47s | branch-1 passed with JDK v1.8.0_252 |
| +1 :green_heart: | javadoc | 0m 40s | branch-1 passed with JDK v1.7.0_262 |
| +0 :ok: | spotbugs | 2m 58s | Used deprecated FindBugs config; considering switching to SpotBugs. |
| +1 :green_heart: | findbugs | 2m 55s | branch-1 passed |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 1m 51s | the patch passed |
| +1 :green_heart: | compile | 0m 39s | the patch passed with JDK v1.8.0_252 |
| +1 :green_heart: | javac | 0m 39s | the patch passed |
| +1 :green_heart: | compile | 0m 43s | the patch passed with JDK v1.7.0_262 |
| +1 :green_heart: | javac | 0m 43s | the patch passed |
| -1 :x: | checkstyle | 1m 31s | hbase-server: The patch generated 2 new + 9 unchanged - 0 fixed = 11 total (was 9) |
| +1 :green_heart: | whitespace | 0m 0s | The patch has no whitespace issues. |
| +1 :green_heart: | shadedjars | 2m 47s | patch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | hadoopcheck | 4m 34s | Patch does not cause any errors with Hadoop 2.8.5 2.9.2. |
| +1 :green_heart: | javadoc | 0m 30s | the patch passed with JDK v1.8.0_252 |
| +1 :green_heart: | javadoc | 0m 40s | the patch passed with JDK v1.7.0_262 |
| +1 :green_heart: | findbugs | 2m 51s | the patch passed |
||| _ Other Tests _ |
| -1 :x: | unit | 128m 38s | hbase-server in the patch failed. |
| +1 :green_heart: | asflicense | 0m 37s | The patch does not generate ASF License warnings. |
| | | 169m 40s | |
| Reason | Tests |
|-------:|:------|
| Failed junit tests | hadoop.hbase.client.TestAdmin2 |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | Client=19.03.12 Server=19.03.12 base: https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1999/5/artifact/out/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/1999 |
| Optional Tests | dupname asflicense javac javadoc unit spotbugs findbugs shadedjars hadoopcheck hbaseanti checkstyle compile |
| uname | Linux f48ac15dbad6 4.15.0-58-generic #64-Ubuntu SMP Tue Aug 6 11:12:41 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | /home/jenkins/jenkins-slave/workspace/Base-PreCommit-GitHub-PR_PR-1999/out/precommit/personality/provided.sh |
| git revision | branch-1 / 32690e1 |
| Default Java | 1.7.0_262 |
| Multi-JDK versions | /usr/lib/jvm/zulu-8-amd64:1.8.0_252 /usr/lib/jvm/zulu-7-amd64:1.7.0_262 |
| checkstyle | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1999/5/artifact/out/diff-checkstyle-hbase-server.txt |
| unit | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1999/5/artifact/out/patch-unit-hbase-server.txt |
| Test Results | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1999/5/testReport/ |
| Max. process+thread count | 5077 (vs. ulimit of 10000) |
| modules | C: hbase-server U: hbase-server |
| Console output | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1999/5/console |
| versions | git=1.9.1 maven=3.0.5 findbugs=3.0.1 |
| Powered by | Apache Yetus 0.11.1 https://yetus.apache.org |
This message was automatically generated.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] d-c-manning edited a comment on pull request #1999: HBASE-24657 fix JsonBean.java from HBASE-23015 HBASE-20571
Posted by GitBox <gi...@apache.org>.
d-c-manning edited a comment on pull request #1999:
URL: https://github.com/apache/hbase/pull/1999#issuecomment-651645788
> Meanwhile, we can have one additional PR for master branch with test coverage. I will take care of all backports.
Shall I create that PR @virajjasani or will you?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] Apache-HBase commented on pull request #1999: HBASE-24657 fix JsonBean.java from HBASE-23015 HBASE-20571
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #1999:
URL: https://github.com/apache/hbase/pull/1999#issuecomment-652281201
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 6m 53s | Docker mode activated. |
||| _ Prechecks _ |
| +1 :green_heart: | dupname | 0m 0s | No case conflicting files found. |
| +1 :green_heart: | hbaseanti | 0m 0s | Patch does not have any anti-patterns. |
| +1 :green_heart: | @author | 0m 0s | The patch does not contain any @author tags. |
| +1 :green_heart: | test4tests | 0m 0s | The patch appears to include 1 new or modified test files. |
||| _ branch-1 Compile Tests _ |
| +1 :green_heart: | mvninstall | 9m 57s | branch-1 passed |
| +1 :green_heart: | compile | 0m 39s | branch-1 passed with JDK v1.8.0_252 |
| +1 :green_heart: | compile | 0m 45s | branch-1 passed with JDK v1.7.0_262 |
| +1 :green_heart: | checkstyle | 1m 43s | branch-1 passed |
| +1 :green_heart: | shadedjars | 3m 3s | branch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 48s | branch-1 passed with JDK v1.8.0_252 |
| +1 :green_heart: | javadoc | 0m 40s | branch-1 passed with JDK v1.7.0_262 |
| +0 :ok: | spotbugs | 3m 2s | Used deprecated FindBugs config; considering switching to SpotBugs. |
| +1 :green_heart: | findbugs | 3m 0s | branch-1 passed |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 1m 55s | the patch passed |
| +1 :green_heart: | compile | 0m 40s | the patch passed with JDK v1.8.0_252 |
| +1 :green_heart: | javac | 0m 40s | the patch passed |
| +1 :green_heart: | compile | 0m 44s | the patch passed with JDK v1.7.0_262 |
| +1 :green_heart: | javac | 0m 44s | the patch passed |
| +1 :green_heart: | checkstyle | 1m 28s | the patch passed |
| +1 :green_heart: | whitespace | 0m 0s | The patch has no whitespace issues. |
| +1 :green_heart: | shadedjars | 2m 48s | patch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | hadoopcheck | 4m 33s | Patch does not cause any errors with Hadoop 2.8.5 2.9.2. |
| +1 :green_heart: | javadoc | 0m 32s | the patch passed with JDK v1.8.0_252 |
| +1 :green_heart: | javadoc | 0m 40s | the patch passed with JDK v1.7.0_262 |
| +1 :green_heart: | findbugs | 2m 51s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | unit | 129m 2s | hbase-server in the patch passed. |
| +1 :green_heart: | asflicense | 0m 38s | The patch does not generate ASF License warnings. |
| | | 176m 52s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | Client=19.03.12 Server=19.03.12 base: https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1999/6/artifact/out/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/1999 |
| Optional Tests | dupname asflicense javac javadoc unit spotbugs findbugs shadedjars hadoopcheck hbaseanti checkstyle compile |
| uname | Linux 789a13661ce7 4.15.0-60-generic #67-Ubuntu SMP Thu Aug 22 16:55:30 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | /home/jenkins/jenkins-slave/workspace/Base-PreCommit-GitHub-PR_PR-1999/out/precommit/personality/provided.sh |
| git revision | branch-1 / 32690e1 |
| Default Java | 1.7.0_262 |
| Multi-JDK versions | /usr/lib/jvm/zulu-8-amd64:1.8.0_252 /usr/lib/jvm/zulu-7-amd64:1.7.0_262 |
| Test Results | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1999/6/testReport/ |
| Max. process+thread count | 4610 (vs. ulimit of 10000) |
| modules | C: hbase-server U: hbase-server |
| Console output | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1999/6/console |
| versions | git=1.9.1 maven=3.0.5 findbugs=3.0.1 |
| Powered by | Apache Yetus 0.11.1 https://yetus.apache.org |
This message was automatically generated.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] bharathv commented on a change in pull request #1999: HBASE-24657 fix JsonBean.java from HBASE-23015 HBASE-20571
Posted by GitBox <gi...@apache.org>.
bharathv commented on a change in pull request #1999:
URL: https://github.com/apache/hbase/pull/1999#discussion_r447291625
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/util/JSONBean.java
##########
@@ -334,7 +334,7 @@ private static void writeObject(final JsonWriter writer, final Object value) thr
} else if(value instanceof Number) {
Number n = (Number)value;
double doubleValue = n.doubleValue();
- if (Double.isNaN(doubleValue) || Double.isInfinite(doubleValue)) {
+ if (!Double.isNaN(doubleValue) && !Double.isInfinite(doubleValue)) {
Review comment:
I think its fine to do it under the same jira since they are all related.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] Apache-HBase commented on pull request #1999: HBASE-24657 fix JsonBean.java from HBASE-23015 HBASE-20571
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #1999:
URL: https://github.com/apache/hbase/pull/1999#issuecomment-651721708
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 35s | Docker mode activated. |
||| _ Prechecks _ |
| +1 :green_heart: | dupname | 0m 0s | No case conflicting files found. |
| +1 :green_heart: | hbaseanti | 0m 0s | Patch does not have any anti-patterns. |
| +1 :green_heart: | @author | 0m 0s | The patch does not contain any @author tags. |
| +1 :green_heart: | test4tests | 0m 0s | The patch appears to include 1 new or modified test files. |
||| _ branch-1 Compile Tests _ |
| +1 :green_heart: | mvninstall | 9m 43s | branch-1 passed |
| +1 :green_heart: | compile | 0m 39s | branch-1 passed with JDK v1.8.0_252 |
| +1 :green_heart: | compile | 0m 44s | branch-1 passed with JDK v1.7.0_262 |
| +1 :green_heart: | checkstyle | 1m 44s | branch-1 passed |
| +1 :green_heart: | shadedjars | 3m 4s | branch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 47s | branch-1 passed with JDK v1.8.0_252 |
| +1 :green_heart: | javadoc | 0m 40s | branch-1 passed with JDK v1.7.0_262 |
| +0 :ok: | spotbugs | 3m 0s | Used deprecated FindBugs config; considering switching to SpotBugs. |
| +1 :green_heart: | findbugs | 2m 57s | branch-1 passed |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 1m 51s | the patch passed |
| +1 :green_heart: | compile | 0m 39s | the patch passed with JDK v1.8.0_252 |
| +1 :green_heart: | javac | 0m 39s | the patch passed |
| +1 :green_heart: | compile | 0m 46s | the patch passed with JDK v1.7.0_262 |
| +1 :green_heart: | javac | 0m 46s | the patch passed |
| +1 :green_heart: | checkstyle | 1m 28s | the patch passed |
| +1 :green_heart: | whitespace | 0m 0s | The patch has no whitespace issues. |
| +1 :green_heart: | shadedjars | 2m 49s | patch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | hadoopcheck | 4m 36s | Patch does not cause any errors with Hadoop 2.8.5 2.9.2. |
| +1 :green_heart: | javadoc | 0m 29s | the patch passed with JDK v1.8.0_252 |
| +1 :green_heart: | javadoc | 0m 40s | the patch passed with JDK v1.7.0_262 |
| +1 :green_heart: | findbugs | 2m 48s | the patch passed |
||| _ Other Tests _ |
| -1 :x: | unit | 129m 13s | hbase-server in the patch failed. |
| +1 :green_heart: | asflicense | 0m 38s | The patch does not generate ASF License warnings. |
| | | 170m 17s | |
| Reason | Tests |
|-------:|:------|
| Failed junit tests | hadoop.hbase.client.TestAdmin2 |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | Client=19.03.12 Server=19.03.12 base: https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1999/3/artifact/out/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/1999 |
| Optional Tests | dupname asflicense javac javadoc unit spotbugs findbugs shadedjars hadoopcheck hbaseanti checkstyle compile |
| uname | Linux 39d3be92ed7d 4.15.0-58-generic #64-Ubuntu SMP Tue Aug 6 11:12:41 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | /home/jenkins/jenkins-slave/workspace/Base-PreCommit-GitHub-PR_PR-1999/out/precommit/personality/provided.sh |
| git revision | branch-1 / 32690e1 |
| Default Java | 1.7.0_262 |
| Multi-JDK versions | /usr/lib/jvm/zulu-8-amd64:1.8.0_252 /usr/lib/jvm/zulu-7-amd64:1.7.0_262 |
| unit | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1999/3/artifact/out/patch-unit-hbase-server.txt |
| Test Results | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1999/3/testReport/ |
| Max. process+thread count | 4881 (vs. ulimit of 10000) |
| modules | C: hbase-server U: hbase-server |
| Console output | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1999/3/console |
| versions | git=1.9.1 maven=3.0.5 findbugs=3.0.1 |
| Powered by | Apache Yetus 0.11.1 https://yetus.apache.org |
This message was automatically generated.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] Apache-HBase commented on pull request #1999: HBASE-24657 fix JsonBean.java from HBASE-23015 HBASE-20571
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #1999:
URL: https://github.com/apache/hbase/pull/1999#issuecomment-651417235
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 6m 48s | Docker mode activated. |
||| _ Prechecks _ |
| +1 :green_heart: | dupname | 0m 0s | No case conflicting files found. |
| +1 :green_heart: | hbaseanti | 0m 0s | Patch does not have any anti-patterns. |
| +1 :green_heart: | @author | 0m 0s | The patch does not contain any @author tags. |
| -0 :warning: | test4tests | 0m 0s | The patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch. |
||| _ branch-1 Compile Tests _ |
| +1 :green_heart: | mvninstall | 10m 0s | branch-1 passed |
| +1 :green_heart: | compile | 0m 39s | branch-1 passed with JDK v1.8.0_252 |
| +1 :green_heart: | compile | 0m 45s | branch-1 passed with JDK v1.7.0_262 |
| +1 :green_heart: | checkstyle | 1m 42s | branch-1 passed |
| +1 :green_heart: | shadedjars | 3m 3s | branch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 47s | branch-1 passed with JDK v1.8.0_252 |
| +1 :green_heart: | javadoc | 0m 40s | branch-1 passed with JDK v1.7.0_262 |
| +0 :ok: | spotbugs | 3m 1s | Used deprecated FindBugs config; considering switching to SpotBugs. |
| +1 :green_heart: | findbugs | 2m 58s | branch-1 passed |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 1m 55s | the patch passed |
| +1 :green_heart: | compile | 0m 40s | the patch passed with JDK v1.8.0_252 |
| +1 :green_heart: | javac | 0m 40s | the patch passed |
| +1 :green_heart: | compile | 0m 44s | the patch passed with JDK v1.7.0_262 |
| +1 :green_heart: | javac | 0m 44s | the patch passed |
| +1 :green_heart: | checkstyle | 1m 31s | the patch passed |
| +1 :green_heart: | whitespace | 0m 0s | The patch has no whitespace issues. |
| +1 :green_heart: | shadedjars | 2m 49s | patch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | hadoopcheck | 4m 37s | Patch does not cause any errors with Hadoop 2.8.5 2.9.2. |
| +1 :green_heart: | javadoc | 0m 29s | the patch passed with JDK v1.8.0_252 |
| +1 :green_heart: | javadoc | 0m 40s | the patch passed with JDK v1.7.0_262 |
| +1 :green_heart: | findbugs | 2m 54s | the patch passed |
||| _ Other Tests _ |
| -1 :x: | unit | 128m 34s | hbase-server in the patch passed. |
| +1 :green_heart: | asflicense | 0m 36s | The patch does not generate ASF License warnings. |
| | | 176m 28s | |
| Reason | Tests |
|-------:|:------|
| Timed out junit tests | org.apache.hadoop.hbase.master.TestGetInfoPort |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | Client=19.03.12 Server=19.03.12 base: https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1999/1/artifact/out/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/1999 |
| Optional Tests | dupname asflicense javac javadoc unit spotbugs findbugs shadedjars hadoopcheck hbaseanti checkstyle compile |
| uname | Linux 06f805af0a59 4.15.0-58-generic #64-Ubuntu SMP Tue Aug 6 11:12:41 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | /home/jenkins/jenkins-slave/workspace/Base-PreCommit-GitHub-PR_PR-1999/out/precommit/personality/provided.sh |
| git revision | branch-1 / 32690e1 |
| Default Java | 1.7.0_262 |
| Multi-JDK versions | /usr/lib/jvm/zulu-8-amd64:1.8.0_252 /usr/lib/jvm/zulu-7-amd64:1.7.0_262 |
| unit | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1999/1/artifact/out/patch-unit-hbase-server.txt |
| Test Results | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1999/1/testReport/ |
| Max. process+thread count | 4923 (vs. ulimit of 10000) |
| modules | C: hbase-server U: hbase-server |
| Console output | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1999/1/console |
| versions | git=1.9.1 maven=3.0.5 findbugs=3.0.1 |
| Powered by | Apache Yetus 0.11.1 https://yetus.apache.org |
This message was automatically generated.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] d-c-manning commented on pull request #1999: HBASE-24657 fix JsonBean.java from HBASE-23015 HBASE-20571
Posted by GitBox <gi...@apache.org>.
d-c-manning commented on pull request #1999:
URL: https://github.com/apache/hbase/pull/1999#issuecomment-652085805
@virajjasani I created a new PR to add only the test to `master`: https://github.com/apache/hbase/pull/2004 - hopefully it would also cleanly apply to `branch-2`.
This PR will still be required for `branch-1`, as it has the fix for `JSONBean.java` and the same test cannot run in both `master` and `branch-1` without changes.
I made some changes to more closely align the tests (getting rid of dependency on `ObjectMapper` and replacing with `gson`) but there are still differences.
This should be good to go now, using these two PRs.
FYI @bharathv @apurtell
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] bharathv commented on a change in pull request #1999: HBASE-24657 fix JsonBean.java from HBASE-23015 HBASE-20571
Posted by GitBox <gi...@apache.org>.
bharathv commented on a change in pull request #1999:
URL: https://github.com/apache/hbase/pull/1999#discussion_r447251613
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/util/JSONBean.java
##########
@@ -334,7 +334,7 @@ private static void writeObject(final JsonWriter writer, final Object value) thr
} else if(value instanceof Number) {
Number n = (Number)value;
double doubleValue = n.doubleValue();
- if (Double.isNaN(doubleValue) || Double.isInfinite(doubleValue)) {
+ if (!Double.isNaN(doubleValue) && !Double.isInfinite(doubleValue)) {
Review comment:
Make the check consistent with other branches using Double.isFinite()? That seems simple.
Also, it looks like there is some test gap here? Mind adding a quick test?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] Apache-HBase commented on pull request #1999: HBASE-24657 fix JsonBean.java from HBASE-23015 HBASE-20571
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #1999:
URL: https://github.com/apache/hbase/pull/1999#issuecomment-651525200
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 35s | Docker mode activated. |
||| _ Prechecks _ |
| +1 :green_heart: | dupname | 0m 1s | No case conflicting files found. |
| +1 :green_heart: | hbaseanti | 0m 0s | Patch does not have any anti-patterns. |
| +1 :green_heart: | @author | 0m 0s | The patch does not contain any @author tags. |
| +1 :green_heart: | test4tests | 0m 0s | The patch appears to include 1 new or modified test files. |
||| _ branch-1 Compile Tests _ |
| +1 :green_heart: | mvninstall | 9m 50s | branch-1 passed |
| +1 :green_heart: | compile | 0m 41s | branch-1 passed with JDK v1.8.0_252 |
| +1 :green_heart: | compile | 0m 45s | branch-1 passed with JDK v1.7.0_262 |
| +1 :green_heart: | checkstyle | 1m 40s | branch-1 passed |
| +1 :green_heart: | shadedjars | 2m 59s | branch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 47s | branch-1 passed with JDK v1.8.0_252 |
| +1 :green_heart: | javadoc | 0m 39s | branch-1 passed with JDK v1.7.0_262 |
| +0 :ok: | spotbugs | 2m 59s | Used deprecated FindBugs config; considering switching to SpotBugs. |
| +1 :green_heart: | findbugs | 2m 56s | branch-1 passed |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 1m 55s | the patch passed |
| +1 :green_heart: | compile | 0m 41s | the patch passed with JDK v1.8.0_252 |
| +1 :green_heart: | javac | 0m 41s | the patch passed |
| +1 :green_heart: | compile | 0m 45s | the patch passed with JDK v1.7.0_262 |
| +1 :green_heart: | javac | 0m 45s | the patch passed |
| +1 :green_heart: | checkstyle | 1m 33s | the patch passed |
| +1 :green_heart: | whitespace | 0m 0s | The patch has no whitespace issues. |
| +1 :green_heart: | shadedjars | 2m 51s | patch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | hadoopcheck | 4m 38s | Patch does not cause any errors with Hadoop 2.8.5 2.9.2. |
| +1 :green_heart: | javadoc | 0m 31s | the patch passed with JDK v1.8.0_252 |
| +1 :green_heart: | javadoc | 0m 40s | the patch passed with JDK v1.7.0_262 |
| +1 :green_heart: | findbugs | 2m 49s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | unit | 128m 44s | hbase-server in the patch passed. |
| +1 :green_heart: | asflicense | 0m 38s | The patch does not generate ASF License warnings. |
| | | 169m 59s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | Client=19.03.12 Server=19.03.12 base: https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1999/2/artifact/out/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/1999 |
| Optional Tests | dupname asflicense javac javadoc unit spotbugs findbugs shadedjars hadoopcheck hbaseanti checkstyle compile |
| uname | Linux 7d057328a0b6 4.15.0-58-generic #64-Ubuntu SMP Tue Aug 6 11:12:41 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | /home/jenkins/jenkins-slave/workspace/Base-PreCommit-GitHub-PR_PR-1999/out/precommit/personality/provided.sh |
| git revision | branch-1 / 32690e1 |
| Default Java | 1.7.0_262 |
| Multi-JDK versions | /usr/lib/jvm/zulu-8-amd64:1.8.0_252 /usr/lib/jvm/zulu-7-amd64:1.7.0_262 |
| Test Results | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1999/2/testReport/ |
| Max. process+thread count | 4892 (vs. ulimit of 10000) |
| modules | C: hbase-server U: hbase-server |
| Console output | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1999/2/console |
| versions | git=1.9.1 maven=3.0.5 findbugs=3.0.1 |
| Powered by | Apache Yetus 0.11.1 https://yetus.apache.org |
This message was automatically generated.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] d-c-manning commented on a change in pull request #1999: HBASE-24657 fix JsonBean.java from HBASE-23015 HBASE-20571
Posted by GitBox <gi...@apache.org>.
d-c-manning commented on a change in pull request #1999:
URL: https://github.com/apache/hbase/pull/1999#discussion_r447491922
##########
File path: hbase-server/src/test/java/org/apache/hadoop/hbase/util/TestJSONBean.java
##########
@@ -0,0 +1,104 @@
+/**
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.hadoop.hbase.util;
+
+import static org.junit.Assert.assertEquals;
+import static org.mockito.Matchers.eq;
+import static org.mockito.Mockito.any;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
+import java.io.PrintWriter;
+import java.io.StringWriter;
+import java.util.HashSet;
+import java.util.Set;
+import javax.management.MBeanAttributeInfo;
+import javax.management.MBeanInfo;
+import javax.management.MBeanServer;
+import javax.management.ObjectName;
+import javax.management.QueryExp;
+import org.apache.hadoop.hbase.testclassification.SmallTests;
+import org.codehaus.jackson.map.ObjectMapper;
+import org.junit.Test;
+import org.junit.experimental.categories.Category;
+
+/**
+ * Test {@link JSONBean}.
+ */
+@Category(SmallTests.class)
+public class TestJSONBean {
+ private MBeanServer getMockMBeanServer() throws Exception {
+ MBeanServer mbeanServer = mock(MBeanServer.class);
+ Set<ObjectName> names = new HashSet<>();
+ names.add(new ObjectName("test1:type=test2"));
+ when(mbeanServer.queryNames(any(ObjectName.class), any(QueryExp.class))).thenReturn(names);
+ MBeanInfo mbeanInfo = mock(MBeanInfo.class);
+ when(mbeanInfo.getClassName()).thenReturn("testClassName");
+ String[] attributeNames = new String[] {"intAttr", "nanAttr", "strAttr", "boolAttr"};
+ MBeanAttributeInfo[] attributeInfos = new MBeanAttributeInfo[attributeNames.length];
+ for (int i = 0; i < attributeInfos.length; i++) {
+ attributeInfos[i] = new MBeanAttributeInfo(attributeNames[i],
+ null,
+ null,
+ true,
+ false,
+ false);
+ }
+ when(mbeanInfo.getAttributes()).thenReturn(attributeInfos);
+ when(mbeanServer.getMBeanInfo(any(ObjectName.class))).thenReturn(mbeanInfo);
+ when(mbeanServer.getAttribute(any(ObjectName.class), eq("intAttr"))).thenReturn(3);
Review comment:
I added the `Infinity` as a separate attribute in the same test. Hopefully that's okay.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] bharathv commented on a change in pull request #1999: HBASE-24657 fix JsonBean.java from HBASE-23015 HBASE-20571
Posted by GitBox <gi...@apache.org>.
bharathv commented on a change in pull request #1999:
URL: https://github.com/apache/hbase/pull/1999#discussion_r447276364
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/util/JSONBean.java
##########
@@ -334,7 +334,7 @@ private static void writeObject(final JsonWriter writer, final Object value) thr
} else if(value instanceof Number) {
Number n = (Number)value;
double doubleValue = n.doubleValue();
- if (Double.isNaN(doubleValue) || Double.isInfinite(doubleValue)) {
+ if (!Double.isNaN(doubleValue) && !Double.isInfinite(doubleValue)) {
Review comment:
Oh sorry, didn't realize isFinite() is Java 8 and above.
> I am unfamiliar with the code, so it may take a while to find where to add.
A simple unit test, create a json using this JSONBean utilities and validate that it parses as expected?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] d-c-manning commented on pull request #1999: HBASE-24657 fix JsonBean.java from HBASE-23015 HBASE-20571
Posted by GitBox <gi...@apache.org>.
d-c-manning commented on pull request #1999:
URL: https://github.com/apache/hbase/pull/1999#issuecomment-651645788
> Meanwhile, we can have one additional PR for master branch with test coverage. I will take care of all backports.
Shall I create that PR?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] virajjasani commented on pull request #1999: HBASE-24657 fix JsonBean.java from HBASE-23015 HBASE-20571
Posted by GitBox <gi...@apache.org>.
virajjasani commented on pull request #1999:
URL: https://github.com/apache/hbase/pull/1999#issuecomment-651713706
> > Meanwhile, we can have one additional PR for master branch with test coverage. I will take care of all backports.
>
> Shall I create that PR @virajjasani or will you?
You can create as per you convenience because the order for commit is master -> branch-2 -> all active branch-2.x -> branch-1 :)
Also, in the meanwhile, @bharathv will also get time to review this PR. Once we have approval, we will go in the same commit order from master till branch-1.
Although this was going to be branch-1 specific PR but since we are adding coverage, we can first commit in master.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] virajjasani closed pull request #1999: HBASE-24657 fix JsonBean.java from HBASE-23015 HBASE-20571
Posted by GitBox <gi...@apache.org>.
virajjasani closed pull request #1999:
URL: https://github.com/apache/hbase/pull/1999
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] Apache-HBase commented on pull request #1999: HBASE-24657 fix JsonBean.java from HBASE-23015 HBASE-20571
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #1999:
URL: https://github.com/apache/hbase/pull/1999#issuecomment-652135045
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 1m 6s | Docker mode activated. |
||| _ Prechecks _ |
| +1 :green_heart: | dupname | 0m 0s | No case conflicting files found. |
| +1 :green_heart: | hbaseanti | 0m 0s | Patch does not have any anti-patterns. |
| +1 :green_heart: | @author | 0m 0s | The patch does not contain any @author tags. |
| +1 :green_heart: | test4tests | 0m 0s | The patch appears to include 1 new or modified test files. |
||| _ branch-1 Compile Tests _ |
| +1 :green_heart: | mvninstall | 10m 9s | branch-1 passed |
| +1 :green_heart: | compile | 0m 51s | branch-1 passed with JDK v1.8.0_252 |
| +1 :green_heart: | compile | 0m 59s | branch-1 passed with JDK v1.7.0_262 |
| +1 :green_heart: | checkstyle | 2m 10s | branch-1 passed |
| +1 :green_heart: | shadedjars | 3m 42s | branch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 57s | branch-1 passed with JDK v1.8.0_252 |
| +1 :green_heart: | javadoc | 0m 49s | branch-1 passed with JDK v1.7.0_262 |
| +0 :ok: | spotbugs | 3m 54s | Used deprecated FindBugs config; considering switching to SpotBugs. |
| +1 :green_heart: | findbugs | 3m 51s | branch-1 passed |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 2m 16s | the patch passed |
| +1 :green_heart: | compile | 0m 48s | the patch passed with JDK v1.8.0_252 |
| +1 :green_heart: | javac | 0m 48s | the patch passed |
| +1 :green_heart: | compile | 0m 56s | the patch passed with JDK v1.7.0_262 |
| +1 :green_heart: | javac | 0m 56s | the patch passed |
| -1 :x: | checkstyle | 1m 53s | hbase-server: The patch generated 2 new + 9 unchanged - 0 fixed = 11 total (was 9) |
| +1 :green_heart: | whitespace | 0m 0s | The patch has no whitespace issues. |
| +1 :green_heart: | shadedjars | 3m 28s | patch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | hadoopcheck | 5m 29s | Patch does not cause any errors with Hadoop 2.8.5 2.9.2. |
| +1 :green_heart: | javadoc | 0m 40s | the patch passed with JDK v1.8.0_252 |
| +1 :green_heart: | javadoc | 0m 47s | the patch passed with JDK v1.7.0_262 |
| +1 :green_heart: | findbugs | 3m 37s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | unit | 130m 26s | hbase-server in the patch passed. |
| +1 :green_heart: | asflicense | 0m 37s | The patch does not generate ASF License warnings. |
| | | 179m 15s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | Client=19.03.12 Server=19.03.12 base: https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1999/4/artifact/out/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/1999 |
| Optional Tests | dupname asflicense javac javadoc unit spotbugs findbugs shadedjars hadoopcheck hbaseanti checkstyle compile |
| uname | Linux 729bcfcc81de 4.15.0-58-generic #64-Ubuntu SMP Tue Aug 6 11:12:41 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | /home/jenkins/jenkins-slave/workspace/Base-PreCommit-GitHub-PR_PR-1999/out/precommit/personality/provided.sh |
| git revision | branch-1 / 32690e1 |
| Default Java | 1.7.0_262 |
| Multi-JDK versions | /usr/lib/jvm/zulu-8-amd64:1.8.0_252 /usr/lib/jvm/zulu-7-amd64:1.7.0_262 |
| checkstyle | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1999/4/artifact/out/diff-checkstyle-hbase-server.txt |
| Test Results | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1999/4/testReport/ |
| Max. process+thread count | 4893 (vs. ulimit of 10000) |
| modules | C: hbase-server U: hbase-server |
| Console output | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1999/4/console |
| versions | git=1.9.1 maven=3.0.5 findbugs=3.0.1 |
| Powered by | Apache Yetus 0.11.1 https://yetus.apache.org |
This message was automatically generated.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] virajjasani commented on pull request #1999: HBASE-24657 fix JsonBean.java from HBASE-23015 HBASE-20571
Posted by GitBox <gi...@apache.org>.
virajjasani commented on pull request #1999:
URL: https://github.com/apache/hbase/pull/1999#issuecomment-651643759
Meanwhile, we can have one additional PR for master branch with test coverage. I will take care of all backports.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] virajjasani commented on a change in pull request #1999: HBASE-24657 fix JsonBean.java from HBASE-23015 HBASE-20571
Posted by GitBox <gi...@apache.org>.
virajjasani commented on a change in pull request #1999:
URL: https://github.com/apache/hbase/pull/1999#discussion_r447508191
##########
File path: hbase-server/src/test/java/org/apache/hadoop/hbase/util/TestJSONBean.java
##########
@@ -0,0 +1,104 @@
+/**
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.hadoop.hbase.util;
+
+import static org.junit.Assert.assertEquals;
+import static org.mockito.Matchers.eq;
+import static org.mockito.Mockito.any;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
+import java.io.PrintWriter;
+import java.io.StringWriter;
+import java.util.HashSet;
+import java.util.Set;
+import javax.management.MBeanAttributeInfo;
+import javax.management.MBeanInfo;
+import javax.management.MBeanServer;
+import javax.management.ObjectName;
+import javax.management.QueryExp;
+import org.apache.hadoop.hbase.testclassification.SmallTests;
+import org.codehaus.jackson.map.ObjectMapper;
+import org.junit.Test;
+import org.junit.experimental.categories.Category;
+
+/**
+ * Test {@link JSONBean}.
+ */
+@Category(SmallTests.class)
+public class TestJSONBean {
+ private MBeanServer getMockMBeanServer() throws Exception {
+ MBeanServer mbeanServer = mock(MBeanServer.class);
+ Set<ObjectName> names = new HashSet<>();
+ names.add(new ObjectName("test1:type=test2"));
+ when(mbeanServer.queryNames(any(ObjectName.class), any(QueryExp.class))).thenReturn(names);
+ MBeanInfo mbeanInfo = mock(MBeanInfo.class);
+ when(mbeanInfo.getClassName()).thenReturn("testClassName");
+ String[] attributeNames = new String[] {"intAttr", "nanAttr", "strAttr", "boolAttr"};
+ MBeanAttributeInfo[] attributeInfos = new MBeanAttributeInfo[attributeNames.length];
+ for (int i = 0; i < attributeInfos.length; i++) {
+ attributeInfos[i] = new MBeanAttributeInfo(attributeNames[i],
+ null,
+ null,
+ true,
+ false,
+ false);
+ }
+ when(mbeanInfo.getAttributes()).thenReturn(attributeInfos);
+ when(mbeanServer.getMBeanInfo(any(ObjectName.class))).thenReturn(mbeanInfo);
+ when(mbeanServer.getAttribute(any(ObjectName.class), eq("intAttr"))).thenReturn(3);
Review comment:
Oh yes, much better
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] virajjasani commented on a change in pull request #1999: HBASE-24657 fix JsonBean.java from HBASE-23015 HBASE-20571
Posted by GitBox <gi...@apache.org>.
virajjasani commented on a change in pull request #1999:
URL: https://github.com/apache/hbase/pull/1999#discussion_r447476731
##########
File path: hbase-server/src/test/java/org/apache/hadoop/hbase/util/TestJSONBean.java
##########
@@ -0,0 +1,104 @@
+/**
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.hadoop.hbase.util;
+
+import static org.junit.Assert.assertEquals;
+import static org.mockito.Matchers.eq;
+import static org.mockito.Mockito.any;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
+import java.io.PrintWriter;
+import java.io.StringWriter;
+import java.util.HashSet;
+import java.util.Set;
+import javax.management.MBeanAttributeInfo;
+import javax.management.MBeanInfo;
+import javax.management.MBeanServer;
+import javax.management.ObjectName;
+import javax.management.QueryExp;
+import org.apache.hadoop.hbase.testclassification.SmallTests;
+import org.codehaus.jackson.map.ObjectMapper;
+import org.junit.Test;
+import org.junit.experimental.categories.Category;
+
+/**
+ * Test {@link JSONBean}.
+ */
+@Category(SmallTests.class)
+public class TestJSONBean {
+ private MBeanServer getMockMBeanServer() throws Exception {
+ MBeanServer mbeanServer = mock(MBeanServer.class);
+ Set<ObjectName> names = new HashSet<>();
+ names.add(new ObjectName("test1:type=test2"));
+ when(mbeanServer.queryNames(any(ObjectName.class), any(QueryExp.class))).thenReturn(names);
+ MBeanInfo mbeanInfo = mock(MBeanInfo.class);
+ when(mbeanInfo.getClassName()).thenReturn("testClassName");
+ String[] attributeNames = new String[] {"intAttr", "nanAttr", "strAttr", "boolAttr"};
+ MBeanAttributeInfo[] attributeInfos = new MBeanAttributeInfo[attributeNames.length];
+ for (int i = 0; i < attributeInfos.length; i++) {
+ attributeInfos[i] = new MBeanAttributeInfo(attributeNames[i],
+ null,
+ null,
+ true,
+ false,
+ false);
+ }
+ when(mbeanInfo.getAttributes()).thenReturn(attributeInfos);
+ when(mbeanServer.getMBeanInfo(any(ObjectName.class))).thenReturn(mbeanInfo);
+ when(mbeanServer.getAttribute(any(ObjectName.class), eq("intAttr"))).thenReturn(3);
Review comment:
Another test with infinity coverage here will also be good one to cover our code and that is supposed to print.
Basically we can do something like this:
```
private MBeanServer getMockMBeanServer(Number number) throws Exception {
....
...
when(mbeanServer.getAttribute(any(ObjectName.class), eq("intAttr"))).thenReturn(number);
}
```
```
private String getExpectedJSON(String str) {
...
...
pw.println(" \"intAttr\": "+str+",");
}
```
and then call
```
jsonWriter.write(getMockMBeanServer(10.0/0.0), null, null, false);
assertEquals(mapper.readTree(getExpectedJSON("\"Infinity\"")), mapper.readTree(stringWriter.toString()));
jsonWriter.write(getMockMBeanServer(3), null, null, false);
assertEquals(mapper.readTree(getExpectedJSON("3")), mapper.readTree(stringWriter.toString()));
```
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] virajjasani commented on pull request #1999: HBASE-24657 fix JsonBean.java from HBASE-23015 HBASE-20571
Posted by GitBox <gi...@apache.org>.
virajjasani commented on pull request #1999:
URL: https://github.com/apache/hbase/pull/1999#issuecomment-653010609
Thank you @d-c-manning
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] d-c-manning commented on a change in pull request #1999: HBASE-24657 fix JsonBean.java from HBASE-23015 HBASE-20571
Posted by GitBox <gi...@apache.org>.
d-c-manning commented on a change in pull request #1999:
URL: https://github.com/apache/hbase/pull/1999#discussion_r447289532
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/util/JSONBean.java
##########
@@ -334,7 +334,7 @@ private static void writeObject(final JsonWriter writer, final Object value) thr
} else if(value instanceof Number) {
Number n = (Number)value;
double doubleValue = n.doubleValue();
- if (Double.isNaN(doubleValue) || Double.isInfinite(doubleValue)) {
+ if (!Double.isNaN(doubleValue) && !Double.isInfinite(doubleValue)) {
Review comment:
Makes sense... this regression is limited to branch-1, but the unit test should go into trunk... should I create a separate JIRA for that?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] Apache-HBase commented on pull request #1999: HBASE-24657 fix JsonBean.java from HBASE-23015 HBASE-20571
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #1999:
URL: https://github.com/apache/hbase/pull/1999#issuecomment-652369097
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 46s | Docker mode activated. |
||| _ Prechecks _ |
| +1 :green_heart: | dupname | 0m 0s | No case conflicting files found. |
| +1 :green_heart: | hbaseanti | 0m 0s | Patch does not have any anti-patterns. |
| +1 :green_heart: | @author | 0m 0s | The patch does not contain any @author tags. |
| +1 :green_heart: | test4tests | 0m 0s | The patch appears to include 1 new or modified test files. |
||| _ branch-1 Compile Tests _ |
| +1 :green_heart: | mvninstall | 10m 30s | branch-1 passed |
| +1 :green_heart: | compile | 0m 48s | branch-1 passed with JDK v1.8.0_252 |
| +1 :green_heart: | compile | 0m 52s | branch-1 passed with JDK v1.7.0_262 |
| +1 :green_heart: | checkstyle | 2m 13s | branch-1 passed |
| +1 :green_heart: | shadedjars | 3m 47s | branch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 54s | branch-1 passed with JDK v1.8.0_252 |
| +1 :green_heart: | javadoc | 0m 49s | branch-1 passed with JDK v1.7.0_262 |
| +0 :ok: | spotbugs | 3m 43s | Used deprecated FindBugs config; considering switching to SpotBugs. |
| +1 :green_heart: | findbugs | 3m 40s | branch-1 passed |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 2m 34s | the patch passed |
| +1 :green_heart: | compile | 0m 44s | the patch passed with JDK v1.8.0_252 |
| +1 :green_heart: | javac | 0m 44s | the patch passed |
| +1 :green_heart: | compile | 0m 58s | the patch passed with JDK v1.7.0_262 |
| +1 :green_heart: | javac | 0m 58s | the patch passed |
| +1 :green_heart: | checkstyle | 1m 52s | the patch passed |
| +1 :green_heart: | whitespace | 0m 0s | The patch has no whitespace issues. |
| +1 :green_heart: | shadedjars | 3m 19s | patch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | hadoopcheck | 5m 47s | Patch does not cause any errors with Hadoop 2.8.5 2.9.2. |
| +1 :green_heart: | javadoc | 0m 34s | the patch passed with JDK v1.8.0_252 |
| +1 :green_heart: | javadoc | 0m 45s | the patch passed with JDK v1.7.0_262 |
| +1 :green_heart: | findbugs | 3m 26s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | unit | 130m 9s | hbase-server in the patch passed. |
| +1 :green_heart: | asflicense | 0m 38s | The patch does not generate ASF License warnings. |
| | | 179m 10s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | Client=19.03.12 Server=19.03.12 base: https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1999/7/artifact/out/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/1999 |
| Optional Tests | dupname asflicense javac javadoc unit spotbugs findbugs shadedjars hadoopcheck hbaseanti checkstyle compile |
| uname | Linux ce59843949e2 4.15.0-58-generic #64-Ubuntu SMP Tue Aug 6 11:12:41 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | /home/jenkins/jenkins-slave/workspace/Base-PreCommit-GitHub-PR_PR-1999/out/precommit/personality/provided.sh |
| git revision | branch-1 / 32690e1 |
| Default Java | 1.7.0_262 |
| Multi-JDK versions | /usr/lib/jvm/zulu-8-amd64:1.8.0_252 /usr/lib/jvm/zulu-7-amd64:1.7.0_262 |
| Test Results | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1999/7/testReport/ |
| Max. process+thread count | 4903 (vs. ulimit of 10000) |
| modules | C: hbase-server U: hbase-server |
| Console output | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1999/7/console |
| versions | git=1.9.1 maven=3.0.5 findbugs=3.0.1 |
| Powered by | Apache Yetus 0.11.1 https://yetus.apache.org |
This message was automatically generated.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] virajjasani edited a comment on pull request #1999: HBASE-24657 fix JsonBean.java from HBASE-23015 HBASE-20571
Posted by GitBox <gi...@apache.org>.
virajjasani edited a comment on pull request #1999:
URL: https://github.com/apache/hbase/pull/1999#issuecomment-651713706
> > Meanwhile, we can have one additional PR for master branch with test coverage. I will take care of all backports.
>
> Shall I create that PR @virajjasani or will you?
You can create as per you convenience because the order for commit is master -> branch-2 -> all active branch-2.x -> branch-1 :)
Also, in the meanwhile, @bharathv will also get time to review this PR. Once we have approval, we will go in the same commit order from master till branch-1.
Although this was going to be branch-1 specific PR but since we are adding coverage, we have reason to follow the commit order.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] d-c-manning commented on a change in pull request #1999: HBASE-24657 fix JsonBean.java from HBASE-23015 HBASE-20571
Posted by GitBox <gi...@apache.org>.
d-c-manning commented on a change in pull request #1999:
URL: https://github.com/apache/hbase/pull/1999#discussion_r447263171
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/util/JSONBean.java
##########
@@ -334,7 +334,7 @@ private static void writeObject(final JsonWriter writer, final Object value) thr
} else if(value instanceof Number) {
Number n = (Number)value;
double doubleValue = n.doubleValue();
- if (Double.isNaN(doubleValue) || Double.isInfinite(doubleValue)) {
+ if (!Double.isNaN(doubleValue) && !Double.isInfinite(doubleValue)) {
Review comment:
@bharathv my understanding is that branch-1 is required to have java7 compatibility, and I guess it only exists in java8: https://docs.oracle.com/javase/8/docs/api/java/lang/Double.html#isFinite-double-
I agree there is missing test coverage somewhere. I am unfamiliar with the code, so it may take a while to find where to add.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org