Skip to content

Commit

Permalink
Add persistent_tasks test
Browse files Browse the repository at this point in the history
  • Loading branch information
timholy committed Sep 9, 2023
1 parent 8dc359d commit 1927a41
Show file tree
Hide file tree
Showing 10 changed files with 220 additions and 4 deletions.
7 changes: 4 additions & 3 deletions docs/src/index.md
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
# Aqua.jl:
# Aqua.jl:
## *A*uto *QU*ality *A*ssurance for Julia packages

Aqua.jl provides functions to run a few automatable checks for Julia packages:
Expand All @@ -13,6 +13,7 @@ Aqua.jl provides functions to run a few automatable checks for Julia packages:
`compat` entry.
* `Project.toml` formatting is compatible with Pkg.jl output.
* There are no "obvious" type piracies.
* The package does not create any persistent Tasks that might block precompilation of dependencies.

## Quick usage

Expand Down Expand Up @@ -55,14 +56,14 @@ recommended to add a version bound for Aqua.jl.
test = ["Aqua", "Test"]
```

If your package supports Julia pre-1.2, you need to use the second approach,
If your package supports Julia pre-1.2, you need to use the second approach,
although you can use both approaches at the same time.

!!! warning
In normal use, `Aqua.jl` should not be added to `[deps]` in `YourPackage/Project.toml`!

### ...to your tests?
It is recommended to create a separate file `YourPackage/test/Aqua.jl` that gets included in `YourPackage/test/runtests.jl`
It is recommended to create a separate file `YourPackage/test/Aqua.jl` that gets included in `YourPackage/test/runtests.jl`
with either

```julia
Expand Down
11 changes: 10 additions & 1 deletion src/Aqua.jl
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
module Aqua

using Base: PkgId, UUID
using Pkg: Pkg, TOML
using Pkg: Pkg, TOML, PackageSpec
using Test

try
Expand All @@ -22,6 +22,7 @@ include("stale_deps.jl")
include("deps_compat.jl")
include("project_toml_formatting.jl")
include("piracy.jl")
include("persistent_tasks.jl")

