Inline StringValue into base::Value

base/values_unittest.cc

Index: base/values_unittest.cc
diff --git a/base/values_unittest.cc b/base/values_unittest.cc
index c3f2a5269878cefc65d2b17d570e70068171cd26..fa8cb69eb243b1c587c7684d099b231d124a816b 100644
--- a/base/values_unittest.cc
+++ b/base/values_unittest.cc
@@ -40,6 +40,34 @@ TEST(ValuesTest, ConstructDouble) {
   EXPECT_EQ(-4.655, value.GetDouble());
 }
 
+TEST(ValuesTest, ConstructStringFromStringPiece) {
+  StringPiece str = "foobar";
+  StringValue value(str);
+  EXPECT_EQ(Value::Type::STRING, value.type());
+  EXPECT_EQ(str.as_string(), value.GetString());

[vabr (Chromium), 2016/12/16 17:00:56]

To avoid testing as_string, please use
EXPECT_EQ("foobar", value.GetString());

What looks like an error-prone constant-duplication is actually a good tactic
for tests: if somebody breaks both as_string and GetString in compatible ways,
the current test is fooled. Also, if somebody just breaks as_string(), the
resulting failure here will be confusing. With a hard-coded expectation, the
test is better scoped and clearer. See also go/tott-338.

Also below for the Construct* cases. Note that Copy* cases are good as they are,
there the goal is to compare the result of both constructions.

[jdoerrie, 2016/12/19 11:54:09]

Done. For some reason I thought we needed the exact same types on both sides (ie
two std::strings), but it's enough to have a defined operator== between the
types.

[vabr (Chromium), 2016/12/19 13:00:56]

Ah, actually, I missed that it was taking the expected value from |str|, and not
from the value somehow. So even your current code looks good to me (although
putting in the literal directly looks a little bit more readable to me).
(Not sure how to explain my previous confusion, it was probably too late or
something. :))

+}
+
+TEST(ValuesTest, ConstructStringFromStdString) {
+  std::string str = "foobar";
+  StringValue value(str);
+  EXPECT_EQ(Value::Type::STRING, value.type());
+  EXPECT_EQ(str, value.GetString());
+}
+
+TEST(ValuesTest, ConstructStringFromString16) {
+  string16 str = ASCIIToUTF16("foobar");
+  StringValue value(str);
+  EXPECT_EQ(Value::Type::STRING, value.type());
+  EXPECT_EQ(UTF16ToUTF8(str), value.GetString());
+}
+
+TEST(ValuesTest, ConstructStringFromConstChar) {
+  const char* str = "foobar";
+  StringValue value(str);
+  EXPECT_EQ(Value::Type::STRING, value.type());
+  EXPECT_EQ(std::string(str), value.GetString());
+}
+
 // Group of tests for the copy constructors and copy-assigmnent. For equality
 // checks comparisons of the interesting fields are done instead of relying on
 // Equals being correct.
@@ -97,6 +125,21 @@ TEST(ValuesTest, CopyDouble) {
   EXPECT_EQ(value.GetDouble(), blank.GetDouble());
 }
 
+TEST(ValuesTest, CopyString) {
+  StringValue value("foobar");
+  StringValue copied_value(value);
+  EXPECT_EQ(value.type(), copied_value.type());
+  EXPECT_EQ(value.GetString(), copied_value.GetString());
+
+  // TODO(crbug.com/646113): Create a Type::NONE value once the assignment
+  // operator of base::Value is public.
+  StringValue blank("");
+
+  blank = value;
+  EXPECT_EQ(value.type(), blank.type());
+  EXPECT_EQ(value.GetString(), blank.GetString());
+}
+
 // Group of tests for the move constructors and move-assigmnent.
 TEST(ValuesTest, MoveBool) {
   FundamentalValue true_value(true);
@@ -152,6 +195,22 @@ TEST(ValuesTest, MoveDouble) {
   EXPECT_EQ(654.38, blank.GetDouble());
 }
 
+TEST(ValuesTest, MoveString) {
+  StringValue value("foobar");
+  StringValue moved_value(std::move(value));
+  EXPECT_EQ(Value::Type::STRING, moved_value.type());
+  EXPECT_EQ(std::string("foobar"), moved_value.GetString());
+  EXPECT_EQ(Value::Type::NONE, value.type());
+
+  // TODO(crbug.com/646113): Create a Type::NONE value once the assignment
+  // operator of base::Value is public.
+  StringValue blank("");
+
+  blank = StringValue("foobar");
+  EXPECT_EQ(Value::Type::STRING, blank.type());
+  EXPECT_EQ(std::string("foobar"), blank.GetString());
+}
+
 TEST(ValuesTest, Basic) {
   // Test basic dictionary getting/setting
   DictionaryValue settings;