Skip to content
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

[WIP/RFC] Rewrite testing infrastructure to live in Base #19567

Open
wants to merge 8 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 7 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion .travis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,6 @@ script:
/tmp/julia/bin/julia --check-bounds=yes runtests.jl $TESTSTORUN &&
/tmp/julia/bin/julia --check-bounds=yes runtests.jl libgit2-online pkg
- cd `dirname $TRAVIS_BUILD_DIR` && mv julia2 julia &&
rm -rf julia/deps/scratch/julia-env &&
rm -f julia/deps/scratch/libgit2-*/CMakeFiles/CMakeOutput.log
# uncomment the following if failures are suspected to be due to the out-of-memory killer
# - dmesg
3 changes: 3 additions & 0 deletions base/deprecated.jl
Original file line number Diff line number Diff line change
Expand Up @@ -1134,4 +1134,7 @@ end)
@deprecate cummin(A, dim=1) accumulate(min, A, dim=1)
@deprecate cummax(A, dim=1) accumulate(max, A, dim=1)

# #19567
@deprecate runtests(tests = ["all"], numcores = ceil(Int, Sys.CPU_CORES / 2)) Base.Test.runtests(["all"], test_dir=joinpath(JULIA_HOME, "../../test"), numcores=numcores)

# End deprecations scheduled for 0.6
28 changes: 0 additions & 28 deletions base/interactiveutil.jl
Original file line number Diff line number Diff line change
Expand Up @@ -632,34 +632,6 @@ function workspace()
nothing
end

# testing

"""
runtests([tests=["all"] [, numcores=ceil(Int, Sys.CPU_CORES / 2) ]])

Run the Julia unit tests listed in `tests`, which can be either a string or an array of
strings, using `numcores` processors. (not exported)
"""
function runtests(tests = ["all"], numcores = ceil(Int, Sys.CPU_CORES / 2))
if isa(tests,AbstractString)
tests = split(tests)
end
ENV2 = copy(ENV)
ENV2["JULIA_CPU_CORES"] = "$numcores"
try
run(setenv(`$(julia_cmd()) $(joinpath(JULIA_HOME,
Base.DATAROOTDIR, "julia", "test", "runtests.jl")) $tests`, ENV2))
catch
buf = PipeBuffer()
versioninfo(buf)
error("A test has failed. Please submit a bug report (https://github.com/JuliaLang/julia/issues)\n" *
"including error messages above and the output of versioninfo():\n$(readstring(buf))")
end
end

# testing


"""
whos(io::IO=STDOUT, m::Module=current_module(), pattern::Regex=r"")

Expand Down
220 changes: 219 additions & 1 deletion base/test.jl
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,9 @@ export @testset
export @test_approx_eq, @test_approx_eq_eps, @inferred
export detect_ambiguities
export GenericString
export runtests

include("testdefs.jl")

#-----------------------------------------------------------------------

Expand Down Expand Up @@ -407,7 +410,7 @@ finish(ts::FallbackTestSet) = ts
"""
DefaultTestSet

If using the DefaultTestSet, the test results will be recorded. If there
If using the `DefaultTestSet`, the test results will be recorded. If there
are any `Fail`s or `Error`s, an exception will be thrown only at the end,
along with a summary of the test results.
"""
Expand Down Expand Up @@ -1075,6 +1078,83 @@ function detect_ambiguities(mods...; imported::Bool=false)
return collect(ambs)
end

"""
build_results_testset(results)

Construct a testset on the master node which will hold results from all the
test files run on workers and on node1. The loop goes through the results,
inserting them as children of the overall testset if they are testsets,
handling errors otherwise.

Since the workers don't return information about passing/broken tests, only
errors or failures, those Result types get passed `nothing` for their test
expressions (and expected/received result in the case of Broken).

If a test failed, returning a `RemoteException`, the error is displayed and
the overall testset has a child testset inserted, with the (empty) Passes
and Brokens from the worker and the full information about all errors and
failures encountered running the tests. This information will be displayed
as a summary at the end of the test run.

