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

Are “Cucumber Expressions” supported ? #124

Closed
StyMaar opened this issue Jul 19, 2021 · 10 comments
Closed

Are “Cucumber Expressions” supported ? #124

StyMaar opened this issue Jul 19, 2021 · 10 comments
Assignees
Labels
enhancement Improvement of existing features or bugfix
Milestone

Comments

@StyMaar
Copy link

StyMaar commented Jul 19, 2021

I couldn't find any references to them, in documentation, examples or even implementation code.

For the record, Cucumber Expressions are the usual way to write step definitions in most languages, it's slightly less powerful than Regex but much more readable.

For example consider:

I have {float} cucumbers in my belly

vs

I have ([+-]?([0-9]+([.][0-9]*)?|[.][0-9]+)) cucumbers in my belly

(Float regex matcher taken from Stackoverflow)

@bbqsrc
Copy link
Member

bbqsrc commented Jul 19, 2021

I do not believe so. Might be something for someone to investigate. 😄

@bbqsrc bbqsrc added enhancement Improvement of existing features or bugfix help wanted Extra attention is needed labels Jul 19, 2021
@tyranron
Copy link
Member

tyranron commented Oct 4, 2021

@ilslv what do you think? I think this can be fully done on proc macro layer by simply translating cucumber expresion into regex during macro expansion. We don't even need to touch type machinery here.

@bbqsrc
Copy link
Member

bbqsrc commented Oct 4, 2021

That would be the sanest way to do it.

@tyranron
Copy link
Member

tyranron commented Oct 4, 2021

The proposed syntax may be #[then(expr = "I have {float} cucumbers in my belly")]

@ilslv
Copy link
Member

ilslv commented Oct 4, 2021

@tyranron sounds like the way to do it.

I propose adding support for Parameter types and leaving custom parameters for already existing FromStr feature.

So Kotlin's example

@ParameterType("red|blue|yellow")   // regexp
fun color(color: String): Color {   // name (from method), type
    return Color(color)             // transformer function
}

will be implemented as follows:

#[then(expr = "I have {string} eyes")]
fn eyes_color(w: &mut World, color: Color) {
    // ...
}

enum Color {
    // ...
}

impl FromStr for Color {
    // ...
}

@ilslv
Copy link
Member

ilslv commented Nov 23, 2021

Quoted from #157 (comment)

For custom parameters I propose something like this

enum Animal {
    Cat,
    Dog,
    Ferris,
}

impl FromStr for Animal {
    type Err = &'static str;

    fn from_str(s: &str) -> Result<Self, Self::Err> {
        match s {
            "cat" => Ok(Self::Cat),
            "dog" => Ok(Self::Dog),
            "🦀" => Ok(Self::Ferris),
            _ => Err("expected 'cat', 'dog' or '🦀'"),
        }
    }
}

impl CustomParameter for Animal {
    const NAME = "animal";
    const REGEX = "cat|dog|🦀";
}

#[then(expr = "the {animal} is not hungry")]
async fn cat_is_fed(world: &mut AnimalWorld, animal: Animal) {
    sleep(Duration::from_secs(2)).await;

    match animal {
        Animal::Cat => assert!(!world.cat.hungry),
        Animal::Dog => assert!(!world.dog.hungry),
        Animal::Ferris => assert!(!world.ferris.hungry),
    };
}

With that we can validate that parameter name is right with const assertion. And to validate regex we can even use clippy lint : Playground.
Also, as we have exact type, we can use deref-based specialisation to fallback on default parameter name in our const assertion.

UPD:

Clippy lint will require our users to run it over tests codebase, which is rarely done. If we want to avoid this, we'll have to implement CustomParameter with our own derive macro, as I don't think there is another way of validating regex at compile time.

tyranron pushed a commit that referenced this issue Nov 23, 2021
…#124)

- add `expr` argument to `#[given]`, `#[when]` and `#[then]` attributes
@tyranron
Copy link
Member

tyranron commented Nov 23, 2021

