Skip to content

Commit

Permalink
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.
  • Loading branch information
bzbarsky-apple authored and pull[bot] committed Aug 9, 2022
1 parent 2db3b4d commit e4a9f48
Show file tree
Hide file tree
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
Expand Up @@ -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));
Expand All @@ -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;
}

Expand Down
91 changes: 36 additions & 55 deletions examples/chip-tool/commands/tests/TestCommand.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand All @@ -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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Up @@ -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;
});
Expand Down
6 changes: 3 additions & 3 deletions src/app/tests/suites/TV_AudioOutputCluster.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
14 changes: 12 additions & 2 deletions src/app/tests/suites/TV_MediaInputCluster.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
6 changes: 5 additions & 1 deletion src/app/tests/suites/TV_TargetNavigatorCluster.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
12 changes: 6 additions & 6 deletions src/app/tests/suites/TV_TvChannelCluster.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
23 changes: 20 additions & 3 deletions src/app/tests/suites/TestDescriptorCluster.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
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
Expand Up @@ -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
Loading

0 comments on commit e4a9f48

Please sign in to comment.