search

timholy / Revise.jl

Automatically update function definitions in a running Julia session
https://timholy.github.io/Revise.jl/stable
Other
1.21k stars 110 forks source link

Test error when testing Revise on buildkite #834

Closed KristofferC closed 2 months ago

KristofferC commented 3 months ago

In order to not make Revise break all the time on master it would be a good idea to test it together with Julia CI so fixes can be more proactive.

https://github.com/JuliaCI/julia-buildkite/pull/372 adds such a test, however, the tests fail (https://buildkite.com/julialang/julia-buildkite/builds/1635#01910896-70f5-482d-bce9-c5cf16132e41) with:

Recipes: Error During Test at /root/.julia/packages/Revise/uvGMC/test/runtests.jl:2840
  Got exception outside of a @test
  KeyError: key Base.PkgId(nothing, "Core") not found
  Stacktrace:
    [1] getindex(h::Dict{Base.PkgId, Revise.PkgData}, key::Base.PkgId)
      @ Base ./dict.jl:477
    [2] top-level scope
      @ ~/.julia/packages/Revise/uvGMC/test/runtests.jl:78
    [3] macro expansion
      @ /cache/build/builder-amdci5-3/julialang/julia-buildkite/usr/share/julia/stdlib/v1.12/Test/src/Test.jl:1703 [inlined]
    [4] macro expansion
      @ ~/.julia/packages/Revise/uvGMC/test/runtests.jl:2842 [inlined]
    [5] macro expansion
      @ /cache/build/builder-amdci5-3/julialang/julia-buildkite/usr/share/julia/stdlib/v1.12/Test/src/Test.jl:1703 [inlined]
    [6] macro expansion
      @ ~/.julia/packages/Revise/uvGMC/test/runtests.jl:2887 [inlined]
    [7] include(fname::String)
      @ Main ./sysimg.jl:38
    [8] top-level scope
      @ none:6
    [9] eval
      @ ./boot.jl:438 [inlined]
   [10] exec_options(opts::Base.JLOptions)
      @ Base ./client.jl:290
   [11] _start()
      @ Base ./client.jl:551

It's not clear to me why the tests should fail there but not on the GHA runner used in this repo.

KristofferC commented 3 months ago

Looking at this, I don't think this code

https://github.com/timholy/Revise.jl/blob/13a5eb7986ee1239ff938a92043c13fee04579cc/test/runtests.jl#L2881-L2895

is tested at all on GitHub actions:

https://github.com/timholy/Revise.jl/actions/runs/10177871191/job/28150291970#step:7:230.

Keno commented 3 months ago

FWIW, the test is working for me locally. Does buildkite clone the git repo or not? I.e. which of the two test paths is it supposed to go down?

IanButterworth commented 3 months ago

From debugs on https://github.com/JuliaCI/julia-buildkite/pull/372

  | 2024-07-31 19:36:46 EDT | Revise.juliadir = "/cache/build/builder-amdci4-7/julialang/julia-buildkite/usr/share/julia"
  | 2024-07-31 19:36:46 EDT | Revise.git_repo(Revise.juliadir) = (LibGit2.GitRepo("/cache/build/builder-amdci4-7/julialang/julia-buildkite"), "/cache/build/builder-amdci4-7/julialang/julia-buildkite")
KristofferC commented 3 months ago

Putting the julia directory (non git) into a git directory makes Revise confused and I reprod not the exact failure but something similar doing so:

Recipes: Error During Test at /Users/kristoffercarlsson/.julia/packages/Revise/doDG7/test/runtests.jl:2840
  Got exception outside of a @test
  GitError(Code:ENOTFOUND, Class:Reference, revspec '48d4fd48430af58502699fdf3504b90589df3852' not found)
  Stacktrace:
    [1] macro expansion
      @ ~/Downloads/gitrepo/julia-1.10.4/share/julia/stdlib/v1.10/LibGit2/src/error.jl:112 [inlined]
    [2] LibGit2.GitTree(repo::LibGit2.GitRepo, spec::String)
      @ LibGit2 ~/Downloads/gitrepo/julia-1.10.4/share/julia/stdlib/v1.10/LibGit2/src/repository.jl:142
    [3] git_tree
      @ ~/.julia/packages/Revise/doDG7/src/git.jl:29 [inlined]
    [4] track_subdir_from_git!(pkgdata::Revise.PkgData, subdir::String; commit::String, modified_files::Set{Tuple{Revise.PkgData, String}})
      @ Revise ~/.julia/packages/Revise/doDG7/src/recipes.jl:135
    [5] track_subdir_from_git!
      @ ~/.julia/packages/Revise/doDG7/src/recipes.jl:128 [inlined]
    [6] _track(id::Base.PkgId, modname::Symbol; modified_files::Set{Tuple{Revise.PkgData, String}})
      @ Revise ~/.julia/packages/Revise/doDG7/src/recipes.jl:94

I think Revise should try a bit harder to confirm that the git repo it find is in fact a julia repo.

timholy commented 3 months ago

In the long run I hope that much of Revise can move into julia proper, but not really sensible to even try until the lowering provenance issue is resolved. (Once that's solved I think/hope it will be possible to divorce Revise from JuliaInterpreter & LoweredCodeUtils.) Until then, testing it alongside Julia probably makes sense.