fix(client): Add SameNeighborsBatchApiTest to test suite - #673
fix(client): Add SameNeighborsBatchApiTest to test suite#673sadwitdastreetz wants to merge 3 commits into
Conversation
Add SameNeighborsBatchApiTest to test suite (temporarily @ignore due to missing server API)
b4e0292 to
879f18d
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #673 +/- ##
============================================
- Coverage 62.49% 62.00% -0.50%
+ Complexity 1903 936 -967
============================================
Files 262 93 -169
Lines 9541 4572 -4969
Branches 886 531 -355
============================================
- Hits 5963 2835 -3128
+ Misses 3190 1523 -1667
+ Partials 388 214 -174 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
@imbajin please take a review : ) |
There was a problem hiding this comment.
Pull Request Overview
This pull request adds the SameNeighborsBatchApiTest to the test suite while temporarily ignoring it due to the missing server API.
- Added a new test class with an @ignore annotation referencing the unimplemented API.
- Updated the ApiTestSuite to include the new test class.
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| hugegraph-client/src/test/java/org/apache/hugegraph/api/traverser/SameNeighborsBatchApiTest.java | Added new test class with @ignore to temporarily bypass tests for unimplemented API. |
| hugegraph-client/src/test/java/org/apache/hugegraph/api/ApiTestSuite.java | Updated test suite by including the new SameNeighborsBatchApiTest class. |
Comments suppressed due to low confidence (1)
hugegraph-client/src/test/java/org/apache/hugegraph/api/traverser/SameNeighborsBatchApiTest.java:32
- [nitpick] Consider removing the angle brackets from the URL in the @ignore message to ensure consistent formatting and clarity.
@Ignore("Blocked by server: /traversers/sameneighborsbatch not implemented, see <https://github.com/apache/incubator-hugegraph/issues/2798>")
|
|
||
| import com.google.common.collect.ImmutableList; | ||
|
|
||
| @Ignore("Blocked by server: /traversers/sameneighborsbatch not implemented, see <https://github.com/apache/incubator-hugegraph/issues/2798>") |
Previously, I added ignore to SameNeighborsBatchApi because I found the API wasn't implemanted in server. now, I should delete this @ignore for the completion of SameNeighborsBatchApi in the community version.
|
Due to the lack of activity, the current pr is marked as stale and will be closed after 180 days, any update will remove the stale label |
|
Due to the lack of activity, the current pr is marked as stale and will be closed after 180 days, any update will remove the stale label |
VGalaxies
left a comment
There was a problem hiding this comment.
Review summary
- Blocking: yes
- Summary: The PR enables
SameNeighborsBatchApiTest, but CI’s pinned HugeGraph server does not expose the endpoint that this test class calls. - Evidence:
- Static review of
git diff origin/master...HEAD git diff --check origin/master...HEADpassed.github/workflows/client-ci.ymlrunsmvn test -Dtest=ApiTestSuiteagainst HugeGraph server commit8c1ee71, whose traversal API has/traversers/sameneighborsbut no/traversers/sameneighborsbatch
- Static review of
| CountApiTest.class, | ||
| RingsRaysApiTest.class, | ||
| SameNeighborsApiTest.class, | ||
| SameNeighborsBatchApiTest.class, |
There was a problem hiding this comment.
High: Enabled test suite calls an unsupported server endpoint
hugegraph-client/src/test/java/org/apache/hugegraph/api/ApiTestSuite.java:78
Evidence
SameNeighborsBatchAPI.type()returnssameneighborsbatch, so addingSameNeighborsBatchApiTest.classmakesApiTestSuitepost to/traversers/sameneighborsbatch; the CI workflow runsApiTestSuiteagainst server commit8c1ee71, where onlySameNeighborsAPIat/traversers/sameneighborsexists.
Impact
mvn test -Dtest=ApiTestSuitewill fail in CI once this class is included.
Requested fix
- Align the client/test with the server route and response contract, or keep this test excluded until the CI server revision actually provides the batch endpoint.
| CountApiTest.class, | ||
| RingsRaysApiTest.class, | ||
| SameNeighborsApiTest.class, | ||
| SameNeighborsBatchApiTest.class, |
There was a problem hiding this comment.
Low: Enabled limit coverage reuses a stale request
hugegraph-client/src/test/java/org/apache/hugegraph/api/ApiTestSuite.java:78
Evidence
- In
SameNeighborsBatchApiTest.testSameNeighborsWithLimit(), the laterlimit(2L)andlimit(1L)blocks create builders but post the original request built before those options were applied.
Impact
- Even after the endpoint issue is fixed, the suite can pass without exercising batch limit behavior.
Requested fix
- Rebuild and assign the request after setting each block’s options, then update the
limit(1L)assertions to expect limited results.
|
Due to the lack of activity, the current pr is marked as stale and will be closed after 180 days, any update will remove the stale label |
Add SameNeighborsBatchApiTest to test suite (temporarily Ignore due to missing server API)
temporarily Ignore due to missing server API, related to server Issue#2798
apache/hugegraph#2798
close #672