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

Call the kaleido binary directly (fixes permission errors on Windows and Julia 1.10) #17

Merged
merged 10 commits into from
Mar 3, 2024
Merged

Call the kaleido binary directly (fixes permission errors on Windows and Julia 1.10) #17

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
83b3dfe
point to kaleido.exe directly on windows
disberd Feb 28, 2024
4080fc0
try alternative
disberd Feb 29, 2024
0058f31
remove hanging stuff
disberd Feb 29, 2024
565fcdf
remove `@test_nowarn` on start()
disberd Feb 29, 2024
8d8f725
put bck test_nowarn on non-windows
disberd Mar 1, 2024
c462fc1
warn about windows problem on README
disberd Mar 1, 2024
f678ac4
testnowarn error
disberd Mar 2, 2024
ebe9ff2
revert test_nowarn error
disberd Mar 2, 2024
be759e4
Throw an error with a suggestion fix if kaleido seems to hang
disberd Mar 2, 2024
6fab6e3
fix noblock function as bytesavailable does not work here
disberd Mar 2, 2024
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
7 changes: 4 additions & 3 deletions Project.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,11 @@ julia = "1.6"
Kaleido_jll = "0.1, 0.2"

[extras]
Test = "8dfed614-e22c-5e08-85e1-65c5234f0b40"
PlotlyLight = "ca7969ec-10b3-423e-8d99-40f33abb42bf"
EasyConfig = "acab07b0-f158-46d4-8913-50acef6d41fe"
PlotlyJS = "f0f68f2c-4968-5e81-91da-67840de0976a"
PlotlyLight = "ca7969ec-10b3-423e-8d99-40f33abb42bf"
Test = "8dfed614-e22c-5e08-85e1-65c5234f0b40"
Pkg = "44cfe95a-1eb2-52ea-b672-e2afdf69b78f"

[targets]
test = ["Test", "PlotlyLight", "EasyConfig", "PlotlyJS"]
test = ["Test", "PlotlyLight", "EasyConfig", "PlotlyJS", "Pkg"]
11 changes: 7 additions & 4 deletions src/PlotlyKaleido.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,10 @@ function start(;
kwargs...,
)
is_running() && return
cmd = joinpath(Kaleido_jll.artifact_dir, "kaleido" * (Sys.iswindows() ? ".cmd" : ""))
basic_cmds = [cmd, "plotly"]
# The kaleido executable must be ran from the artifact directory
BIN = Cmd(kaleido(); dir = Kaleido_jll.artifact_dir)
# We push the mandatory plotly flag
push!(BIN.exec, "plotly")
chromium_flags = ["--disable-gpu", Sys.isapple() ? "--single-process" : "--no-sandbox"]
extra_flags = if plotly_version === missing
(; kwargs...)
Expand Down Expand Up @@ -72,12 +74,13 @@ function start(;
push!(user_flags, "--$flag_name=$v")
end
end
BIN = Cmd(vcat(basic_cmds, chromium_flags, user_flags))
# We add the flags to the BIN
append!(BIN.exec, chromium_flags, extra_flags)

kstdin = Pipe()
kstdout = Pipe()
kstderr = Pipe()
kproc =
kproc =
run(pipeline(BIN, stdin = kstdin, stdout = kstdout, stderr = kstderr), wait = false)

process_running(kproc) || error("There was a problem starting up kaleido.")
Expand Down
13 changes: 11 additions & 2 deletions test/runtests.jl
View file
Open in desktop
Copy link
Member

Choose a reason for hiding this comment

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

What was the error message here?

Copy link
Member Author

@disberd disberd Mar 2, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

I guess you are referring to the removal of @test_nowarn, you can check the error in the latest CI on windows here but I will also paste it here:

Error 
Start: Error During Test at D:\a\PlotlyKaleido.jl\PlotlyKaleido.jl\test\runtests.jl:9
  Got exception outside of a @test
  IOError: unlink("C:\\Users\\RUNNER~1\\AppData\\Local\\Temp\\jl_WJuNMdSL23"): resource busy or locked (EBUSY)
  Stacktrace:
    [1] uv_error
      @ .\libuv.jl:100 [inlined]
    [2] unlink(p::String)
      @ Base.Filesystem .\file.jl:978
    [3] rm(path::String; force::Bool, recursive::Bool)
      @ Base.Filesystem .\file.jl:283
    [4] macro expansion
      @ C:\hostedtoolcache\windows\julia\1.10.2\x64\share\julia\stdlib\v1.10\Test\src\Test.jl:906 [inlined]
    [5] macro expansion
      @ D:\a\PlotlyKaleido.jl\PlotlyKaleido.jl\test\runtests.jl:13 [inlined]
    [6] macro expansion
      @ C:\hostedtoolcache\windows\julia\1.10.2\x64\share\julia\stdlib\v1.10\Test\src\Test.jl:1577 [inlined]
    [7] top-level scope
      @ D:\a\PlotlyKaleido.jl\PlotlyKaleido.jl\test\runtests.jl:13
    [8] include(fname::String)
      @ Base.MainInclude .\client.jl:489
    [9] top-level scope
      @ none:6
   [10] eval
      @ .\boot.jl:385 [inlined]
   [11] exec_options(opts::Base.JLOptions)
      @ Base .\client.jl:291
   [12] _start()
      @ Base .\client.jl:552
Test Summary: | Pass  Error  Total  Time
Start         |    1      1      2  6.9s
ERROR: LoadError: Some tests did not pass: 1 passed, 0 failed, 1 errored, 0 broken.
in expression starting at D:\a\PlotlyKaleido.jl\PlotlyKaleido.jl\test\runtests.jl:9
┌ Warning: Failed to clean up temporary path "C:\\Users\\RUNNER~1\\AppData\\Local\\Temp\\jl_WJuNMdSL23"
│ Base.IOError("unlink(\"C:\\\\Users\\\\RUNNER~1\\\\AppData\\\\Local\\\\Temp\\\\jl_WJuNMdSL23\"): resource busy or locked (EBUSY)", -4082)
└ @ Base.Filesystem file.jl:549
ERROR: LoadError: Package PlotlyKaleido errored during testing
Stacktrace:
 [1] pkgerror(msg::String)
   @ Pkg.Types C:\hostedtoolcache\windows\julia\1.10.2\x64\share\julia\stdlib\v1.10\Pkg\src\Types.jl:70
 [2] test(ctx::Pkg.Types.Context, pkgs::Vector{Pkg.Types.PackageSpec}; coverage::Bool, julia_args::Cmd, test_args::Cmd, test_fn::Nothing, force_latest_compatible_version::Bool, allow_earlier_backwards_compatible_versions::Bool, allow_reresolve::Bool)
   @ Pkg.Operations C:\hostedtoolcache\windows\julia\1.10.2\x64\share\julia\stdlib\v1.10\Pkg\src\Operations.jl:2018
 [3] test
   @ C:\hostedtoolcache\windows\julia\1.10.2\x64\share\julia\stdlib\v1.10\Pkg\src\Operations.jl:1899 [inlined]
 [4] test(ctx::Pkg.Types.Context, pkgs::Vector{Pkg.Types.PackageSpec}; coverage::Bool, test_fn::Nothing, julia_args::Vector{String}, test_args::Cmd, force_latest_compatible_version::Bool, allow_earlier_backwards_compatible_versions::Bool, allow_reresolve::Bool, kwargs::@Kwargs{io::IOContext{Base.PipeEndpoint}})
   @ Pkg.API C:\hostedtoolcache\windows\julia\1.10.2\x64\share\julia\stdlib\v1.10\Pkg\src\API.jl:444
 [5] test(pkgs::Vector{Pkg.Types.PackageSpec}; io::IOContext{Base.PipeEndpoint}, kwargs::@Kwargs{coverage::Bool, julia_args::Vector{String}, force_latest_compatible_version::Bool})
   @ Pkg.API C:\hostedtoolcache\windows\julia\1.10.2\x64\share\julia\stdlib\v1.10\Pkg\src\API.jl:159
 [6] test(; name::Nothing, uuid::Nothing, version::Nothing, url::Nothing, rev::Nothing, path::Nothing, mode::Pkg.Types.PackageMode, subdir::Nothing, kwargs::@Kwargs{coverage::Bool, julia_args::Vector{String}, force_latest_compatible_version::Bool})
   @ Pkg.API C:\hostedtoolcache\windows\julia\1.10.2\x64\share\julia\stdlib\v1.10\Pkg\src\API.jl:174
 [7] top-level scope
   @ D:\a\_actions\julia-actions\julia-runtest\v1\test_harness.jl:15
 [8] include(fname::String)
   @ Base.MainInclude .\client.jl:489
 [9] top-level scope
   @ none:1
in expression starting at D:\a\_actions\julia-actions\julia-runtest\v1\test_harness.jl:7
Error: Process completed with exit code 1.

I was also quite baffled by this error and I kept getting it also locally. But calling PlotlyKaleido.start() without @test_nowarn does not produce the warning triggering the error..

At some point I thought it might be some quirk of downgrading kaleido_jll as part of the runtest file but at least locally I tried setting the Project compat on Kaleido_jll to version 0.1 but I still kept getting that error with @test_nowarn

Original file line number Diff line number Diff line change
@@ -1,8 +1,17 @@
using Test
import Pkg
if Sys.iswindows()
# Fix kaleido tests on windows due to [email protected] hanging
Pkg.add(;name = "Kaleido_jll", version = "0.1")
end
@test_nowarn @eval using PlotlyKaleido

@testset "Start" begin
@test_nowarn PlotlyKaleido.start()
@testset "Start" begin
if Sys.iswindows()
PlotlyKaleido.start()
else
@test_nowarn PlotlyKaleido.start()
end
@test PlotlyKaleido.is_running()
end

Expand Down
Loading