hijack: add include=:static option

Fix partially #29.

Author: rfourquet

src/ReTest.jl

@@ -57,6 +57,7 @@ using .Testset: Testset, Format, print_id
 Base.@kwdef mutable struct Options
     verbose::Bool = false # annotated verbosity
     transient_verbose::Bool = false # verbosity for next run
+    static_include::Bool = false # whether to execute include at `replace_ts` time
 end
 
 mutable struct TestsetExpr
@@ -108,13 +109,25 @@ struct _Invalid
     global const invalid = _Invalid.instance
 end
 
+function extract_testsets(dest)
+    function extractor(x)
+        if Meta.isexpr(x, :macrocall) && x.args[1] == Symbol("@testset")
+            push!(dest, x)
+            nothing # @testset move out of the evaluated file
+        else
+            x
+        end
+    end
+end
+
 # replace unqualified `@testset` by TestsetExpr
-function replace_ts(source, mod, x::Expr, parent)
+function replace_ts(source, mod, x::Expr, parent; static_include::Bool)
     if x.head === :macrocall
         name = x.args[1]
         if name === Symbol("@testset")
             @assert x.args[2] isa LineNumberNode
-            ts, hasbroken = parse_ts(x.args[2], mod, Tuple(x.args[3:end]), parent)
+            ts, hasbroken = parse_ts(x.args[2], mod, Tuple(x.args[3:end]), parent;
+                                     static_include=static_include)
             ts !== invalid && parent !== nothing && push!(parent.children, ts)
             ts, false # hasbroken counts only "proper" @test_broken, not recursive ones
         elseif name === Symbol("@test_broken")
@@ -123,7 +136,7 @@ function replace_ts(source, mod, x::Expr, parent)
             # `@test` is generally called a lot, so it's probably worth it to skip
             # the containment test in this case
             x = macroexpand(mod, x, recursive=false)
-            replace_ts(source, mod, x, parent)
+            replace_ts(source, mod, x, parent; static_include=static_include)
         else
             @goto default
         end
@@ -133,18 +146,34 @@ function replace_ts(source, mod, x::Expr, parent)
         x.args[end] = path isa AbstractString ?
             joinpath(sourcepath, path) :
             :(joinpath($sourcepath, $path))
-        x, false
+        if static_include
+            length(x.args) == 2 || error("cannot handle include with two arguments: $x")
+            news = Expr(:block)
+            insert!(x.args, 2, extract_testsets(news.args))
+            try
+                Core.eval(mod, x)
+            catch
+                @warn "could not statically include at $source"
+                deleteat!(x.args, 2)
+                return x, false
+            end
+            replace_ts(source, mod, news, parent; static_include=static_include)
+        else
+            x, false
+        end
     else @label default
-        body_br = map(z -> replace_ts(source, mod, z, parent), x.args)
+        body_br = map(z -> replace_ts(source, mod, z, parent; static_include=static_include),
+                      x.args)
         filter!(x -> first(x) !== invalid, body_br)
         Expr(x.head, first.(body_br)...), any(last.(body_br))
     end
 end
 
-replace_ts(source, mod, x, _) = x, false
+replace_ts(source, mod, x, _1; static_include::Bool) = x, false
 
 # create a TestsetExpr from @testset's args
-function parse_ts(source::LineNumberNode, mod::Module, args::Tuple, parent=nothing)
+function parse_ts(source::LineNumberNode, mod::Module, args::Tuple, parent=nothing;
+                  static_include::Bool=false)
     function tserror(msg)
         @error msg _file=String(source.file) _line=source.line _module=mod
         invalid, false
