Skip to content

Add splitByElement() to Gherkin step definitions for nested collection parsing#3379

Open
spmallette wants to merge 1 commit into3.8-devfrom
fulfill-pr-3216
Open

Add splitByElement() to Gherkin step definitions for nested collection parsing#3379
spmallette wants to merge 1 commit into3.8-devfrom
fulfill-pr-3216

Conversation

@spmallette
Copy link
Copy Markdown
Contributor

@spmallette spmallette commented Apr 8, 2026

This PR completes #3216 which was for Java only but implements it across all the language variants. cc/ @yunlingTao

VOTE +1

…n parsing

Naive split(",") breaks on nested collection tokens like l[1,2,3] inside
s[l[1,2,3],l[4,5,6]]. Adds a bracket-depth-aware splitByElement() helper to
all Gherkin step definition files across Java and every GLV, so that commas
inside nested brackets are not treated as top-level separators.

Also adds two new Gherkin scenarios to Fold.feature that exercise nested
list-of-list results (g.inject([1,2],[3,4]).fold()), along with the
corresponding traversal bindings in each GLV's translation map.

Java implementation follows PR #3216.

(tinkerpop-mxr, tinkerpop-32y, tinkerpop-ghp, tinkerpop-2ur, tinkerpop-70k, tinkerpop-188, tinkerpop-93a)
Copy link
Copy Markdown

@yunlingTao yunlingTao left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is everything needed. Closed #3216.

@xiazcy
Copy link
Copy Markdown
Contributor

xiazcy commented Apr 8, 2026

VOTE +1

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are only tests here for nested list, but this PR also changes set. Should add test for set as well as test for extra-depth in nesting.

@kenhuuu
Copy link
Copy Markdown
Contributor

kenhuuu commented Apr 9, 2026

Consider a CHANGELOG entry? Though I think it might be small enough that it doesn't matter.

@Cole-Greer
Copy link
Copy Markdown
Contributor

Overall I think this change looks good and I'll give it a VOTE +1 as a standalone inclusion.

I do want to point out that even with this change, nested collections in feature tests is sketchy and only works in specific cases. For example the following case looks reasonable, but does not work.

Given the modern graph
And using the parameter xx1 defined as "l[m[{\"name\":\"marko\"}]]"
And the traversal of
  """
  g.inject(xx1).unfold().select("name")
  """
When iterated to list
Then the result should be unordered
  | result |
  | marko |

The primary issue with this example is that the m\[(.*)\] matcher triggers before the l\[(.*)\] matcher, so it tries to parse {\"name\":\"marko\"}] as a map (note the trailing ] as it used closing bracket from the list to terminate the map).

In general mixed collection type nesting does not work and it's not a small change to fix it.

My view on feature test infrastructure is typically that I don't want complexity without necessity. I'm happy to leave these limitations if we don't intend to write tests that require mixed-type nests.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants