cucumber-rs / cucumber

Cucumber testing framework for Rust. Fully native, no external test runners or dependencies.
https://cucumber-rs.github.io/cucumber/main
Apache License 2.0
579 stars 70 forks source link

Alternative method of defining steps with less repetition repetition #325

Closed hakanai closed 7 months ago

hakanai commented 7 months ago

In the documented example...

#[given(expr = "{word} is hungry")] // Cucumber Expression
async fn someone_is_hungry(w: &mut World, user: String) {

I have to type "is hungry" twice. Is there no cleaner API where I can avoid the repetition? Repetition eventually results in inconsistency. Proof:

#[then("she is full")]
async fn is_full(w: &mut World) {

Why would the first one be "someone" is hungry, while the second one is just "is full"?

Can there not be an alternative API using closures to define the steps? In my Kotlin code, I don't have to do this, and Rust does have closures, so I'm not sure how the API ended up being a clone of the bad Java one.

If the names of the functions are truly arbitrary, then the names in the docs should be corrected to some truly arbitrary naming such as f1, f2, f3 to make it clear that this is boilerplate.

ilslv commented 7 months ago

Hi, I think there has been a misunderstanding. Nowhere in the docs mentioned that function name must correlate with expression itself, it's just seems logical to do. Functions can have arbitrary names:

#[given(expr = "{word} is hungry")]
async fn literally_arbitrary_name(w: &mut World, user: String) {}
hakanai commented 7 months ago

I think you're the one with the misunderstanding here.

Firstly,

If the function name doesn't matter, why am I forced to provide it? Generate one automatically for me.

It's the tools and libraries' job to remove meaningless work from the developer. It isn't my job to do the tool's work for it.

Please reopen this, and fix the issue before resolving it, or properly explain why you're not doing the tool developer's job of making the developers' lives easier.

If this is what the rest of the Rust ecosystem is going to be like, then I'll probably stick to other languages. I had high hopes for this one.

Secondly,

If the readme should have used f1, f2, f3, etc. for the function names, as you appear to be saying, then don't close the ticket until you have corrected the readme to make it clear that these names are just boilerplate, and not important code.

ilslv commented 7 months ago

As an open source developer I don't owe you anything. This issue will stay closed until you at least suggest an alternative design in a respectful manner, instead of just whining around.

hakanai commented 7 months ago

Nobody said you did owe me anything, I'm just stating the correct way to manage tickets. If you want to persist in doing it incorrectly, that's totally up to you, you just have to understand that you are doing it wrong.

Like with most things in life, if something is wrong, all we can do is to point it out. It's up to the party in the wrong to actually fix the situation.

(And once I know that a project is managing tickets incorrectly, that's good information too - it means I know that I can't trust that project. Because, if I can't go to a project's issues page and see all the current issues, if they are closing issues without actually fixing them? Then I don't know how many issues a project actually has. For all I know, you might have thousands of issues, which were just closed without fixing them. Some of them might even be security issues. How could I know?)

If you can't fix the actual issue, can you at least recommend me a library which does things better?

Or at a minimum, update the docs. Currently the inconsistency is still in there.

An overhead, to be sure, to keep the names in sync, but you should take that up with whoever decided to design the API in a way which required duplicating the names.

tyranron commented 7 months ago

@hakanai one of the best practices when using attribute macros in Rust is that the code, the attribute is placed onto, remains a valid Rust code and doesn't disappear after the macro expansion.

We could have, in theory, introduced something like this:

#[given(expr = "{word} is hungry")]
async fn(w: &mut World, user: String) {
    sleep(Duration::from_secs(2)).await;

    w.user = Some(user);
}

and generate some unique name in the macro expansion automatically.

However, this is not a valid Rust code (it doesn't allow such function syntax), which is looking weird for rustaceans and will be complained by IDEs all along.

That's why the Rust code friendly way was chosen, to not introduce any additional syntax to the language, making it hard/unexpected for tooling and readers. The fact that the developer should give some names to functions is considered as a "lesser evil" here by us, and didn't bother us in practice. Given this rationale, we won't change this behavior, unless the Rust language makes such syntax valid.

So, you have 3 options here:

  1. Either accept things "as is" and don't bother about names of step functions as we do.
  2. Or do not use proc macros for step functions and provide them as anonymous closures directly.
  3. Or do not use this library at all.
hakanai commented 6 months ago

Option #\2 is looking like the winner at the moment.

I had enough pain with the function names in the older version of the Java edition and wasn't keen to revisit it. Names get out of sync fast once you can have two different values for them because people update one and not the other. Having seen how much better the lambda API was, it was almost like a bad nightmare coming here and finding that Rust was using the older way again.

Now I get the impression that it has both ways, but for whatever reason is only documenting one of them.