-
-
Notifications
You must be signed in to change notification settings - Fork 5.6k
use ScopedValues for TestSets #53462
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Changes from 11 commits
34b945c
7849324
bd44c2a
b9d7ab8
dcf2977
e664d5b
5097261
0a02d40
a645b3b
19706fd
e083c75
cd3f239
449a7ab
6f7625c
15f609e
490ee21
b43e9a4
dddc4ed
47fc0c9
9f8088a
0ca783e
211d304
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -29,6 +29,7 @@ using Random: AbstractRNG, default_rng | |
using InteractiveUtils: gen_call_with_extracted_types | ||
using Base: typesplit, remove_linenums! | ||
using Serialization: Serialization | ||
using Base.ScopedValues: ScopedValue, @with | ||
|
||
const DISPLAY_FAILED = ( | ||
:isequal, | ||
|
@@ -972,7 +973,7 @@ if get_testset_depth() != 0 | |
end | ||
``` | ||
""" | ||
function finish end | ||
finish(ts::AbstractTestSet) = ts | ||
|
||
""" | ||
TestSetException | ||
|
@@ -1007,7 +1008,6 @@ end | |
A simple fallback test set that throws immediately on a failure. | ||
""" | ||
struct FallbackTestSet <: AbstractTestSet end | ||
fallback_testset = FallbackTestSet() | ||
|
||
struct FallbackTestSetException <: Exception | ||
msg::String | ||
|
@@ -1024,8 +1024,6 @@ function record(ts::FallbackTestSet, t::Union{Fail, Error}) | |
println(t) | ||
throw(FallbackTestSetException("There was an error during testing")) | ||
end | ||
# We don't need to do anything as we don't record anything | ||
finish(ts::FallbackTestSet) = ts | ||
|
||
#----------------------------------------------------------------------- | ||
|
||
|
@@ -1642,21 +1640,11 @@ function testset_context(args, ex, source) | |
else | ||
error("Malformed `let` expression is given") | ||
end | ||
reverse!(contexts) | ||
|
||
test_ex = ex.args[2] | ||
|
||
ex.args[2] = quote | ||
$(map(contexts) do context | ||
:($push_testset($(ContextTestSet)($(QuoteNode(context)), $context; $options...))) | ||
end...) | ||
try | ||
$(test_ex) | ||
finally | ||
$(map(_->:($pop_testset()), contexts)...) | ||
end | ||
for context in contexts | ||
test_ex = :($Test.@with_testset($ContextTestSet($(QuoteNode(context)), $context; $options...), $test_ex)) | ||
end | ||
|
||
ex.args[2] = test_ex | ||
return esc(ex) | ||
end | ||
|
||
|
@@ -1690,17 +1678,19 @@ function testset_beginend_call(args, tests, source) | |
else | ||
$(testsettype)($desc; $options...) | ||
end | ||
push_testset(ts) | ||
|
||
# we reproduce the logic of guardseed, but this function | ||
# cannot be used as it changes slightly the semantic of @testset, | ||
# by wrapping the body in a function | ||
local default_rng_orig = copy(default_rng()) | ||
local tls_seed_orig = copy(Random.get_tls_seed()) | ||
try | ||
# default RNG is reset to its state from last `seed!()` to ease reproduce a failed test | ||
copy!(Random.default_rng(), tls_seed_orig) | ||
let | ||
$(esc(tests)) | ||
@with_testset ts begin | ||
# default RNG is reset to its state from last `seed!()` to ease reproduce a failed test | ||
copy!(Random.default_rng(), tls_seed_orig) | ||
let | ||
$(esc(tests)) | ||
end | ||
end | ||
catch err | ||
err isa InterruptException && rethrow() | ||
|
@@ -1715,7 +1705,6 @@ function testset_beginend_call(args, tests, source) | |
finally | ||
copy!(default_rng(), default_rng_orig) | ||
copy!(Random.get_tls_seed(), tls_seed_orig) | ||
pop_testset() | ||
ret = finish(ts) | ||
end | ||
ret | ||
|
@@ -1774,22 +1763,17 @@ function testset_forloop(args, testloop, source) | |
_check_testset($testsettype, $(QuoteNode(testsettype.args[1]))) | ||
# Trick to handle `break` and `continue` in the test code before | ||
# they can be handled properly by `finally` lowering. | ||
simonbyrne marked this conversation as resolved.
Show resolved
Hide resolved
|
||
if !first_iteration | ||
pop_testset() | ||
finish_errored = true | ||
push!(arr, finish(ts)) | ||
finish_errored = false | ||
copy!(default_rng(), tls_seed_orig) | ||
end | ||
ts = if ($testsettype === $DefaultTestSet) && $(isa(source, LineNumberNode)) | ||
$(testsettype)($desc; source=$(QuoteNode(source.file)), $options...) | ||
else | ||
$(testsettype)($desc; $options...) | ||
end | ||
push_testset(ts) | ||
first_iteration = false | ||
try | ||
$(esc(tests)) | ||
@with_testset ts begin | ||
# default RNG is reset to its state from last `seed!()` to ease reproduce a failed test | ||
copy!(Random.default_rng(), tls_seed_orig) | ||
$(esc(tests)) | ||
end | ||
catch err | ||
err isa InterruptException && rethrow() | ||
# Something in the test block threw an error. Count that as an | ||
|
@@ -1798,28 +1782,19 @@ function testset_forloop(args, testloop, source) | |
if !isa(err, FailFastError) | ||
record(ts, Error(:nontest_error, Expr(:tuple), err, Base.current_exceptions(), $(QuoteNode(source)))) | ||
end | ||
finally | ||
copy!(default_rng(), default_rng_orig) | ||
copy!(Random.get_tls_seed(), tls_seed_orig) | ||
push!(arr, finish(ts)) | ||
end | ||
end | ||
quote | ||
local arr = Vector{Any}() | ||
local first_iteration = true | ||
local ts | ||
local finish_errored = false | ||
local default_rng_orig = copy(default_rng()) | ||
local tls_seed_orig = copy(Random.get_tls_seed()) | ||
copy!(Random.default_rng(), tls_seed_orig) | ||
try | ||
let | ||
$(Expr(:for, Expr(:block, [esc(v) for v in loopvars]...), blk)) | ||
end | ||
finally | ||
# Handle `return` in test body | ||
if !first_iteration && !finish_errored | ||
pop_testset() | ||
push!(arr, finish(ts)) | ||
end | ||
copy!(default_rng(), default_rng_orig) | ||
copy!(Random.get_tls_seed(), tls_seed_orig) | ||
local ts | ||
let | ||
$(Expr(:for, Expr(:block, [esc(v) for v in loopvars]...), blk)) | ||
end | ||
arr | ||
end | ||
|
@@ -1868,39 +1843,21 @@ end | |
#----------------------------------------------------------------------- | ||
# Various helper methods for test sets | ||
|
||
const CURRENT_TESTSET = ScopedValue{AbstractTestSet}(FallbackTestSet()) | ||
const TESTSET_DEPTH = ScopedValue{Int}(0) | ||
|
||
macro with_testset(ts, expr) | ||
:(@with(CURRENT_TESTSET => $(esc(ts)), TESTSET_DEPTH => get_testset_depth() + 1, $(esc(expr)))) | ||
end | ||
|
||
""" | ||
get_testset() | ||
|
||
Retrieve the active test set from the task's local storage. If no | ||
test set is active, use the fallback default test set. | ||
""" | ||
function get_testset() | ||
testsets = get(task_local_storage(), :__BASETESTNEXT__, AbstractTestSet[]) | ||
return isempty(testsets) ? fallback_testset : testsets[end] | ||
end | ||
|
||
""" | ||
push_testset(ts::AbstractTestSet) | ||
|
||
Adds the test set to the `task_local_storage`. | ||
""" | ||
function push_testset(ts::AbstractTestSet) | ||
testsets = get(task_local_storage(), :__BASETESTNEXT__, AbstractTestSet[]) | ||
push!(testsets, ts) | ||
setindex!(task_local_storage(), testsets, :__BASETESTNEXT__) | ||
end | ||
Comment on lines
-1952
to
-1961
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Apparently some (testing) packages were using this: https://juliahub.com/ui/Search?type=symbols&q=push_testset&u=use. Presumably it's ok to ask them to adapt their use of internal functions? These aren't documented. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, unfortunately I don't see how we could keep that working There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. How would one update a codebase that is using push_testset(ts)
try
...
finally
pop_testset()
end to @with_testset ts begin
...
end ? (p.s. Very pleased to see this improvement!) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, that should work I think. |
||
|
||
""" | ||
pop_testset() | ||
|
||
Pops the last test set added to the `task_local_storage`. If there are no | ||
active test sets, returns the fallback default test set. | ||
""" | ||
function pop_testset() | ||
testsets = get(task_local_storage(), :__BASETESTNEXT__, AbstractTestSet[]) | ||
ret = isempty(testsets) ? fallback_testset : pop!(testsets) | ||
setindex!(task_local_storage(), testsets, :__BASETESTNEXT__) | ||
return ret | ||
something(Base.ScopedValues.get(CURRENT_TESTSET)) | ||
end | ||
|
||
""" | ||
|
@@ -1909,8 +1866,7 @@ end | |
Return the number of active test sets, not including the default test set | ||
""" | ||
function get_testset_depth() | ||
testsets = get(task_local_storage(), :__BASETESTNEXT__, AbstractTestSet[]) | ||
return length(testsets) | ||
something(Base.ScopedValues.get(TESTSET_DEPTH)) | ||
end | ||
|
||
_args_and_call((args..., f)...; kwargs...) = (args, kwargs, f(args...; kwargs...)) | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,6 +1,7 @@ | ||
# This file is a part of Julia. License is MIT: https://julialang.org/license | ||
|
||
using Base.ScopedValues | ||
using Test | ||
|
||
include("compiler/irutils.jl") | ||
|
||
|
@@ -80,13 +81,13 @@ import Base.Threads: @spawn | |
end | ||
|
||
@testset "show" begin | ||
@test sprint(show, ScopedValue{Int}()) == "Base.ScopedValues.ScopedValue{$Int}(undefined)" | ||
@test sprint(show, sval) == "Base.ScopedValues.ScopedValue{$Int}(1)" | ||
@test sprint(show, Core.current_scope()) == "nothing" | ||
@test sprint(show, ScopedValue{Int}()) == "$ScopedValue{$Int}(undefined)" | ||
@test sprint(show, sval) == "$ScopedValue{$Int}(1)" | ||
# @test sprint(show, Core.current_scope()) == "nothing" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why was this test commented out? Can we move it at the top-level, outside of any testset, to keep the test, or that's still within an implicit scope? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Depending on how the runtests is called, it might be inside a scope. The only way to guarantee it is to run it in a separate process. |
||
with(sval => 2.0) do | ||
@test sprint(show, sval) == "Base.ScopedValues.ScopedValue{$Int}(2)" | ||
@test sprint(show, sval) == "$ScopedValue{$Int}(2)" | ||
objid = sprint(show, Base.objectid(sval)) | ||
@test sprint(show, Core.current_scope()) == "Base.ScopedValues.Scope(Base.ScopedValues.ScopedValue{$Int}@$objid => 2)" | ||
# @test sprint(show, Core.current_scope()) == "Base.ScopedValues.Scope(Base.ScopedValues.ScopedValue{$Int}@$objid => 2)" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Also this: why was it removed? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same, I can't see a way to make this work. |
||
end | ||
end | ||
|
||
|
Uh oh!
There was an error while loading. Please reload this page.