@ilslv well, in code it would be nice to have something like this:

#[derive(cucumber::Parameter)]
#[param(regex = "cat|dog|🦀")] // `name = ` is optional, 
enum Animal {                  // by default we may use `lowercased(ty.ident)`
    Cat,
    Dog,
    Ferris,
}

impl FromStr for Animal {
    type Err = &'static str;

    fn from_str(s: &str) -> Result<Self, Self::Err> {
        match s {
            "cat" => Ok(Self::Cat),
            "dog" => Ok(Self::Dog),
            "🦀" => Ok(Self::Ferris),
            _ => Err("expected 'cat', 'dog' or '🦀'"),
        }
    }
}

Regarding Regex validation, I guess we may not bother about validating the whole Regex during proc macro expansion. A Regex is a constructive thing, so as long as we have valid sub-parts, building the whole one will be valid to. And we can validate all the sub-parts during proc-macro expansion this way:

  • We validate Animal::REGEX when expand #[derive(cucumber::Parameter)].
  • When expand #[then(expr = "the {animal} is not hungry")] we substitute {animal} with \{animal\} for Regex and validate it as a Regex.
  • But in the generated code will construct a &'static str (or something similar) by const-concatenating "the (" + Animal::REGEX + ") is not hungry".
  • Finally, the whole concatenated thing we will feed into Regex::new() should never fail, as all the sub-parts were validated to be correct.

So, when we expand expr = ... we should check, whether it contains non-default params, and if yes, apply all the described above and the machinery you've menitioned.

This way we have compile-time guarantees and cheks, while preserving obvious and ergonomic API for users.

Do I miss something?

@ilslv
Copy link
Member

ilslv commented Nov 24, 2021

@tyranron pretty much yes, but I would change couple of things:

We validate Animal::REGEX when expand #[derive(cucumber::Parameter)].

In addition to validating we should check that regex doesn't contain capture groups itself. Because in case it is, those capture groups will be mapped onto the next step function argument.

When expand #[then(expr = "the {animal} is not hungry")] we substitute {animal} with \{animal\} for Regex and validate it as a Regex.

Or we can just create cucumber_expressions::ParameterProvider which will return "()" on every parameter name, except defaults.

Also we should const assert that animal is equals to the Animal::NAME. But here we have 2 options: fail in case some default parameter name like {word} is supplied to the Animal or allow that to happen. I think it's better to fail at compile time as there is special regex for it. And vice versa: in case argument struct doesn't implement Provider, only FromStr, it should accept any of the default parameter names.

@tyranron
Copy link
Member

@ilslv

In addition to validating we should check that regex doesn't contain capture groups itself. Because in case it is, those capture groups will be mapped onto the next step function argument.

Seems reasonable. But we still should allow (?: ) groups as they allow express things, and be accurate with escaping (\( allowed, while \\( is not, and so forth).

Another way is just silently transform ( ) into (?: ) so they won't capture anything and don't bother user at all about such nitpicks.

Or we can just create cucumber_expressions::ParameterProvider which will return "()" on every parameter name, except defaults.

As a simplest way to do so - yes. But I proposed that way with a better errors redability in mind, if it applies though.

Also we should const assert that animal is equals to the Animal::NAME. But here we have 2 options: fail in case some default parameter name like {word} is supplied to the Animal or allow that to happen. I think it's better to fail at compile time as there is special regex for it. And vice versa: in case argument struct doesn't implement Provider, only FromStr, it should accept any of the default parameter names.

If we allow it to happen, it just won't work soundly. Having a {word} parameter we cannot now inside proc macro whether we should substitute it with Animal::REGEX or not, just because we cannot know whether Animal impls cucumber::Parameter. So, a hard distinction between default and custom-defined parameters is the only sound way here.

tyranron added a commit that referenced this issue Nov 29, 2021
Co-authored-by: Kai Ren <tyranron@gmail.com>
@tyranron
Copy link
Member

With merging #168 now is fully implemented and will be released in 0.11.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Improvement of existing features or bugfix
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants