connectedhomeip
connectedhomeip copied to clipboard
Update the hasValue constraint test by adding a dedicated file
Problem
matter_yamltests
is mixing null
values and missing
values. This PR add the necessary bits to differentiate both and also adds a new attribute of type struct to unittesting
in order to properly validate that optional in struct are handled properly.
This is a draft until #25343 lands since chip-tool
does not clean up optional fields properly at the moment.
Does this match existing YAML semantics in terms of how constraints other than "hasValue" are applied (or not) to missing optional values?
For the moment this is stricter in the sense that a missing value with a a constraint other than hasValue
will generate an error. This is because of the lines at the beginning of BaseConstraint.validate
:
if not value.has_value():
reason = f"The constraint expects a value but there isn't one."
self._raise_error(reason)
Removing those should bring us back to the same behaviour. I would like to run the CI against this PR to check if we do have some constraints on missing fields that were unexpected, but we can definitively remove it afterward.
I would like to run the CI against this PR to check if we do have some constraints on missing fields that were unexpected
Running CI is not enough. We can have YAMLs that have such constraints on fields that all-clusters-app (which is what we use in CI) sends but that other devices might not send. If we change semantics here we need to look at all the relevant places in the YAML and make sure that it's correctly expressing the intent of the test plan.
The real check is: are there optionals that have constraints other than hasValue listed at all?
This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.
This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.
This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.
This is an old PR that does not seem to be updated, is in draft and has several merge conflicts. Closing as stale.