Improve error message display for Schema::Validate()

components/policy/core/common/schema_unittest.cc

 // Copyright 2013 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 "components/policy/core/common/schema.h"
 
 #include "base/memory/scoped_ptr.h"
 #include "components/policy/core/common/schema_internal.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
@@ skipping 103 matching lines @@
 
 void TestSchemaValidation(Schema schema,
                           const base::Value& value,
                           SchemaOnErrorStrategy strategy,
                           bool expected_return_value) {
   std::string error;
   static const char kNoErrorReturned[] = "No error returned.";
 
   // Test that Schema::Validate() works as expected.
   error = kNoErrorReturned;
-  bool returned = schema.Validate(value, strategy, &error);
+  bool returned = schema.Validate(value, strategy, NULL, &error);
   EXPECT_EQ(returned, expected_return_value) << error;
 
   // Test that Schema::Normalize() will return the same value as
   // Schema::Validate().
   error = kNoErrorReturned;
   scoped_ptr<base::Value> cloned_value(value.DeepCopy());
-  returned = schema.Normalize(cloned_value.get(), strategy, &error);
+  returned = schema.Normalize(cloned_value.get(), strategy, NULL, &error);
   EXPECT_EQ(returned, expected_return_value) << error;
 
   // Test that Schema::Normalize() have actually dropped invalid and unknown
   // properties.
   if (expected_return_value) {
-    EXPECT_TRUE(schema.Validate(*cloned_value.get(), SCHEMA_STRICT, &error));
-    EXPECT_TRUE(schema.Normalize(cloned_value.get(), SCHEMA_STRICT, &error));
+    EXPECT_TRUE(
+        schema.Validate(*cloned_value.get(), SCHEMA_STRICT, NULL, &error));
+    EXPECT_TRUE(
+        schema.Normalize(cloned_value.get(), SCHEMA_STRICT, NULL, &error));
   }
 }
 
+void TestSchemaValidationWithPath(Schema schema,
+                                  const base::Value& value,
+                                  const std::string& expected_failure_path) {
+  std::string error_path = "NOT_SET";
+  std::string error;
+
+  bool returned = schema.Validate(value, SCHEMA_STRICT, &error_path, &error);
+  ASSERT_FALSE(returned) << error_path;
+  EXPECT_EQ(error_path, expected_failure_path);
+}
+
 std::string SchemaObjectWrapper(const std::string& subschema) {
   return "{"
          "  \"type\": \"object\","
          "  \"properties\": {"
          "    \"SomePropertyName\":" + subschema +
          "  }"
          "}";
 }
 
 }  // namespace
@@ skipping 491 matching lines @@
 
   bundle.SetInteger("IntegerWithRange", 4);
   TestSchemaValidation(schema, bundle, SCHEMA_STRICT, false);
   bundle.SetInteger("IntegerWithRange", 3);
 
   // Unknown top level property.
   bundle.SetString("boom", "bang");
   TestSchemaValidation(schema, bundle, SCHEMA_STRICT, false);
   TestSchemaValidation(schema, bundle, SCHEMA_ALLOW_UNKNOWN_TOPLEVEL, true);
   TestSchemaValidation(schema, bundle, SCHEMA_ALLOW_UNKNOWN, true);
+  TestSchemaValidationWithPath(schema, bundle, "");
   bundle.Remove("boom", NULL);
 
   // Invalid top level property.
   bundle.SetInteger("Boolean", 12345);
   TestSchemaValidation(schema, bundle, SCHEMA_STRICT, false);
   TestSchemaValidation(schema, bundle, SCHEMA_ALLOW_INVALID_TOPLEVEL, true);
   TestSchemaValidation(schema, bundle, SCHEMA_ALLOW_INVALID, true);
