Replace test_filter/custom_tests by a single testsuite argument.#57
Replace test_filter/custom_tests by a single testsuite argument.#57
Conversation
|
I'll update & check against OpenCL.jl tomorrow. |
|
One thing we should add a test for: Currently I believe it is possible to filter out a test by default and then run it with the command line interface. As an example in Enzyme, we currently have: But IIRC we should still be able to run our by default inactive test suites since the filtering was either command line or function based. With the |
|
Alright, that muddies the API up a little, but I think I've made that possible now. |
giordano
left a comment
There was a problem hiding this comment.
Perhaps a long shot, and mostly tangential to this PR (although the logic for the following may be related to the format of testsuite), but any thoughts on adding ways to for example have some testsets running serially? I think some tests in Julia do that, right? Or customise the order, besides the number of lines of code (for very manual load balancing basically).
Probably a separate |
giordano
left a comment
There was a problem hiding this comment.
Looks good to me, besides the missing docstring
|
I'll merge this but wait with tagging in case @vchuravy has any more comments (or if he wants to redesign |
| custom_tests::Dict{String, Expr}=Dict{String, Expr}(), init_code = :(), | ||
| test_worker = Returns(nothing), stdout = Base.stdout, stderr = Base.stderr) | ||
| function runtests(mod::Module, args::ParsedArgs; | ||
| testsuite::Dict{String,Expr} = find_tests(pwd()), |
There was a problem hiding this comment.
I think pwd() is problematic: JuliaAstro/BackgroundMeshes.jl#19 (comment). I don't see a better alternative though, basically we'd like a call-place @__DIR__. Since we're passing the module, one option could be joinpath(pkgdir(mod), "test"), but that assumes that (1) tests are always in the test subdirectory and that (2) pkgdir(mod) return something sensible, but it can also return nothing in some cases.
There was a problem hiding this comment.
I don't find either assumption unreasonable, and if either doesn't hold the user can provide their own path?
There was a problem hiding this comment.
I had thought of making runtest a macro so that we could do @__DIR__ from the caller's source file, but that seemed unnecessarily breaking.
There was a problem hiding this comment.
Agreed, I figured that one way to address this would be to make runtests itself a macro, but it'd feel weird.
Attempt to simplify the API, after #53 added yet another test-related kwarg.
To be merged, not squashed, as the commits are cleanly separated.