sqlreplay: fix unstable test TestLocalDirWatcher

[djshow832]

What problem does this PR solve?

Issue Number: close #1071

Problem Summary:

• Watcher callbacks write to the shared files map while Eventually checks concurrently read its entries, causing data races in both the initial-directory and newly-created-directory checks.
• The len(files) checks also run concurrently with map writes. These synchronization issues affect the local, mocked S3, and real S3 watcher tests.
• The 30 ms timeout leaves little room for scheduling delays or slow filesystem operations, making the tests flaky in CI.
• Assertion failures can skip cancel() and wg.Wait(), leaving watcher goroutines running after the test.
• The S3 mock allows only four polling calls, although the actual number depends on scheduling.
• Tests modify the global polling interval without restoring its previous value.

What is changed and how it works:

• Added mutex protection around callback writes, map lookups, and length checks in all affected watcher tests.
• Increased the assertion timeout to 5 seconds while preserving the short polling interval.
• Used the shared waitgroup helper and registered cleanup to cancel and wait for watcher goroutines, including on assertion failure.
• Moved watcher error assertions into cleanup, after the goroutine finishes.
• Removed the S3 mock’s fixed polling limit; assertions continue to verify the discovered directories.
• Restored the original global polling interval during test cleanup.

Check List

Tests

• Unit test
• Integration test
• Manual test (add detailed scripts or steps below)
• No code

Notable changes

• Has configuration change
• Has HTTP API interfaces change
• Has tiproxyctl change
• Other user behavior changes

Release note

Please refer to Release Notes Language Style Guide to write a quality release note.

None

Summary by CodeRabbit

• Tests
  • Improved directory watcher test reliability by replacing timing-dependent checks with deterministic lifecycle and synchronization handling.
  • Added thread-safe access to watched file data during tests.
  • Standardized test timeouts and ensured temporary polling configuration is restored after each test.
  • Updated S3 mock expectations to accommodate variable polling behavior.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Team

Run ID: c5c937a9-e19c-4355-844d-64d60e5bfa18

📥 Commits

Reviewing files that changed from the base of the PR and between 5f001eb and cadbefe.

📒 Files selected for processing (1)

• pkg/sqlreplay/store/dir_watcher_test.go

Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review.

---

📝 Walkthrough

Walkthrough

The directory watcher tests now use deterministic goroutine cleanup, mutex-protected file maps, restored polling intervals, and fixed five-second timeouts across local, mocked S3, and real S3 scenarios.

Changes

Directory watcher test stabilization

Layer / File(s)	Summary
Local watcher lifecycle and timing pkg/sqlreplay/store/dir_watcher_test.go	The local watcher test captures Watch errors, cancels through cleanup, protects the files map, restores the polling interval, and uses a fixed timeout.
Mocked S3 watcher lifecycle and polling pkg/sqlreplay/store/dir_watcher_test.go	The mocked S3 test synchronizes file access, captures watcher errors, restores the polling interval, accepts schedule-dependent poll counts, and uses a fixed timeout.
Real S3 watcher lifecycle and file checks pkg/sqlreplay/store/dir_watcher_test.go	The real S3 tests synchronize file access, capture watcher errors, restore the polling interval, and use fixed timeouts for normal and 2K-file scenarios.

Estimated code review effort: 2 (Simple) | ~10 minutes

Merge Risk: ⚪ Minimal · up to cadbe

Directory watcher tests now clean up background watchers deterministically, protect concurrent test state, and use stable timeouts without changing production behavior. No current merge-blocking risk is identified.

Poem

A rabbit reads each line,
The patch grows clear beneath the moon,
Small changes hop in place,
Tests guard the garden path,
Reviews bloom before the dawn.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 3 functions across 1 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description check	✅ Passed	The description identifies issue #1071, explains the instability causes, describes the test synchronization and cleanup changes, records unit-test coverage, and includes the release-note section.
Linked Issues check	✅ Passed	The changes address issue #1071 by fixing race conditions, timing sensitivity, goroutine cleanup, mock polling limits, and global polling-interval restoration in the directory watcher tests.
Out of Scope Changes check	✅ Passed	All changes are confined to directory watcher test stability and directly support the linked issue. No unrelated code changes are identified.
Title check	✅ Passed	The title clearly identifies the sqlreplay area and the main change: fixing the unstable TestLocalDirWatcher test.

• Fix all pre-merge checks with AI

✨ Finishing Touches 💡 1

🛠️ Fix failing CI checks 💡

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests

Warning

Some tools did not complete. Review the errors below.

🔧 golangci-lint (2.13.2)

Error: can't load config: unsupported version of the configuration: "" See https://golangci-lint.run/docs/product/migration-guide for migration instructions
The command is terminated due to an error: can't load config: unsupported version of the configuration: "" See https://golangci-lint.run/docs/product/migration-guide for migration instructions

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[codecov-commenter]

Codecov Report

✅ All modified and coverable lines are covered by tests.
⚠️ Please upload report for BASE (main@5f001eb). Learn more about missing BASE report.

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #1210   +/-   ##
=======================================
  Coverage        ?   67.80%           
=======================================
  Files           ?      151           
  Lines           ?    16883           
  Branches        ?        0           
=======================================
  Hits            ?    11447           
  Misses          ?     4670           
  Partials        ?      766           

Flag	Coverage Δ
unit	67.80% <ø> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[ti-chi-bot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: YangKeao

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details

Needs approval from an approver in each of these files:

• OWNERS [YangKeao]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[ti-chi-bot]

[LGTM Timeline notifier]

Timeline:

• 2026-09-07 08:39:11.309940085 +0000 UTC m=+1700586.481034190: ☑️ agreed by YangKeao.