break/continue in a @testset for loop is broken

[KristofferC]

As @tkelman reported here: KristofferC/NearestNeighbors.jl#25 (and kindly bisected to cd35df536bd) Julia master now does not support break or continue in a @testset for loop

using Base.Test

@testset for x in [1,2,3]
    if x == 1
        continue
    end
end

ERROR: syntax: break or continue outside loop
 in eval(::Module, ::Any) at ./boot.jl:234
 in macro expansion at ./REPL.jl:92 [inlined]
 in (::Base.REPL.##1#2{Base.REPL.REPLBackend})() at ./event.jl:46

cc @JeffBezanson since it got bisected to commit that was authored by you

[tkelman]

Jeff, did you see this one?

[JeffBezanson]

Ah, it looks like testset generates a comprehension expecting to be able to drop a break or continue into it. What did it do in that case --- return an array with undefined elements? Is that needed behavior?

[tkelman]

cc @IainNZ if you know the answer - BaseTestNext may have a still-working implementation on 0.4?

[JeffBezanson]

Here's a patch to try, that just collects results for all iterations that finish:

--- a/base/test.jl
+++ b/base/test.jl
@@ -732,7 +732,12 @@ function testset_forloop(args, testloop)
         pop_testset()
         finish(ts)
     end
-    Expr(:comprehension, blk, [esc(v) for v in loopvars]...)
+    quote
+        arr = Array{Any,1}(0)
+        $(Expr(:for, Expr(:block, [esc(v) for v in loopvars]...),
+               :(push!(arr, $blk))))
+        arr
+    end
 end

[KristofferC]

The syntax looks just like a loop so being able to break is expected. It is also nifty if you want to skip tests based on some condition. Of course not an essential feature but it used to work so...

[JeffBezanson]

Nobody is arguing with that as far as I can tell...

[KristofferC]

I misunderstood your comment about "Is that needed behaviour" as asking about being able to break in a test loop at all. I see now what you meant, my apologies.

[wildart]

for and @testset for are not the same constructs. @testset for is just syntactic sugar for repetitive tests. So no continue or break. This must be stated in the unit test documentation.

[tkelman]

why not? it's useful, I think jeff's patch and the minimal test here make sense.