Skip to content
This repository was archived by the owner on Feb 6, 2020. It is now read-only.

Commit e2eb462

Browse files
committed
Clarified assertions where necessary
This patch clarifies assertions when multiple assertions exist in the same test method, and when the intent may not be immediately clear. This is not as comprehensive as @Maks3w suggested in his feedback, but should be sufficient.
1 parent c43e871 commit e2eb462

File tree

4 files changed

+74
-29
lines changed

4 files changed

+74
-29
lines changed

test/AbstractPluginManagerTest.php

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -156,7 +156,12 @@ function ($container, $name, $callback) {
156156
]);
157157

158158
$instance = $pluginManager->get(stdClass::class);
159-
$this->assertEquals($config['option'], $instance->option);
159+
$this->assertTrue(isset($instance->option), 'Delegator-injected option was not found');
160+
$this->assertEquals(
161+
$config['option'],
162+
$instance->option,
163+
'Delegator-injected option does not match configuration'
164+
);
160165
$this->assertEquals('bar', $instance->foo);
161166
}
162167
}

test/CommonServiceLocatorBehaviorsTrait.php

Lines changed: 36 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -257,11 +257,7 @@ public function testCanCreateNewLocatorWithMergedConfig()
257257
$this->assertTrue($newServiceManager->has(stdClass::class));
258258

259259
// Make sure the context has been updated for the new container
260-
261-
$reflectionProperty = new \ReflectionProperty($newServiceManager, 'creationContext');
262-
$reflectionProperty->setAccessible(true);
263-
264-
$this->assertSame($newServiceManager, $reflectionProperty->getValue($newServiceManager));
260+
$this->assertAttributeSame($newServiceManager, 'creationContext', $newServiceManager);
265261
}
266262

267263
public function testOverrideConfigWhenMerged()
@@ -346,24 +342,24 @@ public function testHasDoesNotCheckAbstractFactoriesByDefault()
346342
public function abstractFactories()
347343
{
348344
return [
349-
'simple' => [new SimpleAbstractFactory(), 'assertTrue'],
350-
'failing' => [new FailingAbstractFactory(), 'assertFalse'],
345+
'simple' => [new SimpleAbstractFactory(), true],
346+
'failing' => [new FailingAbstractFactory(), false],
351347
];
352348
}
353349

354350
/**
355351
* @group has
356352
* @dataProvider abstractFactories
357353
*/
358-
public function testHasCanCheckAbstractFactoriesWhenRequested($abstractFactory, $assertion)
354+
public function testHasCanCheckAbstractFactoriesWhenRequested($abstractFactory, $expected)
359355
{
360356
$serviceManager = $this->createContainer([
361357
'abstract_factories' => [
362358
$abstractFactory,
363359
],
364360
]);
365361

366-
$this->{$assertion}($serviceManager->has(DateTime::class, true));
362+
$this->assertSame($expected, $serviceManager->has(DateTime::class, true));
367363
}
368364

