Skip to content

Commit

Permalink
Use ScopedValue to fix async issue (#112)
Browse files Browse the repository at this point in the history
* Use ContextVariablesX to fix async issue

* Use ScopedValues rather than ContextVariables

* Apply suggestions from code review

Co-authored-by: Curtis Vogt <[email protected]>

* Define missing PATCH_ENV

* Refactor tasks testset into async scope

* Import ScopedValue alongside other imports

* Use VERSION check instead

---------

Co-authored-by: Curtis Vogt <[email protected]>
  • Loading branch information
oxinabox and omus committed Jul 16, 2024
1 parent 3d17650 commit 9bfeb8f
Show file tree
Hide file tree
Showing 6 changed files with 52 additions and 25 deletions.
7 changes: 7 additions & 0 deletions src/Mocking.jl
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,13 @@ module Mocking
using Compat: mergewith
using ExprTools: splitdef, combinedef

# Available in Julia 1.11+: https://github.com/JuliaLang/julia/pull/50958
# We cannot use ScopedValues.jl for backwards compatability as that implementation breaks
# `@test_logs`.
if VERSION >= v"1.11.0-DEV.482"
using Base: ScopedValue, with
end

export @patch, @mock, Patch, apply

include("expr.jl")
Expand Down
2 changes: 1 addition & 1 deletion src/mock.jl
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ function get_alternate(pe::PatchEnv, target, args...)
end
end

get_alternate(target, args...) = get_alternate(get_active_env(), target, args...)
get_alternate(target, args...) = get_alternate(PATCH_ENV[], target, args...)

function _debug_msg(method::Union{Method,Nothing}, target, args)
call = "$target($(join(map(arg -> "::$(Core.Typeof(arg))", args), ", ")))"
Expand Down
29 changes: 19 additions & 10 deletions src/patch.jl
Original file line number Diff line number Diff line change
Expand Up @@ -197,19 +197,28 @@ julia> apply(p1) do
function apply end

function apply(body::Function, pe::PatchEnv)
prev_pe = get_active_env()
set_active_env(merge(prev_pe, pe))
try
return body()
finally
set_active_env(prev_pe)
end
merged_pe = merge(PATCH_ENV[], pe)
return with_active_env(body, merged_pe)
end

function apply(body::Function, patches; debug::Bool=false)
return apply(body, PatchEnv(patches, debug))
end

const PATCH_ENV = Ref{PatchEnv}(PatchEnv())
set_active_env(pe::PatchEnv) = (PATCH_ENV[] = pe)
get_active_env() = PATCH_ENV[]
# https://github.com/JuliaLang/julia/pull/50958
if VERSION >= v"1.11.0-DEV.482"
const PATCH_ENV = ScopedValue(PatchEnv())
with_active_env(body::Function, pe::PatchEnv) = with(body, PATCH_ENV => pe)
else
const PATCH_ENV = Ref{PatchEnv}(PatchEnv())

function with_active_env(body::Function, pe::PatchEnv)
old_pe = PATCH_ENV[]
try
PATCH_ENV[] = pe
body()
finally
PATCH_ENV[] = old_pe
end
end
end
17 changes: 14 additions & 3 deletions test/async.jl → test/async-scope.jl
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
@testset "tasks" begin
# Async tasks should consistently use the patch environment (if any) they started with
@testset "async scope" begin
c = Condition()
ch = Channel{String}(1)
f() = "original"
Expand All @@ -20,7 +21,12 @@
@test (@mock f()) == "mocked"

notify(c)
@test_broken take!(ch) == "original"
# https://github.com/JuliaLang/julia/pull/50958
if VERSION >= v"1.11.0-DEV.482"
@test take!(ch) == "original"
else
@test_broken take!(ch) == "original"
end

# Task started inside patched context should call patched functions.
@async background()
Expand All @@ -35,6 +41,11 @@
end

notify(c)
@test_broken take!(ch) == "mocked"
# https://github.com/JuliaLang/julia/pull/50958
if VERSION >= v"1.11.0-DEV.482"
@test take!(ch) == "mocked"
else
@test_broken take!(ch) == "mocked"
end
end
end
20 changes: 10 additions & 10 deletions test/concept.jl
Original file line number Diff line number Diff line change
Expand Up @@ -20,19 +20,19 @@
for p in patches
Mocking.apply!(pe, p)
end
Mocking.set_active_env(pe)

@test (@mock multiply(2)) == 8 # calls mocked `multiply(::Int)`
@test (@mock multiply(0x2)) == 0x6 # calls mocked `multiply(::Integer)`
@test (@mock multiply(2//1)) == 4//1 # calls original `multiply(::Number)`
Mocking.apply(pe) do
@test (@mock multiply(2)) == 8 # calls mocked `multiply(::Int)`
@test (@mock multiply(0x2)) == 0x6 # calls mocked `multiply(::Integer)`
@test (@mock multiply(2//1)) == 4//1 # calls original `multiply(::Number)`

@test (@mock multiply(2)) != multiply(2)
@test (@mock multiply(0x2)) != multiply(0x2)
@test (@mock multiply(2//1)) == multiply(2//1)
@test (@mock multiply(2)) != multiply(2)
@test (@mock multiply(0x2)) != multiply(0x2)
@test (@mock multiply(2//1)) == multiply(2//1)
end

# Clean env
pe = Mocking.PatchEnv()
Mocking.set_active_env(pe)
# Patch environment has been reset back to original clean state
@test Mocking.PATCH_ENV[] == Mocking.PatchEnv()

# Ensure that original behaviour is restored
@test (@mock multiply(2)) == 3
Expand Down
2 changes: 1 addition & 1 deletion test/runtests.jl
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ Mocking.activate()
include("args.jl")
include("merge.jl")
include("nested_apply.jl")
include("async.jl")
include("async-scope.jl")
include("issues.jl")
include("activate.jl")
include("async-world-ages.jl")
Expand Down

0 comments on commit 9bfeb8f

Please sign in to comment.