Fixture generation for derived #36
Conversation
|
@coderabbitai pause |
|
Note Reviews pausedUse the following commands to manage reviews:
WalkthroughA new JSON fixture file representing detailed AirOS device and network status was added. Minor IPv6 address updates were made to two existing fixture files. Additionally, a Python script was introduced to automate the generation of derived AirOS fixture data using the project’s data models and serialization logic. Changes
Sequence Diagram(s)sequenceDiagram
participant Script as generate_ha_fixture.py
participant FileSys as File System
participant AirOS as AirOS.derived_data
participant Model as AirOSData
Script->>FileSys: Read ap-ptp.json
Script->>AirOS: Call derived_data(base_fixture)
AirOS-->>Script: Return derived data
Script->>Model: Instantiate AirOSData(derived_data)
Model-->>Script: AirOSData instance
Script->>FileSys: Write airos_ap-ptp.json with serialized data
Estimated code review effort🎯 2 (Simple) | ⏱️ ~7 minutes Possibly related PRs
Poem
Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. ✨ 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. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
fixtures/sta-ptp.json (1)
1-1: Apply the same pretty-printing guideline as forap-ptp.jsonOne-liner JSON hurts readability and diff-friendliness. Please dump with indentation.
🧹 Nitpick comments (4)
fixtures/ap-ptp.json (1)
1-1: Pretty-print fixture for maintainability & cleaner diffsThe entire JSON blob is stored on a single line. Every subsequent change (like the IPv6 tweak here) forces the whole file to re-flow in PRs, making code-review & blame extremely painful.
Serialize withindent=2(or similar) when writing these fixtures so future diffs are minimal and humans can grep / read them.Example:
-with open(path, "w") as fp: - fp.write(json.dumps(data)) +with open(path, "w") as fp: + json.dump(data, fp, indent=2)script/generate_ha_fixture.py (2)
10-15:sys.pathpatching: push to front & preferpathlibInserting the project root at the end of
sys.pathmeans any globally-installed package namedairoswill shadow the local module.
Add it at index 0 and switch toPathfor clarity.-from airos.airos8 import AirOS, AirOSData # noqa: E402 +from pathlib import Path + +current_script_dir = Path(__file__).resolve().parent +project_root_dir = current_script_dir.parent +sys.path.insert(0, str(project_root_dir)) + +from airos.airos8 import AirOS, AirOSData # noqa: E402
23-27: Consider adding CLI & logging for easier reuseRight now the script can’t be parameterised or give feedback.
argparsefor--input/--outputand a simple_LOGGER.infoafter generation would make it generally useful.fixtures/airos_ap-ptp.json (1)
1-1: Generated fixture should be indentedSame readability concern: dump with
indent=2so future reviews can spot deltas easily.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
fixtures/airos_ap-ptp.json(1 hunks)fixtures/ap-ptp.json(1 hunks)fixtures/sta-ptp.json(1 hunks)script/generate_ha_fixture.py(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
fixtures/airos_ap-ptp.json (1)
Learnt from: CoMPaTech
PR: #14
File: airos/airos8.py:222-260
Timestamp: 2025-07-19T18:12:42.053Z
Learning: In AirOS stakick.cgi API, MAC addresses in the "staid" parameter need to be URL-encoded with colons replaced by %3A, as shown in curl example: --data 'staif=ath0&staid=01%3A23%3A45%3AA4%3A08%3A06'
🧬 Code Graph Analysis (1)
fixtures/airos_ap-ptp.json (2)
airos/airos8data.py (1)
AirOSData(367-431)tests/conftest.py (1)
airos_device(16-21)
🔇 Additional comments (1)
script/generate_ha_fixture.py (1)
18-22:fixture_dirpath building is fragile on Windows
os.path.join(..., "../fixtures")leaves the..in the path on Windows when the script is frozen or cwd differs.
Convert toPath.resolve()orparent / "fixtures".
| with open(base_fixture_path) as source, open(new_fixture_path, "w") as new: | ||
| source_data = json.loads(source.read()) | ||
| derived_data = AirOS.derived_data(None, source_data) | ||
| new_data = AirOSData.from_dict(derived_data) | ||
| new.write(json.dumps(new_data.to_dict())) |
There was a problem hiding this comment.
Opening destination file before derivation risks clobbering on failure
If AirOS.derived_data throws, the target file is already truncated to zero bytes.
Open the source first, derive, then open the dest for writing.
-with open(base_fixture_path) as source, open(new_fixture_path, "w") as new:
- source_data = json.loads(source.read())
- derived_data = AirOS.derived_data(None, source_data)
- new_data = AirOSData.from_dict(derived_data)
- new.write(json.dumps(new_data.to_dict()))
+with open(base_fixture_path) as source:
+ source_data = json.load(source)
+
+derived_data = AirOS.derived_data(None, source_data)
+new_data = AirOSData.from_dict(derived_data)
+
+with open(new_fixture_path, "w") as new:
+ json.dump(new_data.to_dict(), new, indent=2)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| with open(base_fixture_path) as source, open(new_fixture_path, "w") as new: | |
| source_data = json.loads(source.read()) | |
| derived_data = AirOS.derived_data(None, source_data) | |
| new_data = AirOSData.from_dict(derived_data) | |
| new.write(json.dumps(new_data.to_dict())) | |
| with open(base_fixture_path) as source: | |
| source_data = json.load(source) | |
| derived_data = AirOS.derived_data(None, source_data) | |
| new_data = AirOSData.from_dict(derived_data) | |
| with open(new_fixture_path, "w") as new: | |
| json.dump(new_data.to_dict(), new, indent=2) |
🤖 Prompt for AI Agents
In script/generate_ha_fixture.py around lines 23 to 27, the destination file is
opened for writing before the derivation step, which risks truncating it if
AirOS.derived_data throws an exception. To fix this, open and read the source
file first, perform the derivation and data transformation, and only then open
the destination file for writing to avoid clobbering the file on failure.
✅ Actions performedReviews paused. |
Summary by CodeRabbit
New Features
Chores
Bug Fixes