acuminous / yadda

A BDD javascript library
412 stars 73 forks source link

Feature Request: Configurable substitute. #203

Closed PeteOnTheGitHub closed 8 years ago

PeteOnTheGitHub commented 8 years ago

Hi :)

My team is using the yadda test runner for our test suit as part of our end to end test system and it works great! To assist with test writing we are using the intellj cucumber js syntax plugin with auto-complete and rule parsing.

Yadda works well within this framework with the exception of the variables/substitues in rules that reference table data. E.G. In an scenario outline:

When I set [search value] to the "search" input field

Examples:
      | search value         | search result page             |
      | "a street address" | "a results page in a table"  | 

is not consider valid by the cucumber syntax highlighting by default the syntax highlighting expects:

When I set <search value> to the "search" input field

The ideal fix for us would be to give the ability to configure the substitute character for the tests parsing. As far as i'm a aware this would give yadda the ability to become fully compatible with cucumber test and obviously be more flexibility.

The change involves some type of configuration that could by used by FeatureParser.js:

Chaging line 561:

function substitute(example, line) {
        for (var heading in example) {
            line = line.replace(new RegExp('\\[\\s*' + heading + '\\s*\\]', 'g'), StringUtils.rtrim(example[heading].join('\n')));
        }
        return line;
    }

To something like:

function substitute(example, line) {
        for (var heading in example) {
            line = line.replace(new RegExp('\\' + config.substituteLeft + '\\s*' + heading + '\\s*\\' + config.substituteRight + '', 'g'), StringUtils.rtrim(example[heading].join('\n')));
        }
        return line;
    }

If you want I could put up a PR for this, however I am not sure how the configuration would fit into the current program structure.

Regards, Pete

cressie176 commented 8 years ago

Hi Pete, I'm fine with setting the FeatureParser to < and > via a config option. It would need to be passed to the FeatureFileParser too.

In retrospect I should have passed in an options object to the FeatureParser instead of just the language. I think rather than adding a second parameter now I'd add an is_language property to the Language class, then use this to detect whether the first argument was a language or an options object.

FeatureParser(options)
    var language = options.is_language ? options : options.language || Localisation.default;
    var substitute_left = options.substitute_left || '['
    var substitute_right = options.substitute_right || ']'

An alternative I'd also consider (despite not being backwards compatible) is to accept either a square bracket or angle bracket in the regex. I think the biggest reason not to do this is that the less than and greater than signs can be quite useful, e.g.

then x < 1 and x > 2
PeteOnTheGitHub commented 8 years ago

Considering the whole Yadda designed to offer more flexibility thing. I think the first option would be better as it would then make [] available to be used as well if anyone changed the substitute.

I tend to agree that with your reason not to as well and I guess that was the reason for the original design.

Based on this feedback will you be implementing this, or do you require a PR?

cressie176 commented 8 years ago

I'm implementing now

cressie176 commented 8 years ago

Give it a try with 0.17.3

new FeatureFileParser({
  leftPlaceholderChar: '<',
  rightPlaceholderChar: '>'
})
PeteOnTheGitHub commented 8 years ago

Thanks works great.

In case anyone is using mocha integration or something similar to the mocha cucumber boilerplate, the change looks like this in init.js

Yadda.plugins.mocha.StepLevelPlugin.init()

to:

Yadda.plugins.mocha.StepLevelPlugin.init({
    parser: new Yadda.parsers.FeatureFileParser({
                leftPlaceholderChar: '<',
                rightPlaceholderChar: '>'
            })
});