search

sourcegraph / sourcegraph-public-snapshot

Code AI platform with Code Search & Cody
https://sourcegraph.com
Other
10.11k stars 1.27k forks source link

all: use context.AfterFunc #64510

Closed keegancsmith closed 4 weeks ago

keegancsmith commented 1 month ago

This is the result of updating the relevant callsites from

git grep '<-.*Done' | grep -v case

to use context.AfterFunc instead of directly spinning up a goroutine to wait for Done.

Test Plan: close code review and CI

keegancsmith commented 1 month ago

The test failures is in TestPeriodicGoroutine. I can't reproduce locally (even with -count=100 and -race), and in the test dashboard this test is marked as flaky https://buildkite.com/organizations/sourcegraph/analytics/suites/sourcegraph-bazel/tests/45aca421-f657-82a0-a07f-a5c37dd56de2?branch=main#flaky

There isn't any recent test runs it failed, but given it hardly ever changes and bazel is good at caching, looks like we have only tried to run it 5 times in the last month.

So I read the source code for the test, and its pretty clearly flakey to me since it relies on the goruntime being slow at notifying the periodic goroutine to be called. I'll see if I can improve the test, but otherwise I am just gonna rerun and see what happens.