+  TestSchemaValidationWithPath(schema, bundle, "Boolean");
   bundle.SetBoolean("Boolean", true);
 
   // Tests on ObjectOfObject.
   {
     Schema subschema = schema.GetProperty("ObjectOfObject");
     ASSERT_TRUE(subschema.valid());
     base::DictionaryValue root;
 
     // Unknown property.
     root.SetBoolean("Object.three", false);
     TestSchemaValidation(subschema, root, SCHEMA_STRICT, false);
     TestSchemaValidation(subschema, root, SCHEMA_ALLOW_UNKNOWN_TOPLEVEL, false);
     TestSchemaValidation(subschema, root, SCHEMA_ALLOW_UNKNOWN, true);
     TestSchemaValidation(subschema, root, SCHEMA_ALLOW_INVALID_TOPLEVEL, true);
     TestSchemaValidation(subschema, root, SCHEMA_ALLOW_INVALID, true);
+    TestSchemaValidationWithPath(subschema, root, "Object");
     root.Remove("Object.three", NULL);
 
     // Invalid property.
     root.SetInteger("Object.one", 12345);
     TestSchemaValidation(subschema, root, SCHEMA_STRICT, false);
     TestSchemaValidation(subschema, root, SCHEMA_ALLOW_UNKNOWN_TOPLEVEL, false);
     TestSchemaValidation(subschema, root, SCHEMA_ALLOW_UNKNOWN, false);
     TestSchemaValidation(subschema, root, SCHEMA_ALLOW_INVALID_TOPLEVEL, true);
     TestSchemaValidation(subschema, root, SCHEMA_ALLOW_INVALID, true);
+    TestSchemaValidationWithPath(subschema, root, "Object.one");
     root.Remove("Object.one", NULL);
   }
 
   // Tests on ArrayOfObjects.
   {
     Schema subschema = schema.GetProperty("ArrayOfObjects");
     ASSERT_TRUE(subschema.valid());
     base::ListValue root;
 
     // Unknown property.
     base::DictionaryValue* dict_value = new base::DictionaryValue();
     dict_value->SetBoolean("three", true);
     root.Append(dict_value);  // Pass ownership to root.
     TestSchemaValidation(subschema, root, SCHEMA_STRICT, false);
     TestSchemaValidation(subschema, root, SCHEMA_ALLOW_UNKNOWN_TOPLEVEL, false);
     TestSchemaValidation(subschema, root, SCHEMA_ALLOW_UNKNOWN, true);
     TestSchemaValidation(subschema, root, SCHEMA_ALLOW_INVALID_TOPLEVEL, true);
     TestSchemaValidation(subschema, root, SCHEMA_ALLOW_INVALID, true);
+    TestSchemaValidationWithPath(subschema, root, "items[0]");
     root.Remove(root.GetSize() - 1, NULL);
 
     // Invalid property.
     dict_value = new base::DictionaryValue();
     dict_value->SetBoolean("two", true);
     root.Append(dict_value);  // Pass ownership to root.
     TestSchemaValidation(subschema, root, SCHEMA_STRICT, false);
     TestSchemaValidation(subschema, root, SCHEMA_ALLOW_UNKNOWN_TOPLEVEL, false);
     TestSchemaValidation(subschema, root, SCHEMA_ALLOW_UNKNOWN, false);
     TestSchemaValidation(subschema, root, SCHEMA_ALLOW_INVALID_TOPLEVEL, true);
     TestSchemaValidation(subschema, root, SCHEMA_ALLOW_INVALID, true);
+    TestSchemaValidationWithPath(subschema, root, "items[0].two");
     root.Remove(root.GetSize() - 1, NULL);
   }
 
   // Tests on ObjectOfArray.
   {
     Schema subschema = schema.GetProperty("ObjectOfArray");
     ASSERT_TRUE(subschema.valid());
     base::DictionaryValue root;
 
     base::ListValue* list_value = new base::ListValue();
     root.Set("List", list_value);  // Pass ownership to root.
 
-    // No errors.
+    // Test that there are not errors here.
     list_value->Append(new base::FundamentalValue(12345));
     TestSchemaValidation(subschema, root, SCHEMA_STRICT, true);
     TestSchemaValidation(subschema, root, SCHEMA_ALLOW_UNKNOWN_TOPLEVEL, true);
     TestSchemaValidation(subschema, root, SCHEMA_ALLOW_UNKNOWN, true);
     TestSchemaValidation(subschema, root, SCHEMA_ALLOW_INVALID_TOPLEVEL, true);
     TestSchemaValidation(subschema, root, SCHEMA_ALLOW_INVALID, true);
 

[Joao da Silva, 2014/01/24 09:58:39]

What I meant is that we should test that there are no errors at this line.
Something like:

  TestSchemaValidationWithPath(subschema, root, "NOT_SET");

[binjin, 2014/01/24 10:03:25]

I think TestSchemaValidation(subschema, root, SCHEMA_STRICT, true) already did
this work. Also I used ASSERT_FALSE(returned) in TestSchemaValidation(), and
TestSchemaValidation() actually expect failure for input.

[Joao da Silva, 2014/01/27 10:35:52]

OK, sgtm.

     // Invalid list item.
     list_value->Append(new base::StringValue("blabla"));
     TestSchemaValidation(subschema, root, SCHEMA_STRICT, false);
     TestSchemaValidation(subschema, root, SCHEMA_ALLOW_UNKNOWN_TOPLEVEL, false);
     TestSchemaValidation(subschema, root, SCHEMA_ALLOW_UNKNOWN, false);
     TestSchemaValidation(subschema, root, SCHEMA_ALLOW_INVALID_TOPLEVEL, true);
     TestSchemaValidation(subschema, root, SCHEMA_ALLOW_INVALID, true);
+    TestSchemaValidationWithPath(subschema, root, "List.items[1]");
   }
 
   // Tests on ArrayOfObjectOfArray.
   {
     Schema subschema = schema.GetProperty("ArrayOfObjectOfArray");
     ASSERT_TRUE(subschema.valid());
     base::ListValue root;
 
     base::ListValue* list_value = new base::ListValue();
     base::DictionaryValue* dict_value = new base::DictionaryValue();
     dict_value->Set("List", list_value);  // Pass ownership to dict_value.
     root.Append(dict_value);  // Pass ownership to root.
 
-    // No errors.
+    // Test that there are not errors here.
     list_value->Append(new base::StringValue("blabla"));
     TestSchemaValidation(subschema, root, SCHEMA_STRICT, true);
     TestSchemaValidation(subschema, root, SCHEMA_ALLOW_UNKNOWN_TOPLEVEL, true);
     TestSchemaValidation(subschema, root, SCHEMA_ALLOW_UNKNOWN, true);
     TestSchemaValidation(subschema, root, SCHEMA_ALLOW_INVALID_TOPLEVEL, true);
     TestSchemaValidation(subschema, root, SCHEMA_ALLOW_INVALID, true);
 
     // Invalid list item.
     list_value->Append(new base::FundamentalValue(12345));
     TestSchemaValidation(subschema, root, SCHEMA_STRICT, false);
     TestSchemaValidation(subschema, root, SCHEMA_ALLOW_UNKNOWN_TOPLEVEL, false);
     TestSchemaValidation(subschema, root, SCHEMA_ALLOW_UNKNOWN, false);
     TestSchemaValidation(subschema, root, SCHEMA_ALLOW_INVALID_TOPLEVEL, true);
     TestSchemaValidation(subschema, root, SCHEMA_ALLOW_INVALID, true);
+    TestSchemaValidationWithPath(subschema, root, "items[0].List.items[1]");
   }
 
   // Test that integer to double promotion is allowed.
   bundle.SetInteger("Number", 31415);
   TestSchemaValidation(schema, bundle, SCHEMA_STRICT, true);
 }
 
 TEST(SchemaTest, InvalidReferences) {
   // References to undeclared schemas fail.
   EXPECT_TRUE(ParseFails(
@@ skipping 246 matching lines @@
 
   EXPECT_FALSE(ParseFails(SchemaObjectWrapper(
       "{"
       "  \"type\": \"integer\","
       "  \"minimum\": 10,"
       "  \"maximum\": 20"
       "}")));
 }
 
 }  // namespace policy