Skip to content

Commit

Permalink
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Fix YAML checking of return values handle lists and structs correctly (
Browse files Browse the repository at this point in the history
…#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.
bzbarsky-apple authored and pull[bot] committed Dec 14, 2022
1 parent c05915c commit 1703560
Showing 12 changed files with 336 additions and 146 deletions.
14 changes: 2 additions & 12 deletions examples/chip-tool/commands/tests/TestCommand.cpp
Original file line number Diff line number Diff line change
@@ -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;
}

91 changes: 36 additions & 55 deletions examples/chip-tool/commands/tests/TestCommand.h
Original file line number Diff line number Diff line change
@@ -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);
Original file line number Diff line number Diff line change
@@ -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}}
6 changes: 3 additions & 3 deletions src/app/clusters/test-cluster-server/test-cluster-server.cpp
Original file line number Diff line number Diff line change
@@ -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;
});
6 changes: 3 additions & 3 deletions src/app/tests/suites/TV_AudioOutputCluster.yaml
Original file line number Diff line number Diff line change
@@ -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"
14 changes: 12 additions & 2 deletions src/app/tests/suites/TV_MediaInputCluster.yaml
Original file line number Diff line number Diff line change
@@ -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"
6 changes: 5 additions & 1 deletion src/app/tests/suites/TV_TargetNavigatorCluster.yaml
Original file line number Diff line number Diff line change
@@ -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"
12 changes: 6 additions & 6 deletions src/app/tests/suites/TV_TvChannelCluster.yaml
Original file line number Diff line number Diff line change
@@ -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
23 changes: 20 additions & 3 deletions src/app/tests/suites/TestDescriptorCluster.yaml
Original file line number Diff line number Diff line change
@@ -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"
4 changes: 3 additions & 1 deletion src/app/tests/suites/certification/Test_TC_DM_2_2.yaml
Original file line number Diff line number Diff line change
@@ -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
79 changes: 43 additions & 36 deletions src/app/zap-templates/common/ClusterTestGeneration.js
Original file line number Diff line number Diff line change
@@ -467,6 +467,42 @@ function chip_tests_item_response_type(options)
return asPromise.call(this, promise, options);
}

// test_cluster_command_value and test_cluster_value-equals are recursive partials using #each. At some point the |global|
// context is lost and it fails. Make sure to attach the global context as a property of the | value |
// that is evaluated.
function attachGlobal(global, value)
{
if (Array.isArray(value)) {
value = value.map(v => attachGlobal(global, v));
} else if (value instanceof Object) {
for (key in value) {
if (key == "global") {
continue;
}
value[key] = attachGlobal(global, value[key]);
}
} else if (value === null) {
value = new NullObject();
} else {
switch (typeof value) {
case 'number':
value = new Number(value);
break;
case 'string':
value = new String(value);
break;
case 'boolean':
value = new Boolean(value);
break;
default:
throw new Error('Unsupported value: ' + JSON.stringify(value));
}
}

value.global = global;
return value;
}

function chip_tests_item_parameters(options)
{
const commandValues = this.arguments.values;
@@ -494,41 +530,6 @@ function chip_tests_item_parameters(options)
'Missing "' + commandArg.name + '" in arguments list: \n\t* '
+ commandValues.map(command => command.name).join('\n\t* '));
}
// test_cluster_command_value is a recursive partial using #each. At some point the |global|
// context is lost and it fails. Make sure to attach the global context as a property of the | value |
// that is evaluated.
function attachGlobal(global, value)
{
if (Array.isArray(value)) {
value = value.map(v => attachGlobal(global, v));
} else if (value instanceof Object) {
for (key in value) {
if (key == "global") {
continue;
}
value[key] = attachGlobal(global, value[key]);
}
} else if (value === null) {
value = new NullObject();
} else {
switch (typeof value) {
case 'number':
value = new Number(value);
break;
case 'string':
value = new String(value);
break;
case 'boolean':
value = new Boolean(value);
break;
default:
throw new Error('Unsupported value: ' + JSON.stringify(value));
}
}

value.global = global;
return value;
}
commandArg.definedValue = attachGlobal(this.global, expected.value);

return commandArg;
@@ -558,7 +559,7 @@ function chip_tests_item_response_parameters(options)
const expected = responseValues.splice(expectedIndex, 1)[0];
if ('value' in expected) {
responseArg.hasExpectedValue = true;
responseArg.expectedValue = expected.value;
responseArg.expectedValue = attachGlobal(this.global, expected.value);
}

if ('constraints' in expected) {
@@ -590,6 +591,11 @@ function isLiteralNull(value, options)
return (value === null) || (value instanceof NullObject);
}

function expectedValueHasProp(value, name)
{
return name in value;
}

//
// Module exports
//
@@ -601,3 +607,4 @@ exports.chip_tests_item_response_parameters = chip_tests_item_response_parameter
exports.chip_tests_pics = chip_tests_pics;
exports.isTestOnlyCluster = isTestOnlyCluster;
exports.isLiteralNull = isLiteralNull;
exports.expectedValueHasProp = expectedValueHasProp;
195 changes: 178 additions & 17 deletions zzz_generated/chip-tool/zap-generated/test/Commands.h

Large diffs are not rendered by default.

0 comments on commit 1703560

Please sign in to comment.