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.

Sign up for GitHub

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

Add timeout option #2

Merged
merged 1 commit into from
Mar 21, 2023
Merged

Add timeout option #2

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
2 changes: 1 addition & 1 deletion Project.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ Test = "8dfed614-e22c-5e08-85e1-65c5234f0b40"
julia = "1"
JuliaWorkspaces = "1.1"
JSON = "0.21"
JSONRPC = "1.3"
JSONRPC = "1.3.5"
ProgressMeter = "1.7"

[targets]
Expand Down
32 changes: 27 additions & 5 deletions src/TestItemRunner2.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -132,12 +132,20 @@ function get_free_testprocess(testitem, max_num_processes)
end
end

function execute_test(test_process, testitem, testsetups)
function execute_test(test_process, testitem, testsetups, timeout)
Copy link

Choose a reason for hiding this comment

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

@nickrobinson251 / @quinnj / @Drvi and I had been talking about possibly introducing a timeout parameter on the testitems themselves.

What do you think about on the testitem vs on the run_tests call? Maybe we'd want both?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, I think both probably makes sense... The only thing I don't like about that is that timeouts generally seem somewhat machine specific (at least in some cases), and that doesn't seem to fit having them on the test items. But on the other hand, there are probably timeouts that are not of that nature where attaching them to a specific test item does make sense? Say a test that downloads something?


test_process.current_testitem = testitem

return_value = Channel(1)

finished = false

timer = timeout>0 ? Timer(timeout) do i
if !finished
kill(test_process.process)
end
end : nothing

@async try
JSONRPC.send(
test_process.connection,
Expand Down Expand Up @@ -167,19 +175,30 @@ function execute_test(test_process, testitem, testsetups)
)
)

finished = true

timer === nothing || close(timer)

test_process.current_testitem = nothing

notify(SOME_TESTITEM_FINISHED)

push!(return_value, result)
catch err
Base.display_error(err, catch_backtrace())
if err isa InvalidStateException

notify(SOME_TESTITEM_FINISHED)

push!(return_value, (status="timeout", message="The test timed out"))
else
Base.display_error(err, catch_backtrace())
end
end

return return_value
end

function run_tests(path; filter=nothing, verbose=false, max_workers::Int=Sys.CPU_THREADS)
function run_tests(path; filter=nothing, verbose=false, max_workers::Int=Sys.CPU_THREADS, timeout=60*5)
jw = JuliaWorkspace(Set([filepath2uri(path)]))

count(Iterators.flatten(values(jw._testerrors))) > 0 && error("There is an error in your test item or test setup definition, we are aborting.")
Expand Down Expand Up @@ -210,7 +229,7 @@ function run_tests(path; filter=nothing, verbose=false, max_workers::Int=Sys.CPU
for testitem in testitems
test_process = get_free_testprocess(testitem, max_workers)

result_channel = execute_test(test_process, testitem, get(()->Dict{Symbol,Any}(), testsetups, testitem.detail.package_uri))
result_channel = execute_test(test_process, testitem, get(()->Dict{Symbol,Any}(), testsetups, testitem.detail.package_uri), timeout)

progress_reported_channel = Channel(1)

Expand All @@ -234,11 +253,14 @@ function run_tests(path; filter=nothing, verbose=false, max_workers::Int=Sys.CPU
responses = [(testitem=i.testitem, result=take!(i.result)) for i in executed_testitems]

count_success = 0
count_timeout = 0
count_fail = 0

for i in responses
if i.result.status=="passed"
count_success += 1
elseif i.result.status=="timeout"
count_timeout += 1
elseif i.result.message!==missing
count_fail += 1
println("Errors for test $(i.testitem.detail.name)")
Expand All @@ -252,7 +274,7 @@ function run_tests(path; filter=nothing, verbose=false, max_workers::Int=Sys.CPU
wait(i.progress_reported_channel)
end

println("$(length(responses)) tests ran, $(count_success) passed, $(count_fail) failed.")
println("$(length(responses)) tests ran, $(count_success) passed, $(count_fail) failed, $(count_timeout) timed out.")
end

function kill_test_processes()
Expand Down