Inline StringValue into base::Value

base/values_unittest.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "base/values.h"
 
 #include <stddef.h>
 
 #include <limits>
 #include <memory>
@@ skipping 22 matching lines @@
   EXPECT_EQ(Value::Type::INTEGER, value.type());
   EXPECT_EQ(-37, value.GetInt());
 }
 
 TEST(ValuesTest, ConstructDouble) {
   FundamentalValue value(-4.655);
   EXPECT_EQ(Value::Type::DOUBLE, value.type());
   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.
 TEST(ValuesTest, CopyBool) {
   FundamentalValue true_value(true);
   FundamentalValue copied_true_value(true_value);
   EXPECT_EQ(true_value.type(), copied_true_value.type());
   EXPECT_EQ(true_value.GetBool(), copied_true_value.GetBool());
 
   FundamentalValue false_value(false);
@@ skipping 37 matching lines @@
 
   // TODO(crbug.com/646113): Create a Type::NONE value once the assignment
   // operator of base::Value is public.
   FundamentalValue blank(false);
 
   blank = value;
   EXPECT_EQ(value.type(), blank.type());
   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);
   FundamentalValue moved_true_value(std::move(true_value));
   EXPECT_EQ(Value::Type::BOOLEAN, moved_true_value.type());
   EXPECT_TRUE(moved_true_value.GetBool());
 
   FundamentalValue false_value(false);
   FundamentalValue moved_false_value(std::move(false_value));
   EXPECT_EQ(Value::Type::BOOLEAN, moved_false_value.type());
@@ skipping 35 matching lines @@
 
   // TODO(crbug.com/646113): Create a Type::NONE value once the assignment
   // operator of base::Value is public.
   FundamentalValue blank(false);
 
   blank = FundamentalValue(654.38);
   EXPECT_EQ(Value::Type::DOUBLE, blank.type());
   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;
   std::string homepage = "http://google.com";
   ASSERT_FALSE(settings.GetString("global.homepage", &homepage));
   ASSERT_EQ(std::string("http://google.com"), homepage);
 
   ASSERT_FALSE(settings.Get("global", NULL));
   settings.SetBoolean("global", true);
   ASSERT_TRUE(settings.Get("global", NULL));
@@ skipping 1117 matching lines @@
   EXPECT_FALSE(main_list.GetList(1, NULL));
   EXPECT_FALSE(main_list.GetList(2, NULL));
   EXPECT_FALSE(main_list.GetList(3, NULL));
   EXPECT_FALSE(main_list.GetList(4, NULL));
   EXPECT_FALSE(main_list.GetList(5, NULL));
   EXPECT_TRUE(main_list.GetList(6, NULL));
   EXPECT_FALSE(main_list.GetList(7, NULL));
 }
 
 }  // namespace base