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

Make strict behaviour consistent and document it #714

Open
aslakhellesoy opened this issue Sep 26, 2019 · 10 comments
Open

Make strict behaviour consistent and document it #714

aslakhellesoy opened this issue Sep 26, 2019 · 10 comments
Labels
📖 documentation Improvements or additions to documentation platform consolidation 🧷 pinned Tells Stalebot not to close this issue

Comments

@aslakhellesoy
Copy link
Contributor

aslakhellesoy commented Sep 26, 2019

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
Copy link
Contributor Author

@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
Copy link
Contributor

mpkorstanje commented Sep 26, 2019

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

@charlierudolph
Copy link
Member

charlierudolph commented Oct 5, 2019

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.

@aslakhellesoy aslakhellesoy added 📖 documentation Improvements or additions to documentation platform consolidation labels Oct 7, 2019
@mpkorstanje
Copy link
Contributor

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

@mpkorstanje
Copy link
Contributor

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

See: #1788

@mpkorstanje mpkorstanje added the 🧷 pinned Tells Stalebot not to close this issue label Jan 17, 2020
@charlierudolph
Copy link
Member

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

@mpkorstanje
Copy link
Contributor

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

mpkorstanje added a commit to cucumber/cucumber-jvm that referenced this issue Apr 30, 2020
Unlike other implementations Cucumber-JVM defaults to `--non-strict` mode. By defaulting to `--strict` Cucumber follows the same standards as other implementations.

Additionally the option to set strict mode adds complexity for consumers of Cucumbers output. They have to interpret Cucumbers 6 scenario outcomes with this in mind. By removing `--strict/--non-strict` mode we remove this complexity all together.

With PR Cucumber will:
 * default to `--strict` 
 * throw an error on `--non-strict`.  
 * warn when `--strict` is used.
 * only print snippets as part of the exception thrown by JUnit rather then use the summary printer
 * only print snippets as part of the exception thrown by TestNG rather then use  the summary printer

Partial fix for #1788, cucumber/common#714
@davidjgoss
Copy link
Contributor

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

@davidjgoss
Copy link
Contributor

(or even just removing it now...)

@aurelien-reeves
Copy link
Contributor

I would prefer deprecating it first

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
📖 documentation Improvements or additions to documentation platform consolidation 🧷 pinned Tells Stalebot not to close this issue
Projects
Status: To do
Development

No branches or pull requests

5 participants