"""
test_all(testtarget::Module)
Expand All @@ -36,6 +37,7 @@ Run following tests in isolated testset:
* [`test_deps_compat(testtarget)`](@ref test_deps_compat)
* [`test_project_toml_formatting(testtarget)`](@ref test_project_toml_formatting)
* [`test_piracy(testtarget)`](@ref test_piracy)
* [`test_persistent_tasks(testtarget)`](@ref test_persistent_tasks)
The keyword argument `\$x` (e.g., `ambiguities`) can be used to
control whether or not to run `test_\$x` (e.g., `test_ambiguities`).
Expand All @@ -51,6 +53,7 @@ passed to `\$x` to specify the keyword arguments for `test_\$x`.
- `deps_compat = true`
- `project_toml_formatting = true`
- `piracy = true`
- `persistent_tasks = true`
"""
function test_all(
testtarget::Module;
Expand All @@ -62,6 +65,7 @@ function test_all(
deps_compat = true,
project_toml_formatting = true,
piracy = true,
persistent_tasks = true,
)
@testset "Method ambiguity" begin
if ambiguities !== false
Expand Down Expand Up @@ -103,6 +107,11 @@ function test_all(
test_piracy(testtarget; askwargs(piracy)...)
end
end
@testset "Persistent tasks" begin
if persistent_tasks !== false
test_persistent_tasks(testtarget; askwargs(persistent_tasks)...)
end
end
end

end # module
145 changes: 145 additions & 0 deletions src/persistent_tasks.jl
Original file line number Diff line number Diff line change
@@ -0,0 +1,145 @@
"""
Aqua.test_persistent_tasks(package; tmax=5)
Test whether loading `package` creates persistent `Task`s
which may block precompilation of dependent packages.
# Motivation
Julia 1.10 and higher wait for all running `Task`s to finish
before writing out the precompiled (cached) version of the package.
One consequence is that a package that launches
`Task`s in its `__init__` function may precompile successfully,
but block precompilation of any packages that depend on it.
# Example
Let's create a dummy package, `PkgA`, that launches a persistent `Task`:
```julia
```julia
module PkgA
const t = Ref{Any}() # to prevent the Timer from being garbage-collected
__init__() = t[] = Timer(0.1; interval=1) # create a persistent `Timer` `Task`
end
```
`PkgA` will precompile successfully, because `PkgA.__init__()` does not
run when `PkgA` is precompiled. However,
```julia
module PkgB
using PkgA
end
```
fails to precompile: `using PkgA` runs `PkgA.__init__()`, which
leaves the `Timer` `Task` running, and that causes precompilation
of `PkgB` to hang.
# How the test works
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`.
# How to fix failing packages
Often, the easiest fix is to modify the `__init__` function to check whether the
Julia process is precompiling some other package; if so, don't launch the
persistent `Task`s.
```
function __init__()
# Other setup code here
if ccall(:jl_generating_output, Cint, ()) == 0 # if we're not precompiling...
# launch persistent tasks here
end
end
```
In more complex cases, you may need to set up independently-callable functions
to launch the tasks and cleanly shut them down.
# Arguments
- `package`: a top-level `Module` or `Base.PkgId`.
# Keyword Arguments
- `tmax::Real`: the maximum time (in seconds) to wait between loading the
package and forced shutdown of the precompilation process.
"""
function test_persistent_tasks(package::PkgId; tmax=5, fails::Bool=false)

Check warning on line 73 in src/persistent_tasks.jl

View workflow job for this annotation

GitHub Actions / code-style

[JuliaFormatter] reported by reviewdog 🐶 Raw Output: src/persistent_tasks.jl:73:-function test_persistent_tasks(package::PkgId; tmax=5, fails::Bool=false) src/persistent_tasks.jl:73:+function test_persistent_tasks(package::PkgId; tmax = 5, fails::Bool = false)
@testset "$package persistent_tasks" begin
result = root_project_or_failed_lazytest(package)
result isa LazyTestResult && return result
@test fails precompile_wrapper(result, tmax)
end
end

function test_persistent_tasks(package::Module; kwargs...)
test_persistent_tasks(PkgId(package); kwargs...)
end

"""
Aqua.test_persistent_tasks_deps(package; kwargs...)
Test all the dependencies of `package` with [`Aqua.test_persistent_tasks`](@ref).
"""
function test_persistent_tasks_deps(package::PkgId; fails=Dict{String,Bool}(), kwargs...)

Check warning on line 90 in src/persistent_tasks.jl

View workflow job for this annotation

GitHub Actions / code-style

[JuliaFormatter] reported by reviewdog 🐶 Raw Output: src/persistent_tasks.jl:90:-function test_persistent_tasks_deps(package::PkgId; fails=Dict{String,Bool}(), kwargs...) src/persistent_tasks.jl:90:+function test_persistent_tasks_deps(package::PkgId; fails = Dict{String,Bool}(), kwargs...)
result = root_project_or_failed_lazytest(package)
result isa LazyTestResult && return result
prj = TOML.parsefile(result)
deps = get(prj, "deps", nothing)
@testset "$result" begin
if deps === nothing
return LazyTestResult("$package", "`$result` does not have `deps`", true)
else
for (name, uuid) in deps
id = PkgId(UUID(uuid), name)
test_persistent_tasks(id; fails=get(fails, name, false), kwargs...)

Check warning on line 101 in src/persistent_tasks.jl

View workflow job for this annotation

GitHub Actions / code-style

[JuliaFormatter] reported by reviewdog 🐶 Raw Output: src/persistent_tasks.jl:101:- test_persistent_tasks(id; fails=get(fails, name, false), kwargs...) src/persistent_tasks.jl:101:+ test_persistent_tasks(id; fails = get(fails, name, false), kwargs...)

Check warning on line 101 in src/persistent_tasks.jl

View check run for this annotation

Codecov / codecov/patch

src/persistent_tasks.jl#L99-L101

Added lines #L99 - L101 were not covered by tests
end
end
end
end

function test_persistent_tasks_deps(package::Module; kwargs...)
test_persistent_tasks_deps(PkgId(package); kwargs...)

Check warning on line 108 in src/persistent_tasks.jl

View check run for this annotation

Codecov / codecov/patch

src/persistent_tasks.jl#L108

Added line #L108 was not covered by tests
end

function precompile_wrapper(project, tmax)
pkgdir = dirname(project)
pkgname = basename(pkgdir)
wrapperdir = tempname()
wrappername, wrapperuuid = only(Pkg.generate(wrapperdir))
Pkg.activate(wrapperdir)
Pkg.develop(PackageSpec(path=pkgdir))

Check warning on line 117 in src/persistent_tasks.jl

View workflow job for this annotation

GitHub Actions / code-style

[JuliaFormatter] reported by reviewdog 🐶 Raw Output: src/persistent_tasks.jl:117:- Pkg.develop(PackageSpec(path=pkgdir)) src/persistent_tasks.jl:117:+ Pkg.develop(PackageSpec(path = pkgdir))
open(joinpath(wrapperdir, "src", wrappername * ".jl"), "w") do io

Check warning on line 118 in src/persistent_tasks.jl

View check run for this annotation

Codecov / codecov/patch

src/persistent_tasks.jl#L116-L118

Added lines #L116 - L118 were not covered by tests
println(io, """

Check warning on line 119 in src/persistent_tasks.jl

View workflow job for this annotation

GitHub Actions / code-style

[JuliaFormatter] reported by reviewdog 🐶 Raw Output: src/persistent_tasks.jl:119:- println(io, """ src/persistent_tasks.jl:120:- module $wrappername src/persistent_tasks.jl:121:- using $pkgname src/persistent_tasks.jl:122:- # Signal Aqua from the precompilation process that we've finished loading the package src/persistent_tasks.jl:123:- open(joinpath("$wrapperdir", "done.log"), "w") do io src/persistent_tasks.jl:124:- println(io, "done") src/persistent_tasks.jl:125:- end src/persistent_tasks.jl:126:- end src/persistent_tasks.jl:127:- """) src/persistent_tasks.jl:119:+ println( src/persistent_tasks.jl:120:+ io, src/persistent_tasks.jl:121:+ """ src/persistent_tasks.jl:122:+module $wrappername src/persistent_tasks.jl:123:+using $pkgname src/persistent_tasks.jl:124:+# Signal Aqua from the precompilation process that we've finished loading the package src/persistent_tasks.jl:125:+open(joinpath("$wrapperdir", "done.log"), "w") do io src/persistent_tasks.jl:126:+ println(io, "done") src/persistent_tasks.jl:127:+end src/persistent_tasks.jl:128:+end src/persistent_tasks.jl:129:+""", src/persistent_tasks.jl:130:+ )
module $wrappername
using $pkgname
# Signal Aqua from the precompilation process that we've finished loading the package
open(joinpath("$wrapperdir", "done.log"), "w") do io
println(io, "done")
end
end
""")
end
# Precompile the wrapper package
cmd = `$(Base.julia_cmd()) --project=$wrapperdir -e 'using Pkg; Pkg.precompile()'`
proc = run(cmd; wait=false)

Check warning on line 131 in src/persistent_tasks.jl

View workflow job for this annotation

GitHub Actions / code-style

[JuliaFormatter] reported by reviewdog 🐶 Raw Output: src/persistent_tasks.jl:131:- proc = run(cmd; wait=false) src/persistent_tasks.jl:134:+ proc = run(cmd; wait = false)
while !isfile(joinpath(wrapperdir, "done.log"))
sleep(0.1)

Check warning on line 133 in src/persistent_tasks.jl

View check run for this annotation

Codecov / codecov/patch

src/persistent_tasks.jl#L130-L133

Added lines #L130 - L133 were not covered by tests
end
# Check whether precompilation finishes in the required time
t = time()
while process_running(proc) && time() - t < tmax
sleep(0.1)

Check warning on line 138 in src/persistent_tasks.jl

View check run for this annotation

Codecov / codecov/patch

src/persistent_tasks.jl#L136-L138

Added lines #L136 - L138 were not covered by tests
end
success = !process_running(proc)
if !success
kill(proc)

Check warning on line 142 in src/persistent_tasks.jl

View check run for this annotation

Codecov / codecov/patch

src/persistent_tasks.jl#L140-L142

Added lines #L140 - L142 were not covered by tests
end
return success

Check warning on line 144 in src/persistent_tasks.jl

View check run for this annotation

Codecov / codecov/patch

src/persistent_tasks.jl#L144

Added line #L144 was not covered by tests
end
4 changes: 4 additions & 0 deletions test/pkgs/PersistentTasks/PersistentTask/Project.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
name = "PersistentTask"
uuid = "e5c298b6-d81d-47aa-a9ed-5ea983e22986"
authors = ["Tim Holy <[email protected]>"]
version = "0.1.0"
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
module PersistentTask

const t = Ref{Any}()
__init__() = t[] = Timer(0.1; interval=1) # create a persistent `Timer` `Task`

Check warning on line 4 in test/pkgs/PersistentTasks/PersistentTask/src/PersistentTask.jl

View workflow job for this annotation

GitHub Actions / code-style

[JuliaFormatter] reported by reviewdog 🐶 Raw Output: test/pkgs/PersistentTasks/PersistentTask/src/PersistentTask.jl:4:-__init__() = t[] = Timer(0.1; interval=1) # create a persistent `Timer` `Task` test/pkgs/PersistentTasks/PersistentTask/src/PersistentTask.jl:4:+__init__() = t[] = Timer(0.1; interval = 1) # create a persistent `Timer` `Task`

end # module PersistentTask
4 changes: 4 additions & 0 deletions test/pkgs/PersistentTasks/TransientTask/Project.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
name = "TransientTask"
uuid = "94ae9332-58b0-4342-989c-0a7e44abcca9"
authors = ["Tim Holy <[email protected]>"]
version = "0.1.0"
5 changes: 5 additions & 0 deletions test/pkgs/PersistentTasks/TransientTask/src/TransientTask.jl
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
module TransientTask

__init__() = Timer(0.1) # create a transient `Timer` `Task`

end # module TransientTask
8 changes: 8 additions & 0 deletions test/pkgs/PersistentTasks/UsesBoth/Project.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
name = "UsesBoth"
uuid = "96f12b6e-60f8-43dc-b131-049a88a2f499"
authors = ["Tim Holy <[email protected]>"]
version = "0.1.0"

[deps]
PersistentTask = "e5c298b6-d81d-47aa-a9ed-5ea983e22986"
TransientTask = "94ae9332-58b0-4342-989c-0a7e44abcca9"
6 changes: 6 additions & 0 deletions test/pkgs/PersistentTasks/UsesBoth/src/UsesBoth.jl
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
module UsesBoth

using TransientTask
using PersistentTask

end # module UsesBoth
28 changes: 28 additions & 0 deletions test/test_persistent_tasks.jl
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
module TestPersistentTasks

include("preamble.jl")
using Base: PkgId, UUID
using Pkg: TOML

function getid(name)
path = joinpath(@__DIR__, "pkgs", "PersistentTasks", name)
if path LOAD_PATH
pushfirst!(LOAD_PATH, path)
end
prj = TOML.parsefile(joinpath(path, "Project.toml"))
return PkgId(UUID(prj["uuid"]), prj["name"])
end


@testset "PersistentTasks" begin
Aqua.test_persistent_tasks(getid("TransientTask"))
Aqua.test_persistent_tasks_deps(getid("TransientTask"))

if all((Base.VERSION.major, Base.VERSION.minor) .>= (1, 10))
Aqua.test_persistent_tasks(getid("PersistentTask"); fails=true)

Check warning on line 22 in test/test_persistent_tasks.jl

View workflow job for this annotation

GitHub Actions / code-style

[JuliaFormatter] reported by reviewdog 🐶 Raw Output: test/test_persistent_tasks.jl:22:- Aqua.test_persistent_tasks(getid("PersistentTask"); fails=true) test/test_persistent_tasks.jl:23:- Aqua.test_persistent_tasks_deps(getid("UsesBoth"); fails=Dict("PersistentTask" => true)) test/test_persistent_tasks.jl:22:+ Aqua.test_persistent_tasks(getid("PersistentTask"); fails = true) test/test_persistent_tasks.jl:23:+ Aqua.test_persistent_tasks_deps( test/test_persistent_tasks.jl:24:+ getid("UsesBoth"); test/test_persistent_tasks.jl:25:+ fails = Dict("PersistentTask" => true), test/test_persistent_tasks.jl:26:+ )
Aqua.test_persistent_tasks_deps(getid("UsesBoth"); fails=Dict("PersistentTask" => true))
end
filter!(str -> !occursin("PersistentTasks", str), LOAD_PATH)
end

end

0 comments on commit 1927a41

Please sign in to comment.