fix(vfsevents): remove sleep-based races in vfsevents watcher tests

[funkyshu]

Summary

Fixes #340. Also folds in the fix for a CI-observed flake in s3events (was briefly its own PR, #342, now consolidated here since it's the same root-cause pattern).

gcsevents (fixes #340)

Same root cause as #339: TestEnhancedMetadata, TestOverwriteEventSuppression, and TestRetryBackoffTiming in contrib/vfsevents/watchers/gcsevents each start receiveWithRetry in a goroutine and then read test-local state (receivedEvent/receivedEvents) after a fixed time.Sleep(50ms), racing with the goroutine's write. Confirmed with go test -race -run TestGCSWatcherTestSuite ./watchers/gcsevents/... on main — all three fail reliably.

Since RetryConfig.Enabled defaults to false (zero value), receiveWithRetry short-circuits to a single receive() call and returns, matching each test's .Once() mock expectations.

s3events (CI flake found after merging #339)

CI on ubuntu + go1.26 hit:

FAIL: DeleteMessage(string,string)
FAIL: 1 out of 2 expectation(s) were met.

in TestEnhancedMetadata. That test only synchronized on the event handler firing (via an eventReceived channel), not on pollOnce's own completion — a residual race flagged (but not fixed at the time) during review of #339's fix for the sibling TestNonVersionedBucketMetadata test. On CI's scheduler, the subtest could finish (and trigger its mock-expectation teardown) before the background pollOnce goroutine's DeleteMessage call happened.

Changes

• All three gcsevents tests now wait on receiveWithRetry's own completion via an error channel (matching the hardened pattern from fix(vfsevents): remove sleep-based race in TestNonVersionedBucketMetadata #339, including its review feedback) instead of a fixed sleep, and assert the call returns without error. Extracted into a shared waitForReceive suite helper.
• s3events/TestEnhancedMetadata now uses the same completion-based synchronization as TestNonVersionedBucketMetadata, extracted into a shared waitForPoll suite helper.
• Both helpers use time.NewTimer + defer timer.Stop() (not time.After), per review feedback on this PR, to avoid leaving timers pending when the happy path wins the select.
• On the timeout path (either package), the context is canceled immediately (not just via the deferred cancel) and the goroutine gets a short, bounded grace window before the subtest fails — avoiding a lingering goroutine that could still touch the mock after the subtest has already failed.
• Updated contrib/vfsevents/CHANGELOG.md under [Unreleased].

Test plan

• go test -race -run TestGCSWatcherTestSuite -count=200 ./contrib/vfsevents/watchers/gcsevents/... — clean (previously failed on all three named subtests).
• go test -race -run TestS3WatcherTestSuite -count=300 ./contrib/vfsevents/watchers/s3events/... — clean.
• go test -run TestS3WatcherTestSuite -count=1000 ./contrib/vfsevents/watchers/s3events/... (no -race, to stress scheduling/timing rather than just data races) — clean.
• go test -race ./contrib/vfsevents/... — clean.
• go build ./... && go vet ./... for contrib/vfsevents — clean.
• golangci-lint run ./watchers/gcsevents/... ./watchers/s3events/... — only pre-existing, unrelated nolintlint findings (not touched by this PR).

Made with Cursor

[Copilot]

Pull request overview

This PR updates the contrib/vfsevents GCS watcher test suite to remove sleep-based synchronization and eliminate race detector failures by waiting for receiveWithRetry completion, aligning with the stabilization approach used previously in s3events. It also records the fix in the contrib changelog.

Changes:

• Replaced time.Sleep(50ms) waits in three gcsevents watcher tests with channel-based synchronization on receiveWithRetry completion.
• Added bounded timeout handling that cancels the context and attempts a short grace wait to reduce the chance of a lingering goroutine after subtest failure.
• Updated contrib/vfsevents/CHANGELOG.md under [Unreleased] to document the fix (issue #340).

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

File	Description
contrib/vfsevents/watchers/gcsevents/gcsevents_test.go	Removes sleep-based races by synchronizing on receiveWithRetry completion and adding time-bounded failure handling.
contrib/vfsevents/CHANGELOG.md	Documents the test race fix under [Unreleased].

Comments suppressed due to low confidence (1)

contrib/vfsevents/watchers/gcsevents/gcsevents_test.go:550

• time.After(2 * time.Second) in this select can’t be canceled when the done case wins, so repeated runs (e.g., -count=200) will accumulate a large number of pending timers. Prefer time.NewTimer + defer timer.Stop() so the timeout timer is cleaned up immediately on success.

		"eventTime":               "2023-01-01T12:00:00Z",
		"overwrittenByGeneration": "1111111111",
	}

	ctx := s.T().Context()

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.