"and" keyword / prevent step definition repetition?

[ivnsch]

I've a step that's used after both a given and then.

I couldn't find the and keyword and figured that I've to use given (etc.) respectively. In this case this leads to having to write the step definition twice (with given and then), which isn't ideal.

[tyranron]

@ivanschuetz we have a lot of similar situations. You shouldn't write the whole step definition twice, it would be enough to only place 2 attributes on the same function like this:

#[given(regex = "^And I build an application transaction...")]
#[then(regex = "^And I build an application transaction...")]
fn build_app_transaction(w: &mut World, /* captures here */) {
    /* code here */
}

And we unlikely will change this, as we do prefer explicit separation between Given, When and Then types of steps.

---

In addition, I also recomend you to consider using more data tables for the scenarios you've shared.

Instead

And I build an application transaction with the transient account, the current application, suggested params, operation "create", approval-program "programs/big_app_program.teal.tok", clear-program "programs/big_app_program.teal.tok", global-bytes <global-bytes>, global-ints 0, local-bytes <local-bytes>, local-ints 0, app-args "", foreign-apps "", foreign-assets "", app-accounts "", extra-pages 3

do (or similar)

And I build an application transaction with 
  | account     | transient |
  | application | current   |
  | params      | suggested |
  | operation        | create                            |
  | approval-program | programs/big_app_program.teal.tok |
  | clear-program    | programs/big_app_program.teal.tok |
  | global-bytes     | <global-bytes>                    |
  | global-ints      | 0                                 |
  | local-bytes      | <local-bytes>                     |
  | local-ints       | 0                                 |
  | app-args         |                                   |
  | foreign-apps     |                                   |
  | foreign-assets   |                                   |
  | app-accounts     |                                   |
  | extra-pages      | 3                                 |

It would be easier both read/match the step and access the table data via gherkin::Step.

See example here.

[ivnsch]

Okay that makes sense.

Re: tables, I'm not in charge of the feature specifications but will forward, thanks!

[eggyal]

Officially:

Keywords are not taken into account when looking for a step definition. This means you cannot have a Given, When, Then, And or But step with the same text as another step.

Cucumber considers the following steps duplicates:

Given there is money in my account
Then there is money in my account

This might seem like a limitation, but it forces you to come up with a less ambiguous, more clear domain language:

Given my account has a balance of £430
Then my account should have a balance of £430

Personally I think it's wrong to diverge from this approach.

[ilslv]

@eggyal we are aware of the official implementations and explicitly choose another way of doing this, because we think it's more flexible.

[eggyal]

Understood, but one problem such divergence creates is that feature files are not portable across implementations.

[tyranron]

@eggyal

but one problem such divergence creates is that feature files are not portable across implementations.

Could you provide an example where this makes unportable a .feature file exactly?

Because the difference has nothing to do with a .feature file itself, but rather deviates in how you're declaring step matching functions. It doesn't limit you to reuse the same step matching functions for .feature files in any way. The separations only allows you do more, as enlarges the space of possible implementations.

[eggyal]

In this implementation, one can write:

Given there is money in my account
Then there is money in my account

and semantically that's two separate steps with different meanings (albeit the user can implement them with the same code).

However, if that feature file was then ported to another implementation, the semantics would change so that the two statements are necessarily a repetition of one single step.

The rule now has different meanings across the two implementations.

Gherkin is a language, with defined semantics. What is implemented here is something similar to Gherkin, but the semantics are different—so it isn't the same language.

Personally I also feel that "forcing users to come up with a less ambiguous, more clear domain language" is more justifiable than "we think it's more flexible".

[tyranron]

@eggyal

In this implementation, one can write:

Given there is money in my account
Then there is money in my account

and semantically that's two separate steps with different meanings (albeit the user can implement them with the same code).

User may also implement them with different code preserving the same semantics.

However, if that feature file was then ported to another implementation, the semantics would change so that the two statements are necessarily a repetition of one single step.

The rule now has different meanings across the two implementations.

You're claming that semantics of a .feature file are defined by one of its implementation. But this point makes no sense to me, as goes against BDD spirit. A .feature file itself should define the described semantics, while the implementation should check whether it's upholden.

Steps separation only provides additional flexibility in how the rule may be implemented, depending on the inner execution stage, whci should not reflect a .feature semantics in any way.

The implementation part is not portable either way it has steps separation, or not.

Gherkin is a language, with defined semantics. What is implemented here is something similar to Gherkin, but the semantics are different—so it isn't the same language.

Oh, c'mon. The cucumber and gherkin crates will never be the same language, just because neither Gherkin, nor Cucumber, doesn't have any formal spec defining the language, accordingly to which we may reason about we implement it or not. They have only very uncomplete reference and are fully implementation-driven. So, "something similar to Gherkin, but the semantics are different—so it isn't the same language" - is the only reality here, even if we implement the reference word-to-word. Without a formal spec it's all just speculations.

I also feel that "forcing users to come up with a less ambiguous, more clear domain language" is more justifiable than "we think it's more flexible".

This point, however, makes a lot sense to me. It's hard to disagree with.

@ilslv another good point for doing this will be simplification of the library machinery, as we don't need support 3 different type of steps anymore. Will we loose something signficant if we do this?

[ilslv]

@tyranron

In this implementation, one can write:

Given there is money in my account
Then there is money in my account

and semantically that's two separate steps with different meanings (albeit the user can implement them with the same code).

To be fair, all cucumber implementation support passing Step into the callback function, where you can match on the keyword type. So all implementations won't stop you from doing this.

Will we loose something significant if we do this?

