diff --git a/README.md b/README.md index a52bf9d31..879f3362e 100644 --- a/README.md +++ b/README.md @@ -108,6 +108,24 @@ This rule spots definitions that are no longer needed, so you can remove them.
+### 4. Find duplicate scenario names (`duplicate-scenario-names`) + +In Behat, each scenario should have a unique name to ensure clarity and avoid confusion during test execution and later debugging. This rule identifies scenarios that share the same name within your feature files: + +```yaml +Feature: User Authentication + + Scenario: User logs in successfully + When the user enters valid credentials + Then login should be successful + + Scenario: User logs in successfully + When the user enters invalid credentials + Then an error message should be displayed +``` + +
+ *Protip*: Add this command to your CI, to get instant feedback of any changes in every pull-request. That's it! diff --git a/src/Analyzer/DuplicatedScenarioNamesAnalyzer.php b/src/Analyzer/DuplicatedScenarioNamesAnalyzer.php new file mode 100644 index 000000000..ad69fb98b --- /dev/null +++ b/src/Analyzer/DuplicatedScenarioNamesAnalyzer.php @@ -0,0 +1,39 @@ +.*?)\n#'; + + /** + * @param SplFileInfo[] $featureFiles + * @return array + */ + public function analyze(array $featureFiles): array + { + $scenarioNamesToFiles = []; + + foreach ($featureFiles as $featureFile) { + // match Scenario: "" + $matches = Regex::matchAll($featureFile->getContents(), self::SCENARIO_NAME_REGEX); + + foreach ($matches as $match) { + $scenarioName = $match['name']; + $scenarioNamesToFiles[$scenarioName][] = $featureFile->getRealPath(); + } + } + + return array_filter($scenarioNamesToFiles, function (array $files): bool { + return count($files) > 1; + }); + } +} diff --git a/src/Analyzer/UnusedDefinitionsAnalyzer.php b/src/Analyzer/UnusedDefinitionsAnalyzer.php index 44f61579a..7b91dc447 100644 --- a/src/Analyzer/UnusedDefinitionsAnalyzer.php +++ b/src/Analyzer/UnusedDefinitionsAnalyzer.php @@ -28,12 +28,11 @@ public function __construct( } /** - * @param SplFileInfo[] $contextFiles * @param SplFileInfo[] $featureFiles * * @return AbstractPattern[] */ - public function analyse(array $contextFiles, array $featureFiles, PatternCollection $patternCollection): array + public function analyse(array $featureFiles, PatternCollection $patternCollection): array { foreach ($featureFiles as $featureFile) { Assert::endsWith($featureFile->getFilename(), '.feature'); diff --git a/src/Command/AnalyzeCommand.php b/src/Command/AnalyzeCommand.php index c7b607419..8af44e785 100644 --- a/src/Command/AnalyzeCommand.php +++ b/src/Command/AnalyzeCommand.php @@ -43,8 +43,9 @@ public function run(?string $projectDirectory = null, array $skip = []): int $contextFileInfos = BehatMetafilesFinder::findContextFiles([$projectDirectory]); if ($contextFileInfos === []) { $this->outputPrinter->redBackground(sprintf( - 'No *.Context files found in "%s". Please provide correct directory', - $projectDirectory + 'No *.Context files found in "%s".%sPlease provide correct directory', + $projectDirectory, + PHP_EOL )); return ExitCode::ERROR; @@ -53,8 +54,9 @@ public function run(?string $projectDirectory = null, array $skip = []): int $featureFileInfos = BehatMetafilesFinder::findFeatureFiles([$projectDirectory]); if ($featureFileInfos === []) { $this->outputPrinter->redBackground(sprintf( - 'No *.feature files found in "%s". Please provide correct directory', - $projectDirectory + 'No *.feature files found in "%s".%sPlease provide correct directory', + $projectDirectory, + PHP_EOL )); return ExitCode::ERROR; @@ -79,7 +81,7 @@ public function run(?string $projectDirectory = null, array $skip = []): int /** @var RuleError[] $allRuleErrors */ $allRuleErrors = []; foreach ($this->rulesRegistry->all() as $rule) { - if ($skip !== [] && in_array($rule->getIdentifier(), $skip, true)) { + if (in_array($rule->getIdentifier(), $skip, true)) { $this->outputPrinter->writeln(sprintf('Skipping "%s" rule', $rule->getIdentifier())); $this->outputPrinter->newLine(); continue; diff --git a/src/Enum/RuleIdentifier.php b/src/Enum/RuleIdentifier.php index e8f17766f..c508a6552 100644 --- a/src/Enum/RuleIdentifier.php +++ b/src/Enum/RuleIdentifier.php @@ -8,6 +8,8 @@ final class RuleIdentifier { public const string DUPLICATED_CONTENTS = 'duplicated-contents'; + public const string DUPLICATED_SCENARIO_NAMES = 'duplicated-scenario-names'; + public const string DUPLICATED_PATTERNS = 'duplicated-patterns'; public const string UNUSED_DEFINITIONS = 'unused-definitions'; diff --git a/src/Rule/DuplicatedScenarioNamesRule.php b/src/Rule/DuplicatedScenarioNamesRule.php new file mode 100644 index 000000000..209b76e8c --- /dev/null +++ b/src/Rule/DuplicatedScenarioNamesRule.php @@ -0,0 +1,53 @@ +duplicatedScenarioNamesAnalyzer->analyze($featureFiles); + + $ruleErrors = []; + foreach ($scenarioNamesToFiles as $scenarioName => $files) { + // it can be used multiple times in single file + $uniqueFiles = array_unique($files); + $uniqueCount = count($uniqueFiles); + + $errorMessage = sprintf('Scenario name "%s" is duplicated %d-times', $scenarioName, $uniqueCount); + + $ruleErrors[] = new RuleError($errorMessage, $uniqueFiles, $this->getIdentifier()); + } + + return $ruleErrors; + } + + public function getIdentifier(): string + { + return RuleIdentifier::DUPLICATED_SCENARIO_NAMES; + } +} diff --git a/src/Rule/UnusedContextDefinitionsRule.php b/src/Rule/UnusedContextDefinitionsRule.php index 092eb3968..3aa7e110d 100644 --- a/src/Rule/UnusedContextDefinitionsRule.php +++ b/src/Rule/UnusedContextDefinitionsRule.php @@ -29,7 +29,7 @@ public function process( PatternCollection $patternCollection, string $projectDirectory ): array { - $unusedPatterns = $this->unusedDefinitionsAnalyzer->analyse($contextFiles, $featureFiles, $patternCollection); + $unusedPatterns = $this->unusedDefinitionsAnalyzer->analyse($featureFiles, $patternCollection); $ruleErrors = []; diff --git a/src/RulesRegistry.php b/src/RulesRegistry.php index 96e1577e3..1664cf966 100644 --- a/src/RulesRegistry.php +++ b/src/RulesRegistry.php @@ -17,7 +17,7 @@ public function __construct( ) { Assert::allObject($rules); Assert::allIsInstanceOf($rules, RuleInterface::class); - Assert::greaterThan(count($rules), 2); + Assert::greaterThan(count($rules), 3); } /** diff --git a/tests/Analyzer/DuplicatedScenarioNamesAnalyzer/DuplicatedScenarioNamesAnalyzerTest.php b/tests/Analyzer/DuplicatedScenarioNamesAnalyzer/DuplicatedScenarioNamesAnalyzerTest.php new file mode 100644 index 000000000..ec3d2087e --- /dev/null +++ b/tests/Analyzer/DuplicatedScenarioNamesAnalyzer/DuplicatedScenarioNamesAnalyzerTest.php @@ -0,0 +1,36 @@ +duplicatedScenarioNamesAnalyzer = $this->make(DuplicatedScenarioNamesAnalyzer::class); + } + + public function test(): void + { + $featureFiles = BehatMetafilesFinder::findFeatureFiles([__DIR__ . '/Fixture']); + $this->assertCount(2, $featureFiles); + + $duplicatedScenarioNamesToFiles = $this->duplicatedScenarioNamesAnalyzer->analyze($featureFiles); + + $this->assertCount(1, $duplicatedScenarioNamesToFiles); + $this->assertArrayHasKey('Same scenario name', $duplicatedScenarioNamesToFiles); + + $givenFiles = $duplicatedScenarioNamesToFiles['Same scenario name']; + + $this->assertSame([__DIR__ . '/Fixture/some.feature', __DIR__ . '/Fixture/another.feature'], $givenFiles); + } +} diff --git a/tests/Analyzer/DuplicatedScenarioNamesAnalyzer/Fixture/another.feature b/tests/Analyzer/DuplicatedScenarioNamesAnalyzer/Fixture/another.feature new file mode 100644 index 000000000..439738da4 --- /dev/null +++ b/tests/Analyzer/DuplicatedScenarioNamesAnalyzer/Fixture/another.feature @@ -0,0 +1,3 @@ +Feature: Some feature + + Scenario: Same scenario name diff --git a/tests/Analyzer/DuplicatedScenarioNamesAnalyzer/Fixture/some.feature b/tests/Analyzer/DuplicatedScenarioNamesAnalyzer/Fixture/some.feature new file mode 100644 index 000000000..c5db63751 --- /dev/null +++ b/tests/Analyzer/DuplicatedScenarioNamesAnalyzer/Fixture/some.feature @@ -0,0 +1,3 @@ +Feature: Another feature + + Scenario: Same scenario name diff --git a/tests/Analyzer/UnusedDefinitionsAnalyzer/UnusedDefinitionsAnalyzerTest.php b/tests/Analyzer/UnusedDefinitionsAnalyzer/UnusedDefinitionsAnalyzerTest.php index dea0f280a..366b272e2 100644 --- a/tests/Analyzer/UnusedDefinitionsAnalyzer/UnusedDefinitionsAnalyzerTest.php +++ b/tests/Analyzer/UnusedDefinitionsAnalyzer/UnusedDefinitionsAnalyzerTest.php @@ -34,11 +34,7 @@ public function testEverythingUsed(): void $patternCollection = $this->definitionPatternsExtractor->extract($contextFiles); - $unusedDefinitions = $this->unusedDefinitionsAnalyzer->analyse( - $contextFiles, - $featureFiles, - $patternCollection - ); + $unusedDefinitions = $this->unusedDefinitionsAnalyzer->analyse($featureFiles, $patternCollection); $this->assertCount(0, $unusedDefinitions); } @@ -53,7 +49,7 @@ public function testFoundPattern(): void $patternCollection = $this->definitionPatternsExtractor->extract($contextFiles); - $unusedPatterns = $this->unusedDefinitionsAnalyzer->analyse($contextFiles, $featureFiles, $patternCollection); + $unusedPatterns = $this->unusedDefinitionsAnalyzer->analyse($featureFiles, $patternCollection); $this->assertCount(1, $unusedPatterns); $this->assertContainsOnlyInstancesOf(AbstractPattern::class, $unusedPatterns);