"and" & "but" keywords should not be handled as dedicated entities

[esemusa]

Background:
The gherkin keywords „And“ and „But“ are just like a placeholder to connect multiple main keywords (eg „Given“, „When“ and „Then“) together in a human-readable sentence.
For example, if you want to combine three given steps like this:


Given step1
Given step2
Given step3

To make it more readable, it makes more sense to combine the phrases with "And", like:

Given step1
And step2
And step3

Thus, in my opinion, „And“ is not a dedicated keyword, it's more like a replacement for a main keyword. I think this matches with the cucumber documentation and concept. See more here https://cucumber.io/docs/gherkin/reference/#and-but

Issue:
In CucumberSwift, the „And“ keyword is handled as an own entity, it's not used as a replacement for the keyword used in one of the previous steps. For example, given this code
:

And(„test“) {_,_ in}

This gherkin syntax would be totally fine and the „test“ step would be executed in every 'and' phrase:


Given step1
And test
When step2
And test
Then step3
And test

Another problem occurs when defining a 'Given' step and you want it to be used for 'And' steps:

Given(„test123“) {_,_ in}

Given step1
And test123

This does not work, but in my opinion, it should. Sure, you can find workarounds like

Let step = „test123“

Func testCode() {}

Given(step) {_,_ in testCode()}
And(step) {_,_ in testCode()}

But then you would still face the first problem and your 'Given' step could be used for other keywords which might not be intented.

Suggestion:

When you parse the feature file and combine it with the matching step implementation (attachClosureToSteps) you need to check for the „and“ & „but“ keywords, and in that case search for the most-recently parsed main keyword, evaluating a match based on that. Something like that:

private func filterSteps(
        keyword: Step.Keyword?,
        regex: String
    ) -> [Step] {
        var filterIndex = -1
        
        let steps = features
            .flatMap { $0.scenarios.flatMap { $0.steps } }
        
        return steps.filter { step -> Bool in
            filterIndex += 1
            if  let k = keyword {
                if step.keyword.contains(k) {
                    return !step.match.matches(for: regex).isEmpty
                }
                
                if step.keyword.contains(.and) || step.keyword.contains(.but) {
                    let stepsBefore = steps[0..<filterIndex].reversed()
                    guard let nextMainStep = stepsBefore.first(where: { [Step.Keyword.given, .when, .then].contains($0.keyword) }), nextMainStep.keyword.contains(k) else {
                        return false
                    }
                    
                    return !step.match.matches(for: regex).isEmpty
                }
                return false
            } else if keyword == nil { // Is this still needed? I dont see any caller with a potential "nil" as keyword value (attachClosureToSteps)
                return !step.match.matches(for: regex).isEmpty
            }
            return false
        }
    }



func attachClosureToSteps(keyword: Step.Keyword? = nil, regex: String, callback:@escaping (([String], Step) -> Void)) {
        filterSteps(
            keyword: keyword,
            regex: regex
        ).forEach { step in
            step.result = .undefined
            step.execute = callback
            step.regex = regex
        }
    }

This specific solution would have at least one problem. Scenario bounds will be ignored. So if you start a scenario with an 'And', it will still be valid:

scenario: 1:

Given step1

Scenario 2:
And step1

You could probably see this as a gherkin syntax fault and not make it a responsibility of CucumberSwift to handle these edge cases. But im sure there's a fix for this somewhere around...

What do you think about the problem / solution? If you think it's a solution worth to invest in I might be able to find some time in the future to create a PR.

[Tyler-Keith-Thompson]

Hrm, I think you’re right about the behavior of “and” and “but”. Let me look at the spec and some other implementations over the weekend.

If we do implement this feature it’s probably something we would want to approach in the AST where we can enforce an error if there are orphans.

[Tyler-Keith-Thompson]

I've looked at some other implementations and confirmed that CucumberSwift is not following defined and expected behavior with this. I don't want to create a breaking change if I can avoid it so I'm pondering some implementation decisions.

I'm thinking of doing this over a few releases, to start we can implement the behavior in your example of Given("test123") And still ends up being a dedicated keyword but you start to get some of the desired behavior from the library.

Then we'll play with the * keyword which currently does sort of work for everything, but probably needs to be smarter when it should be treated more like an And.

Then we'll deprecate, but not remove And as an entity outside of the DSL.

At some point later on in life when V4 needs to be a thing we can just remove it altogether.

[Tyler-Keith-Thompson]

Update on this: CucumberSwift v 3.3.6 adds support for using Given, When, or Then to match gherkin that started with And or But. It is scoped appropriately.

EXAMPLE GHERKIN:

Feature description
    Scenario: Description
        Given some precondition
        And some other precondition

EXAMPLE SWIFT:

Given("some other precondition") { _, _ in
    // executes the "And" condition
}

NOTE: By scoped appropriately I mean that this Gherkin will not match Given

Feature description
    Scenario: Description
        And some other precondition

Although because the And/But keywords still exist, they can match that.

[esemusa]

Very nice! 😊