33309: Eliminated AspectMock usage from TestGeneratorTest.php #850
Conversation
There was a problem hiding this comment.
Hello @anzin
Thank you for your contribution.
It looks well for me.
bohdan-harniuk
added
Partner: Atwix
partners-contribution
|
@magento import pull request to https://github.com/magento-commerce/magento2-functional-testing-framework |
|
@jilu1 the pull request successfully imported. |
|
@anzin |
bohdan-harniuk
force-pushed
the
improvement/mftf-33309-eliminate-aspect-mock-from-test-generator-test
branch
from
0721011 to
6ab27e6
Compare
bohdan-harniuk
force-pushed
the
improvement/mftf-33309-eliminate-aspect-mock-from-test-generator-test
branch
from
6ab27e6 to
43f9436
Compare
|
@magento import pull request to https://github.com/magento-commerce/magento2-functional-testing-framework |
|
@jilu1 the pull request successfully imported. |
| * @return void | ||
| * @throws TestFrameworkException | ||
| */ | ||
| public function create(string $filename, string $exportDirectory, string $testPhp): void |
There was a problem hiding this comment.
I like the idea of pulling some code out of TestGenerator, but I don't think that creating a singleton that hold no data for a single utility function makes sense.
I would sooner that they change the visibility of createCestFile to public and mock that method instead. This is a lot of code to introduce to mock a single method.
There was a problem hiding this comment.
@KevinBKozan, as for me, in general, the composition is much better than big classes with a variety of functions.
@jilu1, what do you think about that concrete case?
There was a problem hiding this comment.
@bohdan-harniuk Yeah, I see your point in that. I'm just vigilant about creating any classes that will perpetually live in memory, but I suppose that's not much of a concern if there's not a large amount of data being held.
I'm happy to defer to @jilu1 's original review in this case, I'll approve this and we can merge it.
Description
I've eliminated AspectMock usage from
dev/tests/unit/Magento/FunctionalTestFramework/Util/TestGeneratorTest.php.Fixed Issues (if relevant)
Contribution checklist