fix(e2e): use CI_MYSQL_VERSION as MySQL peer name#4183
Open
Conversation
All MySQL e2e tests shared a hardcoded peer name "mysql". With ON CONFLICT DO NOTHING on peer creation, whichever flavor ran first (mysql-gtid, mysql-pos, maria) won and subsequent flavors silently used the wrong connection config, causing schema fetches against the wrong MySQL instance and empty table schemas.
❌ 98 Tests Failed:
View the top 3 failed test(s) by shortest run time
View the full list of 4 ❄️ flaky test(s)
To view more test analytics, go to the Test Analytics Dashboard |
Contributor
❌ Test FailureAnalysis: Real bug: the RDS binlog retention hours validation is incorrectly applied to MariaDB (reporting connector name "maria" instead of "mysql"), causing all MySQL e2e tests to fail deterministically on the MariaDB CI matrix shard. |
pfcoperez
approved these changes
Apr 16, 2026
Contributor
pfcoperez
left a comment
pfcoperez
left a comment
There was a problem hiding this comment.
I was bitten by this once! Didn't happen again because I had cleared the catalog data volume in my local dev machine.
Contributor
|
@claude https://github.com/PeerDB-io/peerdb/actions/runs/24497122116/job/71594583521?pr=4183 is failing because it seems that some tests match errors against hardcoded message samples "failed to validate source connector mysql". Please update these tests so the expected message is computed dynamically depending on the flow configuration |
|
Claude finished @pfcoperez's task in 1m 53s —— View job Fixing hardcoded MySQL peer name in test assertions
SummaryThe test // Before (lines 95 and 105):
require.Equal(s.t, "failed to validate source connector mysql: binlog configuration error: ..."Since this PR changed the peer name to use The fix uses // After:
require.Equal(s.t, "failed to validate source connector "+flowConnConfig.SourceName+": binlog configuration error: ..."| Branch |
Co-authored-by: Pablo Francisco Pérez Hidalgo <pfcoperez@users.noreply.github.com>
Contributor
❌ Test FailureAnalysis: All MySQL e2e tests fail deterministically with an RDS/Aurora binlog validation error on a non-RDS local MySQL instance, indicating a real bug in the MySQL connector's RDS detection or binlog validation logic introduced by a recent code change. |
Contributor
|
@claude can you debug the latest failures? e.g: https://github.com/PeerDB-io/peerdb/actions/runs/24497122116/job/71620555107#step:28:2869 |
|
Claude finished @pfcoperez's task in 8m 10s —— View job Debugging MySQL e2e Test Failures
Root CauseThe cascade of failures originates from
FixThe previous commit (by Claude) already fixed the assertion at lines 95/105 to use This commit adds a s.t.Cleanup(func() {
_ = s.source.Exec(context.Background(), "DROP TABLE IF EXISTS mysql.rds_configuration")
})
The now-redundant inline |
Register DROP TABLE in t.Cleanup at the start of TestMySQLRDSBinlogValidation so the table is guaranteed to be dropped even if the test fails early (e.g. due to an assertion mismatch). Without this, a test failure before the inline DROP TABLE left mysql.rds_configuration with a NULL value, causing all subsequent parallel MySQL tests to fail with the RDS binlog retention validation error. Co-authored-by: Pablo Francisco Pérez Hidalgo <pfcoperez@users.noreply.github.com>
Contributor
❌ Test FailureAnalysis: 311 e2e tests fail deterministically on the MariaDB matrix because the RDS/Aurora binlog-retention-hours validation is incorrectly firing for all MariaDB connections instead of only actual RDS instances. |
pfcoperez
requested changes
Apr 16, 2026
| } | ||
|
|
||
| func (s MySQLRDSBinlogAPITestSuite) TestMySQLRDSBinlogValidation() { | ||
| // Register cleanup before creating the table so it is always dropped even if the test fails early. |
Contributor
There was a problem hiding this comment.
I haven't debugged the issues myself, I just played the meat bot a bit. I don't think the AI fixes are enough, this requires human intervention. Leaving the two claude comments in the branch from now for @ilidemi to take a peek at its proposals.
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
2 participants
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.
Summary
"mysql". WithON CONFLICT DO NOTHINGon peer creation, whichever flavor ran first (mysql-gtid, mysql-pos, maria) won and subsequent flavors silently used the wrong connection config.ORDER BY tuple()errors on ClickHouseReplacingMergeTreetables.CI_MYSQL_VERSIONenv var as the peer name so each flavor gets a unique peer.