Skip to content
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

[1.3] TC-TMP-2.1 uses invalid YAML and has PICS set incorrectly. #32835

Closed
cecille opened this issue Apr 3, 2024 · 6 comments
Closed

[1.3] TC-TMP-2.1 uses invalid YAML and has PICS set incorrectly. #32835

cecille opened this issue Apr 3, 2024 · 6 comments
Labels
bug Something isn't working CI/CD improvements Improvements to our CI/CD pipelines needs triage

Comments

@cecille
Copy link
Contributor

cecille commented Apr 3, 2024

Reproduction steps

Step 3:
    - label: "Step 3: TH reads the MaxMeasuredValue attribute from the DUT"
      PICS: TMP.S.A0002
      command: "readAttribute"
      attribute: "MaxMeasuredValue"
      response:
          saveAs: CurrentMaxMeasured
          constraints:
              type: int16s
              minValue: CurrentMinMeasured+1
              maxValue: 32767

Arithmetic expressions are not supported in yaml, which means "CurrentMinMeasured+1" is invalid. It also means that the CI is not running this step. Probably because the PICS is set incorrectly, even though this is a mandatory attribute.

Bug prevalence

always

GitHub hash of the SDK that was being used

e53dca8

Platform

other

Platform Version(s)

yaml;

Type

Test Improvement

Anything else?

No response

@cecille cecille added bug Something isn't working needs triage labels Apr 3, 2024
@Apollon77
Copy link
Contributor

Thank you for validating

@cecille
Copy link
Contributor Author

cecille commented Apr 3, 2024

@raul-marquez-csa FYI

@cecille
Copy link
Contributor Author

cecille commented Apr 3, 2024

From ci-pics-values:

# Thermostat User Configuration cluster
TMP.S=1
TMP.S.A0000=1
TMP.S.A0001=1
TMP.S.A0002=1
TMP.S.A0003=1
TMP.M.ManuallyControlled=1

So why is this step not failing in the CI?

@cecille cecille added the CI/CD improvements Improvements to our CI/CD pipelines label Apr 3, 2024
@cecille
Copy link
Contributor Author

cecille commented Apr 4, 2024

OK, it looks like what's going on here is that the all clusters app returns null for these two attributes, and the yaml check therefore does not hit the affected line. There are a few things that need to happen as a result of this:

  • we need a spec issue for the constraint on that cluster because it is underdescribed
  • this test needs to be re-implemeted in python. This needs to happen somehow either BEFORE this lands as a part of the 1.3 package or with a CCB to back port to wherever that addition change landed
  • We need to implement a linter to ensure that we're not trying to do arithmetic operations in yaml files. YAML is not the appropriate place to do math. While we're at it, let's also lint for the unit testing cluster in lieu of this because we've seen that workaround before too
  • We need to understand what's going on with the constraints checker around null values, because this seems incorrect to just skip all the checks here.

@cecille
Copy link
Contributor Author

cecille commented Apr 4, 2024

spec issue: https://github.com/CHIP-Specifications/connectedhomeip-spec/issues/9091
CI issue: project-chip/matter-test-scripts#218
tests with arithmetic operations: project-chip/matter-test-scripts#217
test with unit testing cluster: project-chip/matter-test-scripts#216
tracking YAML runner tests when returned value is null: #32851

@cecille
Copy link
Contributor Author

cecille commented Apr 15, 2024

#32881 is the fix for this issue. Other issues will be tracked as above.

@cecille cecille closed this as completed Apr 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working CI/CD improvements Improvements to our CI/CD pipelines needs triage
Projects
None yet
Development

No branches or pull requests

2 participants