Test cases for testing the exceptional behavior of JsonArray get... methods #1909

HiFromAjay · 2021-06-11T16:04:45Z

Test cases for testing the exceptional behavior of get, getAsBoolean, getAsDouble, getAsInt, getAsJsonArray, getAsJsonObject, getAsLong, and getAsString methods of JsonArray class. These test cases, which we wrote according to the specified behavior of each method, that helped us in identifying the documentation bugs in JsonArray and JsonElement classes, which we submitted issues for (Issue #1908). Note that we have adapted these test cases based on similar tests from the JSON-java project.

… getAsDouble, getAsInt, getAsJsonArray, getAsJsonObject, getAsLong, and getAsString methods of JsonArray class. These test cases, which we wrote according to the specified behavior of each method, that helped us in identifying the documentation bugs in JsonArray and JsonElement classes, which we submitted issues for (Issue google#1908). Note that we have adapted these test cases based on similar tests from the JSON-java project (https://github.com/stleary/JSON-java).

google-cla · 2021-06-11T16:04:49Z

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed (or fixed any issues), please reply here with @googlebot I signed it! and we'll verify it.

What to do if you already signed the CLA

Individual signers

It's possible we don't have your GitHub username or you're using a different email address on your commit. Check your existing CLA data and verify that your email is set on your git commits.

Corporate signers

Your company has a Point of Contact who decides which employees are authorized to participate. Ask your POC to be added to the group of authorized contributors. If you don't know who your Point of Contact is, direct the Google project maintainer to go/cla#troubleshoot (Public version).
The email used to register you as an authorized contributor must be the email used for the Git commit. Check your existing CLA data and verify that your email is set on your git commits.
The email used to register you as an authorized contributor must also be attached to your GitHub account.

ℹ️ Googlers: Go here for more info.

HiFromAjay · 2021-06-11T16:09:33Z

@googlebot I signed it!

Marcono1234

I hope the review comments are useful; note that I am not a member of this project so feel free to consider these only as suggestions.

Marcono1234 · 2021-06-12T16:48:20Z

gson/src/test/java/com/google/gson/JsonArrayTest.java

+    JsonArray jsonArray = (JsonArray) JsonParser.parseString(arrayStr);
+    try {
+      jsonArray.get(10).getAsBoolean();
+      assertTrue("expected getBoolean to fail", false);


Instead of assertTrue(..., false) it might be better to use fail(...); applies to other tests as well.

Changed to fail(...)

Marcono1234 · 2021-06-12T16:51:26Z

gson/src/test/java/com/google/gson/JsonArrayTest.java

+              "JsonObject",e.getMessage());
+    }
+    try {
+      jsonArray.get(-1);


Except for this test, all other ones are not specific to JsonArray, it might be better to put them to JsonPrimitiveTest (or maybe there exists another more fitting test class).

Changed how the methods are used.

Thanks! Also my comment was incorrect, I missed that the getAs... methods of JsonArray actually try to convert the array element in case the array contains only a single element.

Marcono1234 · 2021-06-12T16:52:03Z

gson/src/test/java/com/google/gson/JsonArrayTest.java

@@ -99,4 +102,65 @@ public void testDeepCopy() {
    assertEquals(1, original.get(0).getAsJsonArray().size());
    assertEquals(0, copy.get(0).getAsJsonArray().size());
  }
+
+  public void testFailedGetArrayValues() {
+    String arrayStr = "[" + "true," + "false," + "\"true\"," + "\"false\"," + "\"hello\"," + "23.45e-4," + "\"23.45\"," + "42," + "\"43\"," + "[" + "\"world\"" + "]," + "{" + "\"key1\":\"value1\"," + "\"key2\":\"value2\"," + "\"key3\":\"value3\"," + "\"key4\":\"value4\"" + "}," + "0," + "\"-1\"" + "]";


Not all values of this array are used; might be good to remove unused values to make this test easier to understand.

Removed the unused values.

Marcono1234 · 2021-06-12T16:53:39Z

gson/src/test/java/com/google/gson/JsonArrayTest.java

+      assertTrue("expected getBoolean to fail", false);
+    } catch (UnsupportedOperationException e) {
+      assertEquals("Expected an exception message",
+              "JsonObject",e.getMessage());


Really minor, but for consistent code style there should probably be a space:

Suggested change

"JsonObject",e.getMessage());

"JsonObject", e.getMessage());

(applies to other tests as well)

Resolved the formatting issues.

…ods [use fail(...), use JsonArray methods, remove unused values, formatting, google#1909, google#1908]

eamonnmcmanus

Thanks for this! Just one small comment.

eamonnmcmanus · 2021-08-05T13:43:45Z

gson/src/test/java/com/google/gson/JsonArrayTest.java

@@ -20,6 +20,9 @@

 import com.google.gson.common.MoreAsserts;

+import static org.junit.Assert.*;


It's not clear to me why these imports are needed. Aren't assertEquals etc inherited from TestCase?

I have removed those imports.

google-cla bot added the cla: no label Jun 11, 2021

google-cla bot added cla: yes and removed cla: no labels Jun 11, 2021

Marcono1234 reviewed Jun 12, 2021

View reviewed changes

modify test cases for testing the exceptional behavior of get... meth…

2d1981d

…ods [use fail(...), use JsonArray methods, remove unused values, formatting, google#1909, google#1908]

eamonnmcmanus requested changes Aug 5, 2021

View reviewed changes

Remove unused imports [google#1909, google#1908]

01ab13f

eamonnmcmanus approved these changes Aug 7, 2021

View reviewed changes

eamonnmcmanus merged commit 9edaeb3 into google:master Aug 7, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Test cases for testing the exceptional behavior of JsonArray get... methods #1909

Test cases for testing the exceptional behavior of JsonArray get... methods #1909

HiFromAjay commented Jun 11, 2021

google-cla bot commented Jun 11, 2021

HiFromAjay commented Jun 11, 2021

Marcono1234 left a comment

Marcono1234 Jun 12, 2021 •

edited

HiFromAjay Jun 14, 2021

Marcono1234 Jun 12, 2021

HiFromAjay Jun 14, 2021

Marcono1234 Jun 14, 2021

Marcono1234 Jun 12, 2021

HiFromAjay Jun 14, 2021

Marcono1234 Jun 12, 2021

HiFromAjay Jun 14, 2021

eamonnmcmanus left a comment

eamonnmcmanus Aug 5, 2021

HiFromAjay Aug 5, 2021

		@@ -20,6 +20,9 @@

		import com.google.gson.common.MoreAsserts;

		import static org.junit.Assert.*;

Test cases for testing the exceptional behavior of JsonArray get... methods #1909

Test cases for testing the exceptional behavior of JsonArray get... methods #1909

Conversation

HiFromAjay commented Jun 11, 2021

google-cla bot commented Jun 11, 2021

What to do if you already signed the CLA

Individual signers

Corporate signers

HiFromAjay commented Jun 11, 2021

Marcono1234 left a comment

Choose a reason for hiding this comment

Marcono1234 Jun 12, 2021 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

eamonnmcmanus left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Marcono1234 Jun 12, 2021 •

edited