Unit testing: @test_throws regression: throw is not anymore detected

[omlins]

The following MWE shows a @test_throws regression. The throw is not anymore detected (this occurs also with Julia 1.6):

julia> import ParallelStencil.ParallelKernel.parallel_indices

julia> using ParallelStencil.ParallelKernel.Exceptions

julia> using Test

julia> @test_throws ArgumentError parallel_indices(:((ix,iy,iz)), :(f()=(99; if x return y end; return))) 
Test Failed at none:1
  Expression: parallel_indices(:((ix, iy, iz)), :(f() = begin
              99
              if x
                  return y
              end
              return
          end))
    Expected: ArgumentError
      Thrown: UndefVarError
ERROR: There was an error during testing

If we call the function directly with these arguments, we see that it does throw an ArgumentError:

julia> parallel_indices(:((ix,iy,iz)), :(f()=(99; if x return y end; return)))
ERROR: ArgumentError: invalid kernel in @parallel kernel definition: only one return statement is allowed in the kernel and it must return nothing and be the last statement (required to ensure equal behaviour with different packages for parallellization).
Stacktrace:
 [1] remove_return(body::Expr)
   @ ParallelStencil.ParallelKernel ~/.julia/dev/ParallelStencil/src/ParallelKernel/shared.jl:80
 [2] parallel_kernel(package::Symbol, numbertype::DataType, indices::Expr, kernel::Expr)
   @ ParallelStencil.ParallelKernel ~/.julia/dev/ParallelStencil/src/ParallelKernel/parallel.jl:130
 [3] parallel_indices(::Expr, ::Vararg{Expr, N} where N; package::Symbol, async::Bool)
   @ ParallelStencil.ParallelKernel ~/.julia/dev/ParallelStencil/src/ParallelKernel/parallel.jl:113
 [4] parallel_indices(::Expr, ::Vararg{Expr, N} where N)
   @ ParallelStencil.ParallelKernel ~/.julia/dev/ParallelStencil/src/ParallelKernel/parallel.jl:112
 [5] top-level scope
   @ none:1

Note that the other similar tests in the testsuite, it still works fine, e.g.:

julia> @test_throws ArgumentError parallel_indices(:((ix,iy,iz)), :(f()=(99; return something)))
Test Passed
      Thrown: ArgumentError

julia> parallel_indices(:((ix,iy,iz)), :(f()=(99; return something)))
ERROR: ArgumentError: invalid kernel in @parallel kernel definition: the last statement must be a `return nothing` statement ('return' or 'return nothing' or 'nothing') as required for any CUDA kernels.
Stacktrace:
 [1] remove_return(body::Expr)
   @ ParallelStencil.ParallelKernel ~/.julia/dev/ParallelStencil/src/ParallelKernel/shared.jl:76
 [2] parallel_kernel(package::Symbol, numbertype::DataType, indices::Expr, kernel::Expr)
   @ ParallelStencil.ParallelKernel ~/.julia/dev/ParallelStencil/src/ParallelKernel/parallel.jl:130
 [3] parallel_indices(::Expr, ::Vararg{Expr, N} where N; package::Symbol, async::Bool)
   @ ParallelStencil.ParallelKernel ~/.julia/dev/ParallelStencil/src/ParallelKernel/parallel.jl:113
 [4] parallel_indices(::Expr, ::Vararg{Expr, N} where N)
   @ ParallelStencil.ParallelKernel ~/.julia/dev/ParallelStencil/src/ParallelKernel/parallel.jl:112
 [5] top-level scope
   @ none:1

[Liozou]

After a bit of investigation, the culprit seems to be this line in the middle of the Test stdlib:

julia/stdlib/Test/src/Test.jl

Line 640 in 7cd1da3

Base.remove_linenums!(result)

What happens is that this line removes the line number indications in the code exported by the @test macros. However, it incidentally also removes the line number indications from the function you pass as an argument through an Expr (the :(f()=(99; return something)) argument). And as a consequence, this line of your code becomes invalid:
https://github.com/omlins/ParallelStencil.jl/blob/122414c0a55527a47b836abb0a1a25c49430f02c/src/ParallelKernel/shared.jl#L79
since the end-2 line of body.args is not a line number indication anymore, but an actual piece of function f (the if x return y end part I believe).

I think the presence of line number indications is not explicitly documented (except in the Developer Documentation under ASTs) which means that you should not rely on them... But I'm no expert on this, sorry. In any case, for your special purpose here, just make a check whether to drop the end-2 line of body.args (if it is a LineNumberNode) will surely fix the issue.
The complementary approach consists in understanding why is this Base.remove_linenums!(result) line necessary in Test code and to make it so that it does not affect blocks of code given explicitly by the user, so that it does not make a difference whether a function is called in normal scope or within an @test macro.

... and after writing all this, it seems that this issue was already well-known: see #31334 and the corresponding PR #31335. So I'm going to close this one as a duplicate, feel free to comment if I missed something!