Remove wrong test expectation

[markshust]

Description (*)

Removing wrong test expectation which fails unit tests

Fixed Issues (if relevant)

1. Fixes Fails and warnings while running unit tests #12419

Manual testing scenarios (*)

1. Run bin/magento dev:tests:run unit, which fails with:

There was 1 failure:

1) Magento\ProductVideo\Test\Unit\Controller\Adminhtml\Product\Gallery\RetrieveImageTest::testExecute
Expectation failed for method name is equal to <string:getDirectoryWrite> when invoked 1 time(s).
Method was expected to be called 1 times, actually called 0 times.

Contribution checklist (*)

• [ x ] Pull request has a meaningful description of its purpose
• [ x ] All commits are accompanied by meaningful commit messages
• [ x ] All new or changed code is covered with unit/integration tests (if applicable)
• [ x ] All automated tests passed successfully (all builds on Travis CI are green)

[magento-engcom-team]

Hi @markoshust. Thank you for your contribution
Here is some useful tips how you can test your changes using Magento test environment.
Add the comment under your pull request to deploy test or vanilla Magento instance:

• @magento-engcom-team give me test instance - deploy test instance based on PR changes
• @magento-engcom-team give me $VERSION instance - deploy vanilla Magento instance

For more details, please, review the Magento Contributor Assistant documentation

[rogyar]

Hi @markoshust. Thank you for your contribution. The getDirectoryWrite method is being called in the original method. As you can see, if we don't mock this case the unit test attempts to call isExist method on the non-existing variable.

[sidolov]

Hi @markoshust , I am closing this PR now due to inactivity.
Please reopen and update if you wish to continue.
Thank you for the collaboration!

[HenKun]

There is a problem with this test.

1. Given:

The line $this->filesystemMock->expects($this->once())->method('getDirectoryWrite')->willReturn($writeInterface); is NOT removed, like before the commit.

Result:

a) If all unit tests are run together, the test fails, because getDirectoryWrite() is not called while it was expected to be called once.

b) If I only run this test separately, then the test passes, because an exception occurs: Warning: filesize(): stat failed for /t/e/test.jpg in /home/cloudpanel/htdocs/nebelkind.dev/vendor/magento/module-product-video/Controller/Adminhtml/Product/Gallery/RetrieveImage.php:184., which results in getDirectoryWrite() getting called once.

2. Given:

The line $this->filesystemMock->expects($this->once())->method('getDirectoryWrite')->willReturn($writeInterface); is removed, like after the commit.

Result:

a) If all unit tests are run together, the test succeeds, because getDirectoryWrite() is not checked.

b) If I only run this test separately, then the test fails, because an exception occurs: Error : Call to a member function isExist() on null, like @rogyar already mentioned. This is because like in 1b the exception Warning: filesize(): stat failed for /t/e/test.jpg in /home/cloudpanel/htdocs/nebelkind.dev/vendor/magento/module-product-video/Controller/Adminhtml/Product/Gallery/RetrieveImage.php:184. occurs which leads to getDirectoryWrite() beeing called, which in turn is now not mocked anymore.

Conclusion

1. It seems the test is not consistent, it depends on the way the test is run. This might be the reason the failing test was not obvious in Travis.

2. The test testExecute() should also check the return value of $this->image->execute();, the error gets obvious if the 'error' key of the returned array is checked. Currently the test just asserts that no exception is thrown, but a failure in the method is encoded in the return value.

3. As an alternative, the line in this commit should not be removed, but modified to $this->filesystemMock->method('getDirectoryWrite')->willReturn($writeInterface);, but this more or less just hides the problem with this test.