Skip to content

33294 eliminate aspectmock from allureHelperTest #839

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Original file line number Diff line number Diff line change
Expand Up @@ -3,40 +3,48 @@
* Copyright © Magento, Inc. All rights reserved.
* See COPYING.txt for license details.
*/
declare(strict_types=1);

namespace tests\unit\Magento\FunctionalTestFramework\Allure;

use Magento\FunctionalTestingFramework\Allure\AllureHelper;
use Magento\FunctionalTestingFramework\Allure\Event\AddUniqueAttachmentEvent;
use PHPUnit\Framework\TestCase;
use ReflectionProperty;
use Yandex\Allure\Adapter\Allure;
use Yandex\Allure\Adapter\Event\AddAttachmentEvent;
use Yandex\Allure\Adapter\AllureException;
use Yandex\Allure\Adapter\Event\StepFinishedEvent;
use Yandex\Allure\Adapter\Event\StepStartedEvent;
use Yandex\Allure\Adapter\Model\Attachment;
use AspectMock\Test as AspectMock;
use PHPUnit\Framework\TestCase;

class AllureHelperTest extends TestCase
{
const MOCK_FILENAME = 'filename';
private const MOCK_FILENAME = 'filename';

/**
* Clear Allure Lifecycle
* Clear Allure Lifecycle.
*
* @return void
*/
public function tearDown(): void
protected function tearDown(): void
{
Allure::setDefaultLifecycle();
AspectMock::clean();
$instanceProperty = new ReflectionProperty(AddUniqueAttachmentEvent::class, 'instance');
$instanceProperty->setAccessible(true);
$instanceProperty->setValue(null);
}

/**
* AddAtachmentToStep should add an attachment to the current step
* @throws \Yandex\Allure\Adapter\AllureException
* The AddAttachmentToStep should add an attachment to the current step.
*
* @return void
* @throws AllureException
*/
public function testAddAttachmentToStep()
public function testAddAttachmentToStep(): void
{
$this->mockAttachmentWriteEvent();
$expectedData = "string";
$expectedCaption = "caption";
$expectedData = 'string';
$expectedCaption = 'caption';
$this->mockAttachmentWriteEvent($expectedData, $expectedCaption);

//Prepare Allure lifecycle
Allure::lifecycle()->fire(new StepStartedEvent('firstStep'));
Expand All @@ -51,14 +59,16 @@ public function testAddAttachmentToStep()
}

/**
* AddAttachmentToLastStep should add an attachment only to the last step
* @throws \Yandex\Allure\Adapter\AllureException
* The AddAttachmentToLastStep should add an attachment only to the last step.
*
* @return void
* @throws AllureException
*/
public function testAddAttachmentToLastStep()
public function testAddAttachmentToLastStep(): void
{
$this->mockAttachmentWriteEvent();
$expectedData = "string";
$expectedCaption = "caption";
$expectedData = 'string';
$expectedCaption = 'caption';
$this->mockAttachmentWriteEvent($expectedData, $expectedCaption);

//Prepare Allure lifecycle
Allure::lifecycle()->fire(new StepStartedEvent('firstStep'));
Expand Down Expand Up @@ -87,14 +97,16 @@ public function testAddAttachmentToLastStep()
}

/**
* AddAttachment actions should have files with different attachment names
* @throws \Yandex\Allure\Adapter\AllureException
* The AddAttachment actions should have files with different attachment names.
*
* @return void
* @throws AllureException
*/
public function testAddAttachementUniqueName()
public function testAddAttachmentUniqueName(): void
{
$this->mockCopyFile();
$expectedData = "string";
$expectedCaption = "caption";
$expectedData = 'string';
$expectedCaption = 'caption';
$this->mockCopyFile($expectedData, $expectedCaption);

//Prepare Allure lifecycle
Allure::lifecycle()->fire(new StepStartedEvent('firstStep'));
Expand All @@ -111,24 +123,52 @@ public function testAddAttachementUniqueName()
}

/**
* Mock entire attachment writing mechanisms
* @throws \Exception
* Mock entire attachment writing mechanisms.
*
* @param string $filePathOrContents
* @param string $caption
*
* @return void
*/
public function mockAttachmentWriteEvent()
private function mockAttachmentWriteEvent(string $filePathOrContents, string $caption): void
{
AspectMock::double(AddUniqueAttachmentEvent::class, [
"getAttachmentFileName" => self::MOCK_FILENAME
]);
$mockInstance = $this->getMockBuilder(AddUniqueAttachmentEvent::class)
->setConstructorArgs([$filePathOrContents, $caption])
->disallowMockingUnknownTypes()
->onlyMethods(['getAttachmentFileName'])
->getMock();

$mockInstance
->method('getAttachmentFileName')
->willReturn(self::MOCK_FILENAME);

$instanceProperty = new ReflectionProperty(AddUniqueAttachmentEvent::class, 'instance');
$instanceProperty->setAccessible(true);
$instanceProperty->setValue($mockInstance, $mockInstance);
}

/**
* Mock only file writing mechanism
* @throws \Exception
* Mock only file writing mechanism.
*
* @param string $filePathOrContents
* @param string $caption
*
* @return void
*/
public function mockCopyFile()
private function mockCopyFile(string $filePathOrContents, string $caption): void
{
AspectMock::double(AddUniqueAttachmentEvent::class, [
"copyFile" => true
]);
$mockInstance = $this->getMockBuilder(AddUniqueAttachmentEvent::class)
->setConstructorArgs([$filePathOrContents, $caption])
->disallowMockingUnknownTypes()
->onlyMethods(['copyFile'])
->getMock();

$mockInstance
->method('copyFile')
->willReturn(true);

$instanceProperty = new ReflectionProperty(AddUniqueAttachmentEvent::class, 'instance');
$instanceProperty->setAccessible(true);
$instanceProperty->setValue($mockInstance, $mockInstance);
}
}
20 changes: 13 additions & 7 deletions src/Magento/FunctionalTestingFramework/Allure/AllureHelper.php
Original file line number Diff line number Diff line change
Expand Up @@ -3,34 +3,40 @@
* Copyright © Magento, Inc. All rights reserved.
* See COPYING.txt for license details.
*/
declare(strict_types=1);

