kartikey321 commented on PR #3451: URL: https://github.com/apache/tinkerpop/pull/3451#issuecomment-5836705691
Hi @spmallette @Cole-Greer @kenhuuu, Pushed the restructuring we discussed. Summary of where this lands: **Structure** - The driver is now `tinkubator/gremlin-dart/`, a standalone Maven project outside the root reactor (no entry in the root `pom.xml`, other than one RAT exclude for `pubspec.lock`, matching the existing `go.sum` exclude for gremlin-go). - `DartTranslateVisitor` lives inside the Dart project (`tinkubator/gremlin-dart/src/main/java/`), in the same package/namespace gremlin-core's translators use and depending on gremlin-core for the interface, so it can move unchanged if gremlin-dart ever graduates out of Tinkubator. **Feature tests: translator-generated, not a runtime parser** - `build/generate.groovy` runs the translator over every scenario via gmavenplus and writes `test/feature/gremlin.dart`, following the same generated-test model as Python/Go/.NET/JS. - The runtime ANTLR-based parser (`GremlinAntlrToDart`, ~30k lines of checked-in generated grammar) is removed. Fixing two translator gaps (`UUID()` with no argument, `OptionsStrategy` with arbitrary keys) closed the last 3 scenarios that needed it, so nothing depends on it anymore. Happy to add it back as a build-time-generated (not checked-in) artifact in a follow-up if there's a use for it beyond feature-test generation — gremlin-js is the only other GLV with an equivalent, and it generates its parser at build time rather than committing it. - Of 2123 scenarios: 2053 execute, and the test runner now honestly reports the other 70 as *skipped* (unsupported tags) rather than counting them as passed — that was a real gap I found and fixed along the way. **Test results** (against `gremlin-server-test`): 2123/2123 feature scenarios (2053 run, 70 skipped), 22 integration tests, 212 unit tests, `dart analyze` clean. **Both previously-failing tests now pass** - The `0.5f` sack case: the test's expected value was being parsed as a double; it's now rounded to float32, matching what a Java float literal would produce. Not a tolerance — an exact match. - Fixing that surfaced a real driver bug: the server reports errors raised during iteration in the trailing status *after* HTTP 200, and the streaming path was dropping it, so a failing traversal like `g.V().range(2,1)` returned an empty list instead of throwing. Fixed, with a regression test. **Also fixed**, from an independent adversarial review of this round's changes: a 503 response never actually reached the retry logic (`validateStatus` accepted every status, so the retry interceptor's own 503 branch was dead code); `OptionsStrategy` config values generated as typed ints threw on a bad cast; transactions opened through the multi-host `Cluster` API were never tracked, so `Cluster.close()` left them and their connections open; and the Dart translator's string/character escaping only handled the quote character and `$`, silently mishandling octal/unicode/backslash escapes. Let me know if the Tinkubator/Maven shape or anything else needs adjusting. -- 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]
