Skip to content

Commit

Permalink
Actually enable our YAML tests in the python-calls-chip-tool runner. (#…
Browse files Browse the repository at this point in the history
…29096)

* Actually enable our YAML tests in the python-calls-chip-tool runner.

We were now getting the right test list, but for each test we were not finding
the test file.  And the harness did not treat that as an error.

* Address review comments.

* Fix chip-repl CI
  • Loading branch information
bzbarsky-apple authored and pull[bot] committed Nov 30, 2023
1 parent 3afc115 commit 2398608
Show file tree
Hide file tree
Showing 7 changed files with 87 additions and 75 deletions.
1 change: 1 addition & 0 deletions .github/workflows/tests.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -223,6 +223,7 @@ jobs:
--exclude-tags MANUAL \
--exclude-tags FLAKY \
--exclude-tags IN_DEVELOPMENT \
--exclude-tags EXTRA_SLOW \
--exclude-tags SLOW \
run \
--iterations 1 \
Expand Down
37 changes: 33 additions & 4 deletions scripts/tests/chiptest/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,9 @@ class ManualTest:
"PICS_Example.yaml",
"Response_Example.yaml",
"Test_Example.yaml",
"Test_Example_1.yaml",
"Test_Example_2.yaml",
"Test_Example_3.yaml",
}


Expand Down Expand Up @@ -127,6 +130,16 @@ def _GetSlowTests() -> Set[str]:
}


def _GetExtraSlowTests() -> Set[str]:
"""Generally tests using sleep() so much they should never run in CI.
1 minute seems like a good threshold to consider something extra slow
"""
return {
"Test_TC_DGGEN_2_1.yaml", # > 2 hours
}


def _GetInDevelopmentTests() -> Set[str]:
"""Tests that fail in YAML for some reason."""
return {
Expand All @@ -145,6 +158,8 @@ def _GetInDevelopmentTests() -> Set[str]:
# TestEventTriggersEnabled is true, which it's not in CI.
"Test_TC_SMOKECO_2_6.yaml", # chip-repl does not support local timeout (07/20/2023) and test assumes
# TestEventTriggersEnabled is true, which it's not in CI.
"Test_TC_IDM_1_2.yaml", # Broken harness: https://github.com/project-chip/connectedhomeip/issues/29115
"Test_TC_S_2_4.yaml", # https://github.com/project-chip/connectedhomeip/issues/29117
}


Expand Down Expand Up @@ -177,6 +192,8 @@ def _GetChipReplUnsupportedTests() -> Set[str]:
"Test_TC_G_2_4.yaml", # chip-repl does not support EqualityCommands pseudo-cluster
"Test_TC_RVCRUNM_3_1.yaml", # chip-repl does not support EqualityCommands pseudo-cluster
"Test_TC_RVCCLEANM_3_1.yaml", # chip-repl does not support EqualityCommands pseudo-cluster
# chip-repl and chip-tool disagree on what the YAML here should look like: https://github.com/project-chip/connectedhomeip/issues/29110
"TestClusterMultiFabric.yaml",
}


Expand Down Expand Up @@ -243,10 +260,14 @@ def tests_with_command(chip_tool: str, is_manual: bool):
)


def _AllFoundYamlTests(treat_repl_unsupported_as_in_development: bool):
def _AllFoundYamlTests(treat_repl_unsupported_as_in_development: bool, use_short_run_name: bool):
"""
use_short_run_name should be true if we want the run_name to be "Test_ABC" instead of "some/path/Test_ABC.yaml"
"""
manual_tests = _GetManualTests()
flaky_tests = _GetFlakyTests()
slow_tests = _GetSlowTests()
extra_slow_tests = _GetExtraSlowTests()
in_development_tests = _GetInDevelopmentTests()
chip_repl_unsupported_tests = _GetChipReplUnsupportedTests()

Expand All @@ -264,27 +285,35 @@ def _AllFoundYamlTests(treat_repl_unsupported_as_in_development: bool):
if path.name in slow_tests:
tags.add(TestTag.SLOW)

if path.name in extra_slow_tests:
tags.add(TestTag.EXTRA_SLOW)

