cucumber/common

Make `strict` behaviour consistent and document it

aslakhellesoy opened this issue · 10 comments

aslakhellesoy commented

Various cucumber implementations have inconsistent behaviour for strict:

  • Default value true or false?
  • Does strict cause undefined steps to fail?
  • Does strict cause pending steps to fail?

I propose we make strict default to true. By defaulting to true we're actively discouraging users from making a lot of WIP, and I think an opinionated tool like Cucumber should do stuff like that.

I also propose the following behaviour:

strict=true strict=false
undefined step error no error
pending step error no error
failed step error error
passed step no error no error
ambiguous step error error

I haven't looked into whether the various implementations behave like this. Let's agree on the desired behaviour - then create issues for the implementations that deviate so we can fix this.

When all implementations are fixed we should put this up on the docs site.

aslakhellesoy commented

@mpkorstanje told me he thinks perhaps strict=true should still allow undefined to cause no error, because it lets people check in WIP scenarios. Maybe this is a good thing - IDK.

We could also introduce more fine grained control:

  • strict=pending
  • strict=undefined
  • strict=all

That would change --strict command line option from taking no arguments to taking an argument. And the Java annotation would change from boolean to string.

Alternatively we could introduce new options:

  • strict-pending=true|false
  • strict-undefined=true|false
  • strict=true|false - sets both of the above
mpkorstanje commented

@mpkorstanje told me he thinks perhaps strict=true should still allow undefined to cause no error, because it lets people check in WIP scenarios. Maybe this is a good thing - IDK.

I think there is a case for defaulting to strict=false. The definition of strict is fine.

Adding more modes puts a burden on tooling. People expect that their reporting suite processes the results in the same way Cucumber does. So if we are going to default strict to true, perhaps we can get rid of it entirely?

edit: This wasn't clear originally -half this discussion happened in slack- but I'm proposing defaulting to strict mode and removing strict all together.

  • 👍1
  • 👎1
charlierudolph commented

Cucumber-js has strict default to true and having it set to false only allows pending steps to not cause a failure. Undefined and ambiguous are logically the same to me as both amount to being unable to find a step definition. Pending requires more intention whereas undefined is more easily a mistake.

I personally prefer we just remove the concept of strict entirely. I don't think WIP scenarios should be checked in in general (keep it on a branch). If you do check it in, I think the scenario should use a tag to mark it as WIP with the default profile having a tag expression that excludes that tag.

  • 👍2
mpkorstanje commented

So we've reached a consensus on --strict then? I feel like there have been very few people involved.

mpkorstanje commented

Cucumber JVM has started the migration. It will take some time to get everything switched over.

See: #1788

charlierudolph commented

What are you migrating Cucumber JVM to? Not sure I see a consensus in the existing comments

mpkorstanje commented

Migrating to default to strict. Then remove the option eventually.

davidjgoss commented

We have a major release of cucumber-js coming up. Thoughts on deprecating --no-strict and then removing it next time?

  • 👍1
davidjgoss commented

(or even just removing it now...)

aurelien-reeves commented

I would prefer deprecating it first

  • 👍1