If a test failed, returning an `Exception` that is not a `RemoteException`,
it is likely the julia process running the test has encountered some kind
of internal error, such as a segfault. The entire testset is marked as
Errored, and execution continues until the summary at the end of the test
run, where the test file is printed out as the "failed expression".
"""
function build_results_testset(results)
o_ts = DefaultTestSet("Overall")
push_testset(o_ts)
for res in results
if isa(res[2][1], DefaultTestSet)
push_testset(res[2][1])
record(o_ts, res[2][1])
pop_testset()
elseif isa(res[2][1], Tuple{Int,Int})
fake = DefaultTestSet(res[1])
for i in 1:res[2][1][1]
record(fake, Pass(:test, nothing, nothing, nothing))
end
for i in 1:res[2][1][2]
record(fake, Broken(:test, nothing))
end
push_testset(fake)
record(o_ts, fake)
pop_testset()
elseif isa(res[2][1], RemoteException)
println("Worker $(res[2][1].pid) failed running test $(res[1]):")
Base.showerror(STDOUT,res[2][1].captured)
o_ts.anynonpass = true
if isa(res[2][1].captured.ex, TestSetException)
fake = DefaultTestSet(res[1])
for i in 1:res[2][1].captured.ex.pass
record(fake, Pass(:test, nothing, nothing, nothing))
end
for i in 1:res[2][1].captured.ex.broken
record(fake, Broken(:test, nothing))
end
for t in res[2][1].captured.ex.errors_and_fails
record(fake, t)
end
push_testset(fake)
record(o_ts, fake)
pop_testset()
end
elseif isa(res[2][1], Exception)
# If this test raised an exception that is not a RemoteException, that means
# the test runner itself had some problem, so we may have hit a segfault
# or something similar. Record this testset as Errored.
o_ts.anynonpass = true
fake = DefaultTestSet(res[1])
record(fake, Error(:test_error, res[1], res[2][1], []))
push_testset(fake)
record(o_ts, fake)
pop_testset()
end
end
return o_ts
end

"""
The `GenericString` can be used to test generic string APIs that program to
the `AbstractString` interface, in order to ensure that functions can work
Expand All @@ -1087,4 +1167,142 @@ Base.convert(::Type{GenericString}, s::AbstractString) = GenericString(s)
Base.endof(s::GenericString) = endof(s.string)
Base.next(s::GenericString, i::Int) = next(s.string, i)

function move_to_node1!(tests, node1, ts)
for t in ts
if t in tests
splice!(tests, findfirst(tests, t))
push!(node1, t)
end
end
end

function print_test_statistics(test, test_stats, worker, alignments)
name_str = rpad(test*" ($worker)", alignments[1], " ")
time_str = @sprintf("%7.2f", test_stats[2])
time_str = rpad(time_str, alignments[2], " ")
gc_str = @sprintf("%5.2f", test_stats[5].total_time/10^9)
gc_str = rpad(gc_str, alignments[3] ," ")
# since there may be quite a few digits in the percentage,
# the left-padding here is less to make sure everything fits
percent_str = @sprintf("%4.1f",100*test_stats[5].total_time/(10^9*test_stats[2]))
percent_str = rpad(percent_str,alignments[4]," ")
alloc_str = @sprintf("%5.2f",test_stats[3]/2^20)
alloc_str = rpad(alloc_str,alignments[5]," ")
rss_str = @sprintf("%5.2f",test_stats[6]/2^20)
rss_str = rpad(rss_str,alignments[6]," ")
print_with_color(:white, name_str, " | ", time_str, " | ", gc_str, " | ", percent_str, " | ", alloc_str, " | ", rss_str, "\n")
end

