From ae55aa48ae5772061817f52b26ff8328c4746cb3 Mon Sep 17 00:00:00 2001 From: David RACODON Date: Tue, 13 Dec 2016 17:28:35 +0100 Subject: [PATCH] Fix #39 There should be one single When step per scenario --- README.md | 1 + .../checks/OneSingleWhenPerScenarioCheck.java | 65 +++++++++++++++++++ .../gherkin/one-single-when-per-scenario.html | 23 +++++++ .../OneSingleWhenPerScenarioCheckTest.java | 32 +++++++++ .../one-single-when-per-scenario.feature | 20 ++++++ .../one-single-when-per-scenario.feature | 20 ++++++ .../gherkin-one-single-when-per-scenario.json | 21 ++++++ .../test/expected/gherkin-use-and-but.json | 3 + .../gherkin/GherkinRulesDefinition.java | 1 + .../plugins/gherkin/GherkinProfileTest.java | 2 +- .../gherkin/GherkinRulesDefinitionTest.java | 2 +- 11 files changed, 188 insertions(+), 2 deletions(-) create mode 100644 gherkin-checks/src/main/java/org/sonar/gherkin/checks/OneSingleWhenPerScenarioCheck.java create mode 100644 gherkin-checks/src/main/resources/org/sonar/l10n/gherkin/rules/gherkin/one-single-when-per-scenario.html create mode 100644 gherkin-checks/src/test/java/org/sonar/gherkin/checks/OneSingleWhenPerScenarioCheckTest.java create mode 100644 gherkin-checks/src/test/resources/checks/one-single-when-per-scenario.feature create mode 100644 its/ruling/projects/custom/one-single-when-per-scenario.feature create mode 100644 its/ruling/tests/src/test/expected/gherkin-one-single-when-per-scenario.json diff --git a/README.md b/README.md index 2b44e51..5261219 100644 --- a/README.md +++ b/README.md @@ -79,6 +79,7 @@ Number of features. * Tags should comply with a naming convention * Tags should not be set on Examples * Then steps should follow a regular expression + * There should be one single When step per scenario * Unused variables should be removed * Useless tags should be removed * When steps should follow a regular expression diff --git a/gherkin-checks/src/main/java/org/sonar/gherkin/checks/OneSingleWhenPerScenarioCheck.java b/gherkin-checks/src/main/java/org/sonar/gherkin/checks/OneSingleWhenPerScenarioCheck.java new file mode 100644 index 0000000..4d8a3aa --- /dev/null +++ b/gherkin-checks/src/main/java/org/sonar/gherkin/checks/OneSingleWhenPerScenarioCheck.java @@ -0,0 +1,65 @@ +/* + * SonarQube Gherkin Analyzer + * Copyright (C) 2016-2016 David RACODON + * david.racodon@gmail.com + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 3 of the License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with this program; if not, write to the Free Software Foundation, + * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. + */ +package org.sonar.gherkin.checks; + +import com.google.common.collect.ImmutableList; +import org.sonar.check.Priority; +import org.sonar.check.Rule; +import org.sonar.plugins.gherkin.api.tree.BasicScenarioTree; +import org.sonar.plugins.gherkin.api.tree.StepTree; +import org.sonar.plugins.gherkin.api.tree.Tree; +import org.sonar.plugins.gherkin.api.visitors.SubscriptionVisitorCheck; +import org.sonar.plugins.gherkin.api.visitors.issue.PreciseIssue; +import org.sonar.squidbridge.annotations.ActivatedByDefault; +import org.sonar.squidbridge.annotations.SqaleConstantRemediation; + +import java.util.List; +import java.util.stream.Collectors; + +@Rule( + key = "one-single-when-per-scenario", + name = "There should be one single When step per scenario", + priority = Priority.MAJOR, + tags = {Tags.DESIGN}) +@SqaleConstantRemediation("15min") +@ActivatedByDefault +public class OneSingleWhenPerScenarioCheck extends SubscriptionVisitorCheck { + + @Override + public List nodesToVisit() { + return ImmutableList.of(Tree.Kind.SCENARIO, Tree.Kind.SCENARIO_OUTLINE); + } + + @Override + public void visitNode(Tree tree) { + List whenSteps = ((BasicScenarioTree) tree).steps() + .stream() + .filter(s -> s.type() == StepTree.StepType.WHEN) + .collect(Collectors.toList()); + + if (whenSteps.size() > 1) { + PreciseIssue issue = addPreciseIssue(whenSteps.get(0), "Merge all the When steps or split the scenario up in multiple scenarios."); + for (int i = 1; i < whenSteps.size(); i++) { + issue.secondary(whenSteps.get(i), "When step"); + } + } + } + +} diff --git a/gherkin-checks/src/main/resources/org/sonar/l10n/gherkin/rules/gherkin/one-single-when-per-scenario.html b/gherkin-checks/src/main/resources/org/sonar/l10n/gherkin/rules/gherkin/one-single-when-per-scenario.html new file mode 100644 index 0000000..69fc471 --- /dev/null +++ b/gherkin-checks/src/main/resources/org/sonar/l10n/gherkin/rules/gherkin/one-single-when-per-scenario.html @@ -0,0 +1,23 @@ +

