Skip to content

Commit cbe3133

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.
1 parent 9ed1a4c commit cbe3133

File tree

1 file changed

+51
-38
lines changed

1 file changed

+51
-38
lines changed

tests/Unit/RuleSet/RuleSetTest.php

Lines changed: 51 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -51,38 +51,38 @@ public function implementsRuleContainer(): void
5151
}
5252

5353
/**
54-
* @return array<string, array{0: list<Rule>, 1: string, 2: list<string>}>
54+
* @return array<string, array{0: list<string>, 1: string, 2: list<string>}>
5555
*/
56-
public static function provideRulesAndPropertyNameToRemoveAndExpectedRemainingPropertyNames(): array
56+
public static function providePropertyNamesAndPropertyNameToRemoveAndExpectedRemainingPropertyNames(): array
5757
{
5858
return [
5959
'removing single rule' => [
60-
[new Rule('color')],
60+
['color'],
6161
'color',
6262
[],
6363
],
6464
'removing first rule' => [
65-
[new Rule('color'), new Rule('display')],
65+
['color', 'display'],
6666
'color',
6767
['display'],
6868
],
6969
'removing last rule' => [
70-
[new Rule('color'), new Rule('display')],
70+
['color', 'display'],
7171
'display',
7272
['color'],
7373
],
7474
'removing middle rule' => [
75-
[new Rule('color'), new Rule('display'), new Rule('width')],
75+
['color', 'display', 'width'],
7676
'display',
7777
['color', 'width'],
7878
],
7979
'removing multiple rules' => [
80-
[new Rule('color'), new Rule('color')],
80+
['color', 'color'],
8181
'color',
8282
[],
8383
],
8484
'removing multiple rules with another kept' => [
85-
[new Rule('color'), new Rule('color'), new Rule('display')],
85+
['color', 'color', 'display'],
8686
'color',
8787
['display'],
8888
],
@@ -92,7 +92,7 @@ public static function provideRulesAndPropertyNameToRemoveAndExpectedRemainingPr
9292
[],
9393
],
9494
'removing nonexistent rule from nonempty list' => [
95-
[new Rule('color'), new Rule('display')],
95+
['color', 'display'],
9696
'width',
9797
['color', 'display'],
9898
],
@@ -102,60 +102,60 @@ public static function provideRulesAndPropertyNameToRemoveAndExpectedRemainingPr
102102
/**
103103
* @test
104104
*
105-
* @param list<Rule> $rules
105+
* @param list<string> $initialPropertyNames
106106
* @param list<string> $expectedRemainingPropertyNames
107107
*
108-
* @dataProvider provideRulesAndPropertyNameToRemoveAndExpectedRemainingPropertyNames
108+
* @dataProvider providePropertyNamesAndPropertyNameToRemoveAndExpectedRemainingPropertyNames
109109
*/
110110
public function removeMatchingRulesRemovesRulesByPropertyNameAndKeepsOthers(
111-
array $rules,
112-
string $propertyName,
111+
array $initialPropertyNames,
112+
string $propertyNameToRemove,
113113
array $expectedRemainingPropertyNames
114114
): void {
115-
$this->subject->setRules($rules);
115+
$this->setRulesFromPropertyNames($initialPropertyNames);
116116

117-
$this->subject->removeMatchingRules($propertyName);
117+
$this->subject->removeMatchingRules($propertyNameToRemove);
118118

119119
$remainingRules = $this->subject->getRulesAssoc();
120-
self::assertArrayNotHasKey($propertyName, $remainingRules);
120+
self::assertArrayNotHasKey($propertyNameToRemove, $remainingRules);
121121
foreach ($expectedRemainingPropertyNames as $expectedPropertyName) {
122122
self::assertArrayHasKey($expectedPropertyName, $remainingRules);
123123
}
124124
}
125125

126126
/**
127-
* @return array<string, array{0: list<Rule>, 1: string, 2: list<string>}>
127+
* @return array<string, array{0: list<string>, 1: string, 2: list<string>}>
128128
*/
129-
public static function provideRulesAndPropertyNamePrefixToRemoveAndExpectedRemainingPropertyNames(): array
129+
public static function providePropertyNamesAndPropertyNamePrefixToRemoveAndExpectedRemainingPropertyNames(): array
130130
{
131131
return [
132132
'removing shorthand rule' => [
133-
[new Rule('font')],
133+
['font'],
134134
'font',
135135
[],
136136
],
137137
'removing longhand rule' => [
138-
[new Rule('font-size')],
138+
['font-size'],
139139
'font',
140140
[],
141141
],
142142
'removing shorthand and longhand rule' => [
143-
[new Rule('font'), new Rule('font-size')],
143+
['font', 'font-size'],
144144
'font',
145145
[],
146146
],
147147
'removing shorthand rule with another kept' => [
148-
[new Rule('font'), new Rule('color')],
148+
['font', 'color'],
149149
'font',
150150
['color'],
151151
],
152152
'removing longhand rule with another kept' => [
153-
[new Rule('font-size'), new Rule('color')],
153+
['font-size', 'color'],
154154
'font',
155155
['color'],
156156
],
157157
'keeping other rules whose property names begin with the same characters' => [
158-
[new Rule('contain'), new Rule('container'), new Rule('container-type')],
158+
['contain', 'container', 'container-type'],
159159
'contain',
160160
['container', 'container-type'],
161161
],
@@ -165,18 +165,18 @@ public static function provideRulesAndPropertyNamePrefixToRemoveAndExpectedRemai
165165
/**
166166
* @test
167167
*
168-
* @param list<Rule> $rules
168+
* @param list<string> $initialPropertyNames
169169
* @param list<string> $expectedRemainingPropertyNames
170170
*
171-
* @dataProvider provideRulesAndPropertyNamePrefixToRemoveAndExpectedRemainingPropertyNames
171+
* @dataProvider providePropertyNamesAndPropertyNamePrefixToRemoveAndExpectedRemainingPropertyNames
172172
*/
173173
public function removeMatchingRulesRemovesRulesByPropertyNamePrefixAndKeepsOthers(
174-
array $rules,
174+
array $initialPropertyNames,
175175
string $propertyNamePrefix,
176176
array $expectedRemainingPropertyNames
177177
): void {
178178
$propertyNamePrefixWithHyphen = $propertyNamePrefix . '-';
179-
$this->subject->setRules($rules);
179+
$this->setRulesFromPropertyNames($initialPropertyNames);
180180

181181
$this->subject->removeMatchingRules($propertyNamePrefixWithHyphen);
182182

@@ -191,31 +191,44 @@ public function removeMatchingRulesRemovesRulesByPropertyNamePrefixAndKeepsOther
191191
}
192192

193193
/**
194-
* @return array<string, array{0: list<Rule>}>
194+
* @return array<string, array{0: list<string>}>
195195
*/
196-
public static function provideRulesToRemove(): array
196+
public static function providePropertyNamesToRemove(): array
197197
{
198198
return [
199-
'no rules' => [[]],
200-
'one rule' => [[new Rule('color')]],
201-
'two rules for different properties' => [[new Rule('color'), new Rule('display')]],
202-
'two rules for the same property' => [[new Rule('color'), new Rule('color')]],
199+
'no properties' => [[]],
200+
'one property' => [['color']],
201+
'two different properties' => [['color', 'display']],
202+
'two of the same property' => [['color', 'color']],
203203
];
204204
}
205205

206206
/**
207207
* @test
208208
*
209-
* @param list<Rule> $rules
209+
* @param list<string> $propertyNamesToRemove
210210
*
211-
* @dataProvider provideRulesToRemove
211+
* @dataProvider providePropertyNamesToRemove
212212
*/
213-
public function removeAllRulesRemovesAllRules(array $rules): void
213+
public function removeAllRulesRemovesAllRules(array $propertyNamesToRemove): void
214214
{
215-
$this->subject->setRules($rules);
215+
$this->setRulesFromPropertyNames($propertyNamesToRemove);
216216

217217
$this->subject->removeAllRules();
218218

219219
self::assertSame([], $this->subject->getRules());
220220
}
221+
222+
/**
223+
* @param list<string> $propertyNames
224+
*/
225+
private function setRulesFromPropertyNames(array $propertyNames): void
226+
{
227+
$this->subject->setRules(\array_map(
228+
static function (string $propertyName): Rule {
229+
return new Rule($propertyName);
230+
},
231+
$propertyNames
232+
));
233+
}
221234
}

0 commit comments

Comments
 (0)