From 9fb8406b768706fb5e34de38276ec24cad13561d Mon Sep 17 00:00:00 2001 From: Jake Hotson Date: Fri, 13 Jun 2025 00:05:34 +0100 Subject: [PATCH 1/2] [TASK] Add unit tests for `RuleSet::setRules` Also rename a data provider to indicate its (now) more generic purpose. --- tests/Unit/RuleSet/RuleSetTest.php | 168 +++++++++++++++++++++++++++-- 1 file changed, 161 insertions(+), 7 deletions(-) diff --git a/tests/Unit/RuleSet/RuleSetTest.php b/tests/Unit/RuleSet/RuleSetTest.php index 3c9f1bdb..2daa1e4c 100644 --- a/tests/Unit/RuleSet/RuleSetTest.php +++ b/tests/Unit/RuleSet/RuleSetTest.php @@ -54,7 +54,7 @@ public function implementsRuleContainer(): void /** * @return array}> */ - public static function providePropertyNamesToBeSetInitially(): array + public static function providePropertyNames(): array { return [ 'no properties' => [[]], @@ -81,7 +81,7 @@ public static function provideAnotherPropertyName(): array */ public static function provideInitialPropertyNamesAndAnotherPropertyName(): DataProvider { - return DataProvider::cross(self::providePropertyNamesToBeSetInitially(), self::provideAnotherPropertyName()); + return DataProvider::cross(self::providePropertyNames(), self::provideAnotherPropertyName()); } /** @@ -313,7 +313,7 @@ public function addRuleWithCompletePositionWithoutSiblingPreservesPosition( */ public static function provideInitialPropertyNamesAndIndexOfOne(): array { - $initialPropertyNamesSets = self::providePropertyNamesToBeSetInitially(); + $initialPropertyNamesSets = self::providePropertyNames(); // Provide sets with each possible index for the initially set `Rule`s. $initialPropertyNamesAndIndexSets = []; @@ -719,7 +719,7 @@ public function removeMatchingRulesWithPropertyNamePrefixKeepsOtherRules( * * @param list $propertyNamesToRemove * - * @dataProvider providePropertyNamesToBeSetInitially + * @dataProvider providePropertyNames */ public function removeAllRulesRemovesAllRules(array $propertyNamesToRemove): void { @@ -730,16 +730,170 @@ public function removeAllRulesRemovesAllRules(array $propertyNamesToRemove): voi self::assertSame([], $this->subject->getRules()); } + /** + * @test + * + * @param list $propertyNamesToSet + * + * @dataProvider providePropertyNames + */ + public function setRulesOnVirginSetsRules(array $propertyNamesToSet): void + { + $rulesToSet = self::createRulesFromPropertyNames($propertyNamesToSet); + + $this->subject->setRules($rulesToSet); + + self::assertArrayHasSameValues($rulesToSet, $this->subject->getRules()); + } + + /** + * @return DataProvider, 1: list}> + */ + public static function provideInitialPropertyNamesAndPropertyNamesToSet(): DataProvider + { + return DataProvider::cross(self::providePropertyNames(), self::providePropertyNames()); + } + + /** + * @test + * + * @param list $initialPropertyNames + * @param list $propertyNamesToSet + * + * @dataProvider provideInitialPropertyNamesAndPropertyNamesToSet + */ + public function setRulesReplacesRules(array $initialPropertyNames, array $propertyNamesToSet): void + { + $rulesToSet = self::createRulesFromPropertyNames($propertyNamesToSet); + $this->setRulesFromPropertyNames($initialPropertyNames); + + $this->subject->setRules($rulesToSet); + + self::assertArrayHasSameValues($rulesToSet, $this->subject->getRules()); + } + + /** + * @test + */ + public function setRulesWithRuleWithoutPositionSetsValidLineNumber(): void + { + $ruleToSet = new Rule('color'); + + $this->subject->setRules([$ruleToSet]); + + self::assertIsInt($ruleToSet->getLineNumber(), 'line number not set'); + self::assertGreaterThanOrEqual(1, $ruleToSet->getLineNumber(), 'line number not valid'); + } + + /** + * @test + */ + public function setRulesWithRuleWithoutPositionSetsValidColumnNumber(): void + { + $ruleToSet = new Rule('color'); + + $this->subject->setRules([$ruleToSet]); + + self::assertIsInt($ruleToSet->getColumnNumber(), 'column number not set'); + self::assertGreaterThanOrEqual(0, $ruleToSet->getColumnNumber(), 'column number not valid'); + } + + /** + * @test + */ + public function setRulesWithRuleWithOnlyLineNumberSetsColumnNumber(): void + { + $ruleToSet = new Rule('color'); + $ruleToSet->setPosition(42); + + $this->subject->setRules([$ruleToSet]); + + self::assertIsInt($ruleToSet->getColumnNumber(), 'column number not set'); + self::assertGreaterThanOrEqual(0, $ruleToSet->getColumnNumber(), 'column number not valid'); + } + + /** + * @test + */ + public function setRulesWithRuleWithOnlyLineNumberPreservesLineNumber(): void + { + $ruleToSet = new Rule('color'); + $ruleToSet->setPosition(42); + + $this->subject->setRules([$ruleToSet]); + + self::assertSame(42, $ruleToSet->getLineNumber(), 'line number not preserved'); + } + + /** + * @test + */ + public function setRulesWithRuleWithOnlyColumnNumberSetsLineNumber(): void + { + $ruleToSet = new Rule('color'); + $ruleToSet->setPosition(null, 42); + + $this->subject->setRules([$ruleToSet]); + + self::assertIsInt($ruleToSet->getLineNumber(), 'line number not set'); + self::assertGreaterThanOrEqual(1, $ruleToSet->getLineNumber(), 'line number not valid'); + } + + /** + * @test + */ + public function setRulesWithRuleWithOnlyColumnNumberPreservesColumnNumber(): void + { + $ruleToSet = new Rule('color'); + $ruleToSet->setPosition(null, 42); + + $this->subject->setRules([$ruleToSet]); + + self::assertSame(42, $ruleToSet->getColumnNumber(), 'column number not preserved'); + } + + /** + * @test + */ + public function setRulesWithRuleWithCompletePositionPreservesPosition(): void + { + $ruleToSet = new Rule('color'); + $ruleToSet->setPosition(42, 64); + + $this->subject->setRules([$ruleToSet]); + + self::assertSame(42, $ruleToSet->getLineNumber(), 'line number not preserved'); + self::assertSame(64, $ruleToSet->getColumnNumber(), 'column number not preserved'); + } + /** * @param list $propertyNames */ private function setRulesFromPropertyNames(array $propertyNames): void { - $this->subject->setRules(\array_map( - static function (string $propertyName): Rule { + $this->subject->setRules(self::createRulesFromPropertyNames($propertyNames)); + } + + /** + * @param list $propertyNames + * + * @return list + */ + private static function createRulesFromPropertyNames(array $propertyNames): array + { + return \array_map( + function (string $propertyName): Rule { return new Rule($propertyName); }, $propertyNames - )); + ); + } + + private static function assertArrayHasSameValues(array $expected, array $actual, string $message = ''): void + { + self::assertCount(\count($expected), $actual, $message); + foreach ($expected as $expectedElement) { + self::assertContains($expectedElement, $actual, $message); + } } } From e28d0c32c864459cb123c2e760e68b7f75f28cb6 Mon Sep 17 00:00:00 2001 From: Jake Hotson Date: Fri, 13 Jun 2025 18:37:37 +0100 Subject: [PATCH 2/2] Change suggested in review --- tests/Unit/RuleSet/RuleSetTest.php | 14 +++----------- 1 file changed, 3 insertions(+), 11 deletions(-) diff --git a/tests/Unit/RuleSet/RuleSetTest.php b/tests/Unit/RuleSet/RuleSetTest.php index 2daa1e4c..f36db888 100644 --- a/tests/Unit/RuleSet/RuleSetTest.php +++ b/tests/Unit/RuleSet/RuleSetTest.php @@ -737,13 +737,13 @@ public function removeAllRulesRemovesAllRules(array $propertyNamesToRemove): voi * * @dataProvider providePropertyNames */ - public function setRulesOnVirginSetsRules(array $propertyNamesToSet): void + public function setRulesOnVirginSetsRulesWithoutPositionInOrder(array $propertyNamesToSet): void { $rulesToSet = self::createRulesFromPropertyNames($propertyNamesToSet); $this->subject->setRules($rulesToSet); - self::assertArrayHasSameValues($rulesToSet, $this->subject->getRules()); + self::assertSame($rulesToSet, $this->subject->getRules()); } /** @@ -769,7 +769,7 @@ public function setRulesReplacesRules(array $initialPropertyNames, array $proper $this->subject->setRules($rulesToSet); - self::assertArrayHasSameValues($rulesToSet, $this->subject->getRules()); + self::assertSame($rulesToSet, $this->subject->getRules()); } /** @@ -888,12 +888,4 @@ function (string $propertyName): Rule { $propertyNames ); } - - private static function assertArrayHasSameValues(array $expected, array $actual, string $message = ''): void - { - self::assertCount(\count($expected), $actual, $message); - foreach ($expected as $expectedElement) { - self::assertContains($expectedElement, $actual, $message); - } - } }