GumpacG opened a new pull request, #3512:
URL: https://github.com/apache/tinkerpop/pull/3512
# Test coverage improvements
Two related but independent changes that improve test coverage in TinkerPop,
each self-contained:
1. **Grammar parser coverage** - raises `GremlinParser` coverage in
`gremlin-language`.
2. **gremlin-console coverage** - adds unit + end-to-end (pexpect) tests for
the console.
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)**
- Removed the orphaned `booleanArgument`, `dateArgument`, and
`nullableGenericLiteralMap` grammar rules (unreachable from `queryList`
after
their references were dropped in earlier changes), plus the coupled
`gremlin-core` base-visitor stubs, `ArgumentVisitor`/`TranslateVisitor`
methods, and obsolete `ArgumentVisitorTest` cases. Regenerated the
`gremlin-js`
parser and removed its dead `visitBooleanArgument`.
**gremlin-console**
- `Console.groovy`: unused private `writeTraverserToErrorLines`.
- `PluggedIn.groovy`: never-instantiated `GroovyGremlinShellEnvironment`
inner class.
- Deleted the unused `MockGroovyGremlinShellEnvironment` test 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%):
<img width="1171" height="152" alt="coverage-console-before"
src="https://github.com/user-attachments/assets/e8d125de-0f48-43a8-b594-388d34713d62";
/>
after (~83%):
<img width="1169" height="151" alt="coverage-screenshot-2"
src="https://github.com/user-attachments/assets/b70bb882-cdd0-4df9-87e1-6b57df972048";
/>
---
### gremlin-language (grammar / GremlinParser)
- gremlin-language grammar package: 56.98% -> 60.25% (instructions).
- `GremlinParser` main rule class: 76.2% -> 81.2% instructions, 62.8% ->
69.2% branches.
- Aggregate `GremlinParser` (gremlin-language + full gremlin-core suite):
57.5% -> 61.3%.
- ~33 gremlin-core grammar visitor methods moved from 0% to covered.
before (~57%):
<img width="1170" height="124" alt="coverage-screenshot-7"
src="https://github.com/user-attachments/assets/bdfb220d-af53-4c77-9a92-967ce91c5fff";
/>
before (~60%):
<img width="1165" height="137" alt="coverage-screenshot-5"
src="https://github.com/user-attachments/assets/ceacd8a2-1b2c-4762-8333-a4144b1c2d7f";
/>
`GremlinParser` before:
<img width="1379" height="21" alt="coverage-screenshot-8"
src="https://github.com/user-attachments/assets/d0f7b993-b080-436c-935b-bf47b48c54bf";
/>
`GremlinParser` after:
<img width="1387" height="88" alt="coverage-screenshot-6"
src="https://github.com/user-attachments/assets/b5564ede-d9f5-4a6b-bc71-e24f21ce9ee6";
/>
### Why coverage doesn't go higher
**gremlin-console**
- `InstallCommand`'s successful dependency-download branch (the real
`Grape.grab` + post-download plugin load) needs Grape + network, so it can't
run in the offline test environment.
- `RemoteGremlinPlugin` and remote query submission only do real work
against a live Gremlin Server. Covering them from `gremlin-console` would
require standing up an embedded server or adding a test dependency on
`gremlin-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.
- The `StackOverflowError` hint branch in `handleError` stays uncovered:
triggering a real `StackOverflowError` on demand is flaky and depends on the
JVM `-Xss` stack 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, the `Failure` formatter, and the non-interactive `-e` script-error
exit - are covered by the pexpect suite.)
**gremlin-language (`GremlinParser`)**
`GremlinParser` is ANTLR-generated, so the remaining uncovered code is either
unreachable by valid input or only coverable by tests that assert nothing
meaningful. These additions already cover essentially all the reachable,
behaviour-bearing productions:
- ANTLR error-recovery paths - every generated rule method has a
`catch (RecognitionException)` recovery block. The grammar test harness
installs
`BailErrorStrategy`, which aborts on the first error, so recovery is never
entered; 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.
- Grammar-shadowed alternatives - some labeled alternatives are always
pre-empted by an earlier alternative that matches the same input (e.g.
`by(Order)`/`by(T)`, and the nested-traversal `has`/`property`/`hasValue`
forms, which the `*_Object` alternatives subsume). ANTLR never builds those
context nodes, so no input can cover them; making them reachable would
require
reordering the grammar (out of scope).
- Unused generated accessors - `*Context` accessor methods such as
`KeywordContext` and `TraversalGTypeContext` token 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.
- Generated infrastructure - `getSerializedATN`, `getATN`, `sempred`, etc.
are
ANTLR boilerplate; invoking them in a test would colour the lines without
verifying anything, so covering them would only inflate the number.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