In our codebase, we have many cases where given and when steps are implemented in the same function, differentiating only somewhere in the end in the way we store the result of previous calculations. This approach works fine for our needs. New approach will force us to split them, making code harder to follow. To be fair I don't buy the argument forcing users to come up with a less ambiguous, more clear domain language, because given, when or then gives additional meaning to a step, making itself less ambiguous. I don't really see how splitting Given Alice ate 3 cucumbers and When Alice eats 3 cucumbers into 2 different functions will make language less ambiguous. I would even argue otherwise: this approach can lead me to writing Given Alice eats 3 cucumbers and then painfully debugging, what exactly went wrong.

[tyranron]

@ilslv

In our codebase, we have many cases where given and when steps are implemented in the same function, differentiating only somewhere in the end in the way we store the result of previous calculations. This approach works fine for our needs. New approach will force us to split them, making code harder to follow.

What exactly would change? Can you provide a more concrete example? As far as I see, this shouldn't change our existing test suites anyhow, so no split should be required. There are only situations where the same phrase was matched by different function already (so we can match over Step and unit them, or just change the phrase for one of them).

I would even argue otherwise: this approach can lead me to writing Given Alice eats 3 cucumbers and then painfully debugging, what exactly went wrong.

Yup, that makes sense too.

We definitely need more opinions here.

@bbqsrc what would you say?

@tgsmith61591 @rogertorres i would be nice to here your opinions on this issue as well.

[ilslv]

@tyranron

As far as I see, this shouldn't change our existing test suites anyhow, so no split should be required.

Yeah, you are right.

I also found a similar discussion in official cucumber repo: https://github.com/cucumber/common/issues/768 TLDR: there is a proposal to make behaviour similar to ours, but even stricter: disallow declaring steps with same name but different keywords. Good summary from the discussion:

• With step definition: Given("x", ...)
  • Use in a feature file
  • Given x -> matches step definition
  • When x -> no match, wrong keyword
  • Then x -> no match, wrong keyword
  • And x / But x -> keyword need to be transformed to last usage of Given / When / Then
• Multiple step definitions Given("x", ...) and Given("x", ...) lead to ambiguous result

Questions:

• Are multiple step definitions Given("x", ...) and When("x", ...) allowed?
  • These stop leading to ambiguous results and could technically be allowed after keywords are always unambiguous

And from what I can see, maintainers of the official cucumber are agreed, that it's a great direction.

[tyranron]

@ilslv

there is a proposal to make behaviour similar to ours, but even stricter: disallow declaring steps with same name but different keywords.

And from what I can see, maintainers of the official cucumber are agreed, that it's a great direction.

It would be nice if they had designed so from the start. However now, we (and they) are tightened hard by existing and legacy codebases.

If we do so, the issue raised by @ivanschuetz at the start of the topic won't be possible to be resolved at all, because there is an existing large features set abusing this rule and is being used/referenced by many implementations. While with the current our implementation at least he can resolve his issue.

I propose to make such strictness only after upstream will implement it (so we bc break when they bc break). Otherwise, we'll introduce some real and painful incompatibility.

For now, I propose left our implementation as is, because, as @ilslv emphasized, it does much better with preventing footguns and even forces the rule for "users to come up with a less ambiguous, more clear domain language" as they inevitably will consider the separation, so will think about it, instead of just freely reusing anything everywhere without giving it a thought. (That's what actually happened in our codebase, where we came with "was"/"is" differences in description because of considering separation.) At the same time our separation doesn't restrict users to write tests against existing scenarios using this rule unmindly.

[ilslv]

@tyranron

That's what actually happened in our codebase, where we came with "was"/"is" differences in description because of considering separation.

I think it will be useful to include those recommendations inside the Book and revisit all existing code samples to make sure that we follow them.

[ivnsch]

Late addition: this is being quite annoying in some tests I'm implementing, which have often the same Ands after given/when/then (all 3, sometimes 2 or 1) so I've to go through every line and verify that I added all the macros (or wait for the tests to show "skipped" somewhere) + obviously the code repetition is not nice (especially when the expressions are large / complex).

See e.g. "I build an application transaction with" .. https://github.com/algorand/algorand-sdk-testing/blob/master/features/integration/abi.feature https://github.com/algorand/algorand-sdk-testing/blob/master/features/integration/applications.feature https://github.com/algorand/algorand-sdk-testing/blob/master/features/integration/c2c.feature

Macros:

#[given(
    regex = r#"^I build an application transaction with the transient account, the current application, suggested params, operation "([^"]*)", approval-program "([^"]*)", clear-program "([^"]*)", global-bytes (\d+), global-ints (\d+), local-bytes (\d+), local-ints (\d+), app-args "([^"]*)", foreign-apps "([^"]*)", foreign-assets "([^"]*)", app-accounts "([^"]*)", extra-pages (\d+)$"#
)]
#[then(
    regex = r#"^I build an application transaction with the transient account, the current application, suggested params, operation "([^"]*)", approval-program "([^"]*)", clear-program "([^"]*)", global-bytes (\d+), global-ints (\d+), local-bytes (\d+), local-ints (\d+), app-args "([^"]*)", foreign-apps "([^"]*)", foreign-assets "([^"]*)", app-accounts "([^"]*)", extra-pages (\d+)$"#
)]
#[when(
    regex = r#"^I build an application transaction with the transient account, the current application, suggested params, operation "([^"]*)", approval-program "([^"]*)", clear-program "([^"]*)", global-bytes (\d+), global-ints (\d+), local-bytes (\d+), local-ints (\d+), app-args "([^"]*)", foreign-apps "([^"]*)", foreign-assets "([^"]*)", app-accounts "([^"]*)", extra-pages (\d+)$"#
)]

[ivnsch]

Oh, that's exactly the same example that I brought up initially. It can't be rewritten as tables currently and there are many other instances - guess that I'm just irritated about having had to re-run the test so many times now (which isn't quick), because it keeps finding some missing somewhere.