33294 eliminate aspectmock from allureHelperTest #839

SilinMykola · 2021-06-27T15:26:04Z

Description

Eliminate AspectMock usage from dev/tests/unit/Magento/FunctionalTestFramework/Allure/AllureHelperTest.php

Fixed Issues

[MFTF] Eliminate AspectMock from AllureHelperTest (Complex!) magento2#33294: [MFTF] Eliminate AspectMock from AllureHelperTest (Complex!) #33294

Contribution checklist

Pull request has a meaningful description of its purpose
All commits are accompanied by meaningful commit messages
All new or changed code is covered with unit/verification tests (if applicable)
All automated tests passed successfully (all builds on Travis CI are green)
Changes to Framework doesn't have backward incompatible changes for tests or have related Pull Request with fixes to tests

jilu1 · 2021-06-28T19:51:26Z

@magento import pull request to https://github.com/magento-commerce/magento2-functional-testing-framework

jilu1 · 2021-06-28T20:04:45Z

@SilinMykola
Thank you for your pull request! We will review and update you soon.

jilu1 · 2021-06-28T20:21:51Z

@magento import pull request to https://github.com/magento-commerce/magento2-functional-testing-framework

jilu1 · 2021-06-30T16:30:48Z

@SilinMykola
Please fix the 3 unit test failures in this PR?

sivaschenko · 2021-07-14T12:07:54Z

@SilinMykola thanks for the pull request! Would you be able to respond to the code review comments?

SilinMykola · 2021-07-14T12:10:39Z

@sivaschenko hi! Yes, I'll try to fix all tests errors. I need some time for this.

bohdan-harniuk · 2021-07-20T08:54:48Z

Hello, @jilu1

I have a little bit helped for @SilinMykola with the aspect mock replacement here.
Please, proceed with the code review.

cc: @SilinMykola, @sivaschenko

andrewbess

Hello @SilinMykola @bohdan-harniuk
Thank you for your contribution.

jilu1 · 2021-07-21T01:10:47Z

@magento import pull request to https://github.com/magento-commerce/magento2-functional-testing-framework

magento-engcom-team · 2021-07-21T01:11:10Z

@jilu1 the pull request successfully imported.

jilu1 · 2021-07-21T14:53:16Z

src/Magento/FunctionalTestingFramework/Allure/Event/AddUniqueAttachmentEvent.php

+            );
+        }
+        return self::$instance;
+    }


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?

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

@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?

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); }

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.

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

jilu1

@bohdan-harniuk
Have you run this PR with magento platform-health branch? I have seen the following exception when running a build:

[group329] PHP Fatal error:  Uncaught TypeError: file_exists() expects parameter 1 to be a valid path, object given in /var/www/html/vendor/magento/magento2-functional-testing-framework/src/Magento/FunctionalTestingFramework/Allure/Event/AddUniqueAttachmentEvent.php:34
[group329] Stack trace:
[group329] #0 /var/www/html/vendor/magento/magento2-functional-testing-framework/src/Magento/FunctionalTestingFramework/Allure/Event/AddUniqueAttachmentEvent.php(34): file_exists()
[group329] #1 /var/www/html/vendor/allure-framework/allure-php-api/src/Yandex/Allure/Adapter/Event/AddAttachmentEvent.php(34): Magento\FunctionalTestingFramework\Allure\Event\AddUniqueAttachmentEvent->getAttachmentFileName()
[group329] #2 /var/www/html/vendor/allure-framework/allure-php-api/src/Yandex/Allure/Adapter/Allure.php(116): Yandex\Allure\Adapter\Event\AddAttachmentEvent->process()
[group329] #3 /var/www/html/vendor/allure-framework/allure-php-api/src/Yandex/Allure/Adapter/Allure.php(92): Yandex\Allure\Adapter\Allure->processStepEvent()
[group329] #4 /var/www/html/vendor/magento/magento2-functional-testing-framework/src/Magento/ in /var/www/html/vendor/magento/magento2-functional-testing-framework/src/Magento/FunctionalTestingFramework/Allure/Event/AddUniqueAttachmentEvent.php on line 34

