Throwing an error from a transform function kills Cucumber

[aslakhellesoy]

How to reproduce:

addTransform({
  captureGroupRegexps: ['[A-Z]\\w+'],
  transformer: s =>  { throw new Error('I killed cucumber') },
  typeName: 'name'
})

[aslakhellesoy]

I've just started to look at this and discovered that Cucumber Expressions are parsed over and over again. A StepDefinition object only holds a reference to a pattern (a String or a RegExp) and a new CucumberExpression or RegularExpression object is created twice for every gherkin step: First when we check if there is a match (matchesStepName) and second when we extract the arguments (getInvocationParameters).

I haven't benchmarked, but I would imagine this has a significant negative impact on performance.

Consider a project with 100 Gherkin steps and 30 step definitions. Each step definition expression would be parsed 200 times and matched 200 times as well. That's a total of 6000 parses and 6000 match invocations.

A single parse per expression should be sufficient (total 30 instead of 6000) and a single evaluation per step (total 3000 instead of 6000).

We could improve the number of evaluations further by caching results.

Matching expressions in 2 places is also problematic for this issue - an error may be thrown by a transform during the first pass. So I think we may have to change the CucumberExpressions library to defer invoking the transform function. It shouldn't have to be invoked just for matching.

[charlierudolph]

Sounds good. Yes there are some optimizations we could make on our side. I like the logic of two match invocations per step as at first we are finding what steps match the pattern (this should not do any transformer logic) as there might be multiple in matches the case of ambiguous steps. Then when invoking the step, we match again to get the parameters and actually invoke the transformer logic

[charlierudolph]

Fix released in v2.0.0-rc.8

[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.