Make persistent task test more robust

CHANGELOG.md

@@ -10,6 +10,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 ### Changed
 
 - The minimum supported julia version is increased to 1.6. ([#328])
+- `test_persistent_tasks` now reports a precompilation failure as a precompilation
+  error instead of misclassifying it as a persistent task. The default `tmax` is
+  also raised to 30 seconds (still configurable) to reduce false positives when a
+  package without persistent tasks is merely slow to shut down. ([#315])
 
 ## Version [v0.8.16] - 2026-06-05
 
@@ -350,6 +354,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 [#297]: https://github.com/JuliaTesting/Aqua.jl/issues/297
 [#309]: https://github.com/JuliaTesting/Aqua.jl/issues/309
 [#313]: https://github.com/JuliaTesting/Aqua.jl/issues/313
+[#315]: https://github.com/JuliaTesting/Aqua.jl/issues/315
 [#319]: https://github.com/JuliaTesting/Aqua.jl/issues/319
 [#322]: https://github.com/JuliaTesting/Aqua.jl/issues/322
 [#328]: https://github.com/JuliaTesting/Aqua.jl/issues/328

docs/src/persistent_tasks.md

@@ -84,8 +84,18 @@ end)
 
 This test works by launching a Julia process that tries to precompile a
 dummy package similar to `PkgB` above, modified to signal back to Aqua when
-`PkgA` has finished loading. The test fails if the gap between loading `PkgA`
-and finishing precompilation exceeds time `tmax`.
+`PkgA` has finished loading.
+
+Aqua waits — without any time limit — for that signal, so slow precompilation of
+the dependencies never affects the result. Once `PkgA` has loaded, a persistent
+`Task` shows up as the precompilation process being unable to exit: it hangs
+indefinitely. A package without persistent tasks always exits eventually, so Aqua
+waits up to `tmax` seconds for its shutdown before reporting a failure. If a
+package you know to be free of persistent tasks is misreported, increase `tmax`.
+
+If precompilation instead fails outright (for example because a dependency
+cannot be precompiled), that is reported as a precompilation error rather than a
+persistent task, so the failure message points at the real cause.
 
 ## How to fix failing packages
 

src/persistent_tasks.jl

@@ -18,9 +18,11 @@ On Julia version 1.9 and before, this test always succeeds.
 # Keyword Arguments
 - `broken::Bool = false`: If true, it uses `@test_broken` instead of
   `@test`.
-- `tmax::Real = 5`: the maximum time (in seconds) to wait after loading the
-  package before forcibly shutting down the precompilation process (triggering
-  a test failure).
+- `tmax::Real = 30`: the maximum time (in seconds) to wait for the
+  precompilation process to exit *after* `package` has finished loading. Only
+  this shutdown counts against `tmax`, not the time spent loading the
+  dependencies. A persistent `Task` blocks the exit indefinitely, so if a
+  package free of persistent tasks is misreported, increase `tmax`.
 - `expr::Expr = quote end`: An expression to run in the precompile package.
 
 !!! note
@@ -44,7 +46,7 @@ function test_persistent_tasks(package::Module; kwargs...)
     test_persistent_tasks(PkgId(package); kwargs...)
 end
 
-function has_persistent_tasks(package::PkgId; expr::Expr = quote end, tmax = 10)
+function has_persistent_tasks(package::PkgId; expr::Expr = quote end, tmax = 30)
     root_project_path, found = root_project_toml(package)
     found || error("Unable to locate Project.toml")
     return !precompile_wrapper(root_project_path, tmax, expr)
@@ -118,25 +120,44 @@ end
             code = """touch("$(escape_string(statusfile))")"""
             `$(Base.julia_cmd()) -e $code`
         else
-            `$(Base.julia_cmd()) --project=$wrapperdir -e 'push!(LOAD_PATH, "@stdlib"); using Pkg; Pkg.precompile(; io = devnull)'`
+            `$(Base.julia_cmd()) --project=$wrapperdir -e 'push!(LOAD_PATH, "@stdlib"); using Pkg; Pkg.precompile()'`
         end
 
-        cmd = pipeline(cmd; stdout, stderr)
+        # Capture the subprocess's stderr so a genuine precompilation error can be
+        # reported on its own terms instead of masquerading as a persistent task.
+        errlog = joinpath(wrapperdir, "precompile-stderr.log")
+        cmd = pipeline(cmd; stdout = devnull, stderr = errlog)
         proc = run(cmd; wait = false)::Base.Process
-        while !isfile(statusfile) && process_running(proc)
-            sleep(0.5)
-        end
+
+        # Phase 1 (unbounded): wait for the package to finish loading. The wrapper
+        # writes `statusfile` once `using $pkgname` (and any `expr`) has run. Slow
+        # precompilation of the dependencies only prolongs this phase.
+        timedwait(() -> isfile(statusfile) || !process_running(proc), Inf; pollint = 0.5)
         if !isfile(statusfile)
-            @error "Unexpected error: $statusfile was not created, but precompilation exited"
-            return false
-        end
-        # Check whether precompilation finishes in the required time
-        t = time()
-        while process_running(proc) && time() - t < tmax
-            sleep(0.1)
+            # The process exited before the package finished loading: a
+            # precompilation failure, not a persistent task.
+            wait(proc)
+            error(
+                "Loading `$pkgname` for the persistent-task check failed before " *
+                "precompilation completed (process exited with code " *
+                "$(proc.exitcode), signal $(proc.termsignal)). This indicates a " *
+                "precompilation error, not a persistent task. Captured output:\n\n" *
+                (isfile(errlog) ? read(errlog, String) : ""),
+            )
         end
+
+        # Phase 2 (bounded by `tmax`): the package loaded cleanly. A persistent task
+        # keeps the process from exiting, so it hangs indefinitely. A healthy package
+        # exits once its shutdown finishes, so allow up to `tmax` seconds for it.
+        timedwait(() -> !process_running(proc), tmax; pollint = 0.1)
         success = !process_running(proc)
         if !success
+            @warn(
+                "Loading `$pkgname` prevented the precompilation process from " *
+                "exiting within $tmax seconds, which usually means a persistent " *
+                "task is still running. If `$pkgname` is free of persistent tasks, " *
+                "re-run with a larger `tmax` to give its shutdown more time."
+            )
             # SIGKILL to prevent julia from printing the SIG 15 handler, which can
             # misleadingly look like it's caused by an issue in the user's program.
             kill(proc, Base.SIGKILL)

test/pkgs/PersistentTasks/FailsToPrecompile/Project.toml

@@ -0,0 +1,2 @@
+name = "FailsToPrecompile"
+uuid = "f9ce74f8-1127-4e05-8a6a-300e168e70af"

test/pkgs/PersistentTasks/FailsToPrecompile/src/FailsToPrecompile.jl

@@ -0,0 +1,7 @@
+module FailsToPrecompile
+
+# Fail during precompilation so the persistent-task check hits its
+# precompilation-error branch instead of loading the package successfully.
+error("Intentional precompilation failure for testing Aqua's persistent-task check")
+
+end

test/test_persistent_tasks.jl

@@ -21,12 +21,23 @@ end
     @test result == []
 
     if Base.VERSION >= v"1.10-"
-        @test Aqua.has_persistent_tasks(getid("PersistentTask"))
+        @test Aqua.has_persistent_tasks(getid("PersistentTask"); tmax = 2)
 
-        result = Aqua.find_persistent_tasks_deps(getid("UsesBoth"))
+        result = Aqua.find_persistent_tasks_deps(getid("UsesBoth"); tmax = 2)
         @test result == ["PersistentTask"]
     end
-    filter!(str -> !occursin("PersistentTasks", str), LOAD_PATH)
+    filter!(!occursin("PersistentTasks"), LOAD_PATH)
+end
+
+@testset "precompilation failure is reported as an error" begin
+    if Base.VERSION >= v"1.10-"
+        # A package that fails to precompile must be reported as a
+        # precompilation error rather than misclassified as a persistent task.
+        @test_throws "precompilation error" Aqua.has_persistent_tasks(
+            getid("FailsToPrecompile"),
+        )
+    end
+    filter!(!occursin("PersistentTasks"), LOAD_PATH)
 end
 
 @testset "test_persistent_tasks(expr)" begin
@@ -37,11 +48,15 @@ end
                 fetch(Threads.@spawn nothing)
             end,
         )
-        @test Aqua.has_persistent_tasks(getid("TransientTask"), expr = quote
-            Threads.@spawn while true
-                sleep(0.5)
-            end
-        end)
+        @test Aqua.has_persistent_tasks(
+            getid("TransientTask"),
+            tmax = 2,
+            expr = quote
+                Threads.@spawn while true
+                    sleep(0.5)
+                end
+            end,
+        )
     end
 end