Added coverage for gremlin-console and gremlin-language - #3512
Conversation
Assisted-by: Kiro: Claude Opus 4.8
| this.activated = false | ||
| } | ||
|
|
||
| public class GroovyGremlinShellEnvironment implements GremlinShellEnvironment { |
There was a problem hiding this comment.
This is a part of the codebase I'm not too familiar with, but as I understand it this code only existed to support the old remote-console infrastructure. If so, I agree it should be removed.
| @AfterClass | ||
| static void tearDownClass() { | ||
| // These tests write to the real per-user java.util.prefs store. Clear it so developer prefs aren't polluted. | ||
| ShellPreferences.clear() |
There was a problem hiding this comment.
Just confirming that these won't clear existing user preferences?
There was a problem hiding this comment.
I included an in memory preference for testing so that it doesn't conflict with user preferences.
|
VOTE +1 pending comments resolution |
|
This should be merged after beta-3 as to not add risk and other coverage tests in other areas will be after beta-3 as well. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #3512 +/- ##
============================================
+ Coverage 76.35% 76.85% +0.49%
- Complexity 13424 14361 +937
============================================
Files 1012 1036 +24
Lines 60341 64577 +4236
Branches 7075 7652 +577
============================================
+ Hits 46076 49631 +3555
- Misses 11548 11820 +272
- Partials 2717 3126 +409 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Test coverage improvements
Two related but independent changes that improve test coverage in TinkerPop, each self-contained:
GremlinParsercoverage ingremlin-language.Both changes also remove dead code found while raising coverage. Removals are listed first, followed by a consolidated coverage summary with the before/after numbers and an explanation of why coverage doesn't go higher.
Dead code removed
gremlin-language / gremlin-core (grammar)
booleanArgument,dateArgument, andnullableGenericLiteralMapgrammar rules (unreachable fromqueryListaftertheir references were dropped in earlier changes), plus the coupled
gremlin-corebase-visitor stubs,ArgumentVisitor/TranslateVisitormethods, and obsolete
ArgumentVisitorTestcases. Regenerated thegremlin-jsparser and removed its dead
visitBooleanArgument.gremlin-console
Console.groovy: unused privatewriteTraverserToErrorLines.PluggedIn.groovy: never-instantiatedGroovyGremlinShellEnvironmentinner class.MockGroovyGremlinShellEnvironmenttest helper.Coverage
gremlin-console
Caveat: the standard build runs the pexpect tests in Docker without the JaCoCo agent, so CI's JaCoCo run won't reflect the pexpect-driven portion; the 83% figure was obtained by attaching the agent to the console JVM the pexpect tests spawn.
before (~50%):
after (~83%):
gremlin-language (grammar / GremlinParser)
GremlinParsermain rule class: 76.2% -> 81.2% instructions, 62.8% -> 69.2% branches.GremlinParser(gremlin-language + full gremlin-core suite): 57.5% -> 61.3%.before (~57%):


before (~60%):
GremlinParserbefore:GremlinParserafter:Why coverage doesn't go higher
gremlin-console
InstallCommand's successful dependency-download branch (the realGrape.grab+ post-download plugin load) needs Grape + network, so it can't run in the offline test environment.RemoteGremlinPluginand remote query submission only do real work against a live Gremlin Server. Covering them fromgremlin-consolewould require standing up an embedded server or adding a test dependency ongremlin-server/gremlin-driver- out of scope for this module, and already covered by those modules' own integration tests - so those paths are intentionally left unexercised here.StackOverflowErrorhint branch inhandleErrorstays uncovered: triggering a realStackOverflowErroron demand is flaky and depends on the JVM-Xssstack size, so a test for that single hint message would be brittle. (The other interactive error branches - the "Display stack trace? [yN]" y/n answers, theFailureformatter, and the non-interactive-escript-error exit - are covered by the pexpect suite.)gremlin-language (
GremlinParser)GremlinParseris ANTLR-generated, so the remaining uncovered code is eitherunreachable by valid input or only coverable by tests that assert nothing
meaningful. These additions already cover essentially all the reachable,
behaviour-bearing productions:
catch (RecognitionException)recovery block. The grammar test harness installsBailErrorStrategy, which aborts on the first error, so recovery is neverentered; reaching those blocks would mean re-parsing malformed input under a
non-bail error strategy the harness deliberately doesn't use. This is the single
largest uncovered bucket.
pre-empted by an earlier alternative that matches the same input (e.g.
by(Order)/by(T), and the nested-traversalhas/property/hasValueforms, which the
*_Objectalternatives subsume). ANTLR never builds thosecontext nodes, so no input can cover them; making them reachable would require
reordering the grammar (out of scope).
*Contextaccessor methods such asKeywordContextandTraversalGTypeContexttoken getters are never called(consumers read
ctx.getText()), and the generated no-op listener is unused.The only way to cover them is to invoke the generated getters directly, which
asserts nothing about behaviour, so such a test would only game the metric.
getSerializedATN,getATN,sempred, etc. areANTLR boilerplate; invoking them in a test would colour the lines without
verifying anything, so covering them would only inflate the number.