function runtests(names=["all"]; test_dir=joinpath(JULIA_HOME, Base.DATAROOTDIR, "julia/test/"), numcores::Int=ceil(Int, Sys.CPU_CORES / 2))
include(joinpath(test_dir, "choosetests.jl"))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe these includes should just move out of the function. This function shouldnt call include except on the test file

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The includes are in the function in case you want to pass the package's test dir in, and you can have your own choosetests.jl in Pkg.dir()/test

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That doesn't sound like a good API. And it also doesn't apply to testdefs.jl, which should just be part of Base.

If a package needs to customize the tests it wants to run, it should just do that in runtests.jl and pass the test list to Base.runtests().

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AFAICT doing this in choosetests.jl instead of runtests.jl doesn't really make it any easier for the packages. The most useful part of the base choosetests.jl is (IMHO) the --skip option and the all "meta"-test and this currently doesn't make it possible for the package to use it. It might be useful to provide a few helper functions for cmdline parsing but I honestly don't think the one we have in base is a particularly good one and packages are better off using sth like ArgParse.jl for that.

Even if a choosetests type of interface is wanted, it should be implemented as a callback rather than includeing a file.

tests, n1_tests, bigmemtests, net_on = Main.choosetests(names)
tests = unique(tests)
n1_tests = unique(n1_tests)
bigmemtests = unique(bigmemtests)
# In a constrained memory environment, run the tests which may need a lot of memory after all others
const max_worker_rss = if haskey(ENV, "JULIA_TEST_MAXRSS_MB")
parse(Int, ENV["JULIA_TEST_MAXRSS_MB"]) * 2^20
else
typemax(Csize_t)
end
node1_tests = String[]
move_to_node1!(tests, node1_tests, n1_tests)
if max_worker_rss != typemax(Csize_t)
move_to_node1!(tests, node1_tests, bigmemtests)
end

if haskey(ENV, "JULIA_TEST_EXEFLAGS")
const test_exeflags = `$(Base.shell_split(ENV["JULIA_TEST_EXEFLAGS"]))`
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No const

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds good

else
const test_exeflags = `--check-bounds=yes --startup-file=no --depwarn=error`
end

if haskey(ENV, "JULIA_TEST_EXENAME")
const test_exename = `$(Base.shell_split(ENV["JULIA_TEST_EXENAME"]))`
else
const test_exename = `$(joinpath(JULIA_HOME, Base.julia_exename()))`
end

cd(test_dir) do
n = 1
if net_on
n = min(numcores, length(tests))
n > 1 && addprocs(n; exename=test_exename, exeflags=test_exeflags)
BLAS.set_num_threads(1)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this have to be set back to the default after performing the task that requires a single thread?

end

@everywhere include("../base/testdefs.jl")

#pretty print the information about gc and mem usage
name_align = max(length("Test (Worker)"), maximum(map(x -> length(x) + 3 + ndigits(nworkers()), tests)))
elapsed_align = length("Time (s)")
gc_align = length("GC (s)")
percent_align = length("GC %")
alloc_align = length("Alloc (MB)")
rss_align = length("RSS (MB)")
alignments = (name_align, elapsed_align, gc_align, percent_align, alloc_align, rss_align)
print_with_color(:white, rpad("Test (Worker)",name_align," "), " | ")
print_with_color(:white, "Time (s) | GC (s) | GC % | Alloc (MB) | RSS (MB)\n")
results=[]
@sync begin
for p in workers()
@async begin
while length(tests) > 0
test = shift!(tests)
local resp
try
resp = remotecall_fetch(runtest, p, test)
catch e
resp = [e]
end
push!(results, (test, resp))
if (isa(resp[end], Integer) && (resp[end] > max_worker_rss)) || isa(resp, Exception)
if n > 1
rmprocs(p, waitfor=0.5)
p = addprocs(1; exename=test_exename, exeflags=test_exeflags)[1]
remotecall_fetch(()->include("../base/testdefs.jl"), p)
else
# single process testing, bail if mem limit reached, or, on an exception.
isa(resp, Exception) ? rethrow(resp) : error("Halting tests. Memory limit reached : $resp > $max_worker_rss")
end
end
if !isa(resp[1], Exception)
print_test_statistics(test, resp, p, alignments)
end
end
end
end
end
# Free up memory =)
n > 1 && rmprocs(workers(), waitfor=5.0)
for t in node1_tests
# As above, try to run each test
# which must run on node 1. If
# the test fails, catch the error,
# and either way, append the results
# to the overall aggregator
local resp
try
resp = runtest(t)
catch e
resp = [e]
end
push!(results, (t, resp))
if !isa(resp[1], Exception)
print_test_statistics(t, resp, 1, alignments)
end
end
o_ts = build_results_testset(results)
println()
print_test_results(o_ts,1)
if !o_ts.anynonpass
println(" \033[32;1mSUCCESS\033[0m")
else
println(" \033[31;1mFAILURE\033[0m")
print_test_errors(o_ts)
error()
end
end
end

