Skip to content
Open
Show file tree
Hide file tree
Changes from 15 commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
c6908e9
Add automatic device attribute dump on composition test failures:
j-ororke Sep 17, 2025
e2154ab
Restyled by autopep8
restyled-commits Sep 17, 2025
9c36c19
Remove duplicate on_fail() and unused import from TC_DeviceConformance
j-ororke Sep 17, 2025
5edf777
Removing unused logging import
j-ororke Sep 17, 2025
1438550
Change inheritance order to enable automatic device attribute dump on…
j-ororke Sep 17, 2025
9abde9b
Restyled by autopep8
restyled-commits Sep 17, 2025
dc5df9e
resolving linting errors and making sure that on test failures we emi…
j-ororke Sep 17, 2025
6756ca7
Restyled by autopep8
restyled-commits Sep 17, 2025
10f1ca6
Update basic_composition.py
j-ororke Sep 24, 2025
a026a85
Update basic_composition.py
j-ororke Sep 24, 2025
b1b1a4e
Merge branch 'master' into create_global_attr_log_function
j-ororke Sep 24, 2025
b149ff6
Restyled by autopep8
restyled-commits Sep 24, 2025
6992404
Move device attribute dumping from BasicCompositionTests to MatterBas…
j-ororke Oct 1, 2025
fee2649
Restyled by autopep8
restyled-commits Oct 1, 2025
855d881
Merge branch 'master' into create_global_attr_log_function
j-ororke Oct 1, 2025
13fb594
Merge branch 'master' into create_global_attr_log_function
j-ororke Oct 16, 2025
530ac3a
Fix Python 3.11 compatibility and improve test failure User Experience:
j-ororke Oct 16, 2025
f2fc773
removed duplicate log_structured_data function from basic_composition…
j-ororke Nov 6, 2025
7c93c8c
removed duplicate log_structured_data function from basic_composition…
j-ororke Nov 6, 2025
914426f
Restyled by autopep8
restyled-commits Nov 6, 2025
127cb15
Resolving linting error
j-ororke Nov 6, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 10 additions & 13 deletions src/python_testing/TC_DeviceBasicComposition.py
Original file line number Diff line number Diff line change
Expand Up @@ -189,7 +189,7 @@
from matter.clusters.Types import Nullable
from matter.exceptions import ChipStackError
from matter.interaction_model import InteractionModelError, Status
from matter.testing.basic_composition import BasicCompositionTests
from matter.testing.basic_composition import BasicCompositionTests, log_structured_data
from matter.testing.global_attribute_ids import (AttributeIdType, ClusterIdType, CommandIdType, GlobalAttributeIds,
attribute_id_type, cluster_id_type, command_id_type)
from matter.testing.matter_testing import MatterBaseTest, TestStep, async_test_body, default_matter_test_main
Expand Down Expand Up @@ -272,7 +272,7 @@ def check_no_duplicates(obj: Any) -> None:
raise ValueError(f"Value {str(obj)} contains duplicate values")


class TC_DeviceBasicComposition(MatterBaseTest, BasicCompositionTests):
class TC_DeviceBasicComposition(BasicCompositionTests, MatterBaseTest):
@async_test_body
async def setup_class(self):
super().setup_class()
Expand Down Expand Up @@ -604,12 +604,14 @@ class RequiredMandatoryAttribute:
attribute_id=manufacturer_value)
if suffix > attribute_standard_range_max and suffix < global_range_min:
self.record_error(self.get_test_name(), location=location,
problem=f"Manufacturer attribute in undefined range {manufacturer_value} in cluster {cluster_id}",
problem=f"Manufacturer attribute in undefined range {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We're about to update the restyler so this doesn't happen - there was a bug in the restyler that is causing this weirdness. When that lands, would you mind reverting these changes? I think this might break python prior to 3.12.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Merged in master after the PR for your restylizer change got merged and reverted the changes for the f-str lines in this file here now.

manufacturer_value} in cluster {cluster_id}",
spec_location=f"Cluster {cluster_id}")
success = False
elif suffix >= global_range_min:
self.record_error(self.get_test_name(), location=location,
problem=f"Manufacturer attribute in global range {manufacturer_value} in cluster {cluster_id}",
problem=f"Manufacturer attribute in global range {
manufacturer_value} in cluster {cluster_id}",
spec_location=f"Cluster {cluster_id}")
success = False

Expand Down Expand Up @@ -930,7 +932,8 @@ def record_problems(problems):
for ep, problem in problems.items():
location = AttributePathLocation(endpoint_id=ep, cluster_id=Clusters.Descriptor.id,
attribute_id=Clusters.Descriptor.Attributes.TagList.attribute_id)
msg = f'problem on ep {ep}: missing feature = {problem.missing_feature}, missing attribute = {problem.missing_attribute}, duplicates = {problem.duplicates}, same_tags = {problem.same_tag}'
msg = f'problem on ep {ep}: missing feature = {problem.missing_feature}, missing attribute = {
problem.missing_attribute}, duplicates = {problem.duplicates}, same_tags = {problem.same_tag}'
self.record_error(self.get_test_name(), location=location, problem=msg, spec_location="Descriptor TagList")

record_problems(problems)
Expand Down Expand Up @@ -966,13 +969,6 @@ def test_TC_IDM_12_1(self):
json_str, txt_str = self.dump_wildcard(dump_device_composition_path)

# Structured dump so we can pull these back out of the logs
def log_structured_data(start_tag: str, dump_string):
lines = dump_string.splitlines()
logging.info(f'{start_tag}BEGIN ({len(lines)} lines)====')
for line in lines:
logging.info(f'{start_tag}{line}')
logging.info(f'{start_tag}END ====')

