Increase code coverage with unit tests for core classes #224
Conversation
📝 WalkthroughWalkthroughThis pull request adds six new unit test classes to increase test coverage across the FalkorDB Java library. The tests validate behavior of existing classes including Header, GraphException, Edge, Node, Point, and Property through constructors, accessors, equality, hashCode, string representation, and edge cases. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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. Comment |
Co-authored-by: gkorland <753206+gkorland@users.noreply.github.com>
Co-authored-by: gkorland <753206+gkorland@users.noreply.github.com>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #224 +/- ##
============================================
+ Coverage 73.13% 76.34% +3.21%
- Complexity 265 290 +25
============================================
Files 25 25
Lines 871 871
Branches 106 106
============================================
+ Hits 637 665 +28
+ Misses 172 159 -13
+ Partials 62 47 -15 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Pull request overview
This PR adds comprehensive unit tests for previously untested core classes in the JFalkorDB library, increasing code coverage from 16% to 22%. The focus is on testing fundamental graph entities (Node, Edge, Point, Property), exception handling (GraphException), and enum functionality (Header.ResultSetColumnTypes).
Changes:
- Added 78 unit tests covering constructors, getters/setters, equality, hash codes, and edge cases
- Achieved 100% coverage for GraphException and significant improvements for graph entity classes
- All tests follow JUnit 5 conventions with no external dependencies
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| src/test/java/com/falkordb/graph_entities/PropertyTest.java | Tests for Property class including generic type handling, Integer/Long equality, and null safety |
| src/test/java/com/falkordb/graph_entities/PointTest.java | Tests for Point class covering geographic coordinates, epsilon-based equality, and boundary values |
| src/test/java/com/falkordb/graph_entities/NodeTest.java | Tests for Node class focusing on label management and capacity optimization |
| src/test/java/com/falkordb/graph_entities/EdgeTest.java | Tests for Edge class verifying relationship properties and source/destination handling |
| src/test/java/com/falkordb/exceptions/GraphExceptionTest.java | Tests for GraphException covering all constructor variants |
| src/test/java/com/falkordb/HeaderTest.java | Tests for Header.ResultSetColumnTypes enum including valueOf and ordinal methods |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![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
🤖 Fix all issues with AI agents
In `@src/test/java/com/falkordb/graph_entities/PointTest.java`:
- Around line 128-138: The test testHashCodeWithDifferentPoints currently
asserts that two distinct Point instances (Point point1, Point point2) have
different hash codes which is brittle; remove the assertion
assertNotEquals(point1.hashCode(), point2.hashCode()) and keep only the
meaningful assertion assertNotEquals(point1, point2); optionally rename the test
to testPointInequality or testEqualsContract to reflect it checks inequality (or
add a separate test that equal Points produce equal hashCode using Point.equals
and Point.hashCode if needed).
🧹 Nitpick comments (1)
src/test/java/com/falkordb/HeaderTest.java (1)
10-63: Reduce brittleness from enum order/ordinal assertions.These tests will fail on harmless enum growth or reordering. Consider focusing on presence/valueOf/name instead of
values()order, length, andordinal().♻️ Suggested adjustment
public void testResultSetColumnTypesEnumValues() { Header.ResultSetColumnTypes[] types = Header.ResultSetColumnTypes.values(); - - assertEquals(4, types.length); - assertEquals(Header.ResultSetColumnTypes.COLUMN_UNKNOWN, types[0]); - assertEquals(Header.ResultSetColumnTypes.COLUMN_SCALAR, types[1]); - assertEquals(Header.ResultSetColumnTypes.COLUMN_NODE, types[2]); - assertEquals(Header.ResultSetColumnTypes.COLUMN_RELATION, types[3]); + assertTrue(Arrays.asList(types).contains(Header.ResultSetColumnTypes.COLUMN_UNKNOWN)); + assertTrue(Arrays.asList(types).contains(Header.ResultSetColumnTypes.COLUMN_SCALAR)); + assertTrue(Arrays.asList(types).contains(Header.ResultSetColumnTypes.COLUMN_NODE)); + assertTrue(Arrays.asList(types).contains(Header.ResultSetColumnTypes.COLUMN_RELATION)); } @@ - public void testResultSetColumnTypesOrdinal() { - assertEquals(0, Header.ResultSetColumnTypes.COLUMN_UNKNOWN.ordinal()); - assertEquals(1, Header.ResultSetColumnTypes.COLUMN_SCALAR.ordinal()); - assertEquals(2, Header.ResultSetColumnTypes.COLUMN_NODE.ordinal()); - assertEquals(3, Header.ResultSetColumnTypes.COLUMN_RELATION.ordinal()); - }
| public void testHashCodeWithDifferentPoints() { | ||
| Point point1 = new Point(40.7128, -74.0060); | ||
| Point point2 = new Point(51.5074, -0.1278); | ||
|
|
||
| // Different points should have different hash codes (not required, but likely) | ||
| // Main assertion is that they are not equal | ||
| assertNotEquals(point1, point2); | ||
| // We can't guarantee different hash codes, but for these distinct values it's likely | ||
| // This is more of a sanity check than a strict requirement | ||
| assertNotEquals(point1.hashCode(), point2.hashCode()); | ||
| } |
There was a problem hiding this comment.
Avoid asserting hashCode inequality for different points.
Hash collisions are legal; this test can fail even if the implementation is correct.
✅ Safer assertion
public void testHashCodeWithDifferentPoints() {
Point point1 = new Point(40.7128, -74.0060);
Point point2 = new Point(51.5074, -0.1278);
// Different points should have different hash codes (not required, but likely)
// Main assertion is that they are not equal
assertNotEquals(point1, point2);
- // We can't guarantee different hash codes, but for these distinct values it's likely
- // This is more of a sanity check than a strict requirement
- assertNotEquals(point1.hashCode(), point2.hashCode());
}📝 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.
| public void testHashCodeWithDifferentPoints() { | |
| Point point1 = new Point(40.7128, -74.0060); | |
| Point point2 = new Point(51.5074, -0.1278); | |
| // Different points should have different hash codes (not required, but likely) | |
| // Main assertion is that they are not equal | |
| assertNotEquals(point1, point2); | |
| // We can't guarantee different hash codes, but for these distinct values it's likely | |
| // This is more of a sanity check than a strict requirement | |
| assertNotEquals(point1.hashCode(), point2.hashCode()); | |
| } | |
| public void testHashCodeWithDifferentPoints() { | |
| Point point1 = new Point(40.7128, -74.0060); | |
| Point point2 = new Point(51.5074, -0.1278); | |
| // Different points should have different hash codes (not required, but likely) | |
| // Main assertion is that they are not equal | |
| assertNotEquals(point1, point2); | |
| } |
🤖 Prompt for AI Agents
In `@src/test/java/com/falkordb/graph_entities/PointTest.java` around lines 128 -
138, The test testHashCodeWithDifferentPoints currently asserts that two
distinct Point instances (Point point1, Point point2) have different hash codes
which is brittle; remove the assertion assertNotEquals(point1.hashCode(),
point2.hashCode()) and keep only the meaningful assertion
assertNotEquals(point1, point2); optionally rename the test to
testPointInequality or testEqualsContract to reflect it checks inequality (or
add a separate test that equal Points produce equal hashCode using Point.equals
and Point.hashCode if needed).
Added unit tests for previously untested core classes, increasing overall coverage from 16% to 22%.
Coverage improvements
Tests added (78 total)
All tests are self-contained unit tests with no external dependencies. Test patterns follow existing codebase conventions (JUnit 5, EqualsVerifier where applicable).
Original prompt
✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.