This exception happened in multiple test groups.

bohdan-harniuk · 2021-07-30T14:41:54Z

@bohdan-harniuk
Have you run this PR with magento platform-health branch? I have seen the following exception when running a build:

[group329] PHP Fatal error:  Uncaught TypeError: file_exists() expects parameter 1 to be a valid path, object given in /var/www/html/vendor/magento/magento2-functional-testing-framework/src/Magento/FunctionalTestingFramework/Allure/Event/AddUniqueAttachmentEvent.php:34
[group329] Stack trace:
[group329] #0 /var/www/html/vendor/magento/magento2-functional-testing-framework/src/Magento/FunctionalTestingFramework/Allure/Event/AddUniqueAttachmentEvent.php(34): file_exists()
[group329] #1 /var/www/html/vendor/allure-framework/allure-php-api/src/Yandex/Allure/Adapter/Event/AddAttachmentEvent.php(34): Magento\FunctionalTestingFramework\Allure\Event\AddUniqueAttachmentEvent->getAttachmentFileName()
[group329] #2 /var/www/html/vendor/allure-framework/allure-php-api/src/Yandex/Allure/Adapter/Allure.php(116): Yandex\Allure\Adapter\Event\AddAttachmentEvent->process()
[group329] #3 /var/www/html/vendor/allure-framework/allure-php-api/src/Yandex/Allure/Adapter/Allure.php(92): Yandex\Allure\Adapter\Allure->processStepEvent()
[group329] #4 /var/www/html/vendor/magento/magento2-functional-testing-framework/src/Magento/ in /var/www/html/vendor/magento/magento2-functional-testing-framework/src/Magento/FunctionalTestingFramework/Allure/Event/AddUniqueAttachmentEvent.php on line 34

This exception happened in multiple test groups.

Hello, @jilu1!
No, I haven’t checked it because we didn't change any business logic here:

Could you please take a look at it? What do you think, how it is possible that we broke something?

Thanks, Bohdan

jilu1 · 2021-07-30T14:59:28Z

@bohdan-harniuk
Have you run this PR with magento platform-health branch? I have seen the following exception when running a build:

[group329] PHP Fatal error:  Uncaught TypeError: file_exists() expects parameter 1 to be a valid path, object given in /var/www/html/vendor/magento/magento2-functional-testing-framework/src/Magento/FunctionalTestingFramework/Allure/Event/AddUniqueAttachmentEvent.php:34
[group329] Stack trace:
[group329] #0 /var/www/html/vendor/magento/magento2-functional-testing-framework/src/Magento/FunctionalTestingFramework/Allure/Event/AddUniqueAttachmentEvent.php(34): file_exists()
[group329] #1 /var/www/html/vendor/allure-framework/allure-php-api/src/Yandex/Allure/Adapter/Event/AddAttachmentEvent.php(34): Magento\FunctionalTestingFramework\Allure\Event\AddUniqueAttachmentEvent->getAttachmentFileName()
[group329] #2 /var/www/html/vendor/allure-framework/allure-php-api/src/Yandex/Allure/Adapter/Allure.php(116): Yandex\Allure\Adapter\Event\AddAttachmentEvent->process()
[group329] #3 /var/www/html/vendor/allure-framework/allure-php-api/src/Yandex/Allure/Adapter/Allure.php(92): Yandex\Allure\Adapter\Allure->processStepEvent()
[group329] #4 /var/www/html/vendor/magento/magento2-functional-testing-framework/src/Magento/ in /var/www/html/vendor/magento/magento2-functional-testing-framework/src/Magento/FunctionalTestingFramework/Allure/Event/AddUniqueAttachmentEvent.php on line 34

This exception happened in multiple test groups.

Hello, @jilu1!
No, I haven’t checked it because we didn't change any business logic here:

Could you please take a look at it? What do you think, how it is possible that we broke something?

Thanks, Bohdan
@bohdan-harniuk