log_structured_data('==== json: ', json_str)
log_structured_data('==== txt: ', txt_str)

Expand Down Expand Up @@ -1216,7 +1212,8 @@ async def test_TC_DESC_2_1(self):
self.record_error(
self.get_test_name(),
location=location,
problem=f"EndpointUniqueId attribute length is {len(value)} bytes which exceeds the maximum allowed 32 bytes",
problem=f"EndpointUniqueId attribute length is {
len(value)} bytes which exceeds the maximum allowed 32 bytes",
spec_location="EndpointUniqueId attribute"
)
self.fail_current_test(
Expand Down
2 changes: 1 addition & 1 deletion src/python_testing/TC_DeviceConformance.py
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@
from matter.testing.matter_testing import MatterBaseTest, TestStep, async_test_body, default_matter_test_main


class TC_DeviceConformance(MatterBaseTest, DeviceConformanceTests):
class TC_DeviceConformance(DeviceConformanceTests, MatterBaseTest):
@async_test_body
async def setup_class(self):
super().setup_class()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,23 @@
LOGGER = logging.getLogger(__name__)


def log_structured_data(start_tag: str, dump_string: str):
"""Log structured data with a clear start and end marker.

This function is used to output device attribute dumps and other structured
data to logs in a format that can be easily extracted for debugging.

Args:
start_tag: A prefix tag to identify the type of data being logged
dump_string: The data to be logged
"""
lines = dump_string.splitlines()
LOGGER.info(f'{start_tag}BEGIN ({len(lines)} lines)====')
for line in lines:
LOGGER.info(f'{start_tag}{line}')
LOGGER.info(f'{start_tag}END ====')


@dataclass
class ArlData:
have_arl: bool
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -206,7 +206,7 @@ def setup_class(self):
self.stored_global_wildcard = None

def teardown_class(self):
"""Final teardown after all tests: log all problems.
"""Final teardown after all tests: log all problems and dump device attributes if available.
Test authors may overwrite this method in the derived class to perform teardown that is common for all tests
This function is called only once per class. To perform teardown after each test, use teardown_test.
Test authors that implement steps in this function need to be careful of step handling if there is
Expand All @@ -222,8 +222,54 @@ def teardown_class(self):
for problem in self.problems:
LOGGER.info(str(problem))
LOGGER.info("###########################################################")

# Attempt to dump device attribute data for debugging when problems are found
self._dump_device_attributes_on_failure()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you please move this to above the problem dump? That will make the actual problems appear next to the failure message rather than forcing people to scroll up to above the device dump.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated, moved the problem dump to above the problem logging.

super().teardown_class()

def _dump_device_attributes_on_failure(self):
"""
Dump device attribute data when problems are found for debugging purposes.

This method attempts to generate a device attribute dump if the test has
collected endpoint data. It's designed to be safe and not interfere with
the original test failure reporting.
"""
try:
# Check if we have endpoints_tlv data (from BasicCompositionTests or similar)
if hasattr(self, 'endpoints_tlv') and self.endpoints_tlv:
LOGGER.info("MatterBaseTest: Problems detected - generating device attribute dump")

# Check if we have the dump_wildcard method (from BasicCompositionTests)
if hasattr(self, 'dump_wildcard'):
LOGGER.info("Device attribute data available - generating dump")
_, txt_str = self.dump_wildcard(None)
# Only dump the text format - it's more readable for debugging
self._log_structured_data('==== FAILURE_DUMP_txt: ', txt_str)
else:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know if it's worthwhile logging these - it might just confuse people. Ditto with the exception.

Copy link
Contributor Author

@j-ororke j-ororke Oct 16, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah ok, understood, I have removed the logging statements here and the logging statement in the exception below from the code here now.

LOGGER.info("dump_wildcard method not available - skipping device attribute dump")
else:
LOGGER.debug("No device attribute data available (endpoints_tlv not populated)")
except Exception as e:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what would cause an exception here? you should be catching both failure cases with the hasattr checks.

If you do need this, can you please change to be the specific exception rather than Exception?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You are right that the hasattr checks handle the missing attribute cases.
The exception handling is for the data access and serialization that happens inside dump_wildcard() itself, things such as:

  • Dictionary key access when traversing endpoints_tlv
  • JSON serialization of TLV data
  • String formatting with pformat()

I've changed it to catch only the specific exceptions that could realistically occur during those operations: AttributeError, KeyError, ValueError, and TypeError. This way we're not masking unexpected errors, just preventing data dump failures from interfering with the actual test failure reporting.

# Don't let logging errors interfere with the original test failure
LOGGER.warning(f"Failed to generate device attribute dump: {e}")

def _log_structured_data(self, start_tag: str, dump_string: str):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems like a duplicate of the function in basic_composition.py

Copy link
Contributor Author

@j-ororke j-ororke Nov 6, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for catching that!
I’ve removed this function from the basic_composition support module. There’s a test, test_TC_IDM_12_1 in the TC_DeviceBasicComposition module, that uses this function directly; it originally came from there (previously embedded inside that test).
I’ve now updated that test to inherit the function from here, and renamed this function so that it doesn't appear to be a private function anymore.

"""Log structured data with a clear start and end marker.

This function is used to output device attribute dumps and other structured
data to logs in a format that can be easily extracted for debugging.

Args:
start_tag: A prefix tag to identify the type of data being logged
dump_string: The data to be logged
"""
lines = dump_string.splitlines()
LOGGER.info(f'{start_tag}BEGIN ({len(lines)} lines)====')
for line in lines:
LOGGER.info(f'{start_tag}{line}')
LOGGER.info(f'{start_tag}END ====')

def setup_test(self):
"""Set up for each individual test execution.

Expand Down
Loading