if path.name in in_development_tests:
tags.add(TestTag.IN_DEVELOPMENT)

if treat_repl_unsupported_as_in_development and path.name in chip_repl_unsupported_tests:
tags.add(TestTag.IN_DEVELOPMENT)

if use_short_run_name:
run_name = path.stem # `path.stem` converts "some/path/Test_ABC_1.2.yaml" to "Test_ABC.1.2"
else:
run_name = str(path)

yield TestDefinition(
run_name=str(path),
run_name=run_name,
name=path.stem, # `path.stem` converts "some/path/Test_ABC_1.2.yaml" to "Test_ABC.1.2"
target=target_for_name(path.name),
tags=tags,
)


def AllReplYamlTests():
for test in _AllFoundYamlTests(treat_repl_unsupported_as_in_development=True):
for test in _AllFoundYamlTests(treat_repl_unsupported_as_in_development=True, use_short_run_name=False):
yield test


def AllChipToolYamlTests():
for test in _AllFoundYamlTests(treat_repl_unsupported_as_in_development=False):
for test in _AllFoundYamlTests(treat_repl_unsupported_as_in_development=False, use_short_run_name=True):
yield test


Expand Down
1 change: 1 addition & 0 deletions scripts/tests/chiptest/test_definition.py
Original file line number Diff line number Diff line change
Expand Up @@ -218,6 +218,7 @@ class TestTag(Enum):
FLAKY = auto() # test is considered flaky (usually a bug/time dependent issue)
IN_DEVELOPMENT = auto() # test may not pass or undergoes changes
CHIP_TOOL_PYTHON_ONLY = auto() # test uses YAML features only supported by the CHIP_TOOL_PYTHON runner.
EXTRA_SLOW = auto() # test uses Sleep and is generally _very_ slow (>= 60s is a typical threshold)

def to_s(self):
for (k, v) in TestTag.__members__.items():
Expand Down
1 change: 1 addition & 0 deletions scripts/tests/run_test_suite.py
Original file line number Diff line number Diff line change
Expand Up @@ -174,6 +174,7 @@ def main(context, dry_run, log_level, target, target_glob, target_skip_glob,
TestTag.MANUAL,
TestTag.IN_DEVELOPMENT,
TestTag.FLAKY,
TestTag.EXTRA_SLOW
}

