Base.Test does not support continue statement

[rprechelt]

Using Base.Test on master,

@testset "Testing 1..3" for i=1:3
           @test 1 == 1
       end

results in

Test Summary: | Pass  Total
  Testing 1..3 |    1      1
Test Summary: | Pass  Total
  Testing 1..3 |    1      1
Test Summary: | Pass  Total
  Testing 1..3 |    1      1

as expected. However, adding a continue statement set to trigger on i=2, causes the whole testset to stop on i=2 and not continue to i=3.

@testset "Testing 1..3" for i=1:3
           if i%2 == 0
               continue
           end
           @test 1 == 1
       end

results in

Test Summary: | Pass  Total
  Testing 1..3 |    1      1

Furthermore, once I have run one testset in a REPL that contained a call to continue, any regular (non-continued) testset now returns Nothing. i.e.

@testset "Testing 1..3" for i=1:3
           @test 1 == 1
       end

returns

3-element Array{Union{Base.Test.DefaultTestSet,Base.Test.FallbackTestSet,Void},1}:
 nothing
 nothing
 nothing

and does not print any test results to the terminal.

I can confirm that this also happens on release-0.5

Maybe having the continue statement not interact with a testset is by design, but having one testset with a continue make all future REPL testset's not correct is definitely a bug.

[mauro3]

Related #17462?

[KristofferC]

I get the same with BaseTestNext on 0.4.6. Probably means my tests at https://github.com/KristofferC/NearestNeighbors.jl/blob/79cbe1343dfd591b33af5209de729024b16cf99c/test/test_monkey.jl#L12 are horribly broken :P

[tkelman]

also x-ref #17692 - are we getting lucky that we don't have many @testsets in the base tests? What consequences does this have for #17165?

[kshyatt]

The consequences for my PR are that I need to rebase anyway. IIRC we don't have any @testsets in test/ except in test/test.jl.

To expand a bit, most of the changes I made in that PR to @testset deal with how/when it throws if a test fails, and how it prints. So I think changes to how a @testset deals with break/continue/finally shouldn't break things too horribly.

[tkelman]

Does anyone know the @testset code well enough to take a look at this? Would really like to have an assignee here to try and address this, as it means the test system can silently give misleading information.

[yuyichao]

It seems that it'll actually not miss any failing tests at least...

[yuyichao]

And the main issue here is that finally does not catch continue, break (also @goto, which explicitly raises a syntax error) so it's impossible to reliably make sure the clean up code is always run.

julia> f_return() = for i in 1:10
           try
               return
           finally
               println(i)
           end
       end
f_return (generic function with 1 method)

julia> f_break() = for i in 1:10
           try
               break
           finally
               println(i)
           end
       end
f_break (generic function with 1 method)

julia> f_continue() = for i in 1:10
           try
               continue
           finally
               println(i)
           end
       end
f_continue (generic function with 1 method)

julia> f_return()
1

julia> f_break()

julia> f_continue()

[tkelman]

great find @yuyichao. so this is closely related to (near dupe? nasty consequence?) #12806 then

[yuyichao]

Yeah, almost. Once #12806 is fixed the following patch should fix the issue for testset. (The patch currently makes no difference)

diff --git a/base/test.jl b/base/test.jl
index 7d23115..04a4015 100644
--- a/base/test.jl
+++ b/base/test.jl
@@ -728,14 +728,14 @@ function testset_forloop(args, testloop)
             # Something in the test block threw an error. Count that as an
             # error in this test set
             record(ts, Error(:nontest_error, :(), err, catch_backtrace()))
+        finally
+            pop_testset()
+            push!(arr, finish(ts))
         end
-        pop_testset()
-        finish(ts)
     end
     quote
         arr = Array{Any,1}(0)
-        $(Expr(:for, Expr(:block, [esc(v) for v in loopvars]...),
-               :(push!(arr, $blk))))
+        $(Expr(:for, Expr(:block, [esc(v) for v in loopvars]...), blk))
         arr
     end
 end

[yuyichao]

There's probably a way to trick it to do the right thing though, let me have a try.

[tkelman]

x-ref #13660 for past attempt, some discussion and good test cases

[yuyichao]

Yeah I saw that PR.

By trick I actually mean rearrange the code for testset to fix it without fixing lowering. Anyway, seems to be working, just need to figure out a way to test it.