namespace Magento\FunctionalTestingFramework\Allure;

use Magento\FunctionalTestingFramework\Allure\Event\AddUniqueAttachmentEvent;
use Yandex\Allure\Adapter\Allure;
use Yandex\Allure\Adapter\Event\AddAttachmentEvent;
use Yandex\Allure\Adapter\AllureException;

class AllureHelper
{
/**
* Adds attachment to the current step
* Adds attachment to the current step.
*
* @param mixed $data
* @param string $caption
* @throws \Yandex\Allure\Adapter\AllureException
*
* @return void
* @throws AllureException
*/
public static function addAttachmentToCurrentStep($data, $caption)
public static function addAttachmentToCurrentStep($data, $caption): void
{
Allure::lifecycle()->fire(new AddUniqueAttachmentEvent($data, $caption));
Allure::lifecycle()->fire(AddUniqueAttachmentEvent::getInstance($data, $caption));
}

/**
* Adds Attachment to the last executed step.
* Use this when adding attachments outside of an $I->doSomething() step/context.
*
* @param mixed $data
* @param string $caption
*
* @return void
*/
public static function addAttachmentToLastStep($data, $caption)
public static function addAttachmentToLastStep($data, $caption): void
{
$rootStep = Allure::lifecycle()->getStepStorage()->getLast();
$trueLastStep = array_last($rootStep->getSteps());
Expand All @@ -40,7 +46,7 @@ public static function addAttachmentToLastStep($data, $caption)
return;
}

$attachmentEvent = new AddUniqueAttachmentEvent($data, $caption);
$attachmentEvent = AddUniqueAttachmentEvent::getInstance($data, $caption);
$attachmentEvent->process($trueLastStep);
}
}
Original file line number Diff line number Diff line change
@@ -1,35 +1,63 @@
<?php

/**
* Copyright © Magento, Inc. All rights reserved.
* See COPYING.txt for license details.
*/
declare(strict_types=1);

namespace Magento\FunctionalTestingFramework\Allure\Event;

use Symfony\Component\Mime\MimeTypes;
use Yandex\Allure\Adapter\AllureException;
use Yandex\Allure\Adapter\Event\AddAttachmentEvent;

const DEFAULT_FILE_EXTENSION = 'txt';
const DEFAULT_MIME_TYPE = 'text/plain';

