python-test-reviewer

name	python-test-reviewer
description	Expert guidance for reviewing Python tests in the Matter (connectedhomeip) repository. Use this skill when reviewing changes to tests, specifically targeting common pitfalls in async execution, mocking cluster interactions, and assertion quality.

Python Test Reviewer Skill

This skill provides a checklist and set of principles for reviewing Python tests within the Matter repository.

Core Review Principles

• Prefer Real Interactions Over Mocks: For integration tests, prioritize using the real Matter stack (via fixtures) unless the interaction is too complex or slow.
• Explicit Assertions: Never accept tests that "pass by not crashing." Every test must have at least one clear, meaningful assertion.
• Async Hygiene: Matter is heavily asynchronous. Ensure async patterns are used correctly.

Specific Checkpoints

1. Asyncio Usage

• Check: Are all awaitable calls (like devCtrl.ReadAttribute) actually awaited?
• Propagate Exceptions in Parallel Code: When using concurrent.futures or similar parallel execution mechanisms, ensure that exceptions thrown in background threads are not silently ignored. Call .result() on futures to propagate errors.

2. Mocking Cluster Interactions

• Guideline: When mocking cluster responses, ensure the mock structure matches the chip.clusters attributes or commands exactly.
• Pitfall: Avoid mocking the entire ChipDeviceCtrl if you only need to mock a single attribute read.

3. Code Style

• Re-use: Avoid re-writing code that is present in matter/testing or the chip core libraries. If you are creating a class or function that is substantially similar to existing classes or functions, consider whether extension would be preferable to duplication
• Attribute reads: Prefer using read_single_attribute_check_success in matter/testing to the base Read and ReadAttribute functions in ChipDeviceCtrl.py unless there is a good reason. This function contains additional checks that are useful for testing.
• Exceptions: Avoid except Exception - use a specific exception
• try / except can hide bugs: Avoid fixing CI issues with a try: except:. This pattern can hide real bugs that should be fixed and is a code smell. Try except blocks should only be used where the exception is expected and that should normally include an assertion that the exception happens or a comment explaining in detail why the exception is thrown and how this is spec compliant.
• Sleeps can hide bugs: a sleep in a test is a code smell. Ensure sleeps in tests are actually required. If the test is using sleep to wait for the device to do something, consider whether a subscription can be used instead.
• Tolerances should be well considered: If there is a fudge factor built in, ensure the logic is both explained and reasonable.
• Use dataclass: Do not use dicts with string keys. Use dataclasses with named members instead. This helps programmers avoid uncaught typos.
• Avoid Decorative Separators: Remove long lines of hashes (e.g., ###########) or other decorative separators. Use standard spacing or docstrings instead.
• Cleanup of Stale Code: Remove outdated TODO comments, unbound variables, and leftover debugging code.
• Type Hints: Use type hints for function signatures to improve readability and maintainability, especially for functions that are part of the test framework or are likely to be reused.
• TODOs: TODOs should be linked to a tracking issue and used sparingly.

4. Matter-Specific Patterns

• Commissioning: If the test involves commissioning, ensure it uses the standard commissioning fixtures to avoid duplicated setup logic.
• IDs: IDs and values should be taken from the codegen files (ex. Objects.py). Values that are not in the codegen should be flagged for follow up

5. Testing failures

• Test for unexpected successes: If a test is meant to verify that an error condition is properly handled, ensure that it fails if the error condition does not occur.

Feedback Style

• Be concise.
• Reference specific lines.
• If a pattern is repeated across many tests, suggest a helper function or fixture.