Add unit tests for Queryable#826
Conversation
Reviewer's GuideAdds a comprehensive JUnit 5 test suite for org.lambda.query.Queryable plus a small CustomQuery Extendable implementation used as a test fixture, covering constructors, factories, core query operators, numeric aggregations, grouping/joining/splitting, extension mechanism, and a variety of edge cases (empty, nulls, single element, ties, type inference). File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- The
QueryableTestclass is quite large and covers many different behaviors; consider splitting it into smaller test classes (e.g., grouping by feature: construction, selection/filtering, aggregation, grouping, etc.) to keep each class focused and easier to navigate. - Several test patterns (e.g., max/min with selectors, all/any single vs. empty cases) could be refactored into parameterized tests to reduce repetition and make it clearer which input/output combinations are being exercised.
- For edge-case tests that are explicitly pinning surprising behavior (such as
splitdropping the trailing segment andaverageon empty returning NaN/Infinity), consider making the intent even more explicit in the test names (e.g.,testSplitDoesNotIncludeTrailingSegment) to communicate that these semantics are deliberate rather than incidental.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The `QueryableTest` class is quite large and covers many different behaviors; consider splitting it into smaller test classes (e.g., grouping by feature: construction, selection/filtering, aggregation, grouping, etc.) to keep each class focused and easier to navigate.
- Several test patterns (e.g., max/min with selectors, all/any single vs. empty cases) could be refactored into parameterized tests to reduce repetition and make it clearer which input/output combinations are being exercised.
- For edge-case tests that are explicitly pinning surprising behavior (such as `split` dropping the trailing segment and `average` on empty returning NaN/Infinity), consider making the intent even more explicit in the test names (e.g., `testSplitDoesNotIncludeTrailingSegment`) to communicate that these semantics are deliberate rather than incidental.
## Individual Comments
### Comment 1
<location path="approvaltests-util/src/test/java/org/lambda/query/QueryableTest.java" line_range="25-34" />
<code_context>
+ assertEquals(3, result.get(0).intValue());
+ }
+
+ @Test
+ void testSkipMoreThanSize()
+ {
+ Queryable<Integer> q = Queryable.as(1, 2, 3);
+ Queryable<Integer> result = q.skip(10);
+ assertEquals(0, result.size());
+ }
+
+ @Test
+ void testTake()
+ {
+ Queryable<Integer> q = Queryable.as(1, 2, 3, 4, 5);
+ Queryable<Integer> result = q.take(3);
+ assertEquals(3, result.size());
+ assertEquals(1, result.get(0).intValue());
+ }
+
+ @Test
+ void testTakeMoreThanSize()
+ {
+ Queryable<Integer> q = Queryable.as(1, 2, 3);
+ Queryable<Integer> result = q.take(10);
+ assertEquals(3, result.size());
+ }
+
+ @Test
+ void testTakeAndSkipCombined()
+ {
+ Queryable<Integer> q = Queryable.as(1, 2, 3, 4, 5, 6, 7, 8, 9, 10);
</code_context>
<issue_to_address>
**suggestion (testing):** Add tests for skip()/take() with zero and negative counts to fully pin boundary behavior
Current tests cover normal usage and over-skipping/-taking, but not other boundaries. Please add cases for:
- `skip(0)` / `take(0)` to document whether they are no-ops and the resulting size.
- `skip(-1)` / `take(-1)` (if allowed) to verify how negatives are handled (clamped to 0, ignored, or error).
This will clarify the contract for edge arguments and prevent regressions.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| @Test | ||
| void testDefaultConstructor() | ||
| { | ||
| Queryable<String> q = new Queryable<>(); | ||
| assertEquals(0, q.size()); | ||
| } | ||
|
|
||
| @Test | ||
| void testConstructorWithType() | ||
| { |
There was a problem hiding this comment.
suggestion (testing): Add tests for skip()/take() with zero and negative counts to fully pin boundary behavior
Current tests cover normal usage and over-skipping/-taking, but not other boundaries. Please add cases for:
skip(0)/take(0)to document whether they are no-ops and the resulting size.skip(-1)/take(-1)(if allowed) to verify how negatives are handled (clamped to 0, ignored, or error).
This will clarify the contract for edge arguments and prevent regressions.
|
Does this overlap or extend approvaltests-util-tests/src/test/java/org/lambda/query/QueryableTest.java? |
|
Good catch. I looked, and it's both. There's genuine overlap: about 11 of these duplicate methods already in The other ~80 are net-new coverage the existing suite doesn't have: So it overlaps and extends. If the added coverage is useful, I'm glad to rework it properly: fold the net-new tests into the existing |
Extends the existing approvaltests-util-tests/QueryableTest with additive tests covering where, all/any, max/min (with selectors and ties), sum/average, skip/take, split, orderBy, firstOrDefault, the of/as factory variants, and empty/null/single-element edge cases. No production code changed. Signed-off-by: vasiliy-mikhailov <vasiliy-mikhailov@users.noreply.github.com>
vasiliy-mikhailov
force-pushed
the
add-unit-tests-queryable
branch
from
1355e98 to
4596881
Compare
|
Done, reworked. I folded the net-new tests into the existing The added tests cover operators the existing suite didn't: |
2 participants
Additive unit tests for
Queryable— covering edge cases and pinning current behavior with explicit assertions. Includes a smallCustomQuerytest fixture (anExtendableimplementation) used by the extension-mechanism test. No existing test or production code is changed; both are new test files only.Verified green under Java 17:
mvn -pl approvaltests-util test -Dtest=QueryableTest→Tests run: 95, Failures: 0, Errors: 0Summary by Sourcery
Add comprehensive unit tests for Queryable to cover constructors, core query operations, edge cases, and extension mechanisms without changing production code.
Tests: