Prevent Web API functional test installation from dropping an unrelated database (#36291) - #41094
Open
lbajsarowicz wants to merge 1 commit into
Open
Conversation
…ed database Web API functional tests do not use a dedicated configuration directory - WebApiApplication::getCustomDirs() returns an empty array, so app/etc of the installation that serves TESTS_BASE_URL is used. Both setup:uninstall and setup:install therefore act on whatever app/etc/env.php points at, not on the database declared in TESTS_INSTALL_CONFIG_FILE. With TESTS_MAGENTO_INSTALLATION enabled and a test database that differs from the deployed one, the bootstrap silently drops all tables of the deployed database. Abort the installation with an actionable message when the configured test database differs from the one of the already installed application. When both match, behavior is unchanged. Fixes magento#36291
|
Hi @lbajsarowicz. Thank you for your contribution!
Allowed build names are:
You can find more information about the builds here For more details, review the Code Contributions documentation. |
Contributor
Author
|
@magento run all tests |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Running the Web API functional tests with
TESTS_MAGENTO_INSTALLATION=enableddrops all tables of the live database instead of the test database configured indev/tests/api-functional/config/install-config-mysql.php.Root cause
WebApiApplication::getCustomDirs()returns an empty array, so – unlike the integration tests framework, which installs into its owndev/tests/integration/tmp/sandbox-*/etc– the Web API framework resolves its configuration directory toapp/etcof the installation that servesTESTS_BASE_URL.As a result:
Application::cleanup()runsbin/magento setup:uninstallwith--magento-init-paramsthat carry no directory overrides, so the command bootstraps the deployedapp/etc/env.phpand drops every table of the database that file points at — the developer's main database, notmagento_functional_tests.WebApiApplication::install()then runssetup:install, which rewrites that sameapp/etc/env.php(including the encryption key) to the test database.Both steps are inherent to the design: the tests send HTTP requests to the installation under
TESTS_BASE_URL, so the framework must install in place. What is not acceptable is doing that silently against a database the test configuration never names.Fix
WebApiApplication::install()now aborts before any destructive command when the database inTESTS_INSTALL_CONFIG_FILEdiffers from the database of the already installed application, with a message naming both databases and the two ways forward (align the install config, or setTESTS_MAGENTO_INSTALLATIONtodisabled).When the two databases match — the case in CI and in a correctly configured local setup — nothing changes:
setup:uninstallandsetup:installrun exactly as before.Fixed Issues (if relevant)
Manual testing scenarios
Data loss is prevented (mismatching databases)
Install Magento into database
magentoand confirm the tables exist.Copy
dev/tests/api-functional/config/install-config-mysql.php.disttoinstall-config-mysql.php, leavingdb-nameasmagento_functional_tests.Copy
phpunit_graphql.xml.disttophpunit_graphql.xmland setTESTS_MAGENTO_INSTALLATIONtoenabled(TESTS_CLEANUPstaysenabled).Run
cd dev/tests/api-functional && ../../../vendor/bin/phpunit -c phpunit_graphql.xml.Before the fix: the bootstrap runs
setup:uninstalland all tables ofmagentoare dropped.After the fix: the bootstrap fails immediately with
and the
magentodatabase is untouched (verified: table count unchanged).No false positive (matching databases)
db-nameininstall-config-mysql.phpto the database of the installed application.setup:uninstall,setup:install,indexer:set-mode realtimeandindexer:reindexin the same order as before this change.Installation disabled (default)
With
TESTS_MAGENTO_INSTALLATION=disabled(the shipped default)install()is never called, so the guard is not reached and existing workflows are unaffected.Questions or comments
The guard compares database names only. Host comparison was deliberately left out to avoid false positives from equivalent spellings (
localhostvs127.0.0.1) that would break existing setups.No automated test is included:
dev/tests/api-functional/frameworkis not covered by any registered PHPUnit suite (dev/tests/unit/phpunit.xml.distdoes not include it, and the framework has notests/unitharness of its own, unlikedev/tests/integration/framework). Both branches of the guard were verified manually as described above. Happy to add a suite for the Web API framework if maintainers want that scope.Contribution checklist (*)