369365
/**
@@ -409,20 +405,40 @@ function ($container, $instance) {
409405
]);
410406

411407
$dateTime = $serviceManager->get(DateTime::class);
412-
$this->assertInstanceOf(DateTime::class, $dateTime);
408+
$this->assertInstanceOf(DateTime::class, $dateTime, 'DateTime service did not resolve as expected');
413409
$notShared = $serviceManager->get(DateTime::class);
414-
$this->assertInstanceOf(DateTime::class, $notShared);
415-
$this->assertNotSame($dateTime, $notShared);
410+
$this->assertInstanceOf(DateTime::class, $notShared, 'DateTime service did not re-resolve as expected');
411+
$this->assertNotSame(
412+
$dateTime,
413+
$notShared,
414+
'Expected unshared instances for DateTime service but received shared instances'
415+
);
416416

417417
$config = $serviceManager->get('config');
418-
$this->assertInternalType('array', $config);
419-
$this->assertSame($config, $serviceManager->get('config'));
418+
$this->assertInternalType('array', $config, 'Config service did not resolve as expected');
419+
$this->assertSame(
420+
$config,
421+
$serviceManager->get('config'),
422+
'Config service resolved as unshared instead of shared'
423+
);
420424

421425
$stdClass = $serviceManager->get(stdClass::class);
422-
$this->assertInstanceOf(stdClass::class, $stdClass);
423-
$this->assertSame($stdClass, $serviceManager->get(stdClass::class));
424-
$this->assertEquals('bar', $stdClass->foo);
425-
$this->assertEquals('baz', $stdClass->bar);
426+
$this->assertInstanceOf(stdClass::class, $stdClass, 'stdClass service did not resolve as expected');
427+
$this->assertSame(
428+
$stdClass,
429+
$serviceManager->get(stdClass::class),
430+
'stdClass service should be shared, but resolved as unshared'
431+
);
432+
$this->assertTrue(
433+
isset($stdClass->foo),
434+
'Expected delegator to inject "foo" property in stdClass service, but it was not'
435+
);
436+
$this->assertEquals('bar', $stdClass->foo, 'stdClass "foo" property was not injected correctly');
437+
$this->assertTrue(
438+
isset($stdClass->bar),
439+
'Expected initializer to inject "bar" property in stdClass service, but it was not'
440+
);
441+
$this->assertEquals('baz', $stdClass->bar, 'stdClass "bar" property was not injected correctly');
426442
}
427443

428444
/**
@@ -482,7 +498,8 @@ public function testCanSpecifyInitializerUsingStringViaConfiguration()
482498

483499
$instance = $serviceManager->get(stdClass::class);
484500
$this->assertInstanceOf(stdClass::class, $instance);
485-
$this->assertEquals('bar', $instance->foo);
501+
$this->assertTrue(isset($instance->foo), '"foo" property was not injected by initializer');
502+
$this->assertEquals('bar', $instance->foo, '"foo" property was not properly injected');
486503
}
487504

488505
/**

test/LazyServiceIntegrationTest.php

Lines changed: 25 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -72,23 +72,25 @@ public function listProxyFiles()
7272
return new RegexIterator($rii, '/^.+\.php$/i', RecursiveRegexIterator::GET_MATCH);
7373
}
7474

75-
public function assertProxyDirEmpty()
75+
public function assertProxyDirEmpty($message = '')
7676
{
77+
$message = $message ?: 'Expected empty proxy directory; found files';
7778
$count = 0;
7879
foreach ($this->listProxyFiles() as $file) {
79-
$this->assertFail('Expected empty proxy directory; found files');
80+
$this->assertFail($message);
8081
}
8182
$this->assertEquals(0, $count);
8283
}
8384

84-
public function assertProxyFileWritten()
85+
public function assertProxyFileWritten($message = '')
8586
{
87+
$message = $message ?: 'Expected ProxyManager to write at least one class file; none found';
8688
$count = 0;
8789
foreach ($this->listProxyFiles() as $file) {
8890
$count += 1;
8991
break;
9092
}
91-
$this->assertNotEquals(0, $count);
93+
$this->assertNotEquals(0, $count, $message);
9294
}
9395

9496
/**
@@ -121,12 +123,27 @@ public function testCanUseLazyServiceFactoryFactoryToCreateLazyServiceFactoryToA
121123
$this->assertProxyFileWritten();
122124

123125
// Test we got a usable proxy
124-
$this->assertInstanceOf(InvokableObject::class, $instance);
125-
$this->assertContains('TestAssetProxy', get_class($instance));
126+
$this->assertInstanceOf(
127+
InvokableObject::class,
128+
$instance,
129+
'Service returned does not extend ' . InvokableObject::class
130+
);
131+
$this->assertContains(
132+
'TestAssetProxy',
133+
get_class($instance),
134+
'Service returned does not contain expected namespace'
135+
);
126136

127137
// Test proxying works as expected
128138
$options = $instance->getOptions();
129-
$this->assertInternalType('array', $options);
130-
$this->assertEquals(['foo' => 'bar'], $options);
139+
$this->assertInternalType(
140+
'array',
141+
$options,
142+
sprintf(
143+
'Expected an array of options; %s received',
144+
(is_object($options) ? get_class($options) : gettype($options))
145+
)
146+
);
147+
$this->assertEquals(['foo' => 'bar'], $options, 'Options returned do not match configuration');
131148
}
132149
}

test/ServiceManagerTest.php

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,7 @@ public function testConfigurationCanBeMerged()
4747
]);
4848

4949
$this->assertTrue($serviceManager->has(DateTime::class));
50+
// stdClass service is inlined in SimpleServiceManager
5051
$this->assertTrue($serviceManager->has(stdClass::class));
5152
}
5253

@@ -94,7 +95,12 @@ function ($container, $name, $callback) {
9495
]);
9596

9697
$instance = $serviceManager->get(stdClass::class);
97-
$this->assertEquals($config['option'], $instance->option);
98+
$this->assertTrue(isset($instance->option), 'Delegator-injected option was not found');
99+
$this->assertEquals(
100+
$config['option'],
101+
$instance->option,
102+
'Delegator-injected option does not match configuration'
103+
);
98104
$this->assertEquals('bar', $instance->foo);
99105
}
100106
}

0 commit comments

Comments
 (0)