Skip to content

[BUGFIX] Don't return objects from data providers #1267

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
Jun 3, 2025
Merged

[BUGFIX] Don't return objects from data providers #1267

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
92 changes: 53 additions & 39 deletions tests/Unit/RuleSet/RuleSetTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
use PHPUnit\Framework\TestCase;
use Sabberworm\CSS\CSSElement;
use Sabberworm\CSS\Rule\Rule;
use Sabberworm\CSS\RuleSet\RuleSet;
use Sabberworm\CSS\Tests\Unit\RuleSet\Fixtures\ConcreteRuleSet;

/**
Expand All @@ -27,38 +28,38 @@ public function implementsCSSElement()
}

/**
* @return array<string, array{0: list<Rule>, 1: string, 2: list<string>}>
* @return array<string, array{0: list<string>, 1: string, 2: list<string>}>
*/
public static function provideRulesAndPropertyNameToRemoveAndExpectedRemainingPropertyNames()
public static function providePropertyNamesAndPropertyNameToRemoveAndExpectedRemainingPropertyNames()
{
return [
'removing single rule' => [
[new Rule('color')],
['color'],
'color',
[],
],
'removing first rule' => [
[new Rule('color'), new Rule('display')],
['color', 'display'],
'color',
['display'],
],
'removing last rule' => [
[new Rule('color'), new Rule('display')],
['color', 'display'],
'display',
['color'],
],
'removing middle rule' => [
[new Rule('color'), new Rule('display'), new Rule('width')],
['color', 'display', 'width'],
'display',
['color', 'width'],
],
'removing multiple rules' => [
[new Rule('color'), new Rule('color')],
['color', 'color'],
'color',
[],
],
'removing multiple rules with another kept' => [
[new Rule('color'), new Rule('color'), new Rule('display')],
['color', 'color', 'display'],
'color',
['display'],
],
Expand All @@ -68,7 +69,7 @@ public static function provideRulesAndPropertyNameToRemoveAndExpectedRemainingPr
[],
],
'removing nonexistent rule from nonempty list' => [
[new Rule('color'), new Rule('display')],
['color', 'display'],
'width',
['color', 'display'],
],
Expand All @@ -78,62 +79,62 @@ public static function provideRulesAndPropertyNameToRemoveAndExpectedRemainingPr
/**
* @test
*
* @param list<Rule> $rules
* @param string $propertyName
* @param list<string> $initialPropertyNames
* @param string $propertyNameToRemove
* @param list<string> $expectedRemainingPropertyNames
*
* @dataProvider provideRulesAndPropertyNameToRemoveAndExpectedRemainingPropertyNames
* @dataProvider providePropertyNamesAndPropertyNameToRemoveAndExpectedRemainingPropertyNames
*/
public function removeMatchingRulesRemovesRulesByPropertyNameAndKeepsOthers(
array $rules,
$propertyName,
array $initialPropertyNames,
$propertyNameToRemove,
array $expectedRemainingPropertyNames
) {
$subject = new ConcreteRuleSet();
$subject->setRules($rules);
self::setRulesFromPropertyNames($subject, $initialPropertyNames);

$subject->removeMatchingRules($propertyName);
$subject->removeMatchingRules($propertyNameToRemove);

$remainingRules = $subject->getRulesAssoc();
self::assertArrayNotHasKey($propertyName, $remainingRules);
self::assertArrayNotHasKey($propertyNameToRemove, $remainingRules);
foreach ($expectedRemainingPropertyNames as $expectedPropertyName) {
self::assertArrayHasKey($expectedPropertyName, $remainingRules);
}
}

/**
* @return array<string, array{0: list<Rule>, 1: string, 2: list<string>}>
* @return array<string, array{0: list<string>, 1: string, 2: list<string>}>
*/
public static function provideRulesAndPropertyNamePrefixToRemoveAndExpectedRemainingPropertyNames()
public static function providePropertyNamesAndPropertyNamePrefixToRemoveAndExpectedRemainingPropertyNames()
{
return [
'removing shorthand rule' => [
[new Rule('font')],
['font'],
'font',
[],
],
'removing longhand rule' => [
[new Rule('font-size')],
['font-size'],
'font',
[],
],
'removing shorthand and longhand rule' => [
[new Rule('font'), new Rule('font-size')],
['font', 'font-size'],
'font',
[],
],
'removing shorthand rule with another kept' => [
[new Rule('font'), new Rule('color')],
['font', 'color'],
'font',
['color'],
],
'removing longhand rule with another kept' => [
[new Rule('font-size'), new Rule('color')],
['font-size', 'color'],
'font',
['color'],
],
'keeping other rules whose property names begin with the same characters' => [
[new Rule('contain'), new Rule('container'), new Rule('container-type')],
['contain', 'container', 'container-type'],
'contain',
['container', 'container-type'],
],
Expand All @@ -143,20 +144,20 @@ public static function provideRulesAndPropertyNamePrefixToRemoveAndExpectedRemai
/**
* @test
*
* @param list<Rule> $rules
* @param list<string> $initialPropertyNames
* @param string $propertyNamePrefix
* @param list<string> $expectedRemainingPropertyNames
*
* @dataProvider provideRulesAndPropertyNamePrefixToRemoveAndExpectedRemainingPropertyNames
* @dataProvider providePropertyNamesAndPropertyNamePrefixToRemoveAndExpectedRemainingPropertyNames
*/
public function removeMatchingRulesRemovesRulesByPropertyNamePrefixAndKeepsOthers(
array $rules,
array $initialPropertyNames,
$propertyNamePrefix,
array $expectedRemainingPropertyNames
) {
$propertyNamePrefixWithHyphen = $propertyNamePrefix . '-';
$subject = new ConcreteRuleSet();
$subject->setRules($rules);
self::setRulesFromPropertyNames($subject, $initialPropertyNames);

$subject->removeMatchingRules($propertyNamePrefixWithHyphen);

Expand All @@ -171,32 +172,45 @@ public function removeMatchingRulesRemovesRulesByPropertyNamePrefixAndKeepsOther
}

/**
* @return array<string, array{0: list<Rule>}>
* @return array<string, array{0: list<string>}>
*/
public static function provideRulesToRemove()
public static function providePropertyNamesToRemove()
{
return [
'no rules' => [[]],
'one rule' => [[new Rule('color')]],
'two rules for different properties' => [[new Rule('color'), new Rule('display')]],
'two rules for the same property' => [[new Rule('color'), new Rule('color')]],
'no properties' => [[]],
'one property' => [['color']],
'two different properties' => [['color', 'display']],
'two of the same property' => [['color', 'color']],
];
}

/**
* @test
*
* @param list<Rule> $rules
* @param list<string> $propertyNamesToRemove
*
* @dataProvider provideRulesToRemove
* @dataProvider providePropertyNamesToRemove
*/
public function removeAllRulesRemovesAllRules(array $rules)
public function removeAllRulesRemovesAllRules(array $propertyNamesToRemove)
{
$subject = new ConcreteRuleSet();
$subject->setRules($rules);
self::setRulesFromPropertyNames($subject, $propertyNamesToRemove);

$subject->removeAllRules();

self::assertSame([], $subject->getRules());
}

/**
* @param list<string> $propertyNames
*/
private static function setRulesFromPropertyNames(RuleSet $subject, array $propertyNames)
{
$subject->setRules(\array_map(
function ($propertyName) {
return new Rule($propertyName);
},
$propertyNames
));
}
}