Skip to content

Commit e2bcca1

Browse files
JacobBrownAustinJacobBrownAustin
committed
ACPT-1552: WIP
* Changes to Collector and Comparator to avoid issues when object gets destructed during _resetState. * Comparator/Collector will now output class name instead of the "end of recursion level" * Now always showing objectIdBefore even when objectIdBefore == objectIdAfter (This is useful when looking in memory dumps) * Fixed issue where skipped classes weren't getting reset
1 parent 78b77f4 commit e2bcca1

File tree

6 files changed

+64
-18
lines changed

6 files changed

+64
-18
lines changed

‎lib/internal/Magento/Framework/TestFramework/ApplicationStateComparator/CollectedObject.php

Lines changed: 20 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,8 @@
77

88
namespace Magento\Framework\TestFramework\ApplicationStateComparator;
99

10+
use WeakReference;
11+
1012
/**
1113
* Immutable recursive data structure that holds copy of properties from collected objects. Created by Collector.
1214
*/
@@ -31,6 +33,7 @@ public function __construct(
3133
private readonly string $className,
3234
private readonly array $properties,
3335
private readonly int $objectId,
36+
private ?WeakReference $weakReference,
3437
) {
3538
}
3639

@@ -64,6 +67,16 @@ public function getObjectId() : int
6467
return $this->objectId;
6568
}
6669

70+
/**
71+
* Returns the weak reference to object
72+
*
73+
* @return WeakReference|null
74+
*/
75+
public function getWeakReference() : ?WeakReference
76+
{
77+
return $this->weakReference;
78+
}
79+
6780
/**
6881
* Returns a special object that is used to mark a skipped object.
6982
*
@@ -72,7 +85,7 @@ public function getObjectId() : int
7285
public static function getSkippedObject() : CollectedObject
7386
{
7487
if (!self::$skippedObject) {
75-
self::$skippedObject = new CollectedObject('(collected object - skipped)', [], 0);
88+
self::$skippedObject = new CollectedObject('(collected object - skipped)', [], 0, null);
7689
}
7790
return self::$skippedObject;
7891
}
@@ -85,7 +98,12 @@ public static function getSkippedObject() : CollectedObject
8598
public static function getRecursionEndObject() : CollectedObject
8699
{
87100
if (!self::$recursionEndObject) {
88-
self::$recursionEndObject = new CollectedObject('(collected object - end of recursion level)', [], 0);
101+
self::$recursionEndObject = new CollectedObject(
102+
'(collected object - end of recursion level)',
103+
[],
104+
0,
105+
null,
106+
);
89107
}
90108
return self::$recursionEndObject;
91109
}

‎lib/internal/Magento/Framework/TestFramework/ApplicationStateComparator/CollectedObjectConstructedAndCurrent.php

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -7,19 +7,21 @@
77

88
namespace Magento\Framework\TestFramework\ApplicationStateComparator;
99

10+
use WeakReference;
11+
1012
/**
1113
* Returned by Collector
1214
*/
1315
class CollectedObjectConstructedAndCurrent
1416
{
1517

1618
/**
17-
* @param object $object
19+
* @param object $weakReference
1820
* @param CollectedObject $constructedCollected
1921
* @param CollectedObject $currentCollected
2022
*/
2123
public function __construct(
22-
private readonly object $object,
24+
private readonly WeakReference $weakReference,
2325
private readonly CollectedObject $constructedCollected,
2426
private readonly CollectedObject $currentCollected,
2527
) {
@@ -28,11 +30,11 @@ public function __construct(
2830
/**
2931
* Returns the object
3032
*
31-
* @return object
33+
* @return WeakReference
3234
*/
33-
public function getObject() : object
35+
public function getWeakReference() : WeakReference
3436
{
35-
return $this->object;
37+
return $this->weakReference;
3638
}
3739

3840
/**

‎lib/internal/Magento/Framework/TestFramework/ApplicationStateComparator/Collector.php

Lines changed: 20 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
use Magento\Framework\ObjectManager\ResetAfterRequestInterface;
1111
use Magento\Framework\ObjectManagerInterface;
1212
use Magento\Framework\TestFramework\ApplicationStateComparator\ObjectManagerInterface as StateObjectManagerInterface;
13+
use WeakReference;
1314

1415
/**
1516
* Collects shared objects from ObjectManager and copies properties for later comparison
@@ -141,9 +142,18 @@ public function getPropertiesConstructedAndCurrent(): array
141142
// Calling _resetState helps us avoid adding skip/filter for these classes.
142143
$objectManager->_resetState();
143144
$objects = [];
145+
$weakReferenceList = [];
144146
foreach ($objectManager->getWeakMap() as $object => $propertiesBefore) {
147+
$weakReferenceList[] = WeakReference::create($object);
148+
}
149+
foreach ($weakReferenceList as $weakReference) {
150+
$object = $weakReference->get();
151+
if (!$object) {
152+
continue;
153+
}
154+
$propertiesBefore = $objectManager->getWeakMap()[$object];
145155
$objects[] = new CollectedObjectConstructedAndCurrent(
146-
$object,
156+
WeakReference::create($object),
147157
$propertiesBefore,
148158
$this->getPropertiesFromObject($object, CompareType::CompareConstructedAgainstCurrent),
149159
);
@@ -179,7 +189,14 @@ public function getPropertiesFromObject(
179189
}
180190
}
181191
if ($recursionLevel < 0) {
182-
return CollectedObject::getRecursionEndObject();
192+
/* Note: When we reach end of recursionLevel, skip getting properties, but still get the name, object id,
193+
* and WeakReference, so that we can compare if they have changed. */
194+
return new CollectedObject(
195+
$className,
196+
[],
197+
spl_object_id($object),
198+
WeakReference::create($object),
199+
);
183200
}
184201
$objReflection = new \ReflectionObject($object);
185202
$properties = [];
@@ -210,6 +227,7 @@ public function getPropertiesFromObject(
210227
$className,
211228
$properties,
212229
spl_object_id($object),
230+
WeakReference::create($object),
213231
);
214232
}
215233
}

