Add support for minRevision/maxRevision to YAML test framework (project-chip#41706)

* Add support for minRevision/maxRevision to YAML test framework

Changes done:

- Add minRevision and maxRevision fields to YAML tests processing
- Add code to auto-augment the ClusterRevision reading in the
  test execution set of steps if any other steps have
  minRevision, maxRevision.
- Update runner to honor skipping on min/maxRevision
- Update docs to show usage.

Testing done:
- Added necessary unit tests to existing test_yaml_parser.py
- Added an integration test in TestBasicInformation.yaml
- All other unit and integration tests pass

* Restyled by prettier-yaml

* Restyled by autopep8

* Restyled by isort

* Fix return statement

* Fix lint

* Fix parser error, add docs

* Restyled by prettier-markdown

* Address review comments

* Restyled by autopep8

* Restyled by isort

---------

Co-authored-by: Restyled.io <[email protected]>

Author: tcarmelveilleux

docs/testing/yaml.md

@@ -271,12 +271,33 @@ the test harness. Note that full-test gating is not currently implemented in the
 local runner or in the CI.
 
 Some test steps need to be gated on values from earlier in the test. In these
-cases, PICS cannot be used. Instead, the runIf: tag can be used. This tag
+cases, PICS cannot be used. Instead, the `runIf` tag can be used. This tag
 requires a boolean value. To convert values to booleans, the TestEqualities
 function can be use. See
 [TestEqualities](https://github.com/project-chip/connectedhomeip/blob/master/src/app/tests/suites/TestEqualities.yaml)
 for an example of how to use this pseudo-cluster.
 
+In addition to the `PICS` and `runIf` tags, there are the `minRevision` and
+`maxRevision` tags which use the step's `cluster`'s `ClusterRevision` attribute
+to determine if a step should be skipped because it only applies to older or
+newer versions. A step will be skipped if the cluster's `ClusterRevision`
+attribute is < `minRevision` (if present), or > `maxRevision` (if present).
+
+Here is an example of only checking for the value of a given attribute if
+`ClusterRevision` >= 3 using `minRevision`.
+
+```yaml
+- label: "Verify the minimum-to-support Max Paths Per Invoke value"
+  command: "readAttribute"
+  attribute: "MaxPathsPerInvoke"
+  minRevision:
+      3 # Attribute was added in revision 3, so this step applies
+      # to revision >= 3.
+  response:
+      constraints:
+          minValue: 1
+```
+
 #### Setting step timeouts
 
 The timeout argument can be used for each individual test step to set the time

docs/testing/yaml_schema.md

@@ -76,3 +76,5 @@ YAML schema
 |  dataVersion |list,int|Y|
 |  busyWaitMs |int||
 |  wait |str||
+|  minRevision |int|Y|
+|  maxRevision |int|Y|

scripts/py_matter_yamltests/matter/yamltests/parser.py

@@ -18,7 +18,7 @@
 import re
 from dataclasses import dataclass, field
 from enum import Enum, auto
-from typing import Optional
+from typing import Any, Optional
 
 from . import fixes
 from .constraints import get_constraints, is_typed_constraint, is_variable_aware_constraint
@@ -46,6 +46,11 @@
 STRICT_ENUM_VALUE_CHECK = False
 
 
+def build_revision_var_name(endpoint, cluster) -> str:
+    """Helper to create a unique variable name for the cluster revision read-out."""
+    return f'__cluster_revision_read_EP_{endpoint}_CL_{cluster}'
+
+
 class UnknownPathQualifierError(TestStepError):
     """Raise when an attribute/command/event name is not found in the definitions."""
 
@@ -301,6 +306,8 @@ def __init__(self, test: dict, config: dict, definitions: SpecDefinitions, pics_
 
         self.identity = _value_or_none(test, 'identity')
         self.fabric_filtered = _value_or_none(test, 'fabricFiltered')
+        self.min_revision = _value_or_none(test, 'minRevision')
+        self.max_revision = _value_or_none(test, 'maxRevision')
         self.min_interval = _value_or_none(test, 'minInterval')
         self.max_interval = _value_or_none(test, 'maxInterval')
         self.keep_subscriptions = _value_or_none(test, 'keepSubscriptions')
@@ -748,6 +755,10 @@ def __init__(self, test: _TestStepWithPlaceholders, step_index: int, runtime_con
                 self._test.group_id)
             self._test.node_id = self._config_variable_substitution(
                 self._test.node_id)
+            self._test.min_revision = self._config_variable_substitution(
+                self._test.min_revision)
+            self._test.max_revision = self._config_variable_substitution(
+                self._test.max_revision)
             test.update_arguments(self.arguments)
             test.update_responses(self.responses)
 
@@ -855,6 +866,60 @@ def event_number(self, value):
     def pics(self):
         return self._test.pics
 
+    @property
+    def min_revision(self):
+        return self._test.min_revision
+
+    @property
+    def max_revision(self):
+        return self._test.max_revision
+
+    @property
+    def is_revision_condition_passed(self) -> bool:
+        """Checks if the revision conditions passed properly based on min/maxRevision.
+
+        This is evaluated at runtime by the runner.
+
+        Returns True if step can be run, False if it cannot be run, or raises an exception
+        if there was an error processing revision (i.e. internal error).
+        """
+        # If no revision checks are defined, the step is OK to run.
+        if self.min_revision is None and self.max_revision is None:
+            return True
+
+        endpoint = self.endpoint
+        cluster = self.cluster
+
+        # The runner will have executed a prior step to read the revision, based
+        # on the parser having injected that step (search for minRevision in
+        # `YamlTests` class).
+        var_name = build_revision_var_name(endpoint, cluster)
+
+        cluster_revision = self.get_runtime_variable(var_name)
+
+        if cluster_revision is None:
+            raise KeyError(
+                f"Step '{self.label}': Cannot check min/maxRevision. "
+                f"ClusterRevision variable '{var_name}' is not set (read step may have been skipped or failed)."
+            )
+
+        # Perform the checks
+        try:
+            # Ensure values are integers for comparison
+            cluster_revision = int(cluster_revision)
+
+            return all([
+                (self.min_revision is None) or (cluster_revision >= int(self.min_revision)),
+                (self.max_revision is None) or (cluster_revision <= int(self.max_revision)),
+            ])
+        except (ValueError, TypeError) as e:
+            # Failed to convert revision to int. This can happen with malformed YAML.
+            raise ValueError(
+                f"Step '{self.label}': Error checking min/maxRevision. "
+                f"cluster_revision='{cluster_revision}', min='{self.min_revision}', max='{self.max_revision}'. "
+                f"Error: {e}."
+            )
+
     def _get_last_event_number(self, responses) -> Optional[int]:
         if not self.is_event:
             return None
@@ -877,6 +942,10 @@ def _get_last_event_number(self, responses) -> Optional[int]:
 
         return event_number
 
+    def get_runtime_variable(self, name: str) -> Any:
+        """Gets a runtime variable from the test context, or None if missing."""
+        return self._runtime_config_variable_storage.get(name)
+
     def post_process_response(self, received_responses):
         result = PostProcessResponseResult()
 
@@ -1159,7 +1228,8 @@ def _response_constraints_validation(self, expected_response, received_response,
 
             for constraint in constraints:
                 try:
-                    constraint.validate(received_value, response_type_name, self._runtime_config_variable_storage)
+                    constraint.validate(
+                        received_value, response_type_name, self._runtime_config_variable_storage)
                     result.success(check_type, error_success)
                 except TestStepError as e:
                     e.update_context(expected_response, self.step_index)
@@ -1265,9 +1335,19 @@ class YamlTests:
 
     def __init__(self, parsing_config_variable_storage: dict, definitions: SpecDefinitions, pics_checker: PICSChecker, tests: dict):
         self._parsing_config_variable_storage = parsing_config_variable_storage
+        self._runtime_config_variable_storage = copy.deepcopy(
+            parsing_config_variable_storage)
+        self._definitions = definitions
+        self._pics_checker = pics_checker
+
+        tests_with_cluster_revision_injections = self._tests_with_cluster_revision_checks(
+            tests)
+
+        # Build list of enabled tests from the starting point where pseudo steps are added/injected for
+        # things like min/maxRevision tests.
         enabled_tests = []
         try:
-            for step_index, step in enumerate(tests):
+            for step_index, step in enumerate(tests_with_cluster_revision_injections):
                 test_with_placeholders = _TestStepWithPlaceholders(
                     step, self._parsing_config_variable_storage, definitions, pics_checker)
                 if test_with_placeholders.is_enabled:
@@ -1278,12 +1358,93 @@ def __init__(self, parsing_config_variable_storage: dict, definitions: SpecDefin
 
         fixes.try_update_yaml_node_id_test_runner_state(
             enabled_tests, self._parsing_config_variable_storage)
-        self._runtime_config_variable_storage = copy.deepcopy(
-            parsing_config_variable_storage)
+
         self._tests = enabled_tests
         self._index = 0
         self.count = len(self._tests)
 
+    def _tests_with_cluster_revision_checks(self, tests: dict) -> list[TestStep]:
+        """Injection Logic for synthetic steps needed to process cluster revision checks."""
+
+        # Pre-pass: Find all (endpoint, cluster) pairs that *need* a revision check.
+        default_endpoint = self._get_config_value(
+            self._parsing_config_variable_storage, 'endpoint')
+        default_cluster = self._get_config_value(
+            self._parsing_config_variable_storage, 'cluster')
+
+        needed_revisions = set()  # Set of (endpoint, cluster) tuples
+        for step in tests:
+            # A step needs a revision check if it has min/maxRevision and is not disabled.
+            if 'disabled' in step and step['disabled']:
+                continue
+
+            if 'minRevision' in step or 'maxRevision' in step:
+                endpoint = step.get('endpoint', default_endpoint)
+                cluster = step.get('cluster', default_cluster)
+
+                if endpoint is not None and cluster is not None:
+                    needed_revisions.add((endpoint, cluster))
+
+        # Main pass: Inject steps
+        injected_tests = []
+        seen_injections = set()  # Set of (endpoint, cluster) tuples
+
+        for step in tests:
+            endpoint = step.get('endpoint', default_endpoint)
+            cluster = step.get('cluster', default_cluster)
+            key = (endpoint, cluster)
+            is_disabled = 'disabled' in step and step['disabled']
+            step_has_revision_condition = 'minRevision' in step or 'maxRevision' in step
+
+            # Determine if an enabled step is the first that would need a cluster revision not yet read.
+            # Prepend the read to such steps.
+            if step_has_revision_condition and (key in needed_revisions) and (key not in seen_injections) and (not is_disabled):
+                # Inject the step *before* the current step.
+                revision_var_name = build_revision_var_name(endpoint, cluster)
+
+                # Add to config storage so it's a known variable
+                if revision_var_name not in self._parsing_config_variable_storage:
+                    self._parsing_config_variable_storage[revision_var_name] = None
+
+                # Also update the runtime storage, since it was already copied.
+                if revision_var_name not in self._runtime_config_variable_storage:
+                    self._runtime_config_variable_storage[revision_var_name] = None
+
+                injected_step = {
+                    'label': f'Read ClusterRevision for conditions (EP: {endpoint}, CL: {cluster})',
+                    'cluster': cluster,
+                    'endpoint': endpoint,
+                    'command': 'readAttribute',
+                    'attribute': 'ClusterRevision',
+                    'response': {
+                        'saveAs': revision_var_name,
+                        'constraints': {
+                            'type': 'int16u'
+                        }
+                    }
+                }
+
+                injected_tests.append(injected_step)
+                seen_injections.add(key)
+
+            injected_tests.append(step)
+
+        return injected_tests
+
+    def _get_config_value(self, config_storage, key):
+        value = config_storage.get(key)
+        if isinstance(value, dict) and 'defaultValue' in value:
+            return value['defaultValue']
+        return value
+
+    def set_runtime_variable(self, name: str, value: Any) -> None:
+        """Sets a runtime variable in the test context."""
+        self._runtime_config_variable_storage[name] = value
+
+    def get_runtime_variable(self, name: str) -> Any:
+        """Gets a runtime variable from the test context, or None if missing."""
+        return self._runtime_config_variable_storage.get(name)
+
     def __iter__(self):
         return self
 

scripts/py_matter_yamltests/matter/yamltests/runner.py

@@ -22,7 +22,7 @@
 
 from .adapter import TestAdapter
 from .hooks import TestRunnerHooks
-from .parser import TestParser
+from .parser import TestParser, build_revision_var_name
 from .parser_builder import TestParserBuilder, TestParserBuilderConfig
 from .pseudo_clusters.pseudo_clusters import PseudoClusters
 
@@ -188,10 +188,29 @@ async def _run(self, parser: TestParser, config: TestRunnerConfig):
 
             test_duration = 0
             for idx, request in enumerate(parser.tests):
+                # Handle skipping tests where PICS do not apply.
                 if not request.is_pics_enabled:
                     hooks.step_skipped(request.label, request.pics)
                     continue
-                elif not config.adapter:
+
+                # Handle skipping steps where ClusterRevision does not apply.
+                if not request.is_revision_condition_passed:
+                    # Try to get the var name and value for a more informative message
+                    try:
+                        var_name = build_revision_var_name(
+                            request.endpoint, request.cluster)
+                        current_val = request.get_runtime_variable(var_name)
+                    except (ValueError, IndexError, KeyError):
+                        current_val = "unknown"
+
+                    reason = (f"Step skipped due to ClusterRevision range not matching (val={current_val}, "
+                              f"min={request.min_revision}, "
+                              f"max={request.max_revision})")
+                    hooks.step_skipped(request.label, reason)
+                    continue
+
+                # Handle normal flows of execution after condition skipping above.
+                if not config.adapter:
                     hooks.step_start(request)
                     hooks.step_unknown()
                     continue

scripts/py_matter_yamltests/matter/yamltests/yaml_loader.py

@@ -56,16 +56,18 @@
     'verification': str,
     'PICS': str,
     'arguments': dict,
-    'response': (dict, list, str),  # Can be a variable
+    'response': (dict, list, str),  # Can be a variable.
     'saveResponseAs': str,
     'minInterval': int,
     'maxInterval': int,
     'keepSubscriptions': bool,
     'timeout': int,
     'timedInteractionTimeoutMs': int,
-    'dataVersion': (list, int, str),  # Can be a variable
+    'dataVersion': (list, int, str),  # Can be a variable.
     'busyWaitMs': int,
     'wait': str,
+    'minRevision': (int, str),  # Can be a variable.
+    'maxRevision': (int, str),  # Can be a variable.
 }
 
 _TEST_STEP_ARGUMENTS_SCHEMA = {