Skip to content

Commit 76b032c

Browse files
simon-mundysimon-mundy
authored
Test modernization (#76)
* 100% PHPUnit line/file coverage Modernize test suite and remove dead v2 ServiceLocator code Ensure deprecated code can still be tested where possible Dead code removal Test modernization: - Replace try/catch blocks with expectException() for cleaner tests - Use data providers where multiple similar test cases exist - Use more concise assertion methods (assertTrue, assertFalse, etc.) - Expand test coverage for deprecated methods to ensure robustness - Enable previously skipped test (ConstructedNavigationFactory is mockable) Dead code removal: - Remove unreachable v2 ServiceLocator code path from HelperConfig - The v2 branch in getParentContainer() could never execute because laminas-navigation requires PHP 8.2+ which is incompatible with ServiceManager v2 (which only supports PHP 5.5-7.x) Signed-off-by: Simon Mundy <simon.mundy@peptolab.com>
1 parent ae85dac commit 76b032c

11 files changed

+1134
-365
lines changed

‎psalm-baseline.xml‎

Lines changed: 126 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -15,11 +15,6 @@
1515
<code><![CDATA[$pages instanceof AbstractContainer]]></code>
1616
</TypeDoesNotContainType>
1717
</file>
18-
<file src="src/Module.php">
19-
<UnusedClass>
20-
<code><![CDATA[Module]]></code>
21-
</UnusedClass>
22-
</file>
2318
<file src="src/Page/AbstractPage.php">
2419
<DocblockTypeContradiction>
2520
<code><![CDATA[is_array($options)]]></code>
@@ -123,6 +118,9 @@
123118
* ...<string, mixed>
124119
* }]]></code>
125120
</LessSpecificImplementedReturnType>
121+
<UnusedPsalmSuppress>
122+
<code><![CDATA[PossiblyUnusedMethod]]></code>
123+
</UnusedPsalmSuppress>
126124
</file>
127125
<file src="src/Page/Uri.php">
128126
<LessSpecificImplementedReturnType>
@@ -215,12 +213,6 @@
215213
<DeprecatedInterface>
216214
<code><![CDATA[HelperConfig]]></code>
217215
</DeprecatedInterface>
218-
<DeprecatedMethod>
219-
<code><![CDATA[getServiceLocator]]></code>
220-
</DeprecatedMethod>
221-
<LessSpecificReturnStatement>
222-
<code><![CDATA[$container->getServiceLocator() ?: $container]]></code>
223-
</LessSpecificReturnStatement>
224216
<MixedArgument>
225217
<code><![CDATA[$config]]></code>
226218
<code><![CDATA[$pluginManager]]></code>
@@ -259,9 +251,6 @@
259251
<PossiblyUnusedReturnValue>
260252
<code><![CDATA[ServiceManager]]></code>
261253
</PossiblyUnusedReturnValue>
262-
<RedundantConditionGivenDocblockType>
263-
<code><![CDATA[$container->getServiceLocator()]]></code>
264-
</RedundantConditionGivenDocblockType>
265254
</file>
266255
<file src="src/View/NavigationHelperFactory.php">
267256
<DeprecatedClass>
@@ -291,6 +280,13 @@
291280
</ParamNameMismatch>
292281
</file>
293282
<file src="test/ContainerTest.php">
283+
<InvalidArgument>
284+
<code><![CDATA['invalid string']]></code>
285+
<code><![CDATA['invalid string']]></code>
286+
<code><![CDATA['ok']]></code>
287+
<code><![CDATA[1337]]></code>
288+
<code><![CDATA[new stdClass()]]></code>
289+
</InvalidArgument>
294290
<MixedArgument>
295291
<code><![CDATA[$found]]></code>
296292
<code><![CDATA[$found]]></code>
@@ -306,6 +302,14 @@
306302
<MixedMethodCall>
307303
<code><![CDATA[getLabel]]></code>
308304
</MixedMethodCall>
305+
<PossiblyNullArgument>
306+
<code><![CDATA[$container->findOneBy('route', 'bar')]]></code>
307+
<code><![CDATA[$container->findOneBy('route', 'baz')]]></code>
308+
</PossiblyNullArgument>
309+
<PossiblyNullReference>
310+
<code><![CDATA[hasChildren]]></code>
311+
<code><![CDATA[hasChildren]]></code>
312+
</PossiblyNullReference>
309313
<UndefinedMagicMethod>
310314
<code><![CDATA[findAllByAction]]></code>
311315
<code><![CDATA[findAllById]]></code>
@@ -317,7 +321,15 @@
317321
<code><![CDATA[getPagez]]></code>
318322
</UndefinedMagicMethod>
319323
</file>
324+
<file src="test/ModuleTest.php">
325+
<RedundantConditionGivenDocblockType>
326+
<code><![CDATA[assertIsArray]]></code>
327+
</RedundantConditionGivenDocblockType>
328+
</file>
320329
<file src="test/Page/MvcTest.php">
330+
<ArgumentTypeCoercion>
331+
<code><![CDATA[(object) ['invalid' => 'object']]]></code>
332+
</ArgumentTypeCoercion>
321333
<InvalidArgument>
322334
<code><![CDATA[$newRouter]]></code>
323335
<code><![CDATA[$router]]></code>
@@ -328,13 +340,17 @@
328340
<code><![CDATA[$router]]></code>
329341
<code><![CDATA[$router]]></code>
330342
</InvalidArgument>
343+
<MixedArgument>
344+
<code><![CDATA[$invalid]]></code>
345+
<code><![CDATA[$invalid]]></code>
346+
<code><![CDATA[$invalid]]></code>
347+
</MixedArgument>
331348
<NullArgument>
332349
<code><![CDATA[null]]></code>
333350
</NullArgument>
334-
<PossiblyInvalidCast>
335-
<code><![CDATA[$invalid]]></code>
336-
<code><![CDATA[$invalid]]></code>
337-
</PossiblyInvalidCast>
351+
<RedundantConditionGivenDocblockType>
352+
<code><![CDATA[assertIsString]]></code>
353+
</RedundantConditionGivenDocblockType>
338354
</file>
339355
<file src="test/Page/PageFactoryTest.php">
340356
<ArgumentTypeCoercion>
@@ -348,12 +364,104 @@
348364
</InvalidArgument>
349365
</file>
350366
<file src="test/Page/PageTest.php">
367+
<ArgumentTypeCoercion>
368+
<code><![CDATA['stdClass']]></code>
369+
</ArgumentTypeCoercion>
351370
<DocblockTypeContradiction>
352371
<code><![CDATA[assertSame]]></code>
353372
</DocblockTypeContradiction>
373+
<InvalidArgument>
374+
<code><![CDATA['False']]></code>
375+
<code><![CDATA['alternate']]></code>
376+
<code><![CDATA['alternate']]></code>
377+
<code><![CDATA['true']]></code>
378+
<code><![CDATA[0]]></code>
379+
<code><![CDATA[0]]></code>
380+
<code><![CDATA[1]]></code>
381+
<code><![CDATA[1]]></code>
382+
<code><![CDATA[[]]]></code>
383+
<code><![CDATA[[]]]></code>
384+
</InvalidArgument>
385+
<InvalidCast>
386+
<code><![CDATA[[]]]></code>
387+
<code><![CDATA[[]]]></code>
388+
</InvalidCast>
389+
<MixedArgument>
390+
<code><![CDATA[$invalid]]></code>
391+
<code><![CDATA[$invalid]]></code>
392+
<code><![CDATA[$invalid]]></code>
393+
<code><![CDATA[$invalid]]></code>
394+
<code><![CDATA[$invalid]]></code>
395+
<code><![CDATA[$invalid]]></code>
396+
<code><![CDATA[$invalid]]></code>
397+
<code><![CDATA[$invalid]]></code>
398+
<code><![CDATA[$page->getPermission()]]></code>
399+
</MixedArgument>
400+
<MixedAssignment>
401+
<code><![CDATA[$childPage]]></code>
402+
<code><![CDATA[$childPage]]></code>
403+
</MixedAssignment>
404+
<MixedMethodCall>
405+
<code><![CDATA[getVisible]]></code>
406+
<code><![CDATA[getVisible]]></code>
407+
<code><![CDATA[isVisible]]></code>
408+
<code><![CDATA[isVisible]]></code>
409+
</MixedMethodCall>
410+
<MixedPropertyFetch>
411+
<code><![CDATA[$page->getPermission()->name]]></code>
412+
</MixedPropertyFetch>
413+
<PossiblyNullArgument>
414+
<code><![CDATA[$page->getPermission()]]></code>
415+
</PossiblyNullArgument>
416+
<PossiblyNullPropertyFetch>
417+
<code><![CDATA[$page->getPermission()->name]]></code>
418+
</PossiblyNullPropertyFetch>
354419
<UndefinedMagicMethod>
355420
<code><![CDATA[findOneByLabel]]></code>
356421
<code><![CDATA[findOneByLabel]]></code>
357422
</UndefinedMagicMethod>
358423
</file>
424+
<file src="test/View/HelperConfigTest.php">
425+
<DeprecatedClass>
426+
<code><![CDATA[NavigationHelper::class]]></code>
427+
<code><![CDATA[NavigationHelper::class]]></code>
428+
<code><![CDATA[NavigationHelper::class]]></code>
429+
<code><![CDATA[NavigationHelper\Links::class]]></code>
430+
<code><![CDATA[NavigationHelper\Menu::class]]></code>
431+
<code><![CDATA[NavigationHelper\Menu::class]]></code>
432+
<code><![CDATA[NavigationHelper\Menu::class]]></code>
433+
<code><![CDATA[NavigationHelper\Menu::class]]></code>
434+
</DeprecatedClass>
435+
<MissingClosureParamType>
436+
<code><![CDATA[$services]]></code>
437+
</MissingClosureParamType>
438+
<MixedArgument>
439+
<code><![CDATA[$helpers]]></code>
440+
<code><![CDATA[$services]]></code>
441+
</MixedArgument>
442+
<MixedArrayAccess>
443+
<code><![CDATA[$internalConfig['delegators']]]></code>
444+
<code><![CDATA[$internalConfig['delegators'][NavigationHelperFactory::class]]]></code>
445+
</MixedArrayAccess>
446+
<MixedArrayAssignment>
447+
<code><![CDATA[$internalConfig['delegators']]]></code>
448+
<code><![CDATA[$internalConfig['delegators'][NavigationHelperFactory::class]]]></code>
449+
</MixedArrayAssignment>
450+
<MixedAssignment>
451+
<code><![CDATA[$factory]]></code>
452+
<code><![CDATA[$helpers]]></code>
453+
<code><![CDATA[$internalConfig]]></code>
454+
<code><![CDATA[$internalConfig['delegators'][NavigationHelperFactory::class][]]]></code>
455+
</MixedAssignment>
456+
<MixedMethodCall>
457+
<code><![CDATA[findHelper]]></code>
458+
</MixedMethodCall>
459+
</file>
460+
<file src="test/View/NavigationHelperFactoryTest.php">
461+
<DeprecatedClass>
462+
<code><![CDATA[NavigationHelper::class]]></code>
463+
<code><![CDATA[NavigationHelper::class]]></code>
464+
<code><![CDATA[NavigationHelper::class]]></code>
465+
</DeprecatedClass>
466+
</file>
359467
</files>

‎src/View/HelperConfig.php‎

Lines changed: 2 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,6 @@
1717
use function in_array;
1818
use function is_array;
1919
use function iterator_to_array;
20-
use function method_exists;
2120
use function strtolower;
2221
use function strtr;
2322

@@ -152,17 +151,8 @@ private function mergeHelpersFromConfiguration($config)
152151
*/
153152
private function getParentContainer(ServiceManager $container)
154153
{
155-
// We need the parent container in order to retrieve the config
156-
// service. We should likely revisit how this is done in the future.
157-
//
158-
// v3:
159-
if (method_exists($container, 'configure')) {
160-
$r = new ReflectionProperty($container, 'creationContext');
161-
return $r->getValue($container) ?: $container;
162-
}
163-
164-
// v2:
165-
return $container->getServiceLocator() ?: $container;
154+
$r = new ReflectionProperty($container, 'creationContext');
155+
return $r->getValue($container) ?: $container;
166156
}
167157

168158
/**

0 commit comments

Comments
 (0)