Skip to content

Commit

Permalink
[Yaml] Remove optional keyword supports (#25016)
Browse files Browse the repository at this point in the history
  • Loading branch information
vivien-apple authored and pull[bot] committed Feb 13, 2024
1 parent 54d943c commit 1094156
Show file tree
Hide file tree
Showing 8 changed files with 56 additions and 170 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -15,16 +15,6 @@ switch(mTestSubStepIndex)
{{/if}}
{{/inline~}}

{{~#*inline "maybeReturnOnUnsupported"}}
{{~#if optional}}
if (IsUnsupported(status.mStatus))
{
{{>maybeContinueWithoutWaitingOnDone}}
return;
}
{{/if~}}
{{/inline~}}

{{! --- Test Step Response Content --}}
{{#if ../expectMultipleResponses}}
case {{index}}:
Expand All @@ -33,7 +23,6 @@ switch(mTestSubStepIndex)
VerifyOrReturn(CheckValue("status", chip::to_underlying(status.mStatus), {{error}}));
{{>maybeCheckClusterError}}
{{else}}
{{>maybeReturnOnUnsupported}}
VerifyOrReturn(CheckValue("status", chip::to_underlying(status.mStatus), {{error}}));
{{#if hasSpecificResponse}}
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -210,13 +210,6 @@ class {{filename}}: public TestCommandBridge
{{/if}}
NSLog(@"{{label}} Error: %@", err);

{{#if optional}}
if (err.code == MTRInteractionErrorCodeUnsupportedAttribute) {
NextTest();
return;
}
{{/if}}

{{#chip_tests_item_responses}}
{{#if error}}
VerifyOrReturn(CheckValue("status", err ? ([err.domain isEqualToString:MTRInteractionErrorDomain] ? err.code : EMBER_ZCL_STATUS_FAILURE) : 0, {{error}}));
Expand Down
53 changes: 4 additions & 49 deletions scripts/py_matter_yamltests/matter_yamltests/parser.py
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,6 @@
'verification',
'nodeId',
'attribute',
'optional',
'PICS',
'arguments',
'response',
Expand Down Expand Up @@ -102,7 +101,6 @@ class PostProcessCheckType(Enum):
CONSTRAINT_VALIDATION = auto()
SAVE_AS_VARIABLE = auto()
WAIT_VALIDATION = auto()
OPTIONAL = auto()


class PostProcessCheck:
Expand Down Expand Up @@ -204,7 +202,6 @@ def __init__(self, test: dict, config: dict, definitions: SpecDefinitions, pics_
_check_valid_keys(test, _TEST_SECTION)

self.label = _value_or_none(test, 'label')
self.optional = _value_or_none(test, 'optional')
self.node_id = _value_or_config(test, 'nodeId', config)
self.group_id = _value_or_config(test, 'groupId', config)
self.cluster = _value_or_config(test, 'cluster', config)
Expand Down Expand Up @@ -290,15 +287,10 @@ def __init__(self, test: dict, config: dict, definitions: SpecDefinitions, pics_
self.update_arguments(self.arguments_with_placeholders)
self.update_responses(self.responses_with_placeholders)

# Some keywords, such as "optional" and "wait_for" do not support multiple
# responses.
if len(responses) > 1:
if self.optional:
raise Exception(
'The "optional" keyword can not be used with multiple expected responses')
elif self.wait_for:
raise Exception(
'The "wait_for" keyword can not be used with multiple expected responses')
# The "wait_for" keyword do not support multiple responses.
if len(responses) > 1 and self.wait_for:
raise Exception(
'The "wait_for" keyword can not be used with multiple expected responses')

# This performs a very basic sanity parse time check of constraints. This parsing happens
# again inside post processing response since at that time we will have required variables
Expand Down Expand Up @@ -480,10 +472,6 @@ def is_event(self):
def label(self):
return self._test.label

@property
def optional(self):
return self._test.optional

@property
def node_id(self):
return self._test.node_id
Expand Down Expand Up @@ -557,9 +545,6 @@ def post_process_response(self, received_responses):
self._response_cluster_wait_validation(received_responses, result)
return result

if self._skip_post_processing(received_responses, result):
return result

check_type = PostProcessCheckType.RESPONSE_VALIDATION
error_failure_wrong_response_number = f'The test expects {len(self.responses)} responses but got {len(received_responses)} responses.'

Expand Down Expand Up @@ -649,36 +634,6 @@ def _response_cluster_wait_validation(self, received_responses, result):
result.success(check_type, error_success.format(
wait_for=self.wait_for, cluster=self.cluster, wait_type=expected_wait_type, endpoint=self.endpoint))

def _skip_post_processing(self, received_responses, result) -> bool:
'''Should we skip perform post processing.
Currently we only skip post processing if the test step indicates that sent test step
invokation was expected to be optionally supported. We confirm that it is optional
supported by either validating we got the expected error only then indicate that all
other post processing should be skipped.
'''
if not self.optional:
return False

check_type = PostProcessCheckType.OPTIONAL
error_failure_multiple_responses = 'The test expects a single response but got {len(received_responses)} responses.'

if len(received_responses) > 1:
result.error(check_type, error_failure.multiple_responses)
return False
received_response = received_responses[0]

received_error = received_response.get('error', None)
if received_error is None:
return False

if received_error == 'UNSUPPORTED_ATTRIBUTE' or received_error == 'UNSUPPORTED_COMMAND':
result.warning(PostProcessCheckType.OPTIONAL,
f'The response contains the error: "{received_error}".')
return True

return False

def _response_error_validation(self, expected_response, received_response, result):
check_type = PostProcessCheckType.IM_STATUS
error_success = 'The test expects the "{error}" error which occured successfully.'
Expand Down
6 changes: 3 additions & 3 deletions src/app/tests/suites/TestCluster.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -995,16 +995,16 @@ tests:
- label: "Read attribute UNSUPPORTED"
command: "readAttribute"
attribute: "unsupported"
optional: true
response:
value: 0
error: UNSUPPORTED_ATTRIBUTE

- label: "Writeattribute UNSUPPORTED"
command: "writeAttribute"
attribute: "unsupported"
optional: true
arguments:
value: 0
response:
error: UNSUPPORTED_ATTRIBUTE

- label: "Send Test Command to unsupported endpoint"
command: "Test"
Expand Down
2 changes: 1 addition & 1 deletion src/app/tests/suites/TestDiscovery.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -201,7 +201,7 @@ tests:
# product id ?
- label: "TXT key for Vendor ID and Product ID (VP)"
PICS: MCORE.SC.VP_KEY
optional: true
disabled: true # [TestDiscovery] Enable back TXT key for Vendor ID and Product ID (VP) #25017
cluster: "DiscoveryCommands"
command: "FindCommissionable"
response:
Expand Down
7 changes: 0 additions & 7 deletions src/controller/python/chip/yaml/runner.py
Original file line number Diff line number Diff line change
Expand Up @@ -220,8 +220,6 @@ def __init__(self, test_step, cluster: str, context: _ExecutionContext):
if test_step.fabric_filtered is not None:
self._fabric_filtered = test_step.fabric_filtered

self._possibly_unsupported = bool(test_step.optional)

self._cluster_object = context.data_model_lookup.get_cluster(self._cluster)
if self._cluster_object is None:
raise UnexpectedParsingError(
Expand Down Expand Up @@ -260,11 +258,6 @@ def run_action(self, dev_ctrl: ChipDeviceController) -> _ActionResult:
return self.parse_raw_response(raw_resp)

def parse_raw_response(self, raw_resp) -> _ActionResult:
if self._possibly_unsupported and not raw_resp:
# We have found an unsupported attribute. TestStep provided did specify that it might be
# unsupported, so nothing left to validate. We just return a failure here.
return _ActionResult(status=_ActionStatus.ERROR, response=None)

# TODO Currently there are no checks that this indexing won't fail. Need to add some
# initial validity checks. Coming soon in a future PR.
resp = raw_resp[self._endpoint][self._cluster_object][self._request_object]
Expand Down
Loading

0 comments on commit 1094156

Please sign in to comment.