@@ -158,6 +187,10 @@ function parse_ts(source::LineNumberNode, mod::Module, args::Tuple, parent=nothi
     marks = Marks()
     if parent !== nothing
         append!(marks.hard, parent.marks.hard) # copy! not available in Julia 1.0
+        options.static_include = parent.options.static_include
+        # if static_include was set in parent, it should have been forwarded also
+        # through the parse_ts/replace_ts call chains:
+        @assert static_include == parent.options.static_include
     end
     for arg in args[1:end-1]
         if arg isa String || Meta.isexpr(arg, :string)
@@ -207,7 +240,8 @@ function parse_ts(source::LineNumberNode, mod::Module, args::Tuple, parent=nothi
     end
 
     ts = TestsetExpr(source, mod, desc, options, marks, loops, parent)
-    ts.body, ts.hasbroken = replace_ts(source, mod, tsbody, ts)
+    ts.body, ts.hasbroken = replace_ts(source, mod, tsbody, ts;
+                                       static_include=options.static_include)
     ts, false # hasbroken counts only "proper" @test_broken, not recursive ones
 end
 

src/hijack.jl

@@ -104,7 +104,7 @@ const loaded_testmodules = Dict{Module,Vector{Module}}()
 
 """
     ReTest.hijack(source, [modname];
-                  parentmodule::Module=Main, lazy=false, testset::Bool=false,
+                  parentmodule::Module=Main, lazy=false, [include::Symbol],
                   [revise::Bool])
 
 Given test files defined in `source` using the `Test` package, try to load
@@ -143,11 +143,10 @@ The `lazy` keyword specifies whether some toplevel expressions should be skipped
 * `:brutal` means toplevel `@test*` macros are removed, as well as toplevel
   `begin`, `let`, `for` or `if` blocks.
 
-#### `testset` keyword
+#### `include` keyword
 
-The `testset` keyword can help to handle the case where `@testset`s contain
-`include` expressions (at the "toplevel" of the testset), like in the
-following example:
+The `include` keyword can help to handle the case where `@testset`s contain
+`include` expressions, like in the following example:
 ```julia
 @testset "parent" begin
     @test true
@@ -161,11 +160,28 @@ of the testset which include them. With `ReTest`, the `include` expressions
 would be evaluated only when the parent testsets are run, so that included
 testsets are not run themselves, but only "declared".
 
-It the `testset` keyword
-is `true`, `hijack` inspects `@testset` expressions and puts `include`
-expressions outside of the testset. This is not ideal, but at least allows
-`ReTest` to know about all the testsets right after the call to `hijack`, and
-to not declare new testsets when parent testsets are run.
+If the `include` keyword is set to `:static`, `include(...)` expressions are
+evaluated when `@testset` expressions containing them are parsed, before
+filtering and before testsets are run. Testsets which are declared (within the
+same module) as a side effect of `include(...)` are then inserted in place of
+the call to `include(...)`.
+
+If the `include` keyword is set to `:outline`, `hijack` inspects topelevel
+`@testset` expressions and puts toplevel `include(...)` expressions outside of
+the containing testset, and should therefore be evaluated immediately. This is
+not ideal, but at least allows `ReTest` to know about all the testsets right
+after the call to `hijack`, and to not declare new testsets when parent
+testsets are run.
+
+The `:outline` option might be deprecated in the future, and `include=:static`
+should generally be preferred. One case where `:outline` might work better is
+when the included file defines a submodule: `ReTest` doesn't have the concept
+of a nested testset belonging to a different module than the parent testset,
+so the best that can be done here is to "outline" such nested testsets; with
+`include=:outline`, `hijack` will "process" the content of such submodules
+(replace `using Test` by `using ReTest`, etc.), whereas with
+`include=:static`, the subdmodules will get defined after `hijack` has
+returned (on the first call to `retest` thereafter), so won't be "processed".
 
 #### `revise` keyword
 
@@ -180,8 +196,12 @@ and to `false` otherwise.
 """
 function hijack end
 
+# TODO 0.4: remove `testset` kwarg
+# TODO: maybe deprecate `include=:outline`?
+
 function hijack(path::AbstractString, modname=nothing; parentmodule::Module=Main,
-                lazy=false, testset::Bool=false, revise::Maybe{Bool}=nothing)
+                lazy=false, revise::Maybe{Bool}=nothing,
+                include::Maybe{Symbol}=nothing, testset::Bool=false)
 
     # do first, to error early if necessary
     Revise = get_revise(revise)
@@ -192,22 +212,34 @@ function hijack(path::AbstractString, modname=nothing; parentmodule::Module=Main
     modname = Symbol(modname)
 
     newmod = @eval parentmodule module $modname end
-    populate_mod!(newmod, path; lazy=lazy, testset=testset, Revise=Revise)
+    populate_mod!(newmod, path; lazy=lazy, include=setinclude(include, testset),
+                  Revise=Revise)
     newmod
 end
 
+function setinclude(include, testset)
+    if testset
+        include === nothing || error("cannot specify both `testset` and `include` arguments")
+        :outline
+    else
+        include === nothing || include === :static || include === :outline ||
+            error("`include` keyword only accepts `:static` or `:outline` as value")
+        include
+    end
+end
+
 # this is just a work-around for v"1.5", where @__MODULE__ can't be used in
 # expressions; root_module[] is set equal to @__MODULE__ within modules
 const root_module = Ref{Symbol}()
 
 __init__() = root_module[] = gensym("MODULE")
 
-function populate_mod!(mod::Module, path; lazy, Revise, testset)
+function populate_mod!(mod::Module, path; lazy, Revise, include)
     lazy ∈ (true, false, :brutal) ||
         throw(ArgumentError("the `lazy` keyword must be `true`, `false` or `:brutal`"))
 
     files = Revise === nothing ? nothing : Dict(path => mod)
-    substitute!(x) = substitute_retest!(x, lazy, testset, files)
+    substitute!(x) = substitute_retest!(x, lazy, include, files)
 
     @eval mod begin
         using ReTest # for files which don't have `using Test`
@@ -235,7 +267,8 @@ function revise_track(Revise, files)
 end
 
 function hijack(packagemod::Module, modname=nothing; parentmodule::Module=Main,
-                lazy=false, testset::Bool=false, revise::Maybe{Bool}=nothing)
+                lazy=false, revise::Maybe{Bool}=nothing,
+                include::Maybe{Symbol}=nothing, testset::Bool=false)
     packagepath = pathof(packagemod)
     packagepath === nothing && packagemod !== Base &&
         throw(ArgumentError("$packagemod is not a package"))
@@ -253,13 +286,13 @@ function hijack(packagemod::Module, modname=nothing; parentmodule::Module=Main,
     else
         path = joinpath(dirname(dirname(packagepath)), "test", "runtests.jl")
         hijack(path, modname, parentmodule=parentmodule,
-               lazy=lazy, testset=testset, revise=revise)
+               lazy=lazy, testset=testset, include=include, revise=revise)
     end
 end
 
-function substitute_retest!(ex, lazy, testset, files=nothing;
+function substitute_retest!(ex, lazy, include_, files=nothing;
                             ishijack::Bool=true)
-    substitute!(x) = substitute_retest!(x, lazy, testset, files, ishijack=ishijack)
+    substitute!(x) = substitute_retest!(x, lazy, include_, files, ishijack=ishijack)
 
     if Meta.isexpr(ex, :using)
         ishijack || return ex
@@ -309,19 +342,24 @@ function substitute_retest!(ex, lazy, testset, files=nothing;
         ishijack || return ex
         if lazy != false && ex.args[1] ∈ TEST_MACROS
             empty_expr!(ex)
-        elseif testset && ex.args[1] == Symbol("@testset")
-            # we remove `include` expressions and put them out of the `@testset`
-            body = ex.args[end]
-            if body.head == :for
-                body = body.args[end]
+        elseif include_ !== nothing && ex.args[1] == Symbol("@testset")
+            if include_ === :outline
+                # we remove `include` expressions and put them out of the `@testset`
+                body = ex.args[end]
+                if body.head == :for
+                    body = body.args[end]
+                end
+                includes = splice!(body.args, findall(body.args) do x
+                                                  Meta.isexpr(x, :call) && x.args[1] == :include
+                                              end)
+                map!(substitute!, includes, includes)
+                ex.head = :block
+                newts = Expr(:macrocall, ex.args...)
+                push!(empty!(ex.args), newts, includes...)
+            else # :static
+                pos = ex.args[2] isa LineNumberNode ? 3 : 2
+                insert!(ex.args, pos, :(static_include=true))
             end
-            includes = splice!(body.args, findall(body.args) do x
-                                              Meta.isexpr(x, :call) && x.args[1] == :include
-                                          end)
-            map!(substitute!, includes, includes)
-            ex.head = :block
-            newts = Expr(:macrocall, ex.args...)
-            push!(empty!(ex.args), newts, includes...)
         end
     elseif ex isa Expr && ex.head ∈ (:block, :let, :for, :while, :if, :try)
         if lazy == :brutal

test/Hijack/test/include_static.jl

@@ -0,0 +1,8 @@
+# include = :static
+using Hijack, Test
+
+@testset "include_static" begin
+    @test true
+    push!(Hijack.RUN, 1)
+    include("include_static_included1.jl")
+end

test/Hijack/test/include_static_included1.jl

@@ -0,0 +1,9 @@
+@testset "include_static_included1 $i" for i=1:2
+    @test true
+    @testset "nested include_static_included1" begin
+        @test true
+        push!(Hijack.RUN, 2)
+        # test that `include` kwarg is forwarded
+        include("include_static_included2.jl")
+    end
+end

test/Hijack/test/include_static_included2.jl

@@ -0,0 +1,4 @@
+@testset "include_static_included2" begin
+    @test true
+    push!(Hijack.RUN, 3)
+end

test/runtests.jl

@@ -1790,10 +1790,27 @@ end
         empty!(Hijack.RUN)
         @test_throws ArgumentError ReTest.hijack("./Hijack/test/lazy.jl", :HijackWrong, lazy=:wrong)
 
-        # test testset=true
+        # test include=:outline
         empty!(Hijack.RUN)
-        ReTest.hijack("./Hijack/test/testset.jl", :HijackTestset, testset=true)
+        ReTest.hijack("./Hijack/test/testset.jl", :HijackTestset, include=:outline)
         retest(HijackTestset)
         @test Hijack.RUN == [1, 2, 3]
+
+        # test include=:static
+        empty!(Hijack.RUN)
+        @test_throws ErrorException ReTest.hijack("./Hijack/test/include_static.jl", :HijackInclude, include=:static, testset=true)
+        @test_throws ErrorException ReTest.hijack("./Hijack/test/include_static.jl", :HijackInclude, include=:notvalid)
+        ReTest.hijack("./Hijack/test/include_static.jl", :HijackInclude, include=:static)
+        check(HijackInclude, dry=true, [], output="""
+1| include_static
+2|   include_static_included1 1
+3|     nested include_static_included1
+4|       include_static_included2
+2|   include_static_included1 2
+3|     nested include_static_included1
+4|       include_static_included2
+""")
+        retest(HijackInclude)
+        @test Hijack.RUN == [1, 2, 3, 2, 3]
     end
 end