magento2
magento2 copied to clipboard
Product layout based on attribute_set (PR #36244)
follow up: https://github.com/magento/magento2/pull/36244#issuecomment-2705671813
Related: https://github.com/magento/magento2/issues/102#issuecomment-826058850 https://github.com/Vendic/module-optimize-cache-size/tree/main https://github.com/magento/magento2/issues/189#issuecomment-698430804
Hi @in-session. Thank you for your contribution! Here are some useful tips on how you can test your changes using Magento test environment. :exclamation: Automated tests can be triggered manually with an appropriate comment:
@magento run all tests- run or re-run all required tests against the PR changes@magento run <test-build(s)>- run or re-run specific test build(s) For example:@magento run Unit Tests
<test-build(s)> is a comma-separated list of build names.
Allowed build names are:
Database CompareFunctional Tests CEFunctional Tests EEFunctional Tests B2BIntegration TestsMagento Health IndexSample Data Tests CESample Data Tests EESample Data Tests B2BStatic TestsUnit TestsWebAPI TestsSemantic Version Checker
You can find more information about the builds here :information_source: Run only required test builds during development. Run all test builds before sending your pull request for review.
For more details, review the Code Contributions documentation. Join Magento Community Engineering Slack and ask your questions in #github channel.
@magento run all tests
Static test will be fixed after test run:
<!--
/**
* Copyright 2014 Adobe
* All Rights Reserved.
*/
-->
PHP Code Sniffer detected 1 violation(s): FILE: /var/www/html/app/code/Magento/Catalog/etc/adminhtml/system.xml -------------------------------------------------------------------------- FOUND 0 ERRORS AND 1 WARNING AFFECTING 1 LINE -------------------------------------------------------------------------- | WARNING | Copyright is missing or Copyright content/year is not valid -------------------------------------------------------------------------- FILE: /var/www/html/app/code/Magento/Catalog/etc/config.xml -------------------------------------------------------------------------- FOUND 0 ERRORS AND 1 WARNING AFFECTING 1 LINE -------------------------------------------------------------------------- | WARNING | Copyright is missing or Copyright content/year is not valid -------------------------------------------------------------------------- Failed asserting that 1 matches expected 0.
1 PHP Code Sniffer detected 2 violation(s): FILE: /var/www/html/app/code/Magento/Catalog/Helper/Product/View.php -------------------------------------------------------------------------- FOUND 0 ERRORS AND 4 WARNINGS AFFECTING 4 LINES -------------------------------------------------------------------------- 179 | WARNING | [x] Whitespace found at end of line 184 | WARNING | [x] Whitespace found at end of line 193 | WARNING | [ ] Line exceeds 120 characters; contains 122 characters 213 | WARNING | [ ] Line exceeds 120 characters; contains 122 characters -------------------------------------------------------------------------- PHPCBF CAN FIX THE 2 MARKED SNIFF VIOLATIONS AUTOMATICALLY -------------------------------------------------------------------------- Failed asserting that 2 matches expected 0.
Semantic Version Checker because of XML changes:
System changes (MINOR) | FailureDetailsLevelTarget/LocationCode/ReasonMINORdev/layout_settings/enable_id_handle/app/code/Magento/Catalog/etc/adminhtml/system.xml:0M302 A field-node was addedMINORdev/layout_settings/enable_attribute_set_handle/app/code/Magento/Catalog/etc/adminhtml/system.xml:0M302 A field-node was added | Level | Target/Location | Code/Reason | MINOR | dev/layout_settings/enable_id_handle/app/code/Magento/Catalog/etc/adminhtml/system.xml:0 | M302 A field-node was added | MINOR | dev/layout_settings/enable_attribute_set_handle/app/code/Magento/Catalog/etc/adminhtml/system.xml:0 | M302 A field-node was added -- | -- | -- | -- | -- | -- | -- | -- | -- | -- | --MINOR | dev/layout_settings/enable_id_handle/app/code/Magento/Catalog/etc/adminhtml/system.xml:0 | M302 A field-node was added MINOR | dev/layout_settings/enable_attribute_set_handle/app/code/Magento/Catalog/etc/adminhtml/system.xml:0 | M302 A field-node was added
@magento run all tests
@magento run all tests
@magento run Functional Tests B2B, Functional Tests CE, Functional Tests EE
@magento run all tests
@magento run all tests
@engcom-Charlie PR is ready for review.
- Functional tests are not related to the PR
- Semantic Version Checker due to core XML changes.
Hello @in-session,
Thank you for your contribution!
As this PR is marked as a feature request, we require PO approval to proceed. We have initiated an internal discussion regarding this PR and will be placing it on hold for the time being.
@magento run all tests
@magento run all tests
Hello @in-session,
We have received approval on this PR from the PO. Therefore, we will proceed with further activities related to it.
Thanks
@magento run all tests
@magento run all tests
@magento run all tests
@engcom-Hotel
Unfortunately, I'm not really familiar with the tests. Do you mean something like: dev/tests/integration/testsuite/Magento/Catalog/Helper/Product/ViewLayoutHandleTest.php
Hello @in-session,
Thank you for implementing these tests - they look good! To ensure complete test coverage, could you please add two additional test methods that verify the mixed configuration states:
- When only ID handle is enabled (
enable_id_handle=1, enable_attribute_set_handle=0) - When only attribute set handle is enabled (
enable_id_handle=0, enable_attribute_set_handle=1)
This will ensure all possible configuration combinations are verified.
Thanks
@engcom-Hotel I hope that's OK. Please confirm again.
@magento run all tests
@magento run all tests
@magento run all tests
@magento run all tests
@magento run all tests
@magento run Functional Tests B2B, Static Tests
@magento run all tests
@magento run all tests
@magento run Functional Tests B2B, Functional Tests EE
@magento run Functional Tests B2B