From dbf8afc44be4a00ae90df26f6cc981f58793f032 Mon Sep 17 00:00:00 2001 From: Daniel Vu Date: Thu, 16 Jul 2026 01:20:34 +0700 Subject: [PATCH] 902: don't let an unserializable attachment abort the whole test run AllureHelper::addAttachmentToCurrentStep() caught the exception serialize() throws for values containing a Closure, then re-threw using $data->getMessage() instead of the caught $exception. Since $data at that point is still the original, unserialized value, this either surfaces the wrong message or, when $data has no getMessage() method, throws a new, unhandled error that aborts the entire suite instead of the single failing test. addAttachmentToLastStep() had the identical serialize() call with no exception handling at all, so it failed the same way. Both now fall back to a descriptive placeholder string when serialization fails, so a debugging attachment that can't be losslessly serialized no longer takes the rest of the test run down with it. --- .../Allure/AllureHelperTest.php | 31 +++++++++++++++++++ .../Allure/AllureHelper.php | 29 +++++++++++++---- 2 files changed, 54 insertions(+), 6 deletions(-) diff --git a/dev/tests/unit/Magento/FunctionalTestFramework/Allure/AllureHelperTest.php b/dev/tests/unit/Magento/FunctionalTestFramework/Allure/AllureHelperTest.php index 457f297e8..83d925f69 100644 --- a/dev/tests/unit/Magento/FunctionalTestFramework/Allure/AllureHelperTest.php +++ b/dev/tests/unit/Magento/FunctionalTestFramework/Allure/AllureHelperTest.php @@ -82,6 +82,37 @@ public function testAddAttachmentFileToStep( self::assertSame('.html', $attachment->getFileExtension()); } + /** + * A value that can't be serialize()'d (e.g. one containing a Closure) must not abort the test + * run: addAttachmentToCurrentStep falls back to a descriptive placeholder instead. + */ + public function testAddAttachmentToCurrentStepWithUnserializableDataDoesNotThrow(): void + { + $attachment = new AttachmentResult('a'); + Allure::setLifecycleBuilder( + $this->createLifecycleBuilder($this->createResultFactoryWithAttachment($attachment)), + ); + + AllureHelper::addAttachmentToCurrentStep(['closure' => fn () => null], 'unserializableException'); + + self::assertSame('unserializableException', $attachment->getName()); + } + + /** + * Same guard on the addAttachmentToLastStep path. + */ + public function testAddAttachmentToLastStepWithUnserializableDataDoesNotThrow(): void + { + $attachment = new AttachmentResult('a'); + Allure::setLifecycleBuilder( + $this->createLifecycleBuilder($this->createResultFactoryWithAttachment($attachment)), + ); + + AllureHelper::addAttachmentToLastStep(['closure' => fn () => null], 'unserializableException'); + + self::assertSame('unserializableException', $attachment->getName()); + } + /** * @return iterable */ diff --git a/src/Magento/FunctionalTestingFramework/Allure/AllureHelper.php b/src/Magento/FunctionalTestingFramework/Allure/AllureHelper.php index 4221b5620..8a469b39d 100644 --- a/src/Magento/FunctionalTestingFramework/Allure/AllureHelper.php +++ b/src/Magento/FunctionalTestingFramework/Allure/AllureHelper.php @@ -24,11 +24,7 @@ class AllureHelper public static function addAttachmentToCurrentStep($data, $caption): void { if (!is_string($data)) { - try { - $data = serialize($data); - } catch (\Exception $exception) { - throw new \Exception($data->getMessage()); - } + $data = self::serializeAttachmentData($data); } if (@file_exists($data) && is_file($data)) { Allure::attachmentFile($caption, $data); @@ -49,7 +45,7 @@ public static function addAttachmentToCurrentStep($data, $caption): void public static function addAttachmentToLastStep($data, $caption): void { if (!is_string($data)) { - $data = serialize($data); + $data = self::serializeAttachmentData($data); } if (@file_exists($data) && is_file($data)) { Allure::attachmentFile($caption, $data); @@ -58,6 +54,27 @@ public static function addAttachmentToLastStep($data, $caption): void } } + /** + * Serializes attachment data for the Allure report, falling back to a descriptive placeholder + * instead of letting an unserializable value (e.g. one containing a Closure) abort the whole + * test run: a failure to attach a debugging artifact must not be fatal to the test suite. + * + * @param mixed $data + * @return string + */ + private static function serializeAttachmentData($data): string + { + try { + return serialize($data); + } catch (\Exception $exception) { + return sprintf( + 'Unable to serialize attachment data of type "%s": %s', + is_object($data) ? get_class($data) : gettype($data), + $exception->getMessage() + ); + } + } + /** * @param DataSourceInterface $dataSource * @param string $name