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.
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