end # module
33 changes: 33 additions & 0 deletions base/testdefs.jl
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
# This file is a part of Julia. License is MIT: http://julialang.org/license

function runtest(name, isolate=true)
if isolate
mod_name = Symbol("TestMain_", replace(name, '/', '_'))
m = eval(Main, :(module $mod_name end))
else
m = Main
end
eval(m, :(using Base.Test))
ex = quote
@timed @testset $"$name" begin
include($"$name.jl")
end
end
res_and_time_data = eval(m, ex)
rss = Sys.maxrss()
#res_and_time_data[1] is the testset
passes,fails,error,broken,c_passes,c_fails,c_errors,c_broken = Base.Test.get_test_counts(res_and_time_data[1])
if res_and_time_data[1].anynonpass == false
res_and_time_data = (
(passes+c_passes,broken+c_broken),
res_and_time_data[2],
res_and_time_data[3],
res_and_time_data[4],
res_and_time_data[5])
end
vcat(collect(res_and_time_data), rss)
end

# looking in . messes things up badly
#filter!(x->x!=".", LOAD_PATH)
nothing
2 changes: 1 addition & 1 deletion test/Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ endif
$(TESTS):
@cd $(SRCDIR) && \
$(ULIMIT_TEST) \
$(call PRINT_JULIA, $(call spawn,$(JULIA_EXECUTABLE)) --check-bounds=yes --startup-file=no ./runtests.jl $@)
$(call PRINT_JULIA, $(call spawn,$(JULIA_EXECUTABLE)) --check-bounds=yes --startup-file=no -e "Base.Test.runtests(ARGS)" $@)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we make sure we still have this file? It's very useful to run the tests without going through makefile in order to specify different options.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, I can put it back in.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That will also un-break Travis and AV, probably 😓

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done! runtests.jl is back as a stub


perf:
@$(MAKE) -C $(SRCDIR)/perf all
Expand Down
11 changes: 7 additions & 4 deletions test/choosetests.jl
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
# This file is a part of Julia. License is MIT: http://julialang.org/license

@doc """
"""
choosetests(choices = []) -> tests, node1_tests, bigmemtests, net_on

`tests, net_on = choosetests(choices)` selects a set of tests to be
`choosetests(choices)` selects a set of tests to be
run. `choices` should be a vector of test names; if empty or set to
`["all"]`, all tests are selected.

Expand All @@ -12,7 +13,7 @@ directories.

Upon return, `tests` is a vector of fully-expanded test names, and
`net_on` is true if networking is available (required for some tests).
""" ->
"""
function choosetests(choices = [])
testnames = [
"linalg", "subarray", "core", "inference", "keywordargs", "numbers",
Expand Down Expand Up @@ -162,5 +163,7 @@ function choosetests(choices = [])

filter!(x -> !(x in skip_tests), tests)

tests, net_on
node1_tests = ["compile"]
bigmemtests = ["parallel"]
tests, node1_tests, bigmemtests, net_on
end
Loading