Actaully test recursive_unitless_eltype - #56
Conversation
While working on #55 I noticed, that the tests for the `recursive_unitless_eltype` function are not actually run. They are written down, but without a `@test` macro their result are never checked. So I added them.
| recursive_unitless_eltype(AofuSA) == SVector{2,Float64} | ||
|
|
||
| @inferred recursive_unitless_eltype(AofuSA) | ||
| @test recursive_unitless_eltype(AofuSA) == SVector{2,Float64} |
There was a problem hiding this comment.
@test @inferred(recursive_unitless_eltype(AofuSA)) == SVector{2,Float64}
There was a problem hiding this comment.
Hi @YingboMa, I appreciate your comment, but could you please elaborate?
If you mean to tell me that I would need to add the @inferred back, please explain to me why.
From my understanding, the @inferred is just another way of testing things. But as I added the @test macros, I think we don't need it anymore.
There was a problem hiding this comment.
Sorry about that. I should have explained. @inferred is a stronger test than @test recursive_unitless_eltype(AofuSA) == SVector{2,Float64}. Consider the following case
julia> using Test
julia> foo() = rand() > 2 ? Int : Float64
foo (generic function with 1 method)
julia> @test foo() == Float64
Test Passed
julia> @inferred foo()
ERROR: return type Type{Float64} does not match inferred return type Union{Type{Float64}, Type{Int64}}
Stacktrace:
[1] error(::String) at ./error.jl:33
[2] top-level scope at none:0There was a problem hiding this comment.
Hi @YingboMa, thank you for the clarification! Always nice to learn :) As I understand it now, @inferred is used to ensure type stability.
Would it make sense to add @inferred to all tests?
There was a problem hiding this comment.
as many as possible, yes it would be good to have on any test that's checking output types.
3 participants
While working on #55 I noticed, that the tests for the
recursive_unitless_eltypefunction are not actually run. They arewritten down, but without a
@testmacro their result are neverchecked. So I added the macro.