Skip to content

Commit

Permalink
Fix ZAP variable lookups to not mis-treat non-strings as variable nam…
Browse files Browse the repository at this point in the history
…es. (#15276)

[nodeId] was being treated as a lookup of the nodeId varible and
ending up with a number, not an array containing a number.  Which
silently got treated as empty array!

Also adds sanity-checking so that "treat non-array as empty array"
silent thing does not happen anymore.
  • Loading branch information
bzbarsky-apple authored Feb 17, 2022
1 parent 8be86c5 commit 967546f
Show file tree
Hide file tree
Showing 5 changed files with 20 additions and 0 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
{{/if}}
{{else if isArray}}

{{ensureIsArray definedValue~}}
{{! 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. }}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
{{/if}}
{{/if}}
{{else if isArray}}
{{ensureIsArray expected~}}
{
auto iter_{{depth}} = {{actual}}.begin();
{{#each expected}}
Expand Down
16 changes: 16 additions & 0 deletions src/app/zap-templates/common/ClusterTestGeneration.js
Original file line number Diff line number Diff line change
Expand Up @@ -518,6 +518,12 @@ function chip_tests_items(options)

function getVariable(context, key, name)
{
if (!(typeof name == "string" || (typeof name == "object" && (name instanceof String)))) {
// Non-string key; don't try to look it up. Could end up looking like a
// variable name by accident when stringified.
return null;
}

while (!('variables' in context) && context.parent) {
context = context.parent;
}
Expand Down Expand Up @@ -756,6 +762,15 @@ function if_include_struct_item_value(structValue, name, options)
return options.inverse(this);
}

// To be used to verify that things are actually arrays before trying to use
// #each with them, since that silently treats non-arrays as empty arrays.
function ensureIsArray(value, options)
{
if (!(value instanceof Array)) {
printErrorAndExit(this, `Expected array but instead got ${typeof value}: ${JSON.stringify(value)}\n`);
}
}

//
// Module exports
//
Expand All @@ -775,3 +790,4 @@ exports.isTestOnlyCluster = isTestOnlyCluster;
exports.isLiteralNull = isLiteralNull;
exports.octetStringEscapedForCLiteral = octetStringEscapedForCLiteral;
exports.if_include_struct_item_value = if_include_struct_item_value;
exports.ensureIsArray = ensureIsArray;
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
{{/if}}
{{/if}}
{{else if isArray}}
{{ensureIsArray expected~}}
XCTAssertEqual([{{actual}} count], {{expected.length}});
{{#each expected}}
{{>check_test_value actual=(concat ../actual "[" @index "]") expected=this cluster=../cluster isArray=false type=../type parent=../parent}}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
{{>test_value target=target definedValue=definedValue cluster=cluster isNullable=false depth=(incrementDepth depth)}}
{{/if}}
{{else if isArray}}
{{ensureIsArray definedValue~}}
{
NSMutableArray * temp_{{depth}} = [[NSMutableArray alloc] init];
{{#each definedValue}}
Expand Down

0 comments on commit 967546f

Please sign in to comment.