+ There should be one single When step per scenario. If you feel compelled to add more, it is usually a + sign that you should split the scenario up in multiple scenarios or that some When steps could be + converted into Given steps. +

+ +

Noncompliant Code Example

+
+Scenario: Add a nice bike to my cart
+  Given I am a customer
+  When I go to the nice bike product page
+  And I add the nice bike to my cart           # Noncompliant
+  Then I should see the nice bike in my cart
+
+ +

Compliant Solution

+
+Scenario: Add a nice bike to my cart
+  Given I am a customer
+  And I am on the nice bike product page
+  When I add the nice bike to my cart
+  Then I should see the nice bike in my cart
+
diff --git a/gherkin-checks/src/test/java/org/sonar/gherkin/checks/OneSingleWhenPerScenarioCheckTest.java b/gherkin-checks/src/test/java/org/sonar/gherkin/checks/OneSingleWhenPerScenarioCheckTest.java new file mode 100644 index 0000000..a19608c --- /dev/null +++ b/gherkin-checks/src/test/java/org/sonar/gherkin/checks/OneSingleWhenPerScenarioCheckTest.java @@ -0,0 +1,32 @@ +/* + * SonarQube Gherkin Analyzer + * Copyright (C) 2016-2016 David RACODON + * david.racodon@gmail.com + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 3 of the License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with this program; if not, write to the Free Software Foundation, + * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. + */ +package org.sonar.gherkin.checks; + +import org.junit.Test; +import org.sonar.gherkin.checks.verifier.GherkinCheckVerifier; + +public class OneSingleWhenPerScenarioCheckTest { + + @Test + public void test() { + GherkinCheckVerifier.verify(new OneSingleWhenPerScenarioCheck(), CheckTestUtils.getTestFile("one-single-when-per-scenario.feature")); + } + +} diff --git a/gherkin-checks/src/test/resources/checks/one-single-when-per-scenario.feature b/gherkin-checks/src/test/resources/checks/one-single-when-per-scenario.feature new file mode 100644 index 0000000..5524355 --- /dev/null +++ b/gherkin-checks/src/test/resources/checks/one-single-when-per-scenario.feature @@ -0,0 +1,20 @@ +Feature: My feature One single when step per scenario + Blabla... + + Scenario Outline: Scenario 1 - One single when step per scenario + Given Blabla given... + # Noncompliant [[sc=5;ec=33;secondary=+1]] {{Merge all the When steps or split the scenario up in multiple scenarios.}} + When Blabla when... + And Blabla when and blabla when + Then Blabla then... + Examples: + | number | + | 1 | + | 2 | + + Scenario: Scenario 2 - One single when step per scenario + Given Blabla given... + # Noncompliant [[sc=5;ec=24;secondary=+1]] {{Merge all the When steps or split the scenario up in multiple scenarios.}} + When Blabla when... + When Blabla when and blabla when... + Then Blabla then... diff --git a/its/ruling/projects/custom/one-single-when-per-scenario.feature b/its/ruling/projects/custom/one-single-when-per-scenario.feature new file mode 100644 index 0000000..5524355 --- /dev/null +++ b/its/ruling/projects/custom/one-single-when-per-scenario.feature @@ -0,0 +1,20 @@ +Feature: My feature One single when step per scenario + Blabla... + + Scenario Outline: Scenario 1 - One single when step per scenario + Given Blabla given... + # Noncompliant [[sc=5;ec=33;secondary=+1]] {{Merge all the When steps or split the scenario up in multiple scenarios.}} + When Blabla when... + And Blabla when and blabla when + Then Blabla then... + Examples: + | number | + | 1 | + | 2 | + + Scenario: Scenario 2 - One single when step per scenario + Given Blabla given... + # Noncompliant [[sc=5;ec=24;secondary=+1]] {{Merge all the When steps or split the scenario up in multiple scenarios.}} + When Blabla when... + When Blabla when and blabla when... + Then Blabla then... diff --git a/its/ruling/tests/src/test/expected/gherkin-one-single-when-per-scenario.json b/its/ruling/tests/src/test/expected/gherkin-one-single-when-per-scenario.json new file mode 100644 index 0000000..a5ba79e --- /dev/null +++ b/its/ruling/tests/src/test/expected/gherkin-one-single-when-per-scenario.json @@ -0,0 +1,21 @@ +{ +'project:custom/duplicated-steps/duplicates-in-background-and-scenarios.feature':[ +19, +], +'project:custom/duplicated-steps/duplicates-in-scenarios.feature':[ +17, +], +'project:custom/one-single-when-per-scenario.feature':[ +7, +18, +], +'project:custom/steps-right-order.feature':[ +6, +], +'project:custom/use-and-but.feature':[ +25, +], +'project:custom/when-step-regular-expression.feature':[ +7, +], +} diff --git a/its/ruling/tests/src/test/expected/gherkin-use-and-but.json b/its/ruling/tests/src/test/expected/gherkin-use-and-but.json index 4ce90ed..800fdec 100644 --- a/its/ruling/tests/src/test/expected/gherkin-use-and-but.json +++ b/its/ruling/tests/src/test/expected/gherkin-use-and-but.json @@ -1,4 +1,7 @@ { +'project:custom/one-single-when-per-scenario.feature':[ +19, +], 'project:custom/use-and-but.feature':[ 7, 14, diff --git a/sonar-gherkin-plugin/src/main/java/org/sonar/plugins/gherkin/GherkinRulesDefinition.java b/sonar-gherkin-plugin/src/main/java/org/sonar/plugins/gherkin/GherkinRulesDefinition.java index 2c53a90..52062de 100644 --- a/sonar-gherkin-plugin/src/main/java/org/sonar/plugins/gherkin/GherkinRulesDefinition.java +++ b/sonar-gherkin-plugin/src/main/java/org/sonar/plugins/gherkin/GherkinRulesDefinition.java @@ -70,6 +70,7 @@ public static Collection getChecks() { NoScenarioCheck.class, NoStepCheck.class, NoTagExamplesCheck.class, + OneSingleWhenPerScenarioCheck.class, OnlyGivenStepsInBackgroundCheck.class, ParsingErrorCheck.class, SpellingCheck.class, diff --git a/sonar-gherkin-plugin/src/test/java/org/sonar/plugins/gherkin/GherkinProfileTest.java b/sonar-gherkin-plugin/src/test/java/org/sonar/plugins/gherkin/GherkinProfileTest.java index db9d059..330a867 100644 --- a/sonar-gherkin-plugin/src/test/java/org/sonar/plugins/gherkin/GherkinProfileTest.java +++ b/sonar-gherkin-plugin/src/test/java/org/sonar/plugins/gherkin/GherkinProfileTest.java @@ -42,7 +42,7 @@ public void should_create_sonarqube_way_profile() { assertThat(profile.getName()).isEqualTo("SonarQube Way"); assertThat(profile.getLanguage()).isEqualTo("gherkin"); - assertThat(profile.getActiveRulesByRepository("gherkin")).hasSize(35); + assertThat(profile.getActiveRulesByRepository("gherkin")).hasSize(36); assertThat(validation.hasErrors()).isFalse(); } diff --git a/sonar-gherkin-plugin/src/test/java/org/sonar/plugins/gherkin/GherkinRulesDefinitionTest.java b/sonar-gherkin-plugin/src/test/java/org/sonar/plugins/gherkin/GherkinRulesDefinitionTest.java index c47b1c8..5289c6b 100644 --- a/sonar-gherkin-plugin/src/test/java/org/sonar/plugins/gherkin/GherkinRulesDefinitionTest.java +++ b/sonar-gherkin-plugin/src/test/java/org/sonar/plugins/gherkin/GherkinRulesDefinitionTest.java @@ -37,7 +37,7 @@ public void test() { assertThat(repository.name()).isEqualTo("SonarQube"); assertThat(repository.language()).isEqualTo("gherkin"); - assertThat(repository.rules()).hasSize(44); + assertThat(repository.rules()).hasSize(45); RulesDefinition.Rule lineLengthRule = repository.rule(TabCharacterCheck.class.getAnnotation(Rule.class).key()); assertThat(lineLengthRule).isNotNull();