Skip to content

Commit ee52c08

Browse files
JacobBrownAustinJacobBrownAustin
committed
ACPT-1360: Automated test to validate _resetState methods cause same state as initially constructed
Adding more code to skip classes that need to be skipped Updated Collector and Comparator to compare without cloning and compare dependencies
1 parent 5389fe3 commit ee52c08

File tree

3 files changed

+128
-42
lines changed

3 files changed

+128
-42
lines changed

‎dev/tests/integration/testsuite/Magento/Framework/ObjectManager/ResetAfterRequestTest.php

Lines changed: 107 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
namespace Magento\Framework\ObjectManager;
77

88
use Magento\Framework\App\Utility\Classes;
9+
use Magento\Framework\Exception\RuntimeException;
910
use Magento\Framework\ObjectManager\FactoryInterface as ObjectManagerFactoryInterface;
1011
use Magento\Framework\ObjectManagerInterface;
1112
use Magento\GraphQl\App\State\Collector;
@@ -17,48 +18,33 @@
1718
class ResetAfterRequestTest extends \PHPUnit\Framework\TestCase
1819
{
1920

20-
private static $objectManager;
21-
21+
private ?ObjectManagerInterface $objectManager;
2222
private ?Comparator $comparator;
2323
private ?Collector $collector;
2424

25-
public static function setUpBeforeClass(): void
26-
{
27-
$config = new \Magento\Framework\ObjectManager\Config\Config();
28-
$factory = new Factory\Dynamic\Developer($config);
29-
self::$objectManager = new \Magento\Framework\ObjectManager\ObjectManager($factory, $config);
30-
// self::$objectManager->configure(
31-
// ['preferences' => [self::TEST_INTERFACE => self::TEST_INTERFACE_IMPLEMENTATION]]
32-
// );
33-
$factory->setObjectManager(self::$objectManager);
34-
}
35-
36-
public static function tearDownAfterClass(): void
37-
{
38-
self::$objectManager = null;
39-
}
40-
4125
/**
4226
* @return void
4327
*/
4428
protected function setUp(): void
4529
{
4630
parent::setUp();
47-
$this->comparator = static::$objectManager->create(Comparator::class);
48-
$this->collector = static::$objectManager->create(Collector::class);
31+
$this->objectManager = \Magento\TestFramework\Helper\Bootstrap::getObjectManager();
32+
$this->objectManager->configure(
33+
$this->objectManager->get(ConfigLoaderInterface::class)->load('graphql')
34+
);
35+
$this->comparator = $this->objectManager->create(Comparator::class);
36+
$this->collector = $this->objectManager->create(Collector::class);
4937
}
5038

5139
/**
52-
* Data provider for testNewInstance
53-
*
5440
* Provides list of all classes and virtual classes that implement ResetAfterRequestInterface
5541
*
5642
* @return array
5743
*/
5844
public function resetAfterRequestClassDataProvider()
5945
{
6046
$resetAfterRequestClasses = [];
61-
foreach (Classes::getVirtualClasses() as $name => $type ) {
47+
foreach (Classes::getVirtualClasses() as $name => $type) {
6248
try {
6349
if (!class_exists($type)) {
6450
continue;
@@ -77,19 +63,34 @@ public function resetAfterRequestClassDataProvider()
7763
}
7864
}
7965
foreach (array_keys(Classes::collectModuleClasses('[A-Z][a-z\d][A-Za-z\d\\\\]+')) as $type) {
66+
if (str_contains($type, "_files")) {
67+
continue; // We have to skip the fixture files that collectModuleClasses returns;
68+
}
8069
try {
8170
if (!class_exists($type)) {
8271
continue;
8372
}
84-
if (is_a($type, ObjectManagerInterface::class)) {
73+
if (!is_a($type, ResetAfterRequestInterface::class, true)) {
74+
continue; // We only want to return classes that implement ResetAfterRequestInterface
75+
}
76+
if (is_a($type, ObjectManagerInterface::class, true)) {
8577
continue;
8678
}
87-
if (is_a($type, ObjectManagerFactoryInterface::class)) {
79+
if (is_a($type, ObjectManagerFactoryInterface::class, true)) {
8880
continue;
8981
}
90-
if (is_a($type, ResetAfterRequestInterface::class, true)) {
91-
$resetAfterRequestClasses[] = [$type];
82+
$reflectionClass = new \ReflectionClass($type);
83+
if ($reflectionClass->isAbstract()) {
84+
continue; // We can't test abstract classes since they can't instantiate.
9285
}
86+
if (\Magento\Catalog\Model\ResourceModel\Collection\AbstractCollection::class == $type) {
87+
continue; // This class isn't abstract, but it can't be constructed itself without error
88+
}
89+
if (\Magento\Eav\Model\ResourceModel\Form\Attribute\Collection::class == $type) {
90+
continue; // Note: This class isn't abstract, but it cannot be constructed itself.
91+
// It requires subclass to modify protected $_moduleName to be constructed.
92+
}
93+
$resetAfterRequestClasses[] = [$type];
9394
} catch (\Throwable $throwable) {
9495
continue;
9596
}
@@ -105,18 +106,86 @@ public function resetAfterRequestClassDataProvider()
105106
*/
106107
public function testResetAfterRequestClasses(string $className)
107108
{
108-
/** @var ResetAfterRequestInterface $object */
109-
$object = self::$objectManager->get($className);
110-
$beforeProperties = $this->collector->getPropertiesFromObject($object);
111-
$object->_resetState();
112-
$afterProperties = $this->collector->getPropertiesFromObject($object);
113-
$differences = [];
114-
foreach ($afterProperties as $propertyName => $propertyValue) {
115-
$result = $this->comparator->checkValues($beforeProperties[$propertyName] ?? null, $propertyValue);
116-
if ($result) {
117-
$differences[$propertyName] = $result;
109+
if (\Magento\Backend\Model\Locale\Resolver::class == $className) { // FIXME: ACPT-1369
110+
static::markTestSkipped(
111+
"FIXME: Temporal coupling with Magento\Backend\Model\Locale\Resolver and its _request"
112+
);
113+
}
114+
try {
115+
$object = $this->objectManager->create($className);
116+
} catch (\BadMethodCallException $exception) {
117+
static::markTestSkipped(sprintf(
118+
'The class "%s" cannot be be constructed without proper arguments %s',
119+
$className,
120+
(string)$exception
121+
));
122+
} catch (\ReflectionException $reflectionException) {
123+
static::markTestSkipped(sprintf(
124+
'The class "%s" cannot be constructed. It may require different area. %s',
125+
$className,
126+
(string)$reflectionException
127+
));
128+
} catch (\Error $error) {
129+
static::markTestSkipped(sprintf(
130+
'The class "%s" cannot be constructed. It had Error. %s',
131+
$className,
132+
(string)$error
133+
));
134+
} catch (RuntimeException $exception) {
135+
// TODO: We should find a way to test these classes that require additional run time data/configuration
136+
static::markTestSkipped(sprintf(
137+
'The class "%s" had RuntimeException. %s',
138+
$className,
139+
(string)$exception
140+
));
141+
} catch (\Throwable $throwable) {
142+
throw new \Exception(
143+
sprintf("testResetAfterRequestClasses failed on %s", $className),
144+
0,
145+
$throwable
146+
);
147+
}
148+
try {
149+
/** @var ResetAfterRequestInterface $object */
150+
$beforeProperties = $this->collector->getPropertiesFromObject($object, true);
151+
$object->_resetState();
152+
$afterProperties = $this->collector->getPropertiesFromObject($object, true);
153+
$differences = [];
154+
foreach ($afterProperties as $propertyName => $propertyValue) {
155+
if ($propertyValue instanceof ObjectManagerInterface) {
156+
continue; // We need to skip ObjectManagers
157+
}
158+
if ($propertyValue instanceof \Magento\Framework\Model\ResourceModel\Db\AbstractDb) {
159+
continue; // The _tables array gets added to
160+
}
161+
if ($propertyValue instanceof \Magento\Framework\Model\ResourceModel\Db\VersionControl\Snapshot) {
162+
continue;
163+
}
164+
if ('pluginList' == $propertyName) {
165+
continue; // We can skip plugin List loading from intercepters.
166+
}
167+
if ('_select' == $propertyName) {
168+
continue; // We can skip _select because we load a fresh new Select after reset
169+
}
170+
if ('_regionModels' == $propertyName
171+
&& is_a($className, \Magento\Customer\Model\Address\AbstractAddress::class, true))
172+
{
173+
continue; // AbstractAddress has static property _regionModels, so it would fail this test.
174+
// TODO: Can we convert _regionModels to member variable,
175+
// or move to a dependency injected service class instead?
176+
}
177+
$result = $this->comparator->checkValues($beforeProperties[$propertyName] ?? null, $propertyValue, 3);
178+
if ($result) {
179+
$differences[$propertyName] = $result;
180+
}
118181
}
182+
$this->assertEmpty($differences, var_export($differences, true));
183+
} catch (\Throwable $throwable) {
184+
throw new \Exception(
185+
sprintf("testResetAfterRequestClasses failed on %s", $className),
186+
0,
187+
$throwable
188+
);
119189
}
120-
$this->assertEmpty($differences, var_export($differences, true));
121190
}
122191
}

‎dev/tests/integration/testsuite/Magento/GraphQl/App/State/Collector.php

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -73,26 +73,29 @@ public function getSharedObjects(): array
7373
if ($object instanceof \Magento\Framework\ObjectManagerInterface) {
7474
continue;
7575
}
76-
$properties = $this->getPropertiesFromObject($object);
76+
$properties = $this->getPropertiesFromObject($object, true, $didClone);
7777
$sharedObjects[$serviceName] = [$object, $properties];
7878
}
7979
// Note: We have to check again because sometimes cloning objects can indirectly cause adding to Object Manager
8080
} while ($didClone);
8181
return $sharedObjects;
8282
}
8383

84-
public function getPropertiesFromObject(object $object): array
84+
public function getPropertiesFromObject(object $object, $doClone = false, &$didClone = null): array
8585
{
8686
$objReflection = new \ReflectionObject($object);
8787
$properties = [];
8888
foreach ($objReflection->getProperties() as $property) {
8989
$propName = $property->getName();
9090
$property->setAccessible(true);
9191
$value = $property->getValue($object);
92+
if (!$doClone) {
93+
$properties[$propName] = $value;
94+
continue;
95+
}
9296
if (is_object($value)) {
9397
$didClone = true;
9498
$properties[$propName] = clone $value;
95-
continue;
9699
} elseif (is_array($value)) {
97100
$didClone = true;
98101
$properties[$propName] = $this->cloneArray($value);

‎dev/tests/integration/testsuite/Magento/GraphQl/App/State/Comparator.php

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -203,7 +203,7 @@ private function formatValue($type): array
203203
* @return array
204204
* @SuppressWarnings(PHPMD.CyclomaticComplexity)
205205
*/
206-
public function checkValues($before, $after): array
206+
public function checkValues($before, $after, $nestingLevel = 0): array
207207
{
208208
$result = [];
209209
$typeBefore = gettype($before);
@@ -233,6 +233,20 @@ public function checkValues($before, $after): array
233233
if ($before != $after) {
234234
$result['before'] = get_class($before);
235235
$result['after'] = get_class($after);
236+
if ($nestingLevel) {
237+
$beforeProperties = $this->collector->getPropertiesFromObject($before);
238+
$afterProperties = $this->collector->getPropertiesFromObject($after);
239+
foreach ($afterProperties as $propertyName => $propertyValue) {
240+
$propertyResult = $this->checkValues(
241+
$beforeProperties[$propertyName] ?? null,
242+
$propertyValue,
243+
$nestingLevel - 1
244+
);
245+
if ($propertyResult) {
246+
$result['properties'][$propertyName] = $propertyResult;
247+
}
248+
}
249+
}
236250
}
237251
break;
238252
}

0 commit comments

Comments
 (0)