From b2c3463656f3d2072b1f0dcfcc2797ea058e342a Mon Sep 17 00:00:00 2001 From: Tim Holy Date: Thu, 23 Apr 2020 20:40:49 -0500 Subject: [PATCH] Improve test coverage (#465) This is designed to catch issues like what happened in #460, but also more generally improve test coverage. --- .travis.yml | 11 +++- docs/src/dev_reference.md | 3 +- docs/src/index.md | 2 +- src/Revise.jl | 67 +------------------- src/backedges.jl | 7 +++ src/utils.jl | 18 +----- test/populate_compiled.jl | 16 +++++ test/runtests.jl | 125 ++++++++++++++++++++++++++++++++++++-- 8 files changed, 158 insertions(+), 91 deletions(-) create mode 100644 test/populate_compiled.jl diff --git a/.travis.yml b/.travis.yml index 7beaf820..b3e057ba 100644 --- a/.travis.yml +++ b/.travis.yml @@ -24,9 +24,18 @@ git: script: - export JULIA_PROJECT="" + # Populate the precompile cache with an extraneous file, to catch issues like in #460 + - julia -e 'include(joinpath("test", "populate_compiled.jl"))' + # Run the normal tests - julia --project -e 'using Pkg; Pkg.build(); Pkg.test(; coverage=true);' + # The REPL wasn't initialized, so the "Methods at REPL" tests didn't run. Pick those up now. + - julia --project --code-coverage=user -e 'using InteractiveUtils, REPL, Revise; @async(Base.run_main_repl(true, true, false, true, false)); Revise.async_steal_repl_backend(); include(joinpath("test", "runtests.jl")); REPL.eval_user_input(:(exit()), Base.active_repl_backend)' "Methods at REPL" + # We also need to pick up the Git tests, but for that we need to `dev` the package + - julia -e 'using Pkg; Pkg.develop(PackageSpec(path=".")); include(joinpath("test", "runtests.jl"))' "Git" + # Tests for when using polling - JULIA_REVISE_POLL=1 julia --code-coverage=user --project -e 'using Pkg, Revise; include(joinpath(dirname(pathof(Revise)), "..", "test", "polling.jl"))' - - if [ "$TRAVIS_OS_NAME" = "linux" ]; then echo 4 | sudo tee -a /proc/sys/fs/inotify/max_user_watches; julia --project -e 'using Pkg, Revise; cd(joinpath(dirname(pathof(Revise)), "..", "test")); include("inotify.jl")'; fi + # Running out of inotify storage (see #26) + - if [ "$TRAVIS_OS_NAME" = "linux" ]; then echo 4 | sudo tee -a /proc/sys/fs/inotify/max_user_watches; julia --project --code-coverage=user -e 'using Pkg, Revise; cd(joinpath(dirname(pathof(Revise)), "..", "test")); include("inotify.jl")'; fi after_success: - julia -e 'import Pkg; Pkg.add("Coverage"); using Coverage; Codecov.submit(Codecov.process_folder())' diff --git a/docs/src/dev_reference.md b/docs/src/dev_reference.md index 0d873cd4..19f352d4 100644 --- a/docs/src/dev_reference.md +++ b/docs/src/dev_reference.md @@ -83,10 +83,9 @@ Revise.revise_file_queued Revise.revise_file_now ``` -### Interchange between methods and signatures +### Caching the definition of methods ```@docs -Revise.get_method Revise.get_def ``` diff --git a/docs/src/index.md b/docs/src/index.md index bdb92b2e..39ba39a6 100644 --- a/docs/src/index.md +++ b/docs/src/index.md @@ -176,7 +176,7 @@ If Revise isn't working for you, here are some steps to try: - Try running `test Revise` from the Pkg REPL-mode. If tests pass, check the documentation to make sure you understand how Revise should work. If they fail (especially if it mirrors functionality that you need and isn't working), see - [Fixing a broken or partially-working installation](@ref) for some suggestions. + [Debugging problems with paths](@ref) for one set of suggestions. If you still encounter problems, please [file an issue](https://github.com/timholy/Revise.jl/issues). Especially if you think Revise is making mistakes in adding or deleting methods, please diff --git a/src/Revise.jl b/src/Revise.jl index 071f4dcc..653ddb4e 100644 --- a/src/Revise.jl +++ b/src/Revise.jl @@ -357,7 +357,6 @@ struct CodeTrackingMethodInfo includes::Vector{String} end CodeTrackingMethodInfo(ex::Expr) = CodeTrackingMethodInfo([ex], Any[], Set{Union{GlobalRef,Symbol}}(), String[]) -CodeTrackingMethodInfo(rex::RelocatableExpr) = CodeTrackingMethodInfo(rex.ex) function add_signature!(methodinfo::CodeTrackingMethodInfo, @nospecialize(sig), ln) CodeTracking.method_info[sig] = (fixpath(ln), methodinfo.exprstack[end]) @@ -473,7 +472,6 @@ function init_watching(pkgdata::PkgData, files) end return nothing end -init_watching(files) = init_watching(PkgId(Main), files) """ revise_dir_queued(dirname) @@ -563,9 +561,10 @@ that move from one file to another. `Revise.pkgdatas[id].fileinfos`. """ function revise_file_now(pkgdata::PkgData, file) + # @assert !isabspath(file) i = fileindex(pkgdata, file) if i === nothing - println("Revise is currently tracking the following files in $(pkgdata.id): ", keys(pkgdict)) + println("Revise is currently tracking the following files in $(PkgId(pkgdata)): ", srcfiles(pkgdata)) error(file, " is not currently being tracked.") end mexsnew, mexsold = handle_deletions(pkgdata, file) @@ -850,49 +849,6 @@ silence(pkg::AbstractString) = silence(Symbol(pkg)) ## Utilities -""" - method = get_method(sigt) - -Get the method `method` with signature-type `sigt`. This is used to provide -the method to `Base.delete_method`. - -If `sigt` does not correspond to a method, returns `nothing`. - -# Examples - -```jldoctest; setup = :(using Revise), filter = r"in Main at.*" -julia> mymethod(::Int) = 1 -mymethod (generic function with 1 method) - -julia> mymethod(::AbstractFloat) = 2 -mymethod (generic function with 2 methods) - -julia> Revise.get_method(Tuple{typeof(mymethod), Int}) -mymethod(::Int64) in Main at REPL[0]:1 - -julia> Revise.get_method(Tuple{typeof(mymethod), Float64}) -mymethod(::AbstractFloat) in Main at REPL[1]:1 - -julia> Revise.get_method(Tuple{typeof(mymethod), Number}) - -``` -""" -function get_method(@nospecialize(sigt)) - mths = Base._methods_by_ftype(sigt, -1, typemax(UInt)) - length(mths) == 1 && return mths[1][3] - if !isempty(mths) - # There might be many methods, but the one that should match should be the - # last one, since methods are ordered by specificity - i = lastindex(mths) - while i > 0 - m = mths[i][3] - m.sig == sigt && return m - i -= 1 - end - end - return nothing -end - """ success = get_def(method::Method) @@ -999,25 +955,6 @@ function add_definitions_from_repl(filename) return fi end -function fix_line_statements!(ex::Expr, file::Symbol, line_offset::Int=0) - if ex.head == :line - ex.args[1] += line_offset - ex.args[2] = file - else - for (i, a) in enumerate(ex.args) - if isa(a, Expr) - fix_line_statements!(a::Expr, file, line_offset) - elseif isa(a, LineNumberNode) - ex.args[i] = file_line_statement(a::LineNumberNode, file, line_offset) - end - end - end - ex -end - -file_line_statement(lnn::LineNumberNode, file::Symbol, line_offset) = - LineNumberNode(lnn.line + line_offset, file) - function update_stacktrace_lineno!(trace) local nrep for i = 1:length(trace) diff --git a/src/backedges.jl b/src/backedges.jl index 94f10072..e91faf2b 100644 --- a/src/backedges.jl +++ b/src/backedges.jl @@ -208,6 +208,13 @@ function toplevel_chunks(backedges::BackEdges) return chunks end +""" + hastrackedexpr(src, chunk=1:length(src.code)) + +Detect whether the specified `chunk` of `src` contains a definition tracked +by Revise. By default these are methods and type definitions (the latter because +they might contain constructors). +""" function hastrackedexpr(code::CodeInfo, chunk::AbstractUnitRange=axes(code.code, 1); heads=(:method, :struct_type, :abstract_type, :primitive_type)) for stmtidx in chunk stmt = code.code[stmtidx] diff --git a/src/utils.jl b/src/utils.jl index d7b5910c..df1b9e62 100644 --- a/src/utils.jl +++ b/src/utils.jl @@ -99,22 +99,6 @@ Base.in(file, wl::WatchList) = haskey(wl.trackedfiles, file) newer(mtime, timestamp) = mtime >= timestamp end -function macroreplace!(ex::Expr, filename) - for i = 1:length(ex.args) - ex.args[i] = macroreplace!(ex.args[i], filename) - end - if ex.head == :macrocall - m = ex.args[1] - if m == Symbol("@__FILE__") - return String(filename) - elseif m == Symbol("@__DIR__") - return dirname(String(filename)) - end - end - return ex -end -macroreplace!(s, filename) = s - function printf_maxsize(f::Function, io::IO, args...; maxchars::Integer=500, maxlines::Integer=20) # This is dumb but certain to work iotmp = IOBuffer() @@ -144,7 +128,7 @@ function printf_maxsize(f::Function, io::IO, args...; maxchars::Integer=500, max end end println_maxsize(args...; kwargs...) = println_maxsize(stdout, args...; kwargs...) -println_maxsize(io::IO, args...; kwargs...) = printf_maxsize(println, stdout, args...; kwargs...) +println_maxsize(io::IO, args...; kwargs...) = printf_maxsize(println, io, args...; kwargs...) # Trimming backtraces function trim_toplevel!(bt) diff --git a/test/populate_compiled.jl b/test/populate_compiled.jl new file mode 100644 index 00000000..1f3484ab --- /dev/null +++ b/test/populate_compiled.jl @@ -0,0 +1,16 @@ +# This runs only on Travis. The goal is to populate the `.julia/compiled/v*` directory +# with some additional files, so that `filter_valid_cachefiles` has to run. +# This is to catch problems like #460. + +using Pkg +using Test + +Pkg.add(PackageSpec(name="EponymTuples", version="0.2.0")) +using EponymTuples # force compilation +id = Base.PkgId(EponymTuples) +paths = Base.find_all_in_cache_path(id) +Pkg.rm("EponymTuples") # we don't need it anymore +path = first(paths) +base, ext = splitext(path) +mv(path, base*"blahblah"*ext) +Pkg.add(PackageSpec(name="EponymTuples")) diff --git a/test/runtests.jl b/test/runtests.jl index eaeacf57..c6e75504 100644 --- a/test/runtests.jl +++ b/test/runtests.jl @@ -1,16 +1,23 @@ # REVISE: DO NOT PARSE # For people with JULIA_REVISE_INCLUDE=1 -using Revise, CodeTracking, JuliaInterpreter +using Revise +using Revise.CodeTracking +using Revise.JuliaInterpreter using Test @test isempty(detect_ambiguities(Revise, Base, Core)) using Pkg, Unicode, Distributed, InteractiveUtils, REPL, UUIDs import LibGit2 -using OrderedCollections: OrderedSet +using Revise.OrderedCollections: OrderedSet using Test: collect_test_logs using Base.CoreLogging: Debug,Info -using CodeTracking: line_is_decl +using Revise.CodeTracking: line_is_decl + +# In addition to using this for the "More arg-modifying macros" test below, +# this package is used on Travis to test what happens when you have multiple +# *.ji files for the package. +using EponymTuples include("common.jl") @@ -76,6 +83,12 @@ end @test isempty(Revise.basedir(pd)) end + do_test("Package contents") && @testset "Package contents" begin + id = Base.PkgId(EponymTuples) + path, mods_files_mtimes = Revise.pkg_fileinfo(id) + @test occursin("EponymTuples", path) + end + do_test("LineSkipping") && @testset "LineSkipping" begin rex = Revise.RelocatableExpr(quote f(x) = x^2 @@ -364,17 +377,29 @@ k(x) = 4 Base.include(mod, file) mexs = Revise.parse_source(file, mod) Revise.instantiate_sigs!(mexs) - io = IOBuffer() + # io = IOBuffer() print(IOContext(io, :compact=>true), mexs) str = String(take!(io)) @test str == "OrderedCollections.OrderedDict($mod$(pair_op_compact)ExprsSigs(<1 expressions>, <0 signatures>),$mod.ReviseTest$(pair_op_compact)ExprsSigs(<2 expressions>, <2 signatures>),$mod.ReviseTest.Internal$(pair_op_compact)ExprsSigs(<6 expressions>, <5 signatures>))" exs = mexs[getfield(mod, :ReviseTest)] - io = IOBuffer() + # io = IOBuffer() print(IOContext(io, :compact=>true), exs) @test String(take!(io)) == "ExprsSigs(<2 expressions>, <2 signatures>)" print(IOContext(io, :compact=>false), exs) str = String(take!(io)) @test str == "ExprsSigs with the following expressions: \n :(square(x) = begin\n x ^ 2\n end)\n :(cube(x) = begin\n x ^ 4\n end)" + + sleep(0.1) # wait for EponymTuples to hit the cache + pkgdata = Revise.pkgdatas[Base.PkgId(EponymTuples)] + file = first(Revise.srcfiles(pkgdata)) + Revise.maybe_parse_from_cache!(pkgdata, file) + print(io, pkgdata) + str = String(take!(io)) + @test occursin("EponymTuples.jl\": FileInfo", str) + @test occursin(r"with cachefile.*EponymTuples.*ji", str) + print(IOContext(io, :compact=>true), pkgdata) + str = String(take!(io)) + @test occursin("1/1 parsed files", str) end do_test("File paths") && @testset "File paths" begin @@ -887,6 +912,40 @@ end pop!(LOAD_PATH) end + do_test("Changing docstrings") && @testset "Changing docstring" begin + # Compiled mode covers most docstring changes, so we have to go to + # special effort to test the older interpreter-based solution. + testdir = newtestdir() + dn = joinpath(testdir, "ChangeDocstring", "src") + mkpath(dn) + open(joinpath(dn, "ChangeDocstring.jl"), "w") do io + println(io, """ + module ChangeDocstring + "f" f() = 1 + end + """) + end + sleep(mtimedelay) + @eval using ChangeDocstring + sleep(mtimedelay) + @test ChangeDocstring.f() == 1 + ds = @doc(ChangeDocstring.f) + @test ds.content[1].content[1].content[1].content[1] == "f" + # Now manually change the docstring + ex = quote "g" f() = 1 end + lwr = Meta.lower(ChangeDocstring, ex) + frame = JuliaInterpreter.prepare_thunk(ChangeDocstring, lwr, true) + methodinfo = Revise.MethodInfo() + docexprs = Dict{Module,Vector{Expr}}() + ret = Revise.methods_by_execution!(JuliaInterpreter.finish_and_return!, methodinfo, + docexprs, frame, trues(length(frame.framecode.src.code)); define=false) + ds = @doc(ChangeDocstring.f) + @test ds.content[1].content[1].content[1].content[1] == "g" + + rm_precompile("ChangeDocstring") + pop!(LOAD_PATH) + end + do_test("Undef in docstrings") && @testset "Undef in docstrings" begin fn = Base.find_source_file("abstractset.jl") # has lots of examples of """str""" func1, func2 mexsold = Revise.parse_source(fn, Base) @@ -1426,6 +1485,11 @@ for T in (Int, Float64, String) @eval mytypeof(x::\$T) = \$T end +@generated function firstparam(A::AbstractArray) + T = A.parameters[1] + return :(\$T) +end + end """) end @@ -1458,6 +1522,7 @@ end @test MethDel.mytypeof(1) === Int @test MethDel.mytypeof(1.0) === Float64 @test MethDel.mytypeof("hi") === String + @test MethDel.firstparam(rand(2,2)) === Float64 open(joinpath(dn, "MethDel.jl"), "w") do io println(io, """ module MethDel @@ -1515,6 +1580,7 @@ end @test MethDel.mytypeof(1) === Int @test_throws MethodError MethDel.mytypeof(1.0) @test MethDel.mytypeof("hi") === String + @test_throws MethodError MethDel.firstparam(rand(2,2)) Base.delete_method(first(methods(Base.revisefoo))) @@ -1534,6 +1600,43 @@ end @test m.sig.parameters[2] === Integer end + do_test("revise_file_now") && @testset "revise_file_now" begin + # Very rarely this is used for debugging + testdir = newtestdir() + dn = joinpath(testdir, "ReviseFileNow", "src") + mkpath(dn) + fn = joinpath(dn, "ReviseFileNow.jl") + open(fn, "w") do io + println(io, """ + module ReviseFileNow + f(x) = 1 + end + """) + end + sleep(mtimedelay) + @eval using ReviseFileNow + @test ReviseFileNow.f(0) == 1 + sleep(mtimedelay) + pkgdata = Revise.pkgdatas[Base.PkgId(ReviseFileNow)] + open(fn, "w") do io + println(io, """ + module ReviseFileNow + f(x) = 2 + end + """) + end + try + Revise.revise_file_now(pkgdata, "foo") + catch err + @test isa(err, ErrorException) + @test occursin("not currently being tracked", err.msg) + end + Revise.revise_file_now(pkgdata, relpath(fn, pkgdata)) + @test ReviseFileNow.f(0) == 2 + + rm_precompile("ReviseFileNow") + end + do_test("Evaled toplevel") && @testset "Evaled toplevel" begin testdir = newtestdir() dnA = joinpath(testdir, "ToplevelA", "src"); mkpath(dnA) @@ -2418,6 +2521,8 @@ end pop!(hp.history) pop!(hp.history) + else + @warn "REPL tests skipped" end end @@ -2450,6 +2555,16 @@ end end end +do_test("Utilities") && @testset "Utilities" begin + # Used by Rebugger but still lives here + io = IOBuffer() + Revise.println_maxsize(io, "a"^100; maxchars=50) + str = String(take!(io)) + @test startswith(str, "a"^25) + @test endswith(chomp(chomp(str)), "a"^24) + @test occursin("…", str) +end + do_test("Switching free/dev") && @testset "Switching free/dev" begin function make_a2d(path, val, mode="r"; generate=true) # Create a new "read-only package" (which mimics how Pkg works when you `add` a package)