Fix YAML tests handling of lists of structs that contain lists. (#13142)

The main issue was that comparing to such a value failed because we
ended up with "iter" variables for both levels of list nesting that
name-collided.

The fix for that was to add "depth" disabmbiguation like we have in
the Darwin codegen already.

While adding a test for this, a few other issues were encountered:

1) The "commandValue" partial, when dealing with an optional or
nullable struct, would do an Emplace() or SetNonNull() for every
single member, which would wipe out earlier members.  The fix for that
is to do them once, and then use .Value() for the recursive partial
invocation.

2) When sending a struct with optional fields, we were requiring the
YAML to specify values for all the fields.  Added
if_include_struct_item_value to allow the YAML to not include values
for such fields.

3) The YAML for checking struct values expected struct field names in
the YAML to be lowerCamelCase instead of matching the XML case.  The
code for sending struct values expected struct field names to match
the XML case.  This inconsistency was very confusing, and this PR
makes both expect the XML case.  I verified that the only codegen
changes due to this were in the TestModeSelectCluster and
Test_TC_DM_2_2 tests.  The latter had actually had its struct fields
ignored because they were the "wrong" case....  Both tests fixed.

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

@@ -202,7 +202,7 @@ class {{filename}}: public TestCommand
 
         RequestType request;
         {{#chip_tests_item_parameters}}
-        {{>commandValue ns=parent.cluster container=(concat "request." (asLowerCamelCase label)) definedValue=definedValue}}
+        {{>commandValue ns=parent.cluster container=(concat "request." (asLowerCamelCase label)) definedValue=definedValue depth=0}}
         {{/chip_tests_item_parameters}}
 
         auto success = [](void * context, const typename RequestType::ResponseType & data) {
@@ -229,7 +229,7 @@ class {{filename}}: public TestCommand
 
         {{#chip_tests_item_parameters}}
         {{zapTypeToEncodableClusterObjectType type ns=parent.cluster}} {{asLowerCamelCase name}}Argument;
-        {{>commandValue ns=parent.cluster container=(concat (asLowerCamelCase name) "Argument") definedValue=definedValue}}
+        {{>commandValue ns=parent.cluster container=(concat (asLowerCamelCase name) "Argument") definedValue=definedValue depth=0}}
         {{/chip_tests_item_parameters}}
 
         {{#if isWriteAttribute}}
@@ -305,7 +305,7 @@ class {{filename}}: public TestCommand
           {{#chip_tests_item_response_parameters}}
             {{~#*inline "item"}}{{asLowerCamelCase name}}{{#if isOptional}}.Value(){{/if}}{{/inline}}
             {{#if hasExpectedValue}}
-              {{>valueEquals actual=(asLowerCamelCase name) label=(asLowerCamelCase name) expected=expectedValue}}
+              {{>valueEquals actual=(asLowerCamelCase name) label=(asLowerCamelCase name) expected=expectedValue depth=0}}
             {{/if}}
             {{#if hasExpectedConstraints}}
               {{#if isOptional}}

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

@@ -1,30 +1,36 @@
 {{#if isOptional}}
-  {{>commandValue ns=ns container=(concat container ".Emplace()") definedValue=definedValue type=type isOptional=false}}
+  {{container}}.Emplace();
+  {{>commandValue ns=ns container=(concat container ".Value()") definedValue=definedValue type=type isOptional=false depth=(incrementDepth depth)}}
 {{else if isNullable}}
   {{#if (isLiteralNull definedValue)}}
     {{container}}.SetNull();
   {{else}}
-    {{>commandValue ns=ns container=(concat container ".SetNonNull()") definedValue=definedValue type=type isNullable=false}}
+    {{container}}.SetNonNull();
+    {{>commandValue ns=ns container=(concat container ".Value()") definedValue=definedValue type=type isNullable=false depth=(incrementDepth depth)}}
   {{/if}}
 {{else if isArray}}
 
   {{! forceNotList=true because we really want the type of a single item here.
       Similarly, forceNotOptional=true and forceNotNullable=true because we
       have accounted for those already. }}
   {{#if definedValue.length}}
-  {{zapTypeToEncodableClusterObjectType type ns=ns forceNotList=true forceNotNullable=true forceNotOptional=true}} {{asLowerCamelCase label}}List[{{definedValue.length}}];
+  {{! This should really do heap-allocation with a function-scope-wide
+      auto-free setup, so we could guarantee no name collisions. }}
+  {{zapTypeToEncodableClusterObjectType type ns=ns forceNotList=true forceNotNullable=true forceNotOptional=true}} {{asLowerCamelCase label}}List_{{depth}}[{{definedValue.length}}];
   {{#each definedValue}}
-  {{>commandValue ns=../ns container=(concat (asLowerCamelCase ../label) "List[" @index "]") definedValue=. type=../type}}
+  {{>commandValue ns=../ns container=(concat (asLowerCamelCase ../label) "List_" ../depth "[" @index "]") definedValue=. type=../type depth=(incrementDepth ../depth)}}
   {{/each}}
-  {{container}} = {{asLowerCamelCase label}}List;
+  {{container}} = {{asLowerCamelCase label}}List_{{depth}};
   {{else}}
   {{container}} = chip::app::DataModel::List<{{zapTypeToEncodableClusterObjectType type ns=ns forceNotList=true forceNotNullable=true forceNotOptional=true}}>();
   {{/if}}
 {{else}}
   {{#if_is_struct type}}
 
     {{#zcl_struct_items_by_struct_name type}}
-      {{>commandValue ns=parent.ns container=(concat parent.container "." (asLowerCamelCase label)) definedValue=(lookup parent.definedValue name)}}
+      {{#if_include_struct_item_value ../definedValue name}}
+        {{>commandValue ns=parent.ns container=(concat parent.container "." (asLowerCamelCase label)) definedValue=(lookup ../definedValue name) depth=(incrementDepth ../depth)}}
+      {{/if_include_struct_item_value}}
     {{/zcl_struct_items_by_struct_name}}
 
   {{else}}

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

@@ -1,28 +1,30 @@
 {{#if isOptional}}
   VerifyOrReturn(CheckValuePresent("{{label}}", {{actual}}));
-  {{>valueEquals label=(concat label ".Value()") actual=(concat actual ".Value()") expected=expected isOptional=false}}
+  {{>valueEquals label=(concat label ".Value()") actual=(concat actual ".Value()") expected=expected isOptional=false depth=(incrementDepth depth)}}
 {{else if isNullable}}
   {{#if (isLiteralNull expected)}}
     VerifyOrReturn(CheckValueNull("{{label}}", {{actual}}));
   {{else}}
     VerifyOrReturn(CheckValueNonNull("{{label}}", {{actual}}));
-    {{>valueEquals label=(concat label ".Value()") actual=(concat actual ".Value()") expected=expected isNullable=false}}
+    {{>valueEquals label=(concat label ".Value()") actual=(concat actual ".Value()") expected=expected isNullable=false depth=(incrementDepth depth)}}
   {{/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}}));
+    {
+      auto iter_{{depth}} = {{actual}}.begin();
+      {{#each expected}}
+        VerifyOrReturn(CheckNextListItemDecodes<decltype({{../actual}})>("{{../label}}", iter_{{../depth}}, {{@index}}));
+        {{>valueEquals label=(concat ../label "[" @index "]") actual=(concat "iter_" ../depth ".GetValue()") expected=this isArray=false type=../type chipType=../chipType depth=(incrementDepth depth)}}
+      {{/each}}
+      VerifyOrReturn(CheckNoMoreListItems<decltype({{actual}})>("{{label}}", iter_{{depth}}, {{expected.length}}));
+    }
 {{else}}
   {{#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 (expectedValueHasProp ../expected label)}}
+        {{>valueEquals label=(concat ../label "." (asLowerCamelCase label)) actual=(concat ../actual "." (asLowerCamelCase label)) expected=(lookup ../expected label) depth=(incrementDepth depth)}}
       {{/if}}
     {{/zcl_struct_items_by_struct_name}}
     {{! Maybe we should add a check for properties in the expected object (other

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

@@ -96,6 +96,12 @@ Structs::SimpleStruct::Type gStructAttributeValue = { 0,          false,      Si
                                                       0,          0 };
 NullableStruct::TypeInfo::Type gNullableStructAttributeValue;
 
+// We don't actually support any interesting bits in the struct for now, except
+// for a non-null nullableList member.
+SimpleEnum gSimpleEnums[kAttributeListLength];
+size_t gSimpleEnumCount = 0;
+Structs::NullablesAndOptionalsStruct::Type gNullablesAndOptionalsStruct;
+
 CHIP_ERROR TestAttrAccess::Read(const ConcreteReadAttributePath & aPath, AttributeValueEncoder & aEncoder)
 {
     switch (aPath.mAttributeId)
@@ -334,17 +340,59 @@ CHIP_ERROR TestAttrAccess::WriteListStructOctetStringAttribute(AttributeValueDec
 CHIP_ERROR TestAttrAccess::ReadListNullablesAndOptionalsStructAttribute(AttributeValueEncoder & aEncoder)
 {
     return aEncoder.EncodeList([](const auto & encoder) -> CHIP_ERROR {
-        // Just encode a single default-initialized
-        // entry for now.
-        Structs::NullablesAndOptionalsStruct::Type entry;
-        ReturnErrorOnFailure(encoder.Encode(entry));
+        // Just encode our one struct for now.
+        ReturnErrorOnFailure(encoder.Encode(gNullablesAndOptionalsStruct));
         return CHIP_NO_ERROR;
     });
 }
 
 CHIP_ERROR TestAttrAccess::WriteListNullablesAndOptionalsStructAttribute(AttributeValueDecoder & aDecoder)
 {
-    // TODO Add yaml test case for NullablesAndOptionalsStruct list
+    DataModel::DecodableList<Structs::NullablesAndOptionalsStruct::DecodableType> list;
+    ReturnErrorOnFailure(aDecoder.Decode(list));
+
+    size_t count;
+    ReturnErrorOnFailure(list.ComputeSize(&count));
+    // This should really send proper errors on invalid input!
+    VerifyOrReturnError(count == 1, CHIP_ERROR_INVALID_ARGUMENT);
+
+    auto iter = list.begin();
+    while (iter.Next())
+    {
+        auto & value = iter.GetValue();
+        // We only support some values so far.
+        VerifyOrReturnError(value.nullableString.IsNull(), CHIP_ERROR_INVALID_ARGUMENT);
+        VerifyOrReturnError(value.nullableStruct.IsNull(), CHIP_ERROR_INVALID_ARGUMENT);
+
+        VerifyOrReturnError(!value.optionalString.HasValue(), CHIP_ERROR_INVALID_ARGUMENT);
+        VerifyOrReturnError(!value.nullableOptionalString.HasValue(), CHIP_ERROR_INVALID_ARGUMENT);
+        VerifyOrReturnError(!value.optionalStruct.HasValue(), CHIP_ERROR_INVALID_ARGUMENT);
+        VerifyOrReturnError(!value.nullableOptionalStruct.HasValue(), CHIP_ERROR_INVALID_ARGUMENT);
+        VerifyOrReturnError(!value.optionalList.HasValue(), CHIP_ERROR_INVALID_ARGUMENT);
+        VerifyOrReturnError(!value.nullableOptionalList.HasValue(), CHIP_ERROR_INVALID_ARGUMENT);
+
+        // Start our value off as null, just in case we fail to decode things.
+        gNullablesAndOptionalsStruct.nullableList.SetNull();
+        if (!value.nullableList.IsNull())
+        {
+            ReturnErrorOnFailure(value.nullableList.Value().ComputeSize(&count));
+            VerifyOrReturnError(count <= ArraySize(gSimpleEnums), CHIP_ERROR_INVALID_ARGUMENT);
+            auto iter2       = value.nullableList.Value().begin();
+            gSimpleEnumCount = 0;
+            while (iter2.Next())
+            {
+                gSimpleEnums[gSimpleEnumCount] = iter2.GetValue();
+                ++gSimpleEnumCount;
+            }
+            ReturnErrorOnFailure(iter2.GetStatus());
+            gNullablesAndOptionalsStruct.nullableList.SetNonNull(Span<SimpleEnum>(gSimpleEnums, gSimpleEnumCount));
+        }
+        gNullablesAndOptionalsStruct.nullableInt         = value.nullableInt;
+        gNullablesAndOptionalsStruct.optionalInt         = value.optionalInt;
+        gNullablesAndOptionalsStruct.nullableOptionalInt = value.nullableOptionalInt;
+    }
+
+    ReturnErrorOnFailure(iter.GetStatus());
     return CHIP_NO_ERROR;
 }
 

src/app/tests/suites/TestCluster.yaml

@@ -1567,6 +1567,50 @@ tests:
               - name: "wasPresent"
                 value: false
 
+    - label: "Read list of structs containing nullables and optionals"
+      command: "readAttribute"
+      attribute: "list_nullables_and_optionals_struct"
+      response:
+          # By default, just one item, with everything default-initialized
+          value:
+              [
+                  {
+                      NullableInt: null,
+                      NullableString: null,
+                      NullableStruct: null,
+                      NullableList: null,
+                  },
+              ]
+
+    - label: "Write list of structs containing nullables and optionals"
+      command: "writeAttribute"
+      attribute: "list_nullables_and_optionals_struct"
+      arguments:
+          value:
+              [
+                  {
+                      NullableInt: null,
+                      NullableString: null,
+                      NullableStruct: null,
+                      NullableList: [1, 2],
+                  },
+              ]
+
+    - label:
+          "Read list of structs containing nullables and optionals after writing"
+      command: "readAttribute"
+      attribute: "list_nullables_and_optionals_struct"
+      response:
+          value:
+              [
+                  {
+                      NullableInt: null,
+                      NullableString: null,
+                      NullableStruct: null,
+                      NullableList: [1, 2],
+                  },
+              ]
+
     # Nullable attributes
 
     # Tests for nullable Boolean attribute

src/app/tests/suites/TestModeSelectCluster.yaml

@@ -53,9 +53,9 @@ tests:
       response:
           value:
               [
-                  { label: "Black", mode: 0, semanticTag: 0 },
-                  { label: "Cappuccino", mode: 4, semanticTag: 0 },
-                  { label: "Espresso", mode: 7, semanticTag: 0 },
+                  { Label: "Black", Mode: 0, SemanticTag: 0 },
+                  { Label: "Cappuccino", Mode: 4, SemanticTag: 0 },
+                  { Label: "Espresso", Mode: 7, SemanticTag: 0 },
               ]
           constraints:
               - type: list

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

@@ -50,14 +50,10 @@ tests:
       command: "readAttribute"
       attribute: "fabrics list"
       response:
-          value:
-              [
+          value: [
                   {
-                      FabricIndex: 0,
-                      RootPublicKey: 65,
-                      VendorID: 65521,
-                      FabricID: 0,
-                      NodeID: 305414945,
+                      # We really don't know what values to expect for any of
+                      # the fields, except Label.
                       Label: "",
                   },
               ]

src/app/zap-templates/common/ClusterTestGeneration.js

@@ -609,6 +609,21 @@ function octetStringEscapedForCLiteral(value)
   });
 }
 
+// Structs may not always provide values for optional members.
+function if_include_struct_item_value(structValue, name, options)
+{
+  let hasValue = (name in structValue);
+  if (hasValue) {
+    return options.fn(this);
+  }
+
+  if (!this.isOptional) {
+    throw new Error(`Value not provided for ${name} where one is expected`);
+  }
+
+  return options.inverse(this);
+}
+
 //
 // Module exports
 //
@@ -621,3 +636,4 @@ exports.isTestOnlyCluster                   = isTestOnlyCluster;
 exports.isLiteralNull                       = isLiteralNull;
 exports.expectedValueHasProp                = expectedValueHasProp;
 exports.octetStringEscapedForCLiteral       = octetStringEscapedForCLiteral;
+exports.if_include_struct_item_value        = if_include_struct_item_value;

src/app/zap-templates/templates/app/helper.js

@@ -671,6 +671,11 @@ function isWeaklyTypedEnum(label)
   ].includes(label);
 }
 
+function incrementDepth(depth)
+{
+  return depth + 1;
+}
+
 //
 // Module exports
 //
@@ -689,3 +694,4 @@ exports.zapTypeToPythonClusterObjectType    = zapTypeToPythonClusterObjectType;
 exports.getResponseCommandName              = getResponseCommandName;
 exports.isWeaklyTypedEnum                   = isWeaklyTypedEnum;
 exports.getPythonFieldDefault               = getPythonFieldDefault;
+exports.incrementDepth                      = incrementDepth;

src/app/zap-templates/zcl/data-model/chip/test-cluster.xml

@@ -137,7 +137,7 @@ limitations under the License.
     <attribute side="server" code="0x0022" define="TEST_VENDOR_ID" type="vendor_id"
                writable="true" optional="false" default="0">vendor_id</attribute>
     <attribute side="server" code="0x0023" define="LIST_OF_STRUCTS_WITH_OPTIONALS" type="ARRAY" entryType="NullablesAndOptionalsStruct"
-               writable="false" optional="false">list_nullables_and_optionals_struct</attribute>
+               writable="true" optional="false">list_nullables_and_optionals_struct</attribute>
     <attribute side="server" code="0x0024" define="SIMPLE_ENUM" type="SimpleEnum" writable="true" optional="false">enum_attr</attribute>
     <attribute side="server" code="0x0025" define="STRUCT" type="SimpleStruct" writable="true" optional="false">struct_attr</attribute>
     <attribute side="server" code="0x0026" define="RANGE_RESTRICTED_INT8U" type="int8u" min="20" max="100" writable="true" optional="false" default="70">range_restricted_int8u</attribute>

src/darwin/Framework/CHIP/templates/helper.js

@@ -161,11 +161,6 @@ async function asObjectiveCType(type, cluster, options)
   return typeStr;
 }
 
-function incrementDepth(depth)
-{
-  return depth + 1;
-}
-
 function asStructPropertyName(prop)
 {
   prop = appHelper.asLowerCamelCase(prop);
@@ -206,7 +201,6 @@ exports.asTestIndex                  = asTestIndex;
 exports.asTestValue                  = asTestValue;
 exports.asObjectiveCClass            = asObjectiveCClass;
 exports.asObjectiveCType             = asObjectiveCType;
-exports.incrementDepth               = incrementDepth;
 exports.asStructPropertyName         = asStructPropertyName;
 exports.asGetterName                 = asGetterName;
 exports.commandHasRequiredField      = commandHasRequiredField;

src/darwin/Framework/CHIP/templates/partials/check_test_value.zapt

@@ -20,8 +20,8 @@
         type/optionality/nullability information for them for our recursive
         call. }}
     {{#zcl_struct_items_by_struct_name type}}
-      {{#if (expectedValueHasProp ../expected (asLowerCamelCase label))}}
-        {{>check_test_value actual=(concat "((CHIP" (asUpperCamelCase ../cluster) "Cluster" (asUpperCamelCase ../type) " *)" ../actual ")." (asStructPropertyName label)) expected=(lookup ../expected (asLowerCamelCase label)) cluster=../cluster}}
+      {{#if (expectedValueHasProp ../expected label)}}
+        {{>check_test_value actual=(concat "((CHIP" (asUpperCamelCase ../cluster) "Cluster" (asUpperCamelCase ../type) " *)" ../actual ")." (asStructPropertyName label)) expected=(lookup ../expected label) cluster=../cluster}}
       {{/if}}
     {{/zcl_struct_items_by_struct_name}}
     {{! Maybe we should add a check for properties in the expected object (other

src/darwin/Framework/CHIP/templates/partials/test_value.zapt

@@ -19,9 +19,12 @@
   {{#if_is_struct type}}
     {{target}} = [[CHIP{{asUpperCamelCase cluster}}Cluster{{asUpperCamelCase type}} alloc] init];
     {{#zcl_struct_items_by_struct_name type}}
-      {{! target may be some place where we lost type information (e.g. an id),
-          so add explicit cast when trying to assign to our properties. }}
-      {{>test_value target=(concat "((CHIP" (asUpperCamelCase ../cluster) "Cluster" (asUpperCamelCase ../type) " *)" ../target ")." (asStructPropertyName label)) definedValue=(lookup ../definedValue name) cluster=../cluster depth=(incrementDepth ../depth)}}
+      {{#if_include_struct_item_value ../definedValue name}}
+        {{! target may be some place where we lost type information (e.g. an
+            id), so add explicit cast when trying to assign to our
+            properties. }}
+        {{>test_value target=(concat "((CHIP" (asUpperCamelCase ../cluster) "Cluster" (asUpperCamelCase ../type) " *)" ../target ")." (asStructPropertyName label)) definedValue=(lookup ../definedValue name) cluster=../cluster depth=(incrementDepth ../depth)}}
+      {{/if_include_struct_item_value}}
     {{/zcl_struct_items_by_struct_name}}
 
   {{else if (isCharString type)}}