-
Notifications
You must be signed in to change notification settings - Fork 2
Support custom records #51
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
| end | ||
| catch err | ||
| isa(err, Test.TestSetException) || rethrow() | ||
| function execute(::Type{TestRecord}, mod, f, name, color, custom_args)::TestRecord |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The split here is so that the execute is the only function you need to customize.
|
|
||
| function runtest(RecordType::Type{<:AbstractTestRecord}, f, name, init_code, color, custom_args) | ||
| # generate a temporary module to execute the tests in | ||
| mod = Core.eval(Main, Expr(:module, true, gensym(name), Expr(:block))) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Now that this isn't in the custom_record_init anymore we can write it again as:
| mod = Core.eval(Main, Expr(:module, true, gensym(name), Expr(:block))) | |
| mod = @eval(Main, module $(gensym(name)) end) |
| stats = redirect_stdio(stdout=io, stderr=io) do | ||
| @timed try | ||
| # Since we are in a double quote we need to use this form to escape `$` | ||
| if $(Expr(:$, :say_hello)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Talked with Jeff in person about this, and it is seems to be the "right way".
| # entry point | ||
| # | ||
|
|
||
| function runtest(::Type{TestRecord}, f, name, init_code, color) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why are we passing color here?
| custom_record_init = quote | ||
| import ParallelTestRunner: Test | ||
| struct CustomTestRecord <: ParallelTestRunner.AbstractTestRecord | ||
| # TODO: Would it be better to wrap "ParallelTestRunner.TestRecord " |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@maleadt this is probably the biggest open design question.
|
In https://github.com/JuliaGPU/OpenCL.jl/blob/cdff90d17f662e2746940e42d7997d3d99cc995c/test/runtests.jl#L9 I show how to extract a custom command line flag, but what is missing right now is some sort of |
|
Looking at CUDA.jl the missing features I am seeing are:
|
| """ | ||
| function runtests(mod::Module, ARGS; test_filter = Returns(true), RecordType = TestRecord, | ||
| custom_tests::Dict{String, Expr}=Dict{String, Expr}(), init_code = :(), | ||
| custom_tests::Dict{String, Expr}=Dict{String, Expr}(), init_code = :(), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unrelated whitespace change
| custom_tests::Dict{String, Expr}=Dict{String, Expr}(), init_code = :(), | |
| custom_tests::Dict{String, Expr}=Dict{String, Expr}(), init_code = :(), |
Currently struggling with the fact that nested macros and interpolation are a pain