Skip to content

Commit 632218e

Browse files
committed
[BUGFIX] Don't return objects from data providers
The same objects may be provided to multiple tests. If a test manipulates an object, it will no longer be in the initial expected state for other tests. This is the backport of #1260.
1 parent 009b6ca commit 632218e

File tree

1 file changed

+53
-39
lines changed

1 file changed

+53
-39
lines changed

tests/Unit/RuleSet/RuleSetTest.php

Lines changed: 53 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
use PHPUnit\Framework\TestCase;
88
use Sabberworm\CSS\CSSElement;
99
use Sabberworm\CSS\Rule\Rule;
10+
use Sabberworm\CSS\RuleSet\RuleSet;
1011
use Sabberworm\CSS\Tests\Unit\RuleSet\Fixtures\ConcreteRuleSet;
1112

1213
/**
@@ -27,38 +28,38 @@ public function implementsCSSElement()
2728
}
2829

2930
/**
30-
* @return array<string, array{0: list<Rule>, 1: string, 2: list<string>}>
31+
* @return array<string, array{0: list<string>, 1: string, 2: list<string>}>
3132
*/
32-
public static function provideRulesAndPropertyNameToRemoveAndExpectedRemainingPropertyNames()
33+
public static function providePropertyNamesAndPropertyNameToRemoveAndExpectedRemainingPropertyNames()
3334
{
3435
return [
3536
'removing single rule' => [
36-
[new Rule('color')],
37+
['color'],
3738
'color',
3839
[],
3940
],
4041
'removing first rule' => [
41-
[new Rule('color'), new Rule('display')],
42+
['color', 'display'],
4243
'color',
4344
['display'],
4445
],
4546
'removing last rule' => [
46-
[new Rule('color'), new Rule('display')],
47+
['color', 'display'],
4748
'display',
4849
['color'],
4950
],
5051
'removing middle rule' => [
51-
[new Rule('color'), new Rule('display'), new Rule('width')],
52+
['color', 'display', 'width'],
5253
'display',
5354
['color', 'width'],
5455
],
5556
'removing multiple rules' => [
56-
[new Rule('color'), new Rule('color')],
57+
['color', 'color'],
5758
'color',
5859
[],
5960
],
6061
'removing multiple rules with another kept' => [
61-
[new Rule('color'), new Rule('color'), new Rule('display')],
62+
['color', 'color', 'display'],
6263
'color',
6364
['display'],
6465
],
@@ -68,7 +69,7 @@ public static function provideRulesAndPropertyNameToRemoveAndExpectedRemainingPr
6869
[],
6970
],
7071
'removing nonexistent rule from nonempty list' => [
71-
[new Rule('color'), new Rule('display')],
72+
['color', 'display'],
7273
'width',
7374
['color', 'display'],
7475
],
@@ -78,62 +79,62 @@ public static function provideRulesAndPropertyNameToRemoveAndExpectedRemainingPr
7879
/**
7980
* @test
8081
*
81-
* @param list<Rule> $rules
82-
* @param string $propertyName
82+
* @param list<string> $initialPropertyNames
83+
* @param string $propertyNameToRemove
8384
* @param list<string> $expectedRemainingPropertyNames
8485
*
85-
* @dataProvider provideRulesAndPropertyNameToRemoveAndExpectedRemainingPropertyNames
86+
* @dataProvider providePropertyNamesAndPropertyNameToRemoveAndExpectedRemainingPropertyNames
8687
*/
8788
public function removeMatchingRulesRemovesRulesByPropertyNameAndKeepsOthers(
88-
array $rules,
89-
$propertyName,
89+
array $initialPropertyNames,
90+
$propertyNameToRemove,
9091
array $expectedRemainingPropertyNames
9192
) {
9293
$subject = new ConcreteRuleSet();
93-
$subject->setRules($rules);
94+
self::setRulesFromPropertyNames($subject, $initialPropertyNames);
9495

95-
$subject->removeMatchingRules($propertyName);
96+
$subject->removeMatchingRules($propertyNameToRemove);
9697

9798
$remainingRules = $subject->getRulesAssoc();
98-
self::assertArrayNotHasKey($propertyName, $remainingRules);
99+
self::assertArrayNotHasKey($propertyNameToRemove, $remainingRules);
99100
foreach ($expectedRemainingPropertyNames as $expectedPropertyName) {
100101
self::assertArrayHasKey($expectedPropertyName, $remainingRules);
101102
}
102103
}
103104

104105
/**
105-
* @return array<string, array{0: list<Rule>, 1: string, 2: list<string>}>
106+
* @return array<string, array{0: list<string>, 1: string, 2: list<string>}>
106107
*/
107-
public static function provideRulesAndPropertyNamePrefixToRemoveAndExpectedRemainingPropertyNames()
108+
public static function providePropertyNamesAndPropertyNamePrefixToRemoveAndExpectedRemainingPropertyNames()
108109
{
109110
return [
110111
'removing shorthand rule' => [
111-
[new Rule('font')],
112+
['font'],
112113
'font',
113114
[],
114115
],
115116
'removing longhand rule' => [
116-
[new Rule('font-size')],
117+
['font-size'],
117118
'font',
118119
[],
119120
],
120121
'removing shorthand and longhand rule' => [
121-
[new Rule('font'), new Rule('font-size')],
122+
['font', 'font-size'],
122123
'font',
123124
[],
124125
],
125126
'removing shorthand rule with another kept' => [
126-
[new Rule('font'), new Rule('color')],
127+
['font', 'color'],
127128
'font',
128129
['color'],
129130
],
130131
'removing longhand rule with another kept' => [
131-
[new Rule('font-size'), new Rule('color')],
132+
['font-size', 'color'],
132133
'font',
133134
['color'],
134135
],
135136
'keeping other rules whose property names begin with the same characters' => [
136-
[new Rule('contain'), new Rule('container'), new Rule('container-type')],
137+
['contain', 'container', 'container-type'],
137138
'contain',
138139
['container', 'container-type'],
139140
],
@@ -143,20 +144,20 @@ public static function provideRulesAndPropertyNamePrefixToRemoveAndExpectedRemai
143144
/**
144145
* @test
145146
*
146-
* @param list<Rule> $rules
147+
* @param list<string> $initialPropertyNames
147148
* @param string $propertyNamePrefix
148149
* @param list<string> $expectedRemainingPropertyNames
149150
*
150-
* @dataProvider provideRulesAndPropertyNamePrefixToRemoveAndExpectedRemainingPropertyNames
151+
* @dataProvider providePropertyNamesAndPropertyNamePrefixToRemoveAndExpectedRemainingPropertyNames
151152
*/
152153
public function removeMatchingRulesRemovesRulesByPropertyNamePrefixAndKeepsOthers(
153-
array $rules,
154+
array $initialPropertyNames,
154155
$propertyNamePrefix,
155156
array $expectedRemainingPropertyNames
156157
) {
157158
$propertyNamePrefixWithHyphen = $propertyNamePrefix . '-';
158159
$subject = new ConcreteRuleSet();
159-
$subject->setRules($rules);
160+
self::setRulesFromPropertyNames($subject, $initialPropertyNames);
160161

161162
$subject->removeMatchingRules($propertyNamePrefixWithHyphen);
162163

@@ -171,32 +172,45 @@ public function removeMatchingRulesRemovesRulesByPropertyNamePrefixAndKeepsOther
171172
}
172173

173174
/**
174-
* @return array<string, array{0: list<Rule>}>
175+
* @return array<string, array{0: list<string>}>
175176
*/
176-
public static function provideRulesToRemove()
177+
public static function providePropertyNamesToRemove()
177178
{
178179
return [
179-
'no rules' => [[]],
180-
'one rule' => [[new Rule('color')]],
181-
'two rules for different properties' => [[new Rule('color'), new Rule('display')]],
182-
'two rules for the same property' => [[new Rule('color'), new Rule('color')]],
180+
'no properties' => [[]],
181+
'one property' => [['color']],
182+
'two different properties' => [['color', 'display']],
183+
'two of the same property' => [['color', 'color']],
183184
];
184185
}
185186

186187
/**
187188
* @test
188189
*
189-
* @param list<Rule> $rules
190+
* @param list<string> $propertyNamesToRemove
190191
*
191-
* @dataProvider provideRulesToRemove
192+
* @dataProvider providePropertyNamesToRemove
192193
*/
193-
public function removeAllRulesRemovesAllRules(array $rules)
194+
public function removeAllRulesRemovesAllRules(array $propertyNamesToRemove)
194195
{
195196
$subject = new ConcreteRuleSet();
196-
$subject->setRules($rules);
197+
self::setRulesFromPropertyNames($subject, $propertyNamesToRemove);
197198

198199
$subject->removeAllRules();
199200

200201
self::assertSame([], $subject->getRules());
201202
}
203+
204+
/**
205+
* @param list<string> $propertyNames
206+
*/
207+
private static function setRulesFromPropertyNames(RuleSet $subject, array $propertyNames)
208+
{
209+
$subject->setRules(\array_map(
210+
function (string $propertyName) {
211+
return new Rule($propertyName);
212+
},
213+
$propertyNames
214+
));
215+
}
202216
}

0 commit comments

Comments
 (0)