I did not see anything obviously also but the failing build is observed when I run 5 PRs (#839, #843, #844, #845, #852) in a batch. Please double-check from your side. I will look into it more.

jilu1 · 2021-07-30T19:35:20Z

@SilinMykola @bohdan-harniuk

FYI: The exception is reproducible running this PR branch alone.

…ing-framework into 33294-eliminate-aspectmock-from-allure-helpertest

karyna-t · 2021-08-04T10:06:40Z

Hi @jilu1 , I've just re-checked our PR. It seems that after the branch was actualized the problem that you described is gone. I don't see any errors on my local environment. Can you please check it one more time?

jilu1 · 2021-08-04T16:02:26Z

Hi @jilu1 , I've just re-checked our PR. It seems that after the branch was actualized the problem that you described is gone. I don't see any errors on my local environment. Can you please check it one more time?

@karyna-tsymbal-atwix
Have you run this PR with Magento platform-health branch? To reproduce it, you can create a Magento PR based on platform-health and use mftf from this PR. You should see the problem when running the full CE/EE/B2B build. It happens in some test groups but not all of them.

bohdan-harniuk · 2021-08-09T13:17:19Z

Hello, @jilu1 !

I've reproduced mentioned error! I'll fix it!

Thank you!

…ing-framework into 33294-eliminate-aspectmock-from-allure-helpertest

bohdan-harniuk · 2021-08-09T15:11:45Z

I've successfully reproduced mentioned issue:

My steps to reproduce:

Run any MFTF test without running WebDriver (ChromeDriver)
Got this error

After fixing this issue I got the correct output:

And successfully finished test:

@jilu1, could you please recheck if everything is fine?

Thank you,
Bohdan.

jilu1 · 2021-08-09T16:30:56Z

@magento import pull request to https://github.com/magento-commerce/magento2-functional-testing-framework

magento-engcom-team · 2021-08-09T16:31:38Z

@jilu1 the pull request successfully imported.

33294 eliminate aspectMock from allureHelperTest

9484d85

SilinMykola force-pushed the 33294-eliminate-aspectmock-from-allure-helpertest branch from dee081d to 9484d85 Compare June 27, 2021 19:35

jilu1 added accept community-contribution in progress and removed community-contribution labels Jun 28, 2021

jilu1 added the needs update label Jun 30, 2021

bohdan-harniuk self-requested a review July 19, 2021 12:41

bohdan-harniuk requested review from jilu1 and removed request for bohdan-harniuk and jilu1 July 20, 2021 08:55

bohdan-harniuk force-pushed the 33294-eliminate-aspectmock-from-allure-helpertest branch from d45efff to 804c7c2 Compare July 20, 2021 08:58

33294: Eliminated aspect mock from AllureHelperTest

1a71fe7

bohdan-harniuk force-pushed the 33294-eliminate-aspectmock-from-allure-helpertest branch from 804c7c2 to 1a71fe7 Compare July 20, 2021 09:04

andrewbess previously approved these changes Jul 20, 2021

View reviewed changes

jilu1 removed the needs update label Jul 21, 2021

bohdan-harniuk added Partner: Atwix partners-contribution Pull Request is created by Magento Partner labels Jul 21, 2021

bohdan-harniuk self-requested a review July 21, 2021 14:53

jilu1 reviewed Jul 21, 2021

View reviewed changes

bohdan-harniuk removed their request for review July 21, 2021 14:59

anzin mentioned this pull request Jul 26, 2021

33582: Eliminated AspectMock from MagentoTestCase.php #863

Merged

5 tasks

bohdan-harniuk removed the needs update label Jul 27, 2021

bohdan-harniuk self-assigned this Jul 27, 2021

jilu1 added in progress Award: advanced labels Jul 28, 2021

jilu1 suggested changes Jul 30, 2021

View reviewed changes

jilu1 added the needs update label Jul 30, 2021

Merge branch 'develop' of github.com:magento/magento2-functional-test…

f9da313

…ing-framework into 33294-eliminate-aspectmock-from-allure-helpertest

bohdan-harniuk added 2 commits August 9, 2021 17:45

MFTF-33294: Fixed strict type issue

d920627

Merge branch 'develop' of github.com:magento/magento2-functional-test…

0641788

…ing-framework into 33294-eliminate-aspectmock-from-allure-helpertest

bohdan-harniuk dismissed their stale review via 0641788 August 9, 2021 14:49

bohdan-harniuk requested review from jilu1, bohdan-harniuk and andrewbess August 9, 2021 15:13

bohdan-harniuk previously approved these changes Aug 9, 2021

View reviewed changes

jilu1 removed the needs update label Aug 9, 2021

magento-devops-reposync-svc dismissed bohdan-harniuk’s stale review via 0641788 August 9, 2021 20:02

andrewbess approved these changes Aug 10, 2021

View reviewed changes

magento-devops-reposync-svc merged commit 83e5cc0 into magento:develop Aug 11, 2021

magento-engcom-team removed the Award: advanced label Aug 11, 2021

SilinMykola deleted the 33294-eliminate-aspectmock-from-allure-helpertest branch October 23, 2021 11:40

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

33294 eliminate aspectmock from allureHelperTest #839

33294 eliminate aspectmock from allureHelperTest #839

SilinMykola commented Jun 27, 2021

jilu1 commented Jun 28, 2021

jilu1 commented Jun 28, 2021

jilu1 commented Jun 28, 2021

jilu1 commented Jun 30, 2021

sivaschenko commented Jul 14, 2021

SilinMykola commented Jul 14, 2021

bohdan-harniuk commented Jul 20, 2021

andrewbess left a comment

jilu1 commented Jul 21, 2021

magento-engcom-team commented Jul 21, 2021

jilu1 Jul 21, 2021

bohdan-harniuk Jul 21, 2021

jilu1 Jul 21, 2021 •

edited

Loading

jilu1 Jul 21, 2021

bohdan-harniuk Jul 21, 2021

bohdan-harniuk Jul 22, 2021

jilu1 left a comment

bohdan-harniuk commented Jul 30, 2021

jilu1 commented Jul 30, 2021

jilu1 commented Jul 30, 2021

karyna-t commented Aug 4, 2021

jilu1 commented Aug 4, 2021

bohdan-harniuk commented Aug 9, 2021

bohdan-harniuk commented Aug 9, 2021 •

edited

Loading

jilu1 commented Aug 9, 2021

magento-engcom-team commented Aug 9, 2021

33294 eliminate aspectmock from allureHelperTest #839

33294 eliminate aspectmock from allureHelperTest #839

Conversation

SilinMykola commented Jun 27, 2021

Description

Fixed Issues

Contribution checklist

jilu1 commented Jun 28, 2021

jilu1 commented Jun 28, 2021

jilu1 commented Jun 28, 2021

jilu1 commented Jun 30, 2021

sivaschenko commented Jul 14, 2021

SilinMykola commented Jul 14, 2021

bohdan-harniuk commented Jul 20, 2021

andrewbess left a comment

Choose a reason for hiding this comment

jilu1 commented Jul 21, 2021

magento-engcom-team commented Jul 21, 2021

jilu1 Jul 21, 2021

Choose a reason for hiding this comment

bohdan-harniuk Jul 21, 2021

Choose a reason for hiding this comment

jilu1 Jul 21, 2021 • edited Loading

Choose a reason for hiding this comment

jilu1 Jul 21, 2021

Choose a reason for hiding this comment

bohdan-harniuk Jul 21, 2021

Choose a reason for hiding this comment

bohdan-harniuk Jul 22, 2021

Choose a reason for hiding this comment

jilu1 left a comment

Choose a reason for hiding this comment

bohdan-harniuk commented Jul 30, 2021

jilu1 commented Jul 30, 2021

jilu1 commented Jul 30, 2021

karyna-t commented Aug 4, 2021

jilu1 commented Aug 4, 2021

bohdan-harniuk commented Aug 9, 2021

bohdan-harniuk commented Aug 9, 2021 • edited Loading

jilu1 commented Aug 9, 2021

magento-engcom-team commented Aug 9, 2021

jilu1 Jul 21, 2021 •

edited

Loading

bohdan-harniuk commented Aug 9, 2021 •

edited

Loading