-
Notifications
You must be signed in to change notification settings - Fork 615
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Adds a lightweight CI smoke test to validate the HugeGraph server distribution’s bin/gremlin-console.sh entrypoint (non-interactive) on Linux and macOS, addressing the coverage gap called out in #3015.
Changes:
- Introduce a new Travis/CI helper script that runs Gremlin Console with a tiny Groovy script and asserts an expected marker in output.
- Run the smoke test in Linux Server CI only for the
rocksdbmatrix entry. - Run the same smoke test in the macOS RocksDB CI job (both Intel and Apple Silicon runners).
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
hugegraph-server/hugegraph-dist/src/assembly/travis/run-gremlin-console-smoke-test.sh |
New portable bash smoke test that executes bin/gremlin-console.sh with a generated Groovy script and validates output. |
.github/workflows/server-ci.yml |
Adds CI steps to invoke the new Gremlin Console smoke test in Linux (rocksdb only) and macOS RocksDB jobs. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Review summary
- Blocking: yes
- Summary: The new smoke-test script is not portable to the macOS CI jobs added by this PR.
- Evidence:
git diff --check origin/master...HEADcleanbash -n .../run-gremlin-console-smoke-test.shclean- static review of changed workflow/script
@VGalaxies
VGalaxies
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Review summary
- Blocking: no
- Summary: The new Gremlin Console smoke test only validates local console startup, not connectivity to HugeGraph.
- Evidence:
- Reviewed
git diff origin/master...HEAD git diff --check origin/master...HEADpassedrgfound noremote.yaml,:remote, or remote traversal usage in the new script
- Reviewed
Codecov Report
✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 32.79%. Comparing base (f56462a) to head (1d7dab4).
Additional details and impacted files
@@ Coverage Diff @@ ## master #3040 +/- ## ============================================ - Coverage 35.94% 32.79% -3.16% - Complexity 338 499 +161 ============================================ Files 803 803 Lines 68053 68241 +188 Branches 8907 8965 +58 ============================================ - Hits 24465 22380 -2085 - Misses 40967 43358 +2391 + Partials 2621 2503 -118
☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.
🚀 New features to boost your workflow:
- ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
@imbajin
imbajin
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Blocking: yes. Summary: The new Gremlin Console smoke helper is invoked directly but is checked in without the executable bit. Evidence: latest-head CI fails with Permission denied at run-api-test.sh:103, and git ls-tree shows mode 100644.
🔗 Please check the failed latest-head jobs, for example https://github.com/apache/hugegraph/actions/runs/27470916921/job/81205575322.
@imbajin
imbajin
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, +1. The previous CI smoke-test issues have been addressed on the current head, and the relevant Linux/macOS RocksDB jobs are green with the smoke marker emitted after connecting to the live Gremlin Server.
Non-blocking follow-up: the new smoke test now validates the packaged bin/gremlin-console.sh startup path and a live remote traversal, which covers the main CI gap. One possible future improvement is to align it even more closely with the user-facing console flow documented in README.md, i.e. :remote connect tinkerpop.server conf/remote.yaml followed by :> ..., or to reuse scripts/remote-connect.groovy from the Docker healthcheck path. The current driver-based script is enough for this PR's smoke coverage, so I do not think this should block merge.
Purpose of the PR
Add a focused Gremlin Console smoke test for the server distribution entry point.
This follows up on #3015 and the review note from #3010. The existing macOS RocksDB CI covers server compile, core tests, startup, and API tests, but it does not directly exercise
bin/gremlin-console.sh. This PR adds a lightweight non-interactive smoke test for that path on macOS and Linux.Related context: #3006 reported user-facing build/startup issues and mentioned Gremlin Console behavior on Apple Silicon, but this PR only adds CI validation and does not assume a specific Jansi or Java compatibility root cause.
Main Changes
run-gremlin-console-smoke-test.shbin/gremlin-console.sh -- -e <script>gremlin-console-smoke-okmarkertrapVerifying these changes
bash -n hugegraph-server/hugegraph-dist/src/assembly/travis/run-gremlin-console-smoke-test.shgit diff --checkDoes this PR potentially affect the following parts?
Documentation Status
Doc - TODODoc - DoneDoc - No Need