Skip to content

Commit df750e4

Browse files
committed
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
1 parent fe7dc7d commit df750e4

File tree

2 files changed

+33
-12
lines changed

2 files changed

+33
-12
lines changed

stdlib/REPL/src/REPLCompletions.jl

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -330,7 +330,8 @@ end
330330

331331
const PATH_cache_lock = Base.ReentrantLock()
332332
const PATH_cache = Set{String}()
333-
PATH_cache_task::Union{Task,Nothing} = nothing # used for sync in tests
333+
PATH_cache_task::Union{Task,Nothing} = nothing
334+
PATH_cache_condition::Union{Threads.Condition, Nothing} = nothing # used for sync in tests
334335
next_cache_update::Float64 = 0.0
335336
function maybe_spawn_cache_PATH()
336337
global PATH_cache_task, next_cache_update
@@ -339,7 +340,10 @@ function maybe_spawn_cache_PATH()
339340
time() < next_cache_update && return
340341
PATH_cache_task = Threads.@spawn begin
341342
REPLCompletions.cache_PATH()
342-
@lock PATH_cache_lock PATH_cache_task = nothing # release memory when done
343+
@lock PATH_cache_lock begin
344+
PATH_cache_task = nothing # release memory when done
345+
PATH_cache_condition !== nothing && notify(PATH_cache_condition)
346+
end
343347
end
344348
Base.errormonitor(PATH_cache_task)
345349
end

stdlib/REPL/test/replcompletions.jl

Lines changed: 27 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1073,6 +1073,29 @@ let c, r, res
10731073
@test res === false
10741074
end
10751075

1076+
# A pair of utility function for the REPL completions test to test PATH_cache
1077+
# dependent completions, which ordinarily happen asynchronously.
1078+
# Only to be used from the test suite
1079+
function test_only_arm_cache_refresh()
1080+
@assert REPL.REPLCompletions.PATH_cache_condition === nothing
1081+
@lock REPL.REPLCompletions.PATH_cache_lock begin
1082+
# force the next cache update to happen immediately
1083+
REPL.REPLCompletions.next_cache_update = 0
1084+
# Arm a condition we can wait on
1085+
REPL.REPLCompletions.PATH_cache_condition = Threads.Condition(REPL.REPLCompletions.PATH_cache_lock)
1086+
end
1087+
return REPL.REPLCompletions.PATH_cache_condition
1088+
end
1089+
1090+
function test_only_wait_cache_path_done()
1091+
@lock REPL.REPLCompletions.PATH_cache_lock begin
1092+
while REPL.REPLCompletions.next_cache_update == 0.
1093+
wait(REPL.REPLCompletions.PATH_cache_condition)
1094+
end
1095+
REPL.REPLCompletions.PATH_cache_condition = nothing
1096+
end
1097+
end
1098+
10761099
if Sys.isunix()
10771100
let s, c, r
10781101
#Assume that we can rely on the existence and accessibility of /tmp
@@ -1204,12 +1227,9 @@ let s, c, r
12041227
# Files reachable by PATH are cached async when PATH is seen to have been changed by `complete_path`
12051228
# so changes are unlikely to appear in the first complete. For testing purposes we can wait for
12061229
# caching to finish
1207-
@lock REPL.REPLCompletions.PATH_cache_lock begin
1208-
# force the next cache update to happen immediately
1209-
REPL.REPLCompletions.next_cache_update = 0
1210-
end
1230+
test_only_arm_cache_refresh()
12111231
c,r = test_scomplete(s)
1212-
timedwait(()->REPL.REPLCompletions.next_cache_update != 0, 5) # wait for caching to complete
1232+
test_only_wait_cache_path_done()
12131233
c,r = test_scomplete(s)
12141234
@test "tmp-executable" in c
12151235
@test r == 1:9
@@ -1238,12 +1258,9 @@ let s, c, r
12381258

12391259
withenv("PATH" => string(tempdir(), ":", dir)) do
12401260
s = string("repl-completio")
1241-
@lock REPL.REPLCompletions.PATH_cache_lock begin
1242-
# force the next cache update to happen immediately
1243-
REPL.REPLCompletions.next_cache_update = 0
1244-
end
1261+
test_only_arm_cache_refresh()
12451262
c,r = test_scomplete(s)
1246-
timedwait(()->REPL.REPLCompletions.next_cache_update != 0, 5) # wait for caching to complete
1263+
test_only_wait_cache_path_done()
12471264
c,r = test_scomplete(s)
12481265
@test ["repl-completion"] == c
12491266
@test s[r] == "repl-completio"

0 commit comments

Comments
 (0)