add tests #140
add tests #140
Conversation
WalkthroughThis pull request enhances the test suite by adding new test cases across multiple modules. In the text tests, a shared setup block is introduced to initialize a document and text object for testing various edge cases such as negative indices, formatting, complex delta operations, and text length calculations. The XML element tests now verify length adjustments, insertion order, deprecated behavior, sibling retrieval, attribute handling, and nested structures. Additional tests are added to validate StickyIndex behavior across different data types, including proper error handling for invalid indices. Changes
Possibly related PRs
✨ Finishing Touches
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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: 0
🧹 Nitpick comments (2)
test/sticky_index_test.exs (1)
111-141: Improve assertions in XmlText sticky index test.The test correctly tests the behavior of StickyIndex with XmlText type, but the assertions are somewhat weak - only checking that the result is a map with the expected keys without validating the specific values.
Consider strengthening the assertions by:
- # The actual index will depend on the Yjs implementation - # We'll just verify it returns a result without asserting the exact position - assert is_map(result) - assert Map.has_key?(result, :index) - assert Map.has_key?(result, :assoc) + # Since we're testing a specific case, we should verify the actual position + # Based on the operations: adding "Start: " (+7), deleting "Hello" (-5) + # The index should move to the start of "World" + assert result.index == 7 + assert result.assoc == :beforetest/shared_type/text_test.exs (1)
289-306: Strengthen assertions in negative indices test.The test correctly verifies that negative indices work for deleting text, but the assertions are somewhat weak as they only check that some deletion occurred rather than validating the specific behavior.
Consider strengthening this test with more specific assertions:
- # Just check that some deletion occurred rather than expecting exact results - # since implementation may vary - result = Text.to_string(text) - assert String.length(result) < 11 - assert String.starts_with?(result, "Hello") + # Negative indices should count from the end, so deleting from -5 should remove "World" + result = Text.to_string(text) + assert result == "Hello "
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
test/shared_type/text_test.exs(2 hunks)test/shared_type/xml_element_test.exs(2 hunks)test/sticky_index_test.exs(2 hunks)
🧰 Additional context used
🪛 GitHub Check: Build and test
test/shared_type/xml_element_test.exs
[warning] 200-200:
Yex.XmlElement.get/2 is deprecated. Rename to fetch/2
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: lint
🔇 Additional comments (23)
test/sticky_index_test.exs (6)
3-14: Good addition of explicit alias imports.The aliases are well-organized and make the test module code cleaner by removing the need for fully qualified module names.
43-62: Well-implemented test for:beforeassociation handling.This test effectively validates that the StickyIndex correctly tracks position with the
:beforeassociation when text is modified. The comments describing the test logic and expected position calculations make the test very clear.
64-84: Good test for StickyIndex with Array type.This test properly verifies that inserting an element at the beginning of an array correctly adjusts the sticky index position.
86-109: Test provides good coverage of XmlElement integration.The test thoroughly checks the behavior of sticky indices with XmlElement type, particularly when modifying element children.
143-164: Good test for StickyIndex with XmlFragment type.This test properly verifies the behavior of sticky indices in XML fragments, specifically how they update when elements are inserted.
166-188: Excellent test for error handling with invalid sticky indices.This test thoroughly checks that attempting to get the offset of an invalid sticky index raises an appropriate error. The approach of creating a valid reference first and then crafting an invalid one is clever and provides good coverage of error handling.
test/shared_type/text_test.exs (6)
7-11: Good addition of setup block.The setup block reduces code duplication and provides a common context for multiple tests. This is a good practice that improves test maintainability.
308-326: Well-implemented test for text formatting.This test thoroughly validates that formatting attributes can be applied to specific ranges of text and correctly verifies the resulting delta structure.
328-339: Good edge case test for empty strings.This test properly checks the behavior of TextPrelim with empty strings, including handling of both empty list and single empty insert in delta representation.
341-370: Comprehensive test for complex delta operations.This test thoroughly validates the application of complex delta operations, including retaining, deleting, and inserting text with formatting attributes.
372-385: Well-structured test for inserting with attributes.This test effectively verifies that text can be inserted with specific formatting attributes and properly checks the resulting delta structure.
387-399: Complete test for text length calculation.This test thoroughly checks that the Text.length function returns the correct length after various operations including insertion and deletion.
test/shared_type/xml_element_test.exs (11)
160-175: Good test for XmlElement length calculation.This test effectively verifies that the length function returns the correct count of children and updates properly when children are added or removed.
177-194: Thorough test for element insertion at specific indices.This test properly checks insertion at different positions (beginning, middle, end) and validates the resulting order of elements.
196-202: Correctly testing deprecated API functionality.The test acknowledges that the
getfunction is deprecated (as noted in the static analysis hint) while ensuring it still functions correctly for backward compatibility.🧰 Tools
🪛 GitHub Check: Build and test
[warning] 200-200:
Yex.XmlElement.get/2 is deprecated. Rename tofetch/2
204-210: Good edge case testing for sibling relationships.These tests properly verify the behavior of next_sibling and prev_sibling at element boundaries.
212-218: Good tests for attribute edge cases.These tests correctly verify the behavior when dealing with non-existent attributes.
220-223: Good test for error handling with non-existent attributes.This test ensures that removing a non-existent attribute doesn't raise an error.
225-241: Well-structured test for nested element creation.This test effectively verifies the behavior of inserting elements with multiple children, properly testing both the element count and structure.
243-267: Comprehensive test for the children streaming interface.This test thoroughly validates various stream operations on element children, including mapping, counting, and reducing operations.
269-315: Excellent test for complex nested XML structures.This test provides comprehensive coverage of complex operations including traversal, attribute manipulation, and verification of parent-child relationships.
449-465: Good test for XmlElementPrelim with attributes.The test properly verifies that elements can be created with multiple attributes using direct constructor initialization.
467-474: Good edge case test for empty children list.This test properly verifies that XmlElementPrelim correctly handles elements with an empty children list.
Summary by CodeRabbit