-
Notifications
You must be signed in to change notification settings - Fork 29
#42 - Dynamically fill the image keys #94
#42 - Dynamically fill the image keys #94
Conversation
@VincentMarmiesse thank you for this PR. Would you be able to have a look at the failing unit tests. |
@dmanners Ok tests are failing because of the Thank you for your help. |
@VincentMarmiesse yup you are right about this. These methods and objects will need to be mocked correctly. I think the problem is that the select mock is only expected to be called once per test but now with your change it will need to be setup to run multiple times per test. Without knowing too much about the full example I think you will need to setup another expects such as the one found in https://github.com/magento-engcom/import-export-improvements/blob/2.3-develop/app/code/Magento/CatalogImportExport/Test/Unit/Model/Import/ProductTest.php#L526 but with the correct information passed into the Do you think you would be able to work from that? Otherwise I could always fix one test and see if you can work that fix into all the other tests? |
…tually use the database to load the image information
… remove the _ in the name
Hi @VincentMarmiesse I have refactored the code a bit to create a new image type processor. Basically I needed to do this to make the test easier to fix. What do you think to that approach? Also could you please double check that my changes are working as you desire. Thanks |
Looks good for me. Do we have unit test with new image attribute? How will this behave in the import CSV structure? New column will be added or will it import with additional_attributes. I think we need some more explanation about this issue. What about the path of the image. Thanks! |
@VincentMarmiesse thank you for contributing. Please accept Community Contributors team invitation here to gain extended permissions for this repository. |
Hi @dmanners, Sorry for the delay, I like this approch and it works for me. I'm kind of new to tests writing and didn't see how to test it but your solution seems nice! @PieterCappelle, in my CSV file I want to import, I have a column for each image, so @magento-engcom-team, ok I have accepted the invitation, should I do something more? |
Hello,
Here is a fix for the issue #42, I am open to discussion if necessary.