Issue120 Fixes for On/Off cluster YAMLS #32001
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
2 tasks
|
PR #32001: Size comparison from 71fe503 to cb4980b Decreases (2 builds for efr32)
Full report (73 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, cyw30739, efr32, esp32, k32w, linux, mbed, nrfconnect, psoc6, qpg, stm32, telink)
|
cecille
reviewed
Feb 7, 2024
| @@ -38,7 +38,7 @@ tests: | |||
| command: "readAttribute" | |||
| attribute: "ClusterRevision" | |||
| response: | |||
| value: 5 | |||
| value: 6 | |||
There was a problem hiding this comment.
I think you're going to get some test failures here because now the cluster revision won't match the cluster revision in the app. If you don't get failures, that means the test isn't running. But I think it is. It should be.
There was a problem hiding this comment.
Would it need to also get updated elsewhere? This was specifically requested from the issue raised in the description
There was a problem hiding this comment.
A few things need to happen here. The revision should be 6, that is stated by both spec and test plan. In order for that change to land nicely, we need to get all the examples (zap files) updated to match this cluster rev as well. Leaving it a 5 but still adding in the new feature is not the right solution.
There was a problem hiding this comment.
This runs against the all cluster app in the CI, so that will need to be updated at the least to change the cluster revision. If the LT feature is supported, the attributes also need to be turned on. And you will need to update the ci-pics-values file to reflect that.
There was a problem hiding this comment.
Once all-clusters is done, the relevant owners for the other examples will also need to update their code.
|
PR #32001: Size comparison from 71fe503 to 5baad5f Decreases (2 builds for efr32)
Full report (73 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, cyw30739, efr32, esp32, k32w, linux, mbed, nrfconnect, psoc6, qpg, stm32, telink)
|
cecille
reviewed
Feb 8, 2024
| response: | ||
| constraints: | ||
| type: int16u | ||
| value: 30 |
There was a problem hiding this comment.
This is causing a test failure because value has to be defined at the same level as constraints. Did you run the test locally?
See https://project-chip.github.io/connectedhomeip-doc/testing/yaml.html#running-yaml-tests
| @@ -128,6 +138,17 @@ tests: | |||
| type: list | |||
| contains: [64, 65, 66] | |||
|
|
|||
| - label: | |||
| "Step 6c: TH reads the feature dependent(OO.S.F02) commands in | |||
There was a problem hiding this comment.
Doesn't step 6a above need to be fixed too?
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes project-chip/matter-test-scripts#120
Adds YAMLS test steps for updated test plan
Updates revision number