Skip to content

Accept test_args to specify which files to test#511

Merged
mcabbott merged 4 commits intoJuliaDiff:masterfrom
mcabbott:test_args
Aug 27, 2021
Merged

Accept test_args to specify which files to test#511
mcabbott merged 4 commits intoJuliaDiff:masterfrom
mcabbott:test_args

Conversation

@mcabbott
Copy link
Copy Markdown
Member

This lets you select which test files to run, like this:

Pkg.test("ChainRules", test_args = ["index", "LinearAlgebra"])

Only those files whose name or path contains one of the strings you provide will be run. Unless you provide nothing, in which case it runs everything, as before.

I also had a go at reducing the number of recursive levels of un-needed recursive testsets containing levels of testsets which weren't strictly necessary and contain other levels of testsets. Possibly this should go with a quick audit of the names of testsets (containing testsets) in the files being included.

@time Base.include(@__MODULE__(), path) do ex
Meta.isexpr(ex, :macrocall) && ex.args[1] == Symbol("@testset") || return ex
return :(@interpret (() -> $ex)()) # interpret testsets using JuliaInterpreter
function include_test(path)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

At this point i think this function could do with a docstring.

test/runtests.jl Outdated
@testset "ChainRules" begin # one overall @testset ensures it keeps going after failures
include("test_helpers.jl")
println()
# @testset "rulesets" begin
Copy link
Copy Markdown
Member

@oxinabox oxinabox Aug 25, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This one can definitely go.
Remove this and un-indent everything else.

Probably we can remove that level of the file hierarchy too.
It's a legacy from when ChainRules and ChainRulesCore was one package.
Probably that should be a seperate PR.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you move files without destroying git blame? Not sure how much that matters for tests, but sometimes nice to have. Although I would vote for less of a hierarchy, all else equal.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you can.
It is crucial IMO that we don't destroy git-blame.

May need some research as to how to do it.
Or git mv might just do it.

test/runtests.jl Outdated
include("test_helpers.jl")
println()
# @testset "rulesets" begin
# @testset "Base" begin
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Historically I have found these levels pretty useful.
but that may be less true, now that we are breaking the API less

@oxinabox
Copy link
Copy Markdown
Member

Mostly my need for this has gone way down since i have TestEnv.jl
So can just activate the test enviroment and then highlight a block of tests and Shift+Enter in vs-code, or copy and past into the REPL.

But I am not at all opposed to this.

I don't love having the commented out test set blocks
But maybe they are the clearest way still to demarcate the sections,
even if we don't want the system to actually include those layers.
Not 100% sure we don't want those layers though

Copy link
Copy Markdown
Member

@oxinabox oxinabox left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I trust you to make appropriate changes as needed and merge when your happy that this is the most useful way.
Not really much to review here 😁

@mcabbott
Copy link
Copy Markdown
Member Author

mcabbott commented Aug 25, 2021

Maybe this should be marked RFC. This is a pretty simple mechanism, given that it already has a function which handles includeing each path. Could also consider higher-tech things like https://github.com/JuliaTesting/ReTest.jl .

Or, as you say, I could learn to use https://github.com/JuliaTesting/TestEnv.jl . You still have to remember to run the include("test_helpers.jl") line, right? And if your session is a mess then perhaps things can go wrong, or go right when they wouldn't on CI.

@oxinabox
Copy link
Copy Markdown
Member

oxinabox commented Aug 25, 2021

Or, as you say, I could learn to use https://github.com/JuliaTesting/TestEnv.jl . You still have to remember to run the include("test_helpers.jl") line, right? And if your session is a mess then perhaps things can go wrong, or go right when they wouldn't on CI.

These statements are all true.
Also true is that you get to skip a ton of compile-time etc because you are continuing to use the same session, and Revise.

@mcabbott mcabbott merged commit 7caf869 into JuliaDiff:master Aug 27, 2021
@mcabbott mcabbott deleted the test_args branch August 27, 2021 02:42
@mcabbott
Copy link
Copy Markdown
Member Author

Let's try this out. We can add more levels of testsets back if this is too few.

No version bump, won't tag this, since it's mostly for the purpose of local testing on master.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants