allow "cartesian" testest-for loops, e.g. for i=I, j=J

docs/src/index.md

@@ -79,9 +79,6 @@ should be fixable:
   the testset subject usually can't be known statically and the testset can't
   be filtered out with a `Regex`).
 
-* "testsets-for" currently accept only "non-cartesian" looping (e.g. `for i=I,
-  j=J` is not supported, PRs welcome!)
-
 * Testsets can not be "custom testsets" (cf. `Test` documentation).
 
 * Nested testsets can't be "qualified" (i.e. written as `ReTest.@testset`).

src/ReTest.jl

@@ -46,11 +46,18 @@ mutable struct TestsetExpr
     mod::String # enclosing module
     desc::Union{String,Expr}
     options::Options
-    loops::Union{Expr,Nothing}
+    # loops: the original loop expression, if any, but where each `x=...` is
+    # pulled out into a vector
+    loops::Union{Vector{Expr},Nothing}
     parent::Union{TestsetExpr,Nothing}
     children::Vector{TestsetExpr}
     strings::Vector{String}
-    loopvalues::Any
+    # loopvalues & loopiters: when successful in evaluating loop values in resolve!,
+    # we "flatten" the nested for loops into a single loop, with loopvalues
+    # containing tuples of values, and loopiters the tuples of variables to which the
+    # values are assigned
+    loopvalues::Union{Nothing,Vector{Any}}
+    loopiters::Union{Nothing,Expr}
     hasbroken::Bool
     hasbrokenrec::Bool # recursive hasbroken, transiently
     run::Bool
@@ -101,8 +108,16 @@ function parse_ts(source, mod, args::Tuple, parent=nothing)
     body = args[end]
     isa(body, Expr) || error("Expected begin/end block or for loop as argument to @testset")
     if body.head === :for
-        loops = body.args[1]
         tsbody = body.args[2]
+        loops = body.args[1]
+        if loops.head == :(=)
+            loops = Expr[loops]
+        else
+            @assert loops.head == :block
+            @assert all(arg -> Meta.isexpr(arg, :(=)), loops.args)
+            loops = loops.args
+        end
+
     elseif body.head === :block
         loops = nothing
         tsbody = body
@@ -122,6 +137,7 @@ function resolve!(mod::Module, ts::TestsetExpr, rx::Regex;
     desc = ts.desc
     ts.run = force || isempty(rx.pattern)
     ts.loopvalues = nothing # unnecessary ?
+    ts.loopiters = nothing
 
     parentstrs = ts.parent === nothing ? [""] : ts.parent.strings
     ts.descwidth = 0
@@ -144,16 +160,31 @@ function resolve!(mod::Module, ts::TestsetExpr, rx::Regex;
         loops = ts.loops
         @assert loops !== nothing
         xs = ()
+        loopiters = Expr(:tuple, (arg.args[1] for arg in loops)...)
+
         try
-            xs = Core.eval(mod, loops.args[2])
-            if !(xs isa Union{Array,Tuple}) # being conservative on target type
-                # this catches e.g. the case where xs is a generator, then collect
-                # fails because of a world-age problem (the function in xs is too "new")
-                xs = collect(xs)
+            # we need to evaluate roughly the following:
+            # xsgen = Expr(:comprehension, Expr(:generator, loopiters, loops...))
+            # but a comprehension expression returns an array, i.e. loop variables
+            # can't depend on previous ones; the correct way is therefore to
+            # construct nested generators flattened with a :flatten Expr, or to
+            # simply construct directly a for-loop as below
+            xssym = gensym() # to not risk to shadow a global variable on which
+                             # the iteration expression depends
+            xsgen = quote
+                let $xssym = []
+                    $(Expr(:for, Expr(:block, loops...),
+                           Expr(:call, Expr(:., :Base, QuoteNode(:push!)),
+                                xssym, loopiters)))
+                    $xssym
+                end
             end
+            xs = Core.eval(mod, xsgen)
+            @assert xs isa Vector
             ts.loopvalues = xs
+            ts.loopiters = loopiters
         catch
-            xs = () # xs might have been assigned before the collect call
+            @assert xs == ()
             if !ts.run
                 @warn "could not evaluate testset-for iterator, default to inclusion"
             end
@@ -165,7 +196,7 @@ function resolve!(mod::Module, ts::TestsetExpr, rx::Regex;
             end
         end
         for x in xs # empty loop if eval above threw
-            Core.eval(mod, Expr(:(=), loops.args[1], x))
+            Core.eval(mod, Expr(:(=), loopiters, x))
             descx = Core.eval(mod, desc)::String
             if shown
                 ts.descwidth = max(ts.descwidth, textwidth(descx) + 2*depth)
@@ -220,9 +251,15 @@ function make_ts(ts::TestsetExpr, rx::Regex, stats, chan)
             @testset $(ts.mod) $(isfinal(ts)) $rx $(ts.desc) $(ts.options) $stats $chan $body
         end
     else
-        loopvals = something(ts.loopvalues, ts.loops.args[2])
+        c = count(x -> x === nothing, (ts.loopvalues, ts.loopiters))
+        @assert c == 0 || c == 2
+        if c == 0
+            loops = [Expr(:(=), ts.loopiters, ts.loopvalues)]
+        else
+            loops = ts.loops
+        end
         quote
-            @testset $(ts.mod) $(isfinal(ts)) $rx $(ts.desc) $(ts.options) $stats $chan $(ts.loops.args[1]) $loopvals $body
+            @testset $(ts.mod) $(isfinal(ts)) $rx $(ts.desc) $(ts.options) $stats $chan $loops $body
         end
     end
 end
@@ -837,14 +874,14 @@ function dryrun(mod::Module, ts::TestsetExpr, rx::Regex, align::Int=0, parentsub
             dryrun(mod, tsc, rx, align + 2, subject)
         end
     else
-        loopvals = ts.loopvalues
-        if loopvals === nothing
+        loopvalues = ts.loopvalues
+        if loopvalues === nothing
             println(' '^align, desc)
             @warn "could not evaluate testset-for iterator, default to inclusion"
             return
         end
-        for x in loopvals
-            Core.eval(mod, Expr(:(=), ts.loops.args[1], x))
+        for x in loopvalues
+            Core.eval(mod, Expr(:(=), ts.loopiters, x))
             descx = Core.eval(mod, desc)::String
             # avoid repeating ourselves, transform this iteration into a "begin/end" testset
             beginend = TestsetExpr(ts.source, ts.mod, descx, ts.options, nothing,

src/testset.jl

@@ -431,9 +431,9 @@ macro testset(mod::String, isfinal::Bool, rx::Regex, desc::String, options,
 end
 
 macro testset(mod::String, isfinal::Bool, rx::Regex, desc::Union{String,Expr}, options,
-              stats::Bool, chan, loopiter, loopvals, body)
+              stats::Bool, chan, loops, body)
     Testset.testset_forloop(mod, isfinal, rx, desc, options,
-                            stats, chan, loopiter, loopvals, body, __source__)
+                            stats, chan, loops, body, __source__)
 end
 
 """
@@ -495,12 +495,8 @@ end
 Generate the code for a `@testset` with a `for` loop argument
 """
 function testset_forloop(mod::String, isfinal::Bool, rx::Regex, desc::Union{String,Expr},
-                         options, stats, chan, loopiter, loopvals, tests, source)
+                         options, stats, chan, loops, tests, source)
 
-    # Pull out the loop variables. We might need them for generating the
-    # description and we'll definitely need them for generating the
-    # comprehension expression at the end
-    loopvars = Expr[Expr(:(=), loopiter, loopvals)]
     blk = quote
         local current_str
         if $isfinal
@@ -543,7 +539,7 @@ function testset_forloop(mod::String, isfinal::Bool, rx::Regex, desc::Union{Stri
         local tmprng = copy(RNG)
         try
             let
-                $(Expr(:for, Expr(:block, [esc(v) for v in loopvars]...), blk))
+                $(Expr(:for, Expr(:block, [esc(v) for v in loops]...), blk))
             end
         finally
             # Handle `return` in test body

test/runtests.jl

@@ -317,9 +317,13 @@ RUN = []
 end
 end
 
-@test_logs (:warn, r"could not evaluate testset-for iterator.*") retest(Loops2, r"asd")
-@test Loops2.RUN == [1, 0, 2, 0]
-empty!(Loops2.RUN)
+# the test below has been solved
+# TODO: check whether another run could lead to the same result, i.e. RUN == [1, 0, 2, 0] ?
+# @test_logs (:warn, r"could not evaluate testset-for iterator.*") retest(Loops2, r"asd")
+# @test Loops2.RUN == [1, 0, 2, 0]
+# empty!(Loops2.RUN)
+@test isempty(Loops2.RUN)
+
 retest(Loops2)
 @test Loops2.RUN == [1, 0, -1, 2, 0, -1]
 
@@ -351,6 +355,24 @@ empty!(Loops3.RUN)
 retest(Loops3)
 @test Loops3.RUN == [1, 0, -1, 2, 0, -1]
 
+module MultiLoops
+using ReTest
+
+RUN = []
+C1, C2 = 1:2 # check that iteration has access to these values
+
+@testset "multiloops $x $y $z" for (x, y) in zip(1:C2, 1:2), z in C1:x
+    push!(RUN, (x, z))
+    @test true
+end
+end
+
+retest(MultiLoops)
+@test MultiLoops.RUN == [(1, 1), (2, 1), (2, 2)]
+empty!(MultiLoops.RUN)
+retest(MultiLoops, "1 1")
+@test MultiLoops.RUN == [(1, 1)]
+
 ### Failing ##################################################################
 
 module Failing