Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

cucumber-expressions: Invalid snippets generated for escaped optional #994

Closed
mpkorstanje opened this issue May 11, 2020 · 4 comments
Closed
Assignees
Labels
🐛 bug Defect / Bug 🧷 pinned Tells Stalebot not to close this issue

Comments

@mpkorstanje
Copy link
Contributor

mpkorstanje commented May 11, 2020

Spotted on stack overflow:

https://stackoverflow.com/questions/61725664/is-it-not-possible-to-have-a-step-definition-with-no-parameters

The following scenario:

Feature: Spheres
  Scenario: A ray intersects a sphere at two points
    Given r ← ray(point(0, 0, -5), vector(0, 0, 1))
    And s ← sphere()
    When xs ← intersect(s, r)
    Then xs.count = 2
    And xs[0] = 4.0
    And xs[1] = 6.0

Trying to keep it simple, I thought I'd write the step definition literally to start off:

import io.cucumber.java8.En

class SphereStepDefinitions: En {
    private val epsilon: Double = 0.00001
    lateinit var s: Sphere
    lateinit var xs : List<Double>

    init {
        Given("s ← sphere\\()") {
            s = Sphere()
        }
    }
}

The result,

The step "s ← sphere()" is undefined. You can implement it using the snippet(s) below:

Given("s ← sphere\\()", () -> {
    // Write code here that turns the phrase above into concrete actions
    throw new io.cucumber.java8.PendingException();
});

The correct step definition is:

s ← sphere()

Because

    private static final Pattern OPTIONAL_PATTERN = Pattern.compile("(\\\\\\\\)?\\(([^)]+)\\)");

Does not allow empty capture groups.

Probably solved by implementing the parser for Cucumber Expressions rather then the current rewriting solution but we should check the snippet generation when we do.

@lav16kosh
Copy link

lav16kosh commented Jul 1, 2020

@mpkorstanje I changed the optional_pattern to (\\\\)?\(([^)]*)\) to include step definition with no parameters.
image

And I created a test in CucumberExpressionTest.java to verify with no parameters. It worked fine.

image

Do we have to do any other check to resolve this issue?

@mpkorstanje
Copy link
Contributor Author

That would work but it would be breaking changes in cucumber expressions. If breaking changes are to be made I'd rather bundle them all up as part of #771

@mpkorstanje mpkorstanje self-assigned this Jul 2, 2020
@stale
Copy link

stale bot commented Aug 31, 2020

This issue has been automatically marked as stale because it has not had recent activity. It will be closed in a week if no further activity occurs.

@stale stale bot added the ⌛ stale Will soon be closed by stalebot unless there is activity label Aug 31, 2020
@mpkorstanje mpkorstanje added the 🧷 pinned Tells Stalebot not to close this issue label Aug 31, 2020
@stale stale bot removed the ⌛ stale Will soon be closed by stalebot unless there is activity label Aug 31, 2020
@aslakhellesoy
Copy link
Contributor

@mpkorstanje can we close this now? I believe this was fixed as part of #771.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🐛 bug Defect / Bug 🧷 pinned Tells Stalebot not to close this issue
Projects
No open projects
Development

No branches or pull requests

3 participants