Allow extending MySQL client config in the integration test framework - #315
Open
rhoerr wants to merge 4 commits into
Open
Allow extending MySQL client config in the integration test framework#315rhoerr wants to merge 4 commits into
rhoerr wants to merge 4 commits into
Conversation
This commit allows for extending the original client configuration (hard-coded) with an optional `etc/mysql-client-config.php` file) allowing to add or override entries.
Contributor
|
@rhoerr @jissereitsma asking your permission to bring the last array merge commit magento/magento2@9532bf8 I can do it but wanted to get your blessing before doing it. |
Member
Author
|
Go for it. Thanks @marcelmtz |
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.
Cherry-pick of magento/magento2#40410 (authored by Jisse Reitsma), plus a follow-up commit stripping trailing whitespace the patch introduced.
Problem
The integration test framework generates
--defaults-extra-filefor itsmysql/mysqldumpinvocations, hardcoded to exactly one[client]section containing user and password:There is no way to add anything else to it. Newer distro images ship MariaDB clients that enable TLS verification by default — Alpine 3.23, which is what
php:8.5-fpm-alpineis based on, is the case reported on the PR — and the integration suite then fails on the dump/restore step with no supported way to passssl=0,ssl-verify-server-cert=0, or any other client option.This is a developer-experience fix for running integration tests on PHP 8.5 era images, not a storefront fix. It touches only
dev/tests/integration/framework.Fix
Build the file from a nested array and merge in an optional
dev/tests/integration/etc/mysql-client-config.phpwhen present, so a developer can supply extra sections without patching the framework:The early
returnwhen the file already exists is also inverted out of the nesting, which is where the whitespace issue below came from.Follow-up commit
The upstream patch leaves a line of trailing whitespace after the new
return;—git amwarns about it on apply and Magento's staticLiveCodeTestflags it on changed files. Stripped in a separate commit so the cherry-pick itself stays faithful. Verified the two diffs are otherwise character-identical:Known limitation, worth a look before merge
array_merge()here is keyed by section name, so an extra file that defines its ownclientsection replaces the generated one rather than adding to it — the user and password would be dropped and the dump would fail to authenticate. In practice that means the escape hatch works cleanly for adding new sections ([mysqldump],[mysql]) but not for the arguably most obvious use, addingssl=0to[client], unless the developer re-specifies credentials themselves.I left this as upstream wrote it rather than fixing it here, since changing the merge semantics would diverge from the PR we are tracking. Flagging it because it will surprise whoever first reaches for this. A recursive merge, or documenting that the extra file must restate
user/passwordif it touches[client], would both resolve it.Backward compatibility
None. The generated file is byte-identical to before when no
mysql-client-config.phpexists, which is the default; the file is not shipped and must be created deliberately. Nothing outside the integration test framework reads this code.Tests
No automated test — this is integration-test scaffolding that only executes during an integration run, and the behaviour it changes is the contents of a generated defaults file. Verified by syntax check and by confirming the no-extra-file path is unchanged:
I did not run the integration suite, which needs a provisioned database this environment does not have.
Upstream status
magento/magento2#40410 is open, not merged —
Progress: pending review/Priority: P3, opened 2026-01-07 and kept current by the author through eight merges from2.4-develop, with no code review yet. Only the two authored commits are carried here; the six2.4-developmerges are omitted as they carry no content of their own. A second developer confirmed on the thread that this patch resolves the failure when moving fromphp8.4-fpm-alpine3.20tophp8.5-fpm-alpine3.23.🤖 Generated with Claude Code