fix flaky test. #1663
fix flaky test. #1663
Conversation
WalkthroughThe pull request modifies the Changes
Possibly related PRs
Suggested reviewers
Poem
✨ Finishing Touches
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
driver_test.go (1)
Line range hint
3611-3638: Consider adding debug logging for better test observability.The fix looks good, but consider these improvements for better debugging and maintainability:
- Add debug logging when the count is reduced due to max_connections limit.
- Add a comment explaining why we need to respect max_connections.
Here's a suggested improvement:
var max int err := dbt.db.QueryRow("SELECT @@max_connections").Scan(&max) if err != nil { dbt.Fatalf("%s", err.Error()) } // disable connection pooling. // data race happens when new connection is created. dbt.db.SetMaxIdleConns(0) // estimate round trip time. start := time.Now() if err := dbt.db.PingContext(context.Background()); err != nil { t.Fatal(err) } rtt := time.Since(start) if rtt <= 0 { // In some environments, rtt may become 0, so set it to at least 1ms. rtt = time.Millisecond } count := 1000 if testing.Short() { count = 10 } +// Ensure we don't exceed the server's max_connections limit to prevent +// "Too many connections" errors that can make the test flaky if count > max { + t.Logf("Reducing connection count from %d to %d due to server's max_connections limit", count, max) count = max }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
driver_test.go(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (15)
- GitHub Check: test (windows-latest, 1.23, mariadb-10.5)
- GitHub Check: test (windows-latest, 1.23, mariadb-10.6)
- GitHub Check: test (windows-latest, 1.23, mariadb-10.11)
- GitHub Check: test (windows-latest, 1.23, mariadb-11.1)
- GitHub Check: test (windows-latest, 1.23, mariadb-11.2)
- GitHub Check: test (windows-latest, 1.23, mariadb-11.4)
- GitHub Check: test (windows-latest, 1.23, 5.7)
- GitHub Check: test (windows-latest, 1.23, 8.0)
- GitHub Check: test (windows-latest, 1.23, 8.4)
- GitHub Check: test (windows-latest, 1.23, 9.0)
- GitHub Check: test (macos-latest, 1.23, mariadb-10.5)
- GitHub Check: test (macos-latest, 1.23, mariadb-10.6)
- GitHub Check: test (macos-latest, 1.23, mariadb-10.11)
- GitHub Check: test (macos-latest, 1.23, mariadb-11.1)
- GitHub Check: test (macos-latest, 1.23, mariadb-11.4)
🔇 Additional comments (2)
driver_test.go (2)
3611-3615: LGTM! Good addition to prevent connection limit issues.Querying the server's max_connections limit is a good approach to ensure the test doesn't exceed the server's capacity.
3636-3638: LGTM! Good safeguard against connection limit exceeded errors.Capping the test count to max_connections ensures the test won't attempt to create more connections than the server allows, fixing the flaky behavior.
Description
TestIssue1567 fails by max_connections error. This makes our CI unhappy.
https://github.com/go-sql-driver/mysql/actions/runs/12904961433/job/35984402310
Checklist
Summary by CodeRabbit