Skip to content

Conversation

@allenporter
Copy link
Contributor

Add tests for #630. I first wrote the tests and made sure they failed without that PR, then confirm they pass with those changes.

The testing approach is:
(1) Have a test with a generic data type reproducing the issue
(2) Test the caching library with the specific objects causing the problem

Copilot AI review requested due to automatic review settings December 5, 2025 03:56
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR adds comprehensive tests to verify fixes for key parsing bugs reported in issue #630. The tests ensure that dictionaries with integer keys are properly serialized and deserialized, both in generic data structures and in the caching library's specific objects.

  • Added nested_int_dict field to test objects to validate integer key handling
  • Created parametrized test to verify both integer and string-integer key parsing
  • Extended cache tests to include CombinedMapInfo with integer keys for real-world validation

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File Description
tests/test_containers.py Adds nested_int_dict field to ComplexObject and creates parametrized test to verify integer key parsing works with both int and string representations
tests/devices/test_file_cache.py Extends cache test data to include home_map_info with integer keys, testing real-world objects (CombinedMapInfo, NamedRoomMapping)
tests/devices/snapshots/test_file_cache.ambr Updates test snapshots to reflect the new home_map_info data in cache test cases

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Lash-L
Lash-L previously approved these changes Dec 5, 2025
@Lash-L
Copy link
Collaborator

Lash-L commented Dec 5, 2025

Oh gotta fix long before this one is merged

@allenporter
Copy link
Contributor Author

Oh gotta fix long before this one is merged

referring to lint errors here?

@Lash-L
Copy link
Collaborator

Lash-L commented Dec 5, 2025

Oh gotta fix long before this one is merged

referring to lint errors here?

Yes sorry - mobile autocorrect

@allenporter allenporter merged commit 87e14a2 into Python-roborock:main Dec 5, 2025
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants