Skip to content

Commit

Permalink
fixup! Giant refactor to move all state into a Kernel struct
Browse files Browse the repository at this point in the history
  • Loading branch information
@JamesWrigley
JamesWrigley committed Feb 12, 2025
1 parent 20dc591 commit 2fd607f
Show file tree
Hide file tree
Showing 5 changed files with 159 additions and 126 deletions.
5 changes: 1 addition & 4 deletions src/IJulia.jl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -73,8 +73,6 @@ mutable struct Msg
end
end

verbose::Bool = IJULIA_DEBUG

@kwdef mutable struct Kernel
verbose::Bool = IJULIA_DEBUG
inited::Bool = false
Expand Down Expand Up @@ -137,7 +135,7 @@ end

function Base.setproperty!(kernel::Kernel, name::Symbol, x)

Check warning on line 136 in src/IJulia.jl

View check run for this annotation

Codecov / codecov/patch

src/IJulia.jl#L136 

Added line #L136 was not covered by tests
# These fields need to be assigned explicitly to their global counterparts
if name ∈ (:ans, :n, :In, :Out, :inited, :verbose)
if name ∈ (:ans, :n, :In, :Out, :inited)
setproperty!(IJulia, name, x)

Check warning on line 139 in src/IJulia.jl

View check run for this annotation

Codecov / codecov/patch

src/IJulia.jl#L138-L139 

Added lines #L138 - L139 were not covered by tests

if name ∈ (:ans, :In, :Out)
Expand Down Expand Up @@ -187,7 +185,6 @@ function Base.close(kernel::Kernel)
kernel.inited = false
IJulia.ans = nothing
IJulia.n = 0
IJulia.verbose = IJULIA_DEBUG
IJulia._default_kernel = nothing

Check warning on line 188 in src/IJulia.jl

View check run for this annotation

Codecov / codecov/patch

src/IJulia.jl#L185-L188 

Added lines #L185 - L188 were not covered by tests

# The waitloop should now be ready to exit
Expand Down
2 changes: 1 addition & 1 deletion src/init.jl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ the path to an existing connection file. If `args` is empty a connection file
will be generated.
"""
function init(args, kernel, profile=nothing)
kernel.inited && error("IJulia is already running")
!isnothing(_default_kernel) && error("IJulia is already running")

Check warning on line 41 in src/init.jl

View check run for this annotation

Codecov / codecov/patch

src/init.jl#L40-L41 

Added lines #L40 - L41 were not covered by tests
if length(args) > 0
merge!(kernel.profile, open(JSON.parse,args[1]))
kernel.verbose && println("PROFILE = $profile")
Expand Down
2 changes: 1 addition & 1 deletion src/kernel.jl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ ENV["LINES"] = get(ENV, "LINES", 30)
ENV["COLUMNS"] = get(ENV, "COLUMNS", 80)

println("Starting kernel event loops.")
IJulia.init(ARGS, IJulia._default_kernel)
IJulia.init(ARGS, IJulia.Kernel())

let startupfile = !isempty(DEPOT_PATH) ? abspath(DEPOT_PATH[1], "config", "startup_ijulia.jl") : ""
isfile(startupfile) && Base.JLOptions().startupfile != 2 && Base.include(Main, startupfile)
Expand Down
6 changes: 3 additions & 3 deletions src/stdio.jl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,15 +37,15 @@ end

macro vprintln(x...)
quote
if verbose::Bool
if _default_kernel.verbose
println(orig_stdout[], get_log_preface(), $(map(esc, x)...))
end
end
end

macro verror_show(e, bt)
quote
if verbose::Bool
if _default_kernel.verbose
showerror(orig_stderr[], $(esc(e)), $(esc(bt)))
end
end
Expand Down Expand Up @@ -103,7 +103,7 @@ function watch_stream(rd::IO, name::AbstractString, kernel)
end

function send_stdio(name, kernel)
if verbose::Bool && !haskey(task_local_storage(), :IJulia_task)
if kernel.verbose && !haskey(task_local_storage(), :IJulia_task)

Check warning on line 106 in src/stdio.jl

View check run for this annotation

Codecov / codecov/patch

src/stdio.jl#L105-L106 

Added lines #L105 - L106 were not covered by tests
task_local_storage(:IJulia_task, "send $name task")
end
send_stream(name, kernel)

Check warning on line 109 in src/stdio.jl

View check run for this annotation

Codecov / codecov/patch

src/stdio.jl#L109 

Added line #L109 was not covered by tests
Expand Down
270 changes: 153 additions & 117 deletions test/waitloop.jl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,10 @@ jupyter_client_lib.session.protocol_version = "5.4"
const BlockingKernelClient = jupyter_client_lib.BlockingKernelClient

import IJulia: Kernel
# These symbols are imported so that we can test that setproperty!(::Kernel)
# will propagate changes from the corresponding Kernel fields to the
# module-global variables.
import IJulia: ans, In, Out


function getports(port_hint, n)
Expand Down Expand Up @@ -127,13 +131,19 @@ function jupyter_client(f, profile)
end

@testset "Kernel" begin
let kernel = Kernel()
profile = create_profile()
profile_kwargs = Dict([Symbol(key) => value for (key, value) in profile])
profile_kwargs[:key] = pystr(profile_kwargs[:key]).encode()

@testset "getproperty()/setproperty!()" begin
kernel = Kernel()

# Test setting special fields that should be mirrored to global variables
for field in (:n, :ans, :inited, :verbose)
for field in (:n, :ans, :inited)
# Save the old value so we can restore them afterwards
old_value = getproperty(kernel, field)

test_value = field ∈ (:inited, :verbose) ? true : 10
test_value = field === :inited ? true : 10
setproperty!(kernel, field, test_value)
@test getproperty(IJulia, field) == test_value
@test getproperty(kernel, field) == test_value
Expand All @@ -142,138 +152,164 @@ end
end
end

profile = create_profile()
shutdown_called = false
Kernel(profile; capture_stdout=false, capture_stderr=false, shutdown=() -> shutdown_called = true) do kernel
jupyter_client(profile) do client
# Test clear_history()
for i in 1:10
@test msg_ok(execute(client, "$(i)"))
end
@test length(kernel.In) == 10
@test msg_ok(execute(client, "IJulia.clear_history(-1:5)"))
@test Set(keys(kernel.In)) == Set(6:11) # The 11th entry is the call to clear_history()
@test msg_ok(execute(client, "IJulia.clear_history()"))
@test isempty(kernel.In)
@test isempty(kernel.Out)

# Test load()/load_string()
mktemp() do path, _
write(path, "42")

msg = execute(client, """IJulia.load("$(path)")""")
@test msg_ok(msg)
@test length(msg["content"]["payload"]) == 1
end

# Test hooks
let
preexecute = false
postexecute = false
posterror = false
preexecute_hook = () -> preexecute = !preexecute
postexecute_hook = () -> postexecute = !postexecute
posterror_hook = () -> posterror = !posterror
IJulia.push_preexecute_hook(preexecute_hook)
IJulia.push_postexecute_hook(postexecute_hook)
IJulia.push_posterror_hook(posterror_hook)
@testset "Explicit tests with jupyter_client" begin
shutdown_called = false
Kernel(profile; capture_stdout=false, capture_stderr=false, shutdown=() -> shutdown_called = true) do kernel
jupyter_client(profile) do client
# Test clear_history()
for i in 1:10
@test msg_ok(execute(client, "$(i)"))
end
@test length(kernel.In) == 10
@test msg_ok(execute(client, "IJulia.clear_history(-1:5)"))
@test Set(keys(kernel.In)) == Set(6:11) # The 11th entry is the call to clear_history()
@test msg_ok(execute(client, "IJulia.clear_history()"))
@test isempty(kernel.In)
@test isempty(kernel.Out)

# Test load()/load_string()
mktemp() do path, _
write(path, "42")

msg = execute(client, """IJulia.load("$(path)")""")
@test msg_ok(msg)
@test length(msg["content"]["payload"]) == 1
end

# Test hooks
@testset "Hooks" begin
preexecute = false
postexecute = false
posterror = false
preexecute_hook = () -> preexecute = !preexecute
postexecute_hook = () -> postexecute = !postexecute
posterror_hook = () -> posterror = !posterror
IJulia.push_preexecute_hook(preexecute_hook)
IJulia.push_postexecute_hook(postexecute_hook)
IJulia.push_posterror_hook(posterror_hook)
@test msg_ok(execute(client, "42"))

# The pre/post hooks should've been called but not the posterror hook
@test preexecute
@test postexecute
@test !posterror

# With a throwing cell the posterror hook should be called
@test msg_error(execute(client, "error(42)"))
@test posterror

# After popping the hooks they should no longer be executed
preexecute = false
postexecute = false
posterror = false
IJulia.pop_preexecute_hook(preexecute_hook)
IJulia.pop_postexecute_hook(postexecute_hook)
IJulia.pop_posterror_hook(posterror_hook)
@test msg_ok(execute(client, "42"))
@test msg_error(execute(client, "error(42)"))
@test !preexecute
@test !postexecute
@test !posterror
end

# Smoke tests
@test msg_ok(kernel_info(client))
@test msg_ok(comm_info(client))
@test msg_ok(history(client))
@test msg_ok(execute(client, "IJulia.set_verbose(false)"))
@test msg_ok(execute(client, "flush(stdout)"))

# Test history(). This test requires `capture_stdout=false`.
IJulia.clear_history()
@test msg_ok(execute(client, "1"))
@test msg_ok(execute(client, "42"))
stdout_pipe = Pipe()
redirect_stdout(stdout_pipe) do
IJulia.history()
end
close(stdout_pipe.in)
@test collect(eachline(stdout_pipe)) == ["1", "42"]

# The pre/post hooks should've been called but not the posterror hook
@test preexecute
@test postexecute
@test !posterror

# With a throwing cell the posterror hook should be called
@test msg_error(execute(client, "error(42)"))
@test posterror

# After popping the hooks they should no longer be executed
preexecute = false
postexecute = false
posterror = false
IJulia.pop_preexecute_hook(preexecute_hook)
IJulia.pop_postexecute_hook(postexecute_hook)
IJulia.pop_posterror_hook(posterror_hook)
# Test that certain global variables are updated in kernel.current_module
@test msg_ok(execute(client, "42"))
@test msg_error(execute(client, "error(42)"))
@test !preexecute
@test !postexecute
@test !posterror
end
@test msg_ok(execute(client, "ans == 42"))
@test kernel.ans

# Smoke tests
@test msg_ok(kernel_info(client))
@test msg_ok(comm_info(client))
@test msg_ok(history(client))
@test msg_ok(execute(client, "IJulia.set_verbose(false)"))
@test msg_ok(execute(client, "flush(stdout)"))

# Test history(). This test requires `capture_stdout=false`.
IJulia.clear_history()
@test msg_ok(execute(client, "1"))
@test msg_ok(execute(client, "42"))
stdout_pipe = Pipe()
redirect_stdout(stdout_pipe) do
IJulia.history()
# Test shutdown_request
@test shutdown(client)["content"]["status"] == "ok"
@test timedwait(() -> shutdown_called, 10) == :ok
end
close(stdout_pipe.in)
@test collect(eachline(stdout_pipe)) == ["1", "42"]
end
end

# Test that certain global variables are updated in kernel.current_module
@test msg_ok(execute(client, "42"))
@test msg_ok(execute(client, "ans == 42"))
@test_broken kernel.ans
@testset "jupyter_kernel_test" begin
stdout_pipe = Pipe()
stderr_pipe = Pipe()
Base.link_pipe!(stdout_pipe)
Base.link_pipe!(stderr_pipe)
stdout_str = ""
stderr_str = ""
test_proc = nothing

Kernel(profile; shutdown=Returns(nothing)) do kernel
test_file = joinpath(@__DIR__, "kernel_test.py")

mktemp() do connection_file, io
# Write the connection file
jupyter_client_lib.connect.write_connection_file(; fname=connection_file, profile_kwargs...)

try
# Run jupyter_kernel_test
cmd = ignorestatus(`$(PythonCall.C.python_executable_path()) $(test_file)`)
cmd = addenv(cmd, "IJULIA_TESTS_CONNECTION_FILE" => connection_file)
cmd = pipeline(cmd; stdout=stdout_pipe, stderr=stderr_pipe)
test_proc = run(cmd)
finally
close(stdout_pipe.in)
close(stderr_pipe.in)
stdout_str = read(stdout_pipe, String)
stderr_str = read(stderr_pipe, String)
close(stdout_pipe)
close(stderr_pipe)
end
end
end

# Test shutdown_request
@test shutdown(client)["content"]["status"] == "ok"
@test timedwait(() -> shutdown_called, 10) == :ok
if !isempty(stdout_str)
@info "jupyter_kernel_test stdout:"
println(stdout_str)
end
if !isempty(stderr_str)
@info "jupyter_kernel_test stderr:"
println(stderr_str)
end
if !success(test_proc)
error("jupyter_kernel_test failed")
end
end

stdout_pipe = Pipe()
stderr_pipe = Pipe()
Base.link_pipe!(stdout_pipe)
Base.link_pipe!(stderr_pipe)
stdout_str = ""
stderr_str = ""
test_proc = nothing

Kernel(profile; shutdown=Returns(nothing)) do kernel
test_file = joinpath(@__DIR__, "kernel_test.py")
# kernel.jl is the script that's actually run by Jupyter
@testset "kernel.jl" begin
kernel_jl = joinpath(@__DIR__, "..", "src", "kernel.jl")
julia = joinpath(Sys.BINDIR, "julia")

mktemp() do connection_file, io
# Write the connection file
profile_kwargs = Dict([Symbol(key) => value for (key, value) in profile])
profile_kwargs[:key] = pystr(profile_kwargs[:key]).encode()
jupyter_client_lib.connect.write_connection_file(; fname=connection_file, profile_kwargs...)

cmd = `$julia --startup-file=no --project=$(Base.active_project()) $kernel_jl $(connection_file)`
kernel_proc = run(pipeline(cmd; stdout, stderr); wait=false)
try
# Run jupyter_kernel_test
cmd = ignorestatus(`$(PythonCall.C.python_executable_path()) $(test_file)`)
cmd = addenv(cmd, "IJULIA_TESTS_CONNECTION_FILE" => connection_file)
cmd = pipeline(cmd; stdout=stdout_pipe, stderr=stderr_pipe)
test_proc = run(cmd)
jupyter_client(profile) do client
@test msg_ok(kernel_info(client))
@test msg_ok(execute(client, "42"))
@test msg_ok(shutdown(client))
end

@test timedwait(() -> process_exited(kernel_proc), 10) == :ok
finally
close(stdout_pipe.in)
close(stderr_pipe.in)
stdout_str = read(stdout_pipe, String)
stderr_str = read(stderr_pipe, String)
close(stdout_pipe)
close(stderr_pipe)
kill(kernel_proc)
end
end
end

if !isempty(stdout_str)
@info "jupyter_kernel_test stdout:"
println(stdout_str)
end
if !isempty(stderr_str)
@info "jupyter_kernel_test stderr:"
println(stderr_str)
end
if !success(test_proc)
error("jupyter_kernel_test failed")
end
end

0 comments on commit 2fd607f

Please sign in to comment.