Fix invalid preferences

[fredden]

Description

These preferences are defined in XML but the classes they reference do not exist (in production mode). This is blocking progress on magento/magento2#33161

Fixed Issues

• Fixes #296

Manual testing scenarios

No visible changes should be apparent with this change.

Questions or comments

None

Contribution checklist

• [x] Author has signed the Adobe CLA
• [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)
• [ ] All automated tests passed successfully (all builds are green)

[magento-automated-testing[bot]]

The requested builds are added to the queue. You should be able to see them here within a few minutes. Please re-request them if they don't show in a reasonable amount of time.

[magento-automated-testing[bot]]

The requested builds are added to the queue. You should be able to see them here within a few minutes. Please re-request them if they don't show in a reasonable amount of time.

[magento-automated-testing[bot]]

The requested builds are added to the queue. You should be able to see them here within a few minutes. Please re-request them if they don't show in a reasonable amount of time.

[nathanjosiah]

@fredden Thank you for your contribution! I think the PR looks ok except for a comment I left in the di.xml file. Please take a look at that and I think we can move this forward.

[magento-automated-testing[bot]]

The requested builds are added to the queue. You should be able to see them here within a few minutes. Please re-request them if they don't show in a reasonable amount of time.

[nathanjosiah]

@magento run all tests with 2.4.3-develop

[magento-automated-testing[bot]]

The requested builds are added to the queue. You should be able to see them here within a few minutes. Please re-request them if they don't show in a reasonable amount of time.

[nathanjosiah]

@magento run all tests against 2.4.3-develop

[magento-automated-testing[bot]]

The requested builds are added to the queue. You should be able to see them here within a few minutes. Please re-request them if they don't show in a reasonable amount of time.

[nathanjosiah]

@fredden Looks like there are still some errors in the integration tests and the static tests.

[nathanjosiah]

@fredden If you want to work on this let me know, there are still valid failures in the builds which I have highlighted. I will close this for now.

[fredden]

@nathanjosiah Please can you re-open this. I don't seem to have a button to do so myself. This definitely needs to be fixed.

The error showing up that seems related to this change is:

Data set: app/code/Magento/TwoFactorAuth/Plugin/BypassTwoFactorAuthForTestFramework.php Module Magento\TwoFactorAuth has external dependencies: hard [Magento\TestFramework] /var/www/html/dev/tests/static/testsuite/Magento/Test/Integrity/DependencyTest.php:572 /var/www/html/lib/internal/Magento/Framework/App/Utility/AggregateInvoker.php:56 /var/www/html/dev/tests/static/testsuite/Magento/Test/Integrity/DependencyTest.php:575

It doesn't feel right to put a runtime dependency on the test framework. The required class is distributed as part of magento/magento2-base. What's the best approach for resolving this?

I can see that the same test is reporting a number of similar issues in other modules. Perhaps that's something that should be resolved in a separate pull request?

Module Magento\ReCaptchaCheckout has undeclared dependencies: hard [Magento\QuoteGraphQl] Module Magento\ReCaptchaCustomer has undeclared dependencies: hard [Magento\CustomerGraphQl, Magento\Integration] Module Magento\ReCaptchaNewsletter has undeclared dependencies: hard [Magento\NewsletterGraphQl] Module Magento\ReCaptchaPaypal has undeclared dependencies: hard [Magento\PaypalGraphQl] Module Magento\ReCaptchaReview has undeclared dependencies: hard [Magento\ReviewGraphQl] Module Magento\ReCaptchaSendFriend has undeclared dependencies: hard [Magento\SendFriendGraphQl]

[nathanjosiah]

@fredden all of those errors are false-positives. There are no undocumented hard dependencies, if you look at the code for those there are runtime checks for classes to see if they are available first. It's a known issue but one we can safely ignore. You are welcome to continue to contribute to this issue, let's proceed by pulling in the latest develop and run all the tests again. The correct mainline version should be used by default now.

[magento-automated-testing[bot]]

The requested builds are added to the queue. You should be able to see them here within a few minutes. Please re-request them if they don't show in a reasonable amount of time.

[fredden]

It looks like there are more out-of-scope test failures. It's hard to tell if there are any genuine test failures due to the changes within this pull request. How should we proceed?

[magento-automated-testing[bot]]

The requested builds are added to the queue. You should be able to see them here within a few minutes. Please re-request them if they don't show in a reasonable amount of time.

