Separate MySQL and MariaDB tests - #4517
Conversation
❌ 8 Tests Failed:
View the top 3 failed test(s) by shortest run time
To view more test analytics, go to the Test Analytics Dashboard |
❌ Test FailureAnalysis: A real workflow misconfiguration on the separate-mysql-mariadb-tests branch left DB_IMG/DB_ENV empty for some matrix combinations, causing |
🔄 Flaky Test DetectedAnalysis: All failures are MariaDB/MySQL test suites failing instantly at setup with "connection refused"/EOF errors against localhost:3306, indicating the MariaDB service container failed to start or be ready — an infrastructure flake, not a code bug. ✅ Automatically retrying the workflow |
❌ Test FailureAnalysis: The PR's restructured TestMySQLOnlyIntegration* SSH-tunnel tests in flow/connectors/mysql fail deterministically and identically across all three MySQL matrix jobs with instant "ssh: rejected: connect failed (Connection refused)" errors, indicating a real PR-introduced bug in the SSH-tunnel-to-MySQL test setup rather than a flaky/intermittent failure. |
❌ Test FailureAnalysis: Eight newly-added SSH-tunnel MySQL integration tests (TestMySQLOnlyIntegration*, nonexistent on main) fail 100% deterministically across both independent matrix jobs with "ssh: rejected: connect failed (Connection refused)" at initial connection, indicating a real setup/code defect in PR #4517's new tests rather than intermittent flakiness. |
dtunikov
left a comment
There was a problem hiding this comment.
looks good to me
i would maybe add smth to claude.md to make it more likely to catch cases when someone adds a test for mysql and forgets about mariadb (and vice versa), it mostly affects integration tests (not e2e suite):
func TestIntegrationGetTableSchemaCaseSensitiveIdentifiers(t *testing.T) {
for _, tc := range []struct {
name string
}{
{name: "mysql"},
{name: "mariadb"},
} {
t.Run(tc.name, func(t *testing.T) {
| func TestIntegrationMySQLGetMasterGTIDSet(t *testing.T) { | ||
| for _, tc := range []struct { | ||
| name string | ||
| }{ |
There was a problem hiding this comment.
Can't this struct:
{
{name: "mysql"},
{name: "mariadb"},
}Be factored out as a common constant or the table test structure factored out as a test helper?
This way we don't only stop repeating ourselves but also prevent accidentally missing one of the flavors.
There was a problem hiding this comment.
It can but we then would lose the IDE codelenses as the VS Code extensions are not that smart to match through the layers of indirection
pfcoperez
left a comment
There was a problem hiding this comment.
Proceeding to force merge and update the branch protection
Code reviewOne issue found. Checked for bugs and CLAUDE.md compliance. Dependency version change in
|
Addressed in #4518. When this one is merged, I thiiink that one should switch to |
Changes:
Also GitHub seems to not handle indentation diffs very well, most of the line changes are just subtest indentation