Repository navigation
test(map): add v1 map test fixtures and snapshot tests - #969
Merged
allenporter merged 2 commits intoSep 28, 2026
Merged
Conversation
Contributor
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
Add goto_target assertions and correct the fixture documentation before approval.
Review effort: Lite
Findings: 1
Open (1)
What changed in this PR
Adds Roborock V1 map fixtures and snapshot tests covering parsing, rendering, and configuration variants.
Changes:
- Added four real S5/S6 binary map fixtures with attribution.
- Added parser and rendering snapshot tests.
- Added configuration and image-generator coverage.
| File | Summary |
|---|---|
tests/map/testdata/v1/README.md |
Documents fixture sources and coverage; its goto-path description needs correction. |
tests/map/test_map_parser.py |
Adds fixture parsing and rendering tests; should assert goto_target. |
tests/map/__snapshots__/test_map_parser.ambr |
Stores expected parsed-map snapshots. |
💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+28
to
+29
| "charger": md.charger, | ||
| "vacuum_position": md.vacuum_position, |
Contributor
Author
There was a problem hiding this comment.
Good catch! Added goto to the snapshot projection (Point(x=25874, y=26218, a=None) for s5_fw1886_with_goto_target.bin) and updated the fixture description in README.md to accurately reference goto and predicted_path.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
This PR adds real Roborock V1 binary map fixtures (
RRMapformat) and comprehensivesyrupysnapshot tests forroborock.map.map_parser.Previously,
tests/map/test_map_parser.pyonly contained tests for invalid empty payloads (b"",b"12345"), while valid map decoding, image extraction, and rendering options were unexercised in unit tests (and mocked everywhere else).Changes
tests/map/testdata/v1/:s5_fw2008_with_segments.bin: Roborock S5 multi-room map (rooms 1 & 2), charger, vacuum position, and navigation path.s5_fw1886_with_forbidden_zones_and_virtual_walls.bin: Virtual walls, forbidden/no-go areas, and active zones.s5_fw1886_with_goto_target.bin: Pinpoint target location and computed goto path.s6_fw2652_with_active_segment_and_no_mop_zone.bin: Roborock S6 map with 6 segmented rooms, an active segment, and a no-mop zone.tests/map/testdata/v1/README.mddocumenting upstream source (Valetudo), Apache 2.0 license compatibility, and features exercised by each fixture.tests/map/test_map_parser.pyusingsyrupy(SnapshotAssertion) verifying:charger,vacuum_position,rooms,walls,no_go_areas,no_mopping_areas,zones,path_points, header parameters)(width, height),PNGformat) without storing OS-variant zlib binary chunks in snapshot filesMapParserConfigvariants (show_background=False,show_walls=False,show_rooms=False, scaling) and_create_image_generator.roborock/map/map_parser.pytest line coverage to 100%.