common icon indicating copy to clipboard operation
common copied to clipboard

Make `strict` behaviour consistent and document it

Open aslakhellesoy opened this issue 6 years ago • 10 comments

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 avatar Sep 26 '19 16:09 aslakhellesoy

@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

aslakhellesoy avatar Sep 26 '19 16:09 aslakhellesoy

@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.

mpkorstanje avatar Sep 26 '19 16:09 mpkorstanje

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.

charlierudolph avatar Oct 05 '19 15:10 charlierudolph

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

mpkorstanje avatar Dec 03 '19 14:12 mpkorstanje

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

See: #1788

mpkorstanje avatar Jan 17 '20 10:01 mpkorstanje

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

charlierudolph avatar Jan 17 '20 17:01 charlierudolph

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

mpkorstanje avatar Jan 17 '20 17:01 mpkorstanje

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

davidjgoss avatar Jun 29 '21 08:06 davidjgoss

(or even just removing it now...)

davidjgoss avatar Jun 29 '21 12:06 davidjgoss

I would prefer deprecating it first

aurelien-reeves avatar Jun 29 '21 12:06 aurelien-reeves