Fix YAML checking of return values handle lists and structs correctly (…

…project-chip#11570)

* Turn on checking of list contents for YAML tests.

The change to CheckValue is to make it clearer which value is the
actual value and which value is the expected value.

The change to TestAttrAccess::ReadListInt8uAttribute is to make the
actual value it produces match the expected value, in a way that keeps
the values different from the indices.

* Turn on checking of struct fields for YAML tests

The CheckValueAsString change is just to show what the actual value
was that does not match the expected value.

The CheckValue (enum version) change is to make it compile, now that
it's actually being used.

examples/chip-tool/commands/tests/TestCommand.cpp

@@ -109,17 +109,6 @@ bool TestCommand::CheckConstraintMaxLength(const char * itemName, uint64_t curre
     return true;
 }
 
-bool TestCommand::CheckValueAsList(const char * itemName, uint64_t current, uint64_t expected)
-{
-    if (current != expected)
-    {
-        Exit(std::string(itemName) + " count mismatch: " + std::to_string(current) + " != " + std::to_string(expected));
-        return false;
-    }
-
-    return true;
-}
-
 bool TestCommand::CheckValueAsString(const char * itemName, const chip::ByteSpan current, const char * expected)
 {
     const chip::ByteSpan expectedArgument = chip::ByteSpan(chip::Uint8::from_const_char(expected), strlen(expected));
@@ -138,7 +127,8 @@ bool TestCommand::CheckValueAsString(const char * itemName, const chip::CharSpan
     const chip::CharSpan expectedArgument(expected, strlen(expected));
     if (!current.data_equal(expectedArgument))
     {
-        Exit(std::string(itemName) + " value mismatch, expecting " + std::string(expected));
+        Exit(std::string(itemName) + " value mismatch, expected '" + expected + "' but got '" +
+             std::string(current.data(), current.size()) + "'");
         return false;
     }
 

examples/chip-tool/commands/tests/TestCommand.h

@@ -106,7 +106,8 @@ class TestCommand : public CHIPCommand
     {
         if (current != expected)
         {
-            Exit(std::string(itemName) + " value mismatch: " + std::to_string(current) + " != " + std::to_string(expected));
+            Exit(std::string(itemName) + " value mismatch: expected " + std::to_string(expected) + " but got " +
+                 std::to_string(current));
             return false;
         }
 
@@ -116,78 +117,58 @@ class TestCommand : public CHIPCommand
     template <typename T, typename U, typename std::enable_if_t<std::is_enum<T>::value, int> = 0>
     bool CheckValue(const char * itemName, T current, U expected)
     {
-        return CheckValue(itemName, to_underlying(current), expected);
+        return CheckValue(itemName, chip::to_underlying(current), expected);
     }
 
-    bool CheckValueAsList(const char * itemName, uint64_t current, uint64_t expected);
-
-    template <typename T>
-    bool CheckValueAsListHelper(const char * itemName, typename chip::app::DataModel::DecodableList<T>::Iterator iter)
+    /**
+     * Check that the next list item, which is at index "index", exists and
+     * decodes properly.
+     */
+    template <typename ListType>
+    bool CheckNextListItemDecodes(const char * listName, typename std::remove_reference_t<ListType>::Iterator & iter, size_t index)
     {
-        if (iter.Next())
-        {
-            Exit(std::string(itemName) + " value mismatch: expected no more items but found " + std::to_string(iter.GetValue()));
-            return false;
-        }
+        bool hasValue = iter.Next();
         if (iter.GetStatus() != CHIP_NO_ERROR)
         {
-            Exit(std::string(itemName) +
-                 " value mismatch: expected no more items but got an error: " + iter.GetStatus().AsString());
+            Exit(std::string(listName) + " value mismatch: error '" + iter.GetStatus().AsString() + "'decoding item at index " +
+                 std::to_string(index));
             return false;
         }
-        return true;
-    }
 
-    template <typename T, typename U, typename... ValueTypes>
-    bool CheckValueAsListHelper(const char * itemName, typename chip::app::DataModel::DecodableList<T>::Iterator & iter,
-                                const U & firstItem, ValueTypes &&... otherItems)
-    {
-        bool haveValue = iter.Next();
-        if (iter.GetStatus() != CHIP_NO_ERROR)
-        {
-            Exit(std::string(itemName) + " value mismatch: expected " + std::to_string(firstItem) +
-                 " but got error: " + iter.GetStatus().AsString());
-            return false;
-        }
-        if (!haveValue)
-        {
-            Exit(std::string(itemName) + " value mismatch: expected " + std::to_string(firstItem) +
-                 " but found nothing or an error");
-            return false;
-        }
-        if (iter.GetValue() != firstItem)
+        if (hasValue)
         {
-            Exit(std::string(itemName) + " value mismatch: expected " + std::to_string(firstItem) + " but found " +
-                 std::to_string(iter.GetValue()));
-            return false;
+            return true;
         }
-        return CheckValueAsListHelper<T>(itemName, iter, std::forward<ValueTypes>(otherItems)...);
-    }
 
-    template <typename T, typename... ValueTypes>
-    bool CheckValueAsList(const char * itemName, chip::app::DataModel::DecodableList<T> list, ValueTypes &&... items)
-    {
-        auto iter = list.begin();
-        return CheckValueAsListHelper<T>(itemName, iter, std::forward<ValueTypes>(items)...);
+        Exit(std::string(listName) + " value mismatch: should have value at index " + std::to_string(index) +
+             " but doesn't (actual value too short)");
+        return false;
     }
 
-    template <typename T>
-    bool CheckValueAsListLength(const char * itemName, chip::app::DataModel::DecodableList<T> list, uint64_t expectedLength)
+    /**
+     * Check that there are no more list items now that we have seen
+     * "expectedCount" of them.
+     */
+    template <typename ListType>
+    bool CheckNoMoreListItems(const char * listName, typename std::remove_reference_t<ListType>::Iterator & iter,
+                              size_t expectedCount)
     {
-        // We don't just use list.ComputeSize(), because we want to check that
-        // all the values in the list correctly decode to our type too.
-        auto iter      = list.begin();
-        uint64_t count = 0;
-        while (iter.Next())
-        {
-            ++count;
-        }
+        bool hasValue = iter.Next();
         if (iter.GetStatus() != CHIP_NO_ERROR)
         {
-            Exit(std::string(itemName) + " list length mismatch: expected " + std::to_string(expectedLength) + " but got an error");
+            Exit(std::string(listName) + " value mismatch: error '" + iter.GetStatus().AsString() +
+                 "'decoding item after we have seen " + std::to_string(expectedCount) + " items");
             return false;
         }
-        return CheckValueAsList(itemName, count, expectedLength);
+
+        if (!hasValue)
+        {
+            return true;
+        }
+
+        Exit(std::string(listName) + " value mismatch: expected only " + std::to_string(expectedCount) +
+             " items, but have more than that (actual value too long)");
+        return false;
     }
 
     bool CheckValueAsString(const char * itemName, chip::ByteSpan current, const char * expected);

examples/chip-tool/templates/partials/test_cluster_value_equals.zapt

@@ -8,12 +8,30 @@
     VerifyOrReturn(CheckValueNonNull("{{label}}", {{actual}}));
     {{>valueEquals label=(concat label ".Value()") actual=(concat actual ".Value()") expected=expected isNullable=false}}
   {{/if}}
+{{else if isArray}}
+    auto iter = {{actual}}.begin();
+    {{#each expected}}
+      VerifyOrReturn(CheckNextListItemDecodes<decltype({{../actual}})>("{{../label}}", iter, {{@index}}));
+      {{>valueEquals label=(concat ../label "[" @index "]") actual="iter.GetValue()" expected=this isArray=false type=../type chipType=../chipType}}
+    {{/each}}
+    VerifyOrReturn(CheckNoMoreListItems<decltype({{actual}})>("{{label}}", iter, {{expected.length}}));
 {{else}}
-  VerifyOrReturn(CheckValue
-    {{~#if isList}}AsListLength("{{label}}", {{actual}}, {{expected.length}})
-    {{else if isArray}}AsList("{{label}}", {{actual}}{{#if expected.length}}, {{expected}}{{/if}})
-    {{else if (isString type)}}AsString("{{label}}", {{actual}}, "{{expected}}")
-    {{else}}<{{chipType}}>("{{label}}", {{actual}}, {{expected}}{{asTypeLiteralSuffix type}})
-    {{/if}}
-  );
+  {{#if_is_struct type}}
+    {{! Iterate over the actual types in the struct, so we pick up the right
+        type/optionality/nullability information for them for our recursive
+        call. }}
+    {{#zcl_struct_items_by_struct_name type}}
+      {{#if (expectedValueHasProp ../expected (asLowerCamelCase label))}}
+        {{>valueEquals label=(concat ../label "." (asLowerCamelCase label)) actual=(concat ../actual "." (asLowerCamelCase label)) expected=(lookup ../expected (asLowerCamelCase label))}}
+      {{/if}}
+    {{/zcl_struct_items_by_struct_name}}
+    {{! Maybe we should add a check for properties in the expected object (other
+        than "global") that are not present in the struct ? }}
+  {{else}}
+    VerifyOrReturn(CheckValue
+      {{~#if (isString type)}}AsString("{{label}}", {{actual}}, "{{expected}}")
+      {{else}}<{{chipType}}>("{{label}}", {{actual}}, {{expected}}{{asTypeLiteralSuffix type}})
+      {{/if}}
+    );
+  {{/if_is_struct}}
 {{/if}}

src/app/clusters/test-cluster-server/test-cluster-server.cpp

@@ -129,10 +129,10 @@ EmberAfStatus writeTestListInt8uAttribute(EndpointId endpoint)
 CHIP_ERROR TestAttrAccess::ReadListInt8uAttribute(AttributeValueEncoder & aEncoder)
 {
     return aEncoder.EncodeList([](const TagBoundEncoder & encoder) -> CHIP_ERROR {
-        constexpr uint16_t attributeCount = 4;
-        for (uint8_t index = 0; index < attributeCount; index++)
+        constexpr uint8_t maxValue = 4;
+        for (uint8_t value = 1; value <= maxValue; value++)
         {
-            ReturnErrorOnFailure(encoder.Encode(index));
+            ReturnErrorOnFailure(encoder.Encode(value));
         }
         return CHIP_NO_ERROR;
     });

src/app/tests/suites/TV_AudioOutputCluster.yaml

@@ -25,9 +25,9 @@ tests:
       response:
           value:
               [
-                  { index: 1, outputType: 0, name: 12 },
-                  { index: 2, outputType: 0, name: 12 },
-                  { index: 3, outputType: 0, name: 12 },
+                  { index: 1, outputType: 0, name: "exampleName" },
+                  { index: 2, outputType: 0, name: "exampleName" },
+                  { index: 3, outputType: 0, name: "exampleName" },
               ]
 
     - label: "Select Output Command"

src/app/tests/suites/TV_MediaInputCluster.yaml

@@ -25,8 +25,18 @@ tests:
       response:
           value:
               [
-                  { index: 1, inputType: 4, name: 12, description: 19 },
-                  { index: 2, inputType: 4, name: 12, description: 19 },
+                  {
+                      index: 1,
+                      inputType: 4,
+                      name: "exampleName",
+                      description: "exampleDescription",
+                  },
+                  {
+                      index: 2,
+                      inputType: 4,
+                      name: "exampleName",
+                      description: "exampleDescription",
+                  },
               ]
 
     - label: "Select Input Command"

src/app/tests/suites/TV_TargetNavigatorCluster.yaml

@@ -23,7 +23,11 @@ tests:
       command: "readAttribute"
       attribute: "Target Navigator List"
       response:
-          value: [{ identifier: 1, name: 12 }, { identifier: 2, name: 12 }]
+          value:
+              [
+                  { identifier: 1, name: "exampleName" },
+                  { identifier: 2, name: "exampleName" },
+              ]
 
     - label: "Navigate Target Command"
       command: "NavigateTarget"

src/app/tests/suites/TV_TvChannelCluster.yaml

@@ -28,16 +28,16 @@ tests:
                   {
                       majorNumber: 1,
                       minorNumber: 2,
-                      name: 12,
-                      callSign: 13,
-                      affiliateCallSign: 13,
+                      name: "exampleName",
+                      callSign: "exampleCSign",
+                      affiliateCallSign: "exampleASign",
                   },
                   {
                       majorNumber: 2,
                       minorNumber: 3,
-                      name: 12,
-                      callSign: 13,
-                      affiliateCallSign: 13,
+                      name: "exampleName",
+                      callSign: "exampleCSign",
+                      affiliateCallSign: "exampleASign",
                   },
               ]
     # TODO: Enable the test once struct response is supported

src/app/tests/suites/TestDescriptorCluster.yaml

@@ -29,9 +29,26 @@ tests:
       command: "readAttribute"
       attribute: "Server List"
       response:
-          value: [0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0]
-    # CLUSTER_ID value is dummy 0 since yaml compares only the length of the List.
-    # right now
+          value: [
+                  0x0003, # Identify
+                  0x001D, # Descriptor
+                  0x0028, # Basic Information
+                  0x0029, # OTA Software Update Provider
+                  0x002A, # OTA Software Update Requestor
+                  0x0030, # General Commissioning
+                  0x0031, # Network Commissioning
+                  0x0032, # Diagnostic Logs
+                  0x0033, # General Diagnostics
+                  0x0034, # Software Diagnostics
+                  0x0035, # Thread Network Diagnostiscs
+                  0x0036, # WiFi Network Diagnostics
+                  0x0037, # Ethernet Network Diagnostics
+                  0x003C, # Administrator Commissioning
+                  0x003E, # Operational Credentials
+                  0x0405, # Relative Humidity Measurement (why on EP0?)
+                  0xF000, # Binding
+                  0xF004, # Group Key Management
+              ]
 
     - label: "Read attribute Client list"
       command: "readAttribute"

src/app/tests/suites/certification/Test_TC_DM_2_2.yaml

@@ -80,6 +80,8 @@ tests:
       command: "readAttribute"
       attribute: "TrustedRootCertificates"
       response:
-          value: [237]
+          # Can't check for the expected value here, since we don't know it.
+          # TODO: Check the length, at least?
+          # value: [237]
           constraints:
               type: list