class AddUniqueAttachmentEvent extends AddAttachmentEvent
{
private const DEFAULT_FILE_EXTENSION = 'txt';
private const DEFAULT_MIME_TYPE = 'text/plain';

/**
* @var string
* @var AddUniqueAttachmentEvent|null
*/
private $type;
private static $instance;

/**
* Near copy of parent function, added uniqid call for filename to prevent buggy allure behavior
* @param string $filePathOrContents
* An alternative way to instantiate an instance of this class. Used to mock this class object in unit tests.
*
* @param mixed $filePathOrContents
* @param string $caption
* @param string|null $type
*
* @return AddUniqueAttachmentEvent
*/
public static function getInstance(
$filePathOrContents,
string $caption,
?string $type = null
): AddUniqueAttachmentEvent {
if (!self::$instance) {
self::$instance = new AddUniqueAttachmentEvent(
$filePathOrContents,
$caption,
$type
);
}
return self::$instance;
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This alternative way to instantiate an object of this class is only used for unit tests. This should not be used in MFTF. Am I understand it correctly here? If this is the case, can you make it clear in docblock?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jilu1, thank you for the suggestions. I've fixed it.

Copy link
Contributor

@jilu1 jilu1 Jul 21, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bohdan-harniuk
Even with clear php doc, it's still confusing to add this static method in this class. Is there any other way?
The particular test is valuable, if this cannot be mocked by PHPUnit, can we turn this into a functional test https://github.com/magento/magento2-functional-testing-framework/tree/develop/dev/tests/functional?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If copyFile() is the reason that prevents you from mocking the class, you could do something like:

private function copyFile($filePath, $outputPath)
{
    if (MftfApplicationConfig::getConfig()->getPhase() == MftfApplicationConfig::UNIT_TEST_PHASE) {
        return true;
    }
    return copy($filePath, $outputPath);
}

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you, @jilu1 !
It is a great approach for the copyFile method! I will use it for the testAddAttachmentUniqueName testing method!
For other tested methods we have to mock AddUniqueAttachmentEvent with the singleton approach.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hello @jilu1!
I've done that.
Thank you for your suggestion!


/**
* Near copy of parent function, added uniqid call for filename to prevent buggy allure behavior.
*
* @param mixed $filePathOrContents
* @param string $type
*
* @return string
* @throws AllureException
*/
public function getAttachmentFileName($filePathOrContents, $type)
public function getAttachmentFileName($filePathOrContents, $type): string
{
$filePath = $filePathOrContents;

if (!file_exists($filePath) || !is_file($filePath)) {
//Save contents to temporary file
$filePath = tempnam(sys_get_temp_dir(), 'allure-attachment');
Expand All @@ -40,13 +68,11 @@ public function getAttachmentFileName($filePathOrContents, $type)

if (!isset($type)) {
$type = $this->guessFileMimeType($filePath);
$this->type = $type;
}

$fileExtension = $this->guessFileExtension($type);

$fileSha1 = uniqid(sha1_file($filePath));
$outputPath = parent::getOutputPath($fileSha1, $fileExtension);

if (!$this->copyFile($filePath, $outputPath)) {
throw new AllureException("Failed to copy attachment from $filePath to $outputPath.");
}
Expand All @@ -56,51 +82,48 @@ public function getAttachmentFileName($filePathOrContents, $type)

/**
* Copies file from one path to another. Wrapper for mocking in unit test.
*
* @param string $filePath
* @param string $outputPath
*
* @return boolean
*/
private function copyFile($filePath, $outputPath)
public function copyFile(string $filePath, string $outputPath): bool
{
return copy($filePath, $outputPath);
}

/**
* Copy of parent private function
* Copy of parent private function.
*
* @param string $filePath
*
* @return string
*/
private function guessFileMimeType($filePath)
private function guessFileMimeType(string $filePath): string
{
$type = MimeTypes::getDefault()->guessMimeType($filePath);

if (!isset($type)) {
return DEFAULT_MIME_TYPE;
return self::DEFAULT_MIME_TYPE;
}
return $type;
}

/**
* Copy of parent private function
* Copy of parent private function.
*
* @param string $mimeType
*
* @return string
*/
private function guessFileExtension($mimeType)
private function guessFileExtension(string $mimeType): string
{
$candidate = MimeTypes::getDefault()->getExtensions($mimeType);

if (empty($candidate)) {
return DEFAULT_FILE_EXTENSION;
return self::DEFAULT_FILE_EXTENSION;
}
return reset($candidate);
}

/**
* Copy of parent private function
* @param string $sha1
* @param string $extension
* @return string
*/
public function getOutputFileName($sha1, $extension)
{
return $sha1 . '-attachment.' . $extension;
}
}