Fix YAML checking of return values handle lists and structs correctly

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