if runtime != TestRunTime.CHIP_TOOL_PYTHON:
Expand Down
6 changes: 5 additions & 1 deletion scripts/tests/yaml/runner.py
Original file line number Diff line number Diff line change
Expand Up @@ -260,8 +260,12 @@ def runner_base(ctx, configuration_directory: str, test_name: str, configuration
specifications = SpecDefinitionsFromPaths(specifications_paths.split(','), pseudo_clusters)
tests_finder = TestsFinder(configuration_directory, configuration_name)

test_list = tests_finder.get(test_name)
if len(test_list) == 0:
raise Exception(f"No tests found for test name '{test_name}'")

parser_config = TestParserConfig(pics, specifications, kwargs)
parser_builder_config = TestParserBuilderConfig(tests_finder.get(test_name), parser_config, hooks=TestParserLogger())
parser_builder_config = TestParserBuilderConfig(test_list, parser_config, hooks=TestParserLogger())
parser_builder_config.options.stop_on_error = stop_on_error
while ctx:
ctx.obj = ParserGroup(parser_builder_config, pseudo_clusters)
Expand Down
98 changes: 38 additions & 60 deletions src/app/tests/suites/TestClusterMultiFabric.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -284,8 +284,7 @@ tests:
#
# TODO: This can be fixed using the `saveAs` function.
#
value:
[
value: [
{
FabricIndex: 1,
fabricSensitiveInt8u: 33,
Expand Down Expand Up @@ -328,39 +327,29 @@ tests:
},
{
FabricIndex: 2,
fabricSensitiveInt8u: 0,
# These should actually be missing, not null, but right
# now our harness treats those the same, and we have no
# way to indicate "missing" in the YAML.
# https://github.com/project-chip/connectedhomeip/issues/29110
fabricSensitiveInt8u: null,
optionalFabricSensitiveInt8u: null,
nullableFabricSensitiveInt8u: null,
fabricSensitiveCharString: "",
fabricSensitiveStruct:
{
a: 0,
b: false,
c: 0,
d: "",
e: "",
f: 0,
g: 0,
h: 0,
},
fabricSensitiveInt8uList: [],
fabricSensitiveCharString: null,
fabricSensitiveStruct: null,
fabricSensitiveInt8uList: null,
},
{
FabricIndex: 2,
fabricSensitiveInt8u: 0,
# These should actually be missing, not null, but right
# now our harness treats those the same, and we have no
# way to indicate "missing" in the YAML.
# https://github.com/project-chip/connectedhomeip/issues/29110
fabricSensitiveInt8u: null,
optionalFabricSensitiveInt8u: null,
nullableFabricSensitiveInt8u: null,
fabricSensitiveCharString: "",
fabricSensitiveStruct:
{
a: 0,
b: false,
c: 0,
d: "",
e: "",
f: 0,
g: 0,
h: 0,
},
fabricSensitiveInt8uList: [],
fabricSensitiveCharString: null,
fabricSensitiveStruct: null,
fabricSensitiveInt8uList: null,
},
]

Expand All @@ -371,43 +360,32 @@ tests:
fabricFiltered: false
attribute: "list_fabric_scoped"
response:
value:
[
value: [
{
FabricIndex: 1,
fabricSensitiveInt8u: 0,
# These should actually be missing, not null, but right
# now our harness treats those the same, and we have no
# way to indicate "missing" in the YAML.
# https://github.com/project-chip/connectedhomeip/issues/29110
fabricSensitiveInt8u: null,
optionalFabricSensitiveInt8u: null,
nullableFabricSensitiveInt8u: null,
fabricSensitiveCharString: "",
fabricSensitiveStruct:
{
a: 0,
b: false,
c: 0,
d: "",
e: "",
f: 0,
g: 0,
h: 0,
},
fabricSensitiveInt8uList: [],
fabricSensitiveCharString: null,
fabricSensitiveStruct: null,
fabricSensitiveInt8uList: null,
},
{
FabricIndex: 1,
fabricSensitiveInt8u: 0,
# These should actually be missing, not null, but right
# now our harness treats those the same, and we have no
# way to indicate "missing" in the YAML.
# https://github.com/project-chip/connectedhomeip/issues/29110
fabricSensitiveInt8u: null,
optionalFabricSensitiveInt8u: null,
nullableFabricSensitiveInt8u: null,
fabricSensitiveCharString: "",
fabricSensitiveStruct:
{
a: 0,
b: false,
c: 0,
d: "",
e: "",
f: 0,
g: 0,
h: 0,
},
fabricSensitiveInt8uList: [],
fabricSensitiveCharString: null,
fabricSensitiveStruct: null,
fabricSensitiveInt8uList: null,
},
{
FabricIndex: 2,
Expand Down
18 changes: 8 additions & 10 deletions src/app/tests/suites/TestEventsById.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,8 @@ tests:
value: ReadRequestMessage.Cluster
- name: "EventId"
value: ReadRequestMessage.Event
response: []
response:
error: UNSUPPORTED_ENDPOINT

- label:
"Read Request Message with a path that indicates a specific cluster
Expand All @@ -62,11 +63,12 @@ tests:
value: UnsupportedCluster
- name: "EventId"
value: ReadRequestMessage.Event
response: []
response:
error: UNSUPPORTED_CLUSTER

- label:
"Read Request Message with a path that indicates a specific endpoint
that is unsupported"
"Read Request Message with a path that indicates a specific event that
is unsupported"
cluster: "AnyCommands"
command: "ReadEventById"
endpoint: ReadRequestMessage.EndPoint
Expand Down Expand Up @@ -256,9 +258,5 @@ tests:
- name: "EventId"
value: ReadRequestMessage.EndPoint
response:
- values:
- value: { arg1: 1, arg2: 2, arg3: true }
- values:
- value: { arg1: 3, arg2: 1, arg3: false }
- values:
- value: { arg1: 4, arg2: 3, arg3: true }
# Not a valid event request path.
error: FAILURE

0 comments on commit 2398608

Please sign in to comment.