[nathanjosiah]

@magento run all tests against 2.4.3

[magento-automated-testing[bot]]

The requested builds are added to the queue. You should be able to see them here within a few minutes. Please re-request them if they don't show in a reasonable amount of time.

[nathanjosiah]

@magento run all tests against 2.4.4

[magento-automated-testing[bot]]

The requested builds are added to the queue. You should be able to see them here within a few minutes. Please re-request them if they don't show in a reasonable amount of time.

[magento-automated-testing[bot]]

The requested builds are added to the queue. You should be able to see them here within a few minutes. Please re-request them if they don't show in a reasonable amount of time.

[nathanjosiah]

@fredden Looks like everything is green except the static test (https://public-results-storage-prod.magento-testing-service.engineering/reports/magento/security-package/pull/297/72113ff36dd11533a11b1852965b0149/Statics/allure-report-ce/index.html#categories/ace6c7944747e2a66397aa9d447b7430/fa91d33adabf7a98/) which doesn't like the hard dep in your new plugin.

app/code/Magento/TwoFactorAuth/Plugin/BypassTwoFactorAuthForTestFramework.php Module Magento\TwoFactorAuth has external dependencies: hard [Magento\TestFramework] /var/www/html/dev/tests/static/testsuite/Magento/Test/Integrity/DependencyTest.php:572 /var/www/html/lib/internal/Magento/Framework/App/Utility/AggregateInvoker.php:56 /var/www/html/dev/tests/static/testsuite/Magento/Test/Integrity/DependencyTest.php:575

[magento-automated-testing[bot]]

The requested builds are added to the queue. You should be able to see them here within a few minutes. Please re-request them if they don't show in a reasonable amount of time.

[magento-automated-testing[bot]]

The requested builds are added to the queue. You should be able to see them here within a few minutes. Please re-request them if they don't show in a reasonable amount of time.

[fredden]

I've changed the namespace for the plugin to see if this makes the static test happier. This doesn't seem to change the functionality any. (ie, this still fixes the bug in this module, and is compatible with the setup:di:compile check/safe-guard being introduced in the linked pull request.) I don't know if this will fix the static test or not; I suspect there's logic to ignore certain paths as the original Observer has exactly the same dependencies on the TestFramework.

I've triggered test runs for v2.4.4 (as @nathanjosiah has done previously) and the main branches (from the linked pull request so all the other related pull requests are included in the build).

[nathanjosiah]

The 2.4.4 tests are all green and the DB compare job is failing here and here but they are both about the magento_giftregistry_data table which is clearly not from this PR. I am pulling this ticket in for QA and internal processing.

[magento-engcom-team]

@nathanjosiah, an error occurred during the Pull Request import.

[nathanjosiah]

@magento import code to https://github.com/magento-commerce/security-package

[magento-engcom-team]

@nathanjosiah the branch with code successfully imported intomagento-commerce/security-package repository. Branch name: imported-magento-security-package-297.

[magento-engcom-team]

@nathanjosiah the branch with code successfully imported intomagento-commerce/security-package repository. Branch name: imported-magento-security-package-297.

[magento-automated-testing[bot]]

The requested builds are added to the queue. You should be able to see them here within a few minutes. Please re-request them if they don't show in a reasonable amount of time.

[magento-automated-testing[bot]]

The requested builds are added to the queue. You should be able to see them here within a few minutes. Please re-request them if they don't show in a reasonable amount of time.

[magento-automated-testing[bot]]

The requested builds are added to the queue. You should be able to see them here within a few minutes. Please re-request them if they don't show in a reasonable amount of time.

[magento-automated-testing[bot]]

The requested builds are added to the queue. You should be able to see them here within a few minutes. Please re-request them if they don't show in a reasonable amount of time.

[magento-automated-testing[bot]]

The requested builds are added to the queue. You should be able to see them here within a few minutes. Please re-request them if they don't show in a reasonable amount of time.

[magento-automated-testing[bot]]

The requested builds are added to the queue. You should be able to see them here within a few minutes. Please re-request them if they don't show in a reasonable amount of time.

[magento-automated-testing[bot]]

The requested builds are added to the queue. You should be able to see them here within a few minutes. Please message the #magento-devops slack channel if they don't show in a reasonable amount of time and a representative will look into any issues.