Rerun fixtures to test userdata #64
Conversation
WalkthroughAdds a local pre-commit hook to run a fixture-generation script; converts AirOS.derived_data to a staticmethod and updates its call sites; mashumaro-step-debug imports AirOS, derives data before deserialization, and uses _LOGGER.exception for errors; generate_ha_fixture updated to call the new signature. Changes
Sequence Diagram(s)sequenceDiagram
participant Raw as Raw input (response/source_data)
participant AirOS as AirOS.derived_data (static)
participant Parser as AirOS8Data / AirOSData.from_dict
participant Writer as Fixture writer
participant Logger as _LOGGER
Raw->>AirOS: derived_data(response)
AirOS-->>Parser: derived_data
Parser->>Writer: from_dict(derived_data) -> write fixture / continue
alt Error
Parser-->>Logger: raise/exception
Logger-->>Logger: _LOGGER.exception("The program failed") (stack trace)
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~15 minutes Possibly related PRs
Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #64 +/- ##
=======================================
Coverage 96.07% 96.08%
=======================================
Files 9 9
Lines 1352 1353 +1
=======================================
+ Hits 1299 1300 +1
Misses 53 53 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
script/mashumaro-step-debug.py (1)
82-86: Elevate log level on failure and exit non‑zero for automation.Since this is a debugging/validation script, failing with a non‑zero exit code helps CI and local workflows surface issues immediately. Also, use error level for the failure banner.
- except Exception: - _LOGGER.info("\n------------------") - _LOGGER.info("CRITICAL ERROR FOUND!") - _LOGGER.exception("The program failed") - _LOGGER.info("------------------\n") + except Exception: + _LOGGER.error("\n------------------") + _LOGGER.error("CRITICAL ERROR FOUND!") + _LOGGER.exception("The program failed") + _LOGGER.error("------------------\n") + sys.exit(1)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.pre-commit-config.yaml(1 hunks)script/mashumaro-step-debug.py(2 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
script/mashumaro-step-debug.py (4)
airos/airos8.py (2)
AirOS(26-385)derived_data(196-247)airos/data.py (3)
AirOS8Data(559-576)AirOSDataClass(107-110)Derived(543-555)tests/test_airos8.py (2)
test_derived_data_no_interfaces_key(106-110)test_derived_data_no_br0_eth0_ath0(114-124)script/generate_ha_fixture.py (1)
generate_airos_fixtures(22-59)
🔇 Additional comments (1)
script/mashumaro-step-debug.py (1)
79-79: LGTM: Deserialize with derived data.Feeding
AirOS8Data.from_dictwith the response augmented byderivedaligns with the dataclass contract in airos.data and will catch missing interface scenarios early.
|
There was a problem hiding this comment.
Actionable comments posted: 0
🔭 Outside diff range comments (2)
airos/airos8.py (1)
186-188: Bug: logging uses coroutine object instead of response bodyresponse.text is an async coroutine method; you already awaited response_text above. Logging response.text here will print a coroutine repr instead of the actual body.
Apply this diff:
- log = f"Login failed with status {response.status}. Full Response: {response.text}" + log = f"Login failed with status {response.status}. Full Response: {response_text}"script/generate_ha_fixture.py (1)
25-33: Gracefully handle missing userdata directory and ensure deterministic processing
- If fixtures/userdata doesn’t exist (e.g., in a fresh checkout), pre-commit will fail. Handle this early and noop.
- Sort filenames to make output deterministic across platforms.
- Skip non-file entries defensively.
Apply this diff:
@@ - fixture_dir = os.path.join(os.path.dirname(__file__), "../fixtures") - userdata_dir = os.path.join(os.path.dirname(__file__), "../fixtures/userdata") + fixture_dir = os.path.join(os.path.dirname(__file__), "../fixtures") + userdata_dir = os.path.join(os.path.dirname(__file__), "../fixtures/userdata") @@ - # Iterate over all files in the userdata_dir - for filename in os.listdir(userdata_dir): + # If no userdata directory, nothing to generate + if not os.path.isdir(userdata_dir): + _LOGGER.info("No 'userdata' directory found at %s; nothing to generate.", userdata_dir) + return + + # Iterate over all files in a deterministic order + for filename in sorted(os.listdir(userdata_dir)): + full_path = os.path.join(userdata_dir, filename) + if not os.path.isfile(full_path): + continue if "mocked" in filename: continue if filename.endswith(".json"): # Construct the full paths for the base and new fixtures - base_fixture_path = os.path.join(userdata_dir, filename) + base_fixture_path = full_path
🧹 Nitpick comments (2)
airos/airos8.py (1)
271-272: Prefer class-qualified call for clarityNow that derived_data is static, calling it via the class is clearer and avoids any confusion about binding semantics.
Apply this diff:
- adjusted_json = self.derived_data(response_json) + adjusted_json = AirOS.derived_data(response_json)script/generate_ha_fixture.py (1)
45-47: Minor: use json.load for filesjson.load(source) is simpler and slightly more efficient than reading then json.loads.
Apply this diff:
- with open(base_fixture_path, encoding="utf-8") as source: - source_data = json.loads(source.read()) + with open(base_fixture_path, encoding="utf-8") as source: + source_data = json.load(source)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
.pre-commit-config.yaml(1 hunks)airos/airos8.py(1 hunks)script/generate_ha_fixture.py(1 hunks)script/mashumaro-step-debug.py(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- script/mashumaro-step-debug.py
- .pre-commit-config.yaml
🧰 Additional context used
🧬 Code Graph Analysis (2)
script/generate_ha_fixture.py (3)
airos/airos8.py (2)
derived_data(197-248)AirOS(26-386)tests/test_airos8.py (2)
test_derived_data_no_interfaces_key(106-110)test_derived_data_no_br0_eth0_ath0(114-124)airos/exceptions.py (2)
AirOSDataMissingError(16-17)AirOSKeyDataMissingError(20-21)
airos/airos8.py (3)
airos/data.py (3)
AirOSDataClass(107-110)Derived(543-555)AirOS8Data(559-576)tests/test_airos8.py (2)
test_derived_data_no_interfaces_key(106-110)test_derived_data_no_br0_eth0_ath0(114-124)airos/exceptions.py (1)
AirOSDataMissingError(16-17)
🔇 Additional comments (2)
airos/airos8.py (1)
196-197: derived_data staticmethod verified — all call sites pass a single argumentConversion to @staticmethod is appropriate and verified: I scanned the repository and all usages pass exactly one argument.
- airos/airos8.py:197 — definition
- airos/airos8.py:271 — self.derived_data(response_json)
- script/generate_ha_fixture.py:48 — AirOS.derived_data(source_data)
- script/mashumaro-step-debug.py:76 — AirOS.derived_data(data)
- tests/test_airos8.py:110 — airos_device.derived_data({})
- tests/test_airos8.py:122 — airos_device.derived_data(fixture_data)
script/generate_ha_fixture.py (1)
48-49: Updated call aligns with new staticmethod signatureSwitching to AirOS.derived_data(source_data) matches the new one-arg static method. Downstream deserialization remains correct.
1 participant
Summary by CodeRabbit
Chores
Refactor
Bug Fixes