Commit 332d1570 authored by Nigel Tao's avatar Nigel Tao Committed by Commit Bot

Clarify JSONValueDeserializerTest assertion

Prior to this commit, the code looked like:
    etc.Deserialize(&error_code, etc);
    ASSERT_EQ(foo, error_code);
which could be misinterpreted as testing that a side-effect of calling
Deserialize was setting error_code to foo. In fact, error_code was set
to foo before the Deserialize call, and what was actually being tested
was that Deserialize has no effect on error_code when there's no error.
That should be clearer after this commit.

Bug: 1069271
Change-Id: I20045c24c098f792a87622762d58fb085f6fbaaa
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2248395Reviewed-by: default avatarRobert Sesek <rsesek@chromium.org>
Commit-Queue: Nigel Tao <nigeltao@chromium.org>
Cr-Commit-Position: refs/heads/master@{#779796}
parent 29f922da
...@@ -133,12 +133,15 @@ TEST(JSONValueDeserializerTest, ReadJSONWithTrailingCommasFromString) { ...@@ -133,12 +133,15 @@ TEST(JSONValueDeserializerTest, ReadJSONWithTrailingCommasFromString) {
ASSERT_FALSE(value); ASSERT_FALSE(value);
ASSERT_NE(0, error_code); ASSERT_NE(0, error_code);
ASSERT_FALSE(error_message.empty()); ASSERT_FALSE(error_message.empty());
// Repeat with commas allowed. // Repeat with commas allowed. The Deserialize call shouldn't change the
// value of error_code. To test that, we first set it to a nonsense value
// (-789) and ASSERT_EQ that it remains that nonsense value.
error_code = -789;
JSONStringValueDeserializer str_deserializer2(kProperJSONWithCommas, JSONStringValueDeserializer str_deserializer2(kProperJSONWithCommas,
JSON_ALLOW_TRAILING_COMMAS); JSON_ALLOW_TRAILING_COMMAS);
value = str_deserializer2.Deserialize(&error_code, &error_message); value = str_deserializer2.Deserialize(&error_code, &error_message);
ASSERT_TRUE(value); ASSERT_TRUE(value);
ASSERT_EQ(JSONReader::JSON_TRAILING_COMMA, error_code); ASSERT_EQ(-789, error_code);
// Verify if the same JSON is still there. // Verify if the same JSON is still there.
CheckJSONIsStillTheSame(*value); CheckJSONIsStillTheSame(*value);
} }
...@@ -184,12 +187,15 @@ TEST(JSONValueDeserializerTest, ReadJSONWithCommasFromFile) { ...@@ -184,12 +187,15 @@ TEST(JSONValueDeserializerTest, ReadJSONWithCommasFromFile) {
ASSERT_FALSE(value); ASSERT_FALSE(value);
ASSERT_NE(0, error_code); ASSERT_NE(0, error_code);
ASSERT_FALSE(error_message.empty()); ASSERT_FALSE(error_message.empty());
// Repeat with commas allowed. // Repeat with commas allowed. The Deserialize call shouldn't change the
// value of error_code. To test that, we first set it to a nonsense value
// (-789) and ASSERT_EQ that it remains that nonsense value.
error_code = -789;
JSONFileValueDeserializer file_deserializer2(temp_file, JSONFileValueDeserializer file_deserializer2(temp_file,
JSON_ALLOW_TRAILING_COMMAS); JSON_ALLOW_TRAILING_COMMAS);
value = file_deserializer2.Deserialize(&error_code, &error_message); value = file_deserializer2.Deserialize(&error_code, &error_message);
ASSERT_TRUE(value); ASSERT_TRUE(value);
ASSERT_EQ(JSONReader::JSON_TRAILING_COMMA, error_code); ASSERT_EQ(-789, error_code);
// Verify if the same JSON is still there. // Verify if the same JSON is still there.
CheckJSONIsStillTheSame(*value); CheckJSONIsStillTheSame(*value);
} }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment