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

[Core] Default to --strict mode #1960

Merged
merged 22 commits into from
Apr 30, 2020
Merged

[Core] Default to --strict mode #1960

merged 22 commits into from
Apr 30, 2020

Conversation

mpkorstanje
Copy link
Contributor

@mpkorstanje mpkorstanje commented Apr 24, 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

Types of changes

  • Bug fix (non-breaking change which fixes an issue).
  • New feature (non-breaking change which adds functionality).
  • Breaking change (fix or feature that would cause existing functionality to not work as expected).

Checklist:

  • I've added tests for my code.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.

@mpkorstanje mpkorstanje changed the base branch from master to v6.x.x April 24, 2020 19:38
@coveralls
Copy link

coveralls commented Apr 24, 2020

Coverage Status

Coverage decreased (-0.1%) to 87.124% when pulling 08cc8dd on remove-strict into 6c0df8e on v6.x.x.

@mpkorstanje mpkorstanje added this to the 6.0.0 milestone Apr 29, 2020
@mpkorstanje mpkorstanje marked this pull request as ready for review April 30, 2020 11:19
@mpkorstanje mpkorstanje merged commit 5fbae84 into v6.x.x Apr 30, 2020
@mpkorstanje mpkorstanje deleted the remove-strict branch April 30, 2020 11:21
@sebrose
Copy link
Member

sebrose commented Aug 27, 2020

The title of this PR (and the comment in the Changelog) imply only a change of default. It looks like the whole idea of non-strict has been removed - although the help text still says:
cucumber.execution.strict= # true or false. default: false.

Am I missing something?

@mpkorstanje
Copy link
Contributor Author

Internally the implementation is gone. However we're still in the process of phasing out the user facing part.

private static void errorOnNonStrict(Boolean strict) {
if (!strict) {
throw new CucumberException(EXECUTION_STRICT_PROPERTY_NAME
+ "=false is no longer effective. Please use =true (the default) or remove this property");
}
}

mpkorstanje added a commit that referenced this pull request Mar 14, 2021
The teamcity plugin communicates test results to IntelliJ IDEA using teamcity
service messages[1]. Currently the teamcity plugin marks pending steps as
skipped rather then failed. This is inconsistent with the changes from #1960
in which pending steps are considered a test failure.
mpkorstanje added a commit that referenced this pull request Mar 14, 2021
The teamcity plugin communicates test results to IntelliJ IDEA using teamcity
service messages[1]. Currently the teamcity plugin marks pending steps as
skipped rather then failed. This is inconsistent with the changes from #1960
in which pending steps are considered a test failure.

 1: https://www.jetbrains.com/help/teamcity/service-messages.html
mpkorstanje added a commit that referenced this pull request Mar 14, 2021
The teamcity plugin communicates test results to IntelliJ IDEA using teamcity
service messages[1]. Currently the teamcity plugin marks pending steps as
skipped rather then failed. This is inconsistent with the changes from #1960
in which pending steps are considered a test failure.

 1: https://www.jetbrains.com/help/teamcity/service-messages.html
mpkorstanje added a commit that referenced this pull request Mar 14, 2021
The teamcity plugin communicates test results to IntelliJ IDEA using teamcity
service messages[1]. Currently the teamcity plugin marks pending steps as
skipped rather then failed. This is inconsistent with the changes from #1960
in which pending steps are considered a test failure.

 1: https://www.jetbrains.com/help/teamcity/service-messages.html
mpkorstanje added a commit that referenced this pull request Mar 15, 2021
* [Core] Mark pending steps as failed in teamcity plugin

The teamcity plugin communicates test results to IntelliJ IDEA using teamcity
service messages[1]. Currently the teamcity plugin marks pending steps as
skipped rather then failed. This is inconsistent with the changes from #1960
in which pending steps are considered a test failure.

 1: https://www.jetbrains.com/help/teamcity/service-messages.html

* [Core] Fix teamcity link for cucumber-java8 hooks

The teamcity plugin communicates test results to IntelliJ IDEA using teamcity
service messages[1]. These messages can link to files using a url-like protocol.
The code use to convert code locations from cucumber-java8 step definitions
did not correctly extract the method from this string representation.
@Colin-Heyl
Copy link

Hi, wondering if someone can verify something for me?

We're using Cucumber at the moment and a test will not fail, if a feature is not found.

Shouldn't that happen if strict mode is now the default? Or has strict functionality just been stripped out for some reason?

@mpkorstanje
Copy link
Contributor Author

Strict mode only affects translating PENDING, UNDEFINED, AMBIGUOUS steps results into a pass or fail. It does not do anything w.r.t to features being discovered.

If you have a feature request, please create a new issue though.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants