test: replcompletions: Replace timedwait by proper condvar

Replace fragile timing-based synchronization with proper condition
variable signaling to ensure PATH cache updates complete before
testing completions. This eliminates test flakiness on systems
where the cache update takes longer than 5s.

The test failure was seen in CI:
https://buildkite.com/julialang/julia-master/builds/48273

Author: Keno

stdlib/REPL/src/REPLCompletions.jl

@@ -330,7 +330,8 @@ end
 
 const PATH_cache_lock = Base.ReentrantLock()
 const PATH_cache = Set{String}()
-PATH_cache_task::Union{Task,Nothing} = nothing # used for sync in tests
+PATH_cache_task::Union{Task,Nothing} = nothing
+PATH_cache_condition::Union{Threads.Condition, Nothing} = nothing # used for sync in tests
 next_cache_update::Float64 = 0.0
 function maybe_spawn_cache_PATH()
     global PATH_cache_task, next_cache_update
@@ -339,7 +340,11 @@ function maybe_spawn_cache_PATH()
         time() < next_cache_update && return
         PATH_cache_task = Threads.@spawn begin
             REPLCompletions.cache_PATH()
-            @lock PATH_cache_lock PATH_cache_task = nothing # release memory when done
+            @lock PATH_cache_lock begin
+                next_cache_update = time() + 10 # earliest next update can run is 10s after
+                PATH_cache_task = nothing # release memory when done
+                PATH_cache_condition !== nothing && notify(PATH_cache_condition)
+            end
         end
         Base.errormonitor(PATH_cache_task)
     end
@@ -350,8 +355,6 @@ function cache_PATH()
     path = get(ENV, "PATH", nothing)
     path isa String || return
 
-    global next_cache_update
-
     # Calling empty! on PATH_cache would be annoying for async typing hints as completions would temporarily disappear.
     # So keep track of what's added this time and at the end remove any that didn't appear this time from the global cache.
     this_PATH_cache = Set{String}()
@@ -414,7 +417,6 @@ function cache_PATH()
 
     @lock PATH_cache_lock begin
         intersect!(PATH_cache, this_PATH_cache) # remove entries from PATH_cache that weren't found this time
-        next_cache_update = time() + 10 # earliest next update can run is 10s after
     end
 
     @debug "caching PATH files took $t seconds" length(pathdirs) length(PATH_cache)

stdlib/REPL/test/replcompletions.jl

@@ -1073,6 +1073,39 @@ let c, r, res
     @test res === false
 end
 
+# A pair of utility function for the REPL completions test to test PATH_cache
+# dependent completions, which ordinarily happen asynchronously.
+# Only to be used from the test suite
+function test_only_arm_cache_refresh()
+    @lock REPL.REPLCompletions.PATH_cache_lock begin
+        @assert REPL.REPLCompletions.PATH_cache_condition === nothing
+
+        # Arm a condition we can wait on
+        REPL.REPLCompletions.PATH_cache_condition = Threads.Condition(REPL.REPLCompletions.PATH_cache_lock)
+
+        # Check if the previous update is still running - if so, wait for it to finish
+        while REPL.REPLCompletions.PATH_cache_task !== nothing
+            @assert !istaskdone(REPL.REPLCompletions.PATH_cache_task)
+            wait(REPL.REPLCompletions.PATH_cache_condition)
+        end
+
+        # force the next cache update to happen immediately
+        REPL.REPLCompletions.next_cache_update = 0
+    end
+    return REPL.REPLCompletions.PATH_cache_condition
+end
+
+function test_only_wait_cache_path_done()
+    @lock REPL.REPLCompletions.PATH_cache_lock begin
+        @assert REPL.REPLCompletions.PATH_cache_condition !== nothing
+
+        while REPL.REPLCompletions.next_cache_update == 0.
+            wait(REPL.REPLCompletions.PATH_cache_condition)
+        end
+        REPL.REPLCompletions.PATH_cache_condition = nothing
+    end
+end
+
 if Sys.isunix()
 let s, c, r
     #Assume that we can rely on the existence and accessibility of /tmp
@@ -1204,12 +1237,9 @@ let s, c, r
                 # Files reachable by PATH are cached async when PATH is seen to have been changed by `complete_path`
                 # so changes are unlikely to appear in the first complete. For testing purposes we can wait for
                 # caching to finish
-                @lock REPL.REPLCompletions.PATH_cache_lock begin
-                    # force the next cache update to happen immediately
-                    REPL.REPLCompletions.next_cache_update = 0
-                end
+                test_only_arm_cache_refresh()
                 c,r = test_scomplete(s)
-                timedwait(()->REPL.REPLCompletions.next_cache_update != 0, 5) # wait for caching to complete
+                test_only_wait_cache_path_done()
                 c,r = test_scomplete(s)
                 @test "tmp-executable" in c
                 @test r == 1:9
@@ -1238,12 +1268,9 @@ let s, c, r
 
             withenv("PATH" => string(tempdir(), ":", dir)) do
                 s = string("repl-completio")
-                @lock REPL.REPLCompletions.PATH_cache_lock begin
-                    # force the next cache update to happen immediately
-                    REPL.REPLCompletions.next_cache_update = 0
-                end
+                test_only_arm_cache_refresh()
                 c,r = test_scomplete(s)
-                timedwait(()->REPL.REPLCompletions.next_cache_update != 0, 5) # wait for caching to complete
+                test_only_wait_cache_path_done()
                 c,r = test_scomplete(s)
                 @test ["repl-completion"] == c
                 @test s[r] == "repl-completio"