33308: Eliminated AspectMock usage from ModuleResolverTest.php

[anzin]

Description

I've eliminated AspectMock usage from dev/tests/unit/Magento/FunctionalTestFramework/Util/ModuleResolverTest.php

Fixed Issues (if relevant)

1. Fixes magento/magento2#33308: [MFTF] Eliminate AspectMock from ModuleResolverTest (Complex!)

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]

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

[magento-engcom-team]

@jilu1 the pull request successfully imported.

[jilu1]

@anzin Thank you for your PR! Please address unit test failure in ModuleResolverTest.

[jilu1]

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

[magento-engcom-team]

@jilu1 the pull request successfully imported.

[anzin]

Hello @jilu1 , sorry I don't understand your question. I removed these tests because we eliminate AspectMock, and these tests are not necessary, or I misunderstood something?

Thank you.

[jilu1]

Hello @jilu1 , sorry I don't understand your question. I removed these tests because we eliminate AspectMock, and these tests are not necessary, or I misunderstood something?

Thank you.

@anzin Can you explain why these tests are not necessary? In other words, should you add these testAggregateTestModulePathsDevTests, testGetModulePathsLocations, testGetComposerJsonTestModulePathsForPathInvocation, testGetComposerInstalledTestModulePathsForPathInvocation as unit test for new class ModuleResolverService?

[anzin]

ModuleResolverService

Hello, @jilu1!

Yes, I will try to add them!

Thank you!

[anzin]

Hello @jilu1 , sorry I don't understand your question. I removed these tests because we eliminate AspectMock, and these tests are not necessary, or I misunderstood something? Thank you.

@anzin Can you explain why these tests are not necessary? In other words, should you add these testAggregateTestModulePathsDevTests, testGetModulePathsLocations, testGetComposerJsonTestModulePathsForPathInvocation, testGetComposerInstalledTestModulePathsForPathInvocation as unit test for new class ModuleResolverService?

@jilu1, ~could you please help me to understand what does that test do? -> testAggregateTestModulePathsDevTests... because of the same reason.~

I've found what exactly those tests do! I'll rewrite them without using AspectMock!

Thanks!

[anzin]

Hello, @jilu1, I've added new PR where returned deleted tests and fixed them so that they do not use AspectMock, please check if everything is well.

Thanks.

[anzin]

Hello @jilu1, my idea was to mock these methods because one test is did not passed, but then it turned out that it is not necessary to do so I put everything back.

Thanks.

[jilu1]

@anzin

I understand why you moved some methods back. But my above question is actually why you keep wrapper methods like:

    private function getComposerInstalledTestModulePaths($composerFile)
    {
        return ModuleResolverService::getInstance()->getComposerInstalledTestModulePaths($composerFile);
    }

    private function getCustomModulePaths()
    {
        return ModuleResolverService::getInstance()->getCustomModulePaths();
    }

while you can call ModuleResolverService::getInstance()->getComposerInstalledTestModulePaths($composerFile); or ModuleResolverService::getInstance()->getCustomModulePaths(); directly. Is it to help with unit test?

[anzin]

@jilu1 , I understood what you mean, I've corrected the code please check if everything is well.

Thanks for your review.

[anzin]

Hello @jilu1 , I've fixed test 'testGetModulePathsLocations', please check if everything is well.

Thanks.

[jilu1]

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

[magento-engcom-team]

@jilu1 the pull request successfully imported.