cucumber-expressions: regexp bug - parentheses in character class regex

[spicalous]

Summary

A regular expression for a step definition is not generating the correct step parameters

Current Behavior

I have the following regexp for a step in my project

/^an example step: ([A-Z_, ()]+)$/

Should match and return correct parameters for
an example step: VALUE_ONE, VALUE_TWO (ABC)

Currently it matches, but returns undefined for the step parameter

Possible Solution

We are able to change our regexp to match step in passing test (mentioned below), however I feel like this should still be fixed as it used to work in older version of cucumber

Steps to Reproduce (for bugs)

I have managed to produce a failing test in
https://github.com/cucumber/cucumber-expressions-javascript

Add the following lines to https://github.com/cucumber/cucumber-expressions-javascript/blob/master/test/regular_expression_test.js

  it.only('this test fails', function() {
    const parameterRegistry = new ParameterTypeRegistry()
    const expr = /^the chart has settings with following drawings: ([A-Z_, ()]+)$/
    const expression = new RegularExpression(expr, parameterRegistry)

    const args = expression.match(
      'the chart has settings with following drawings: TREND_LINE, TREND_LINE (MACD)'
    )

    assert.equal(args[0].getValue(), 'TREND_LINE, TREND_LINE (MACD)')
  })

  it.only('this test passes', function() {
    const parameterRegistry = new ParameterTypeRegistry()
    const expr = /^the chart has settings with following drawings: (.*)$/
    const expression = new RegularExpression(expr, parameterRegistry)

    const args = expression.match(
      'the chart has settings with following drawings: TREND_LINE, TREND_LINE (MACD)'
    )

    assert.equal(args[0].getValue(), 'TREND_LINE, TREND_LINE (MACD)')
  })

Context & Motivation

We're upgrading from a really old version of cucumber-js (0.5.3)
This used to work with the old version and seems like a regression

Your Environment

[email protected] in the browser

Any help would be appreciated
Thank you in advance!

[aslakhellesoy]

Thanks for the very detailed bug report @spicalous! I have an idea what the problem might be. I'll be working on a fix.

[spicalous]

Thanks @aslakhellesoy ! Let me know if I can be of any help :)

[aslakhellesoy]

Some help would be great! Start in https://github.com/cucumber/cucumber/blob/master/cucumber-expressions/javascript/test/tree_regexp_test.js and see if you can write a failing test.

The TreeRegexp class parses the regexp (yes you read right) and builds a tree representing the (possibly nested) capture groups of the regexp. (This is so we can pull out the right capture groups by index before we pass them to a parameter type transformer).

TreeRegexp currently naively thinks the () inside the character class [] is a capture group, but as we both know, it is not!

Can you fix the parser?

[aslakhellesoy]

@spicalous See above - I've reproduced and fixed the bug (in Java). Do you think you can port it to some of the other languages?

[spicalous]

@aslakhellesoy Nice! I have been a bit busy but I will give it a go tomorrow evening, thanks! 👍

[spicalous]

#463 JS implemention PR opened :)

[lock]

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

[luke-hill]

Wasn't this fixed in 6.0.2?

[aslakhellesoy]

You're right @luke-hill - my bad