-
Notifications
You must be signed in to change notification settings - Fork 2.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[TestConstraints] Add some tests for the hasValue constraints #25087
[TestConstraints] Add some tests for the hasValue constraints #25087
Conversation
PR #25087: Size comparison from d2e4c4a to 73652ba Increases (6 builds for esp32, linux, psoc6, telink)
Decreases (4 builds for bl702, cyw30739, esp32, psoc6)
Full report (33 builds for bl602, bl702, cc13x2_26x2, cc32xx, cyw30739, esp32, linux, mbed, psoc6, qpg, telink)
|
constraints: | ||
hasValue: false |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But.... this is not in fact generating any code to check that there is no value, if you look at the generated code!
More generally, my understanding was that hasValue was about testing for value existence, not null vs not null?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I had assumed python yaml parser is the one that validate these.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Python right now can't tell apart missing and null @andy31415 so can't correctly implement hasValue anyway.
…t-chip#25087) * [TestConstraints] Add some tests for the hasValue constraints * Update generated tests content
Problem
It seems that
TestConstraints.yaml
does not contains any tests for thehasValue
constraints. This PR adds some.