‎lib/internal/Magento/Framework/TestFramework/ApplicationStateComparator/Comparator.php

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -103,7 +103,10 @@ public function compareConstructedAgainstCurrent(string $operationName): array
103103
$skipList = $this->skipListAndFilterList
104104
->getSkipList($operationName, CompareType::CompareConstructedAgainstCurrent);
105105
foreach ($this->collector->getPropertiesConstructedAndCurrent() as $objectAndProperties) {
106-
$object = $objectAndProperties->getObject();
106+
$object = $objectAndProperties->getWeakReference()->get();
107+
if (!$object) {
108+
continue; // object was deconstructed during getPropertiesConstructedAndCurrent
109+
}
107110
$constructedObject = $objectAndProperties->getConstructedCollected();
108111
$currentObject = $objectAndProperties->getCurrentCollected();
109112
if ($object instanceof NoninterceptableInterface) {
@@ -189,8 +192,8 @@ private function compare(
189192
if ($returnValue['objectClassBefore'] !== $after->getClassName()) {
190193
$returnValue['objectClassAfter'] = $after->getClassName();
191194
}
195+
$returnValue['objectIdBefore'] = $before->getObjectId();
192196
if ($before->getObjectId() != $after->getObjectId()) {
193-
$returnValue['objectIdBefore'] = $before->getObjectId();
194197
$returnValue['objectIdAfter'] = $after->getObjectId();
195198
}
196199
return $returnValue;
@@ -282,6 +285,13 @@ public function checkValues(mixed $before, mixed $after, array $skipList): array
282285
return [];
283286
case 'object':
284287
if ($before instanceof CollectedObject) {
288+
if ($after instanceof CollectedObject) {
289+
if ($before->getWeakReference()?->get() === $after->getWeakReference()?->get()) {
290+
/* Note: When comparing composed objects, if they are the same object, we can ignore.
291+
* This is assuming that we are comparing the composed objects elsewhere. */
292+
return [];
293+
}
294+
}
285295
return $this->compare(
286296
$before,
287297
$after,

‎lib/internal/Magento/Framework/TestFramework/ApplicationStateComparator/DynamicFactoryDecorator.php

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -58,10 +58,8 @@ public function __construct(Developer $developer, ObjectManager $objectManager)
5858
public function create($type, array $arguments = [])
5959
{
6060
$object = parent::create($type, $arguments);
61-
if (!array_key_exists(get_class($object), $this->skipList)) {
62-
$this->weakMap[$object] =
63-
$this->collector->getPropertiesFromObject($object, CompareType::CompareConstructedAgainstCurrent);
64-
}
61+
$this->weakMap[$object] =
62+
$this->collector->getPropertiesFromObject($object, CompareType::CompareConstructedAgainstCurrent);
6563
return $object;
6664
}
6765

‎lib/internal/Magento/Framework/TestFramework/ApplicationStateComparator/SkipListAndFilterList.php

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ class SkipListAndFilterList
2727
*/
2828
private ?array $filterList = null;
2929

30-
private readonly $fixturePath =
30+
private const FIXTURE_PATH =
3131
'/dev/tests/integration/framework/Magento/TestFramework/ApplicationStateComparator/_files';
3232

3333
/**
@@ -53,7 +53,7 @@ public function getSkipList(string $operationName, string $compareType): array
5353
{
5454
if ($this->skipList === null) {
5555
$skipListList = [];
56-
foreach (glob(BP . $fixturePath . '/state-skip-list*.php') as $skipListFile) {
56+
foreach (glob(BP . self::FIXTURE_PATH . '/state-skip-list*.php') as $skipListFile) {
5757
$skipListList[] = include($skipListFile);
5858
}
5959
$this->skipList = array_merge_recursive(...$skipListList);
@@ -85,7 +85,7 @@ public function getFilterList(): array
8585
{
8686
if ($this->filterList === null) {
8787
$filterListList = [];
88-
foreach (glob(BP . $fixturePath . '/state-filter-list*.php') as $filterListFile) {
88+
foreach (glob(BP . self::FIXTURE_PATH . '/state-filter-list*.php') as $filterListFile) {
8989
$filterListList[] = include($filterListFile);
9090
}
9191
$this->filterList = array_merge_recursive(...$filterListList);

0 commit comments

Comments
 (0)