Skip to content
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

Add the ability to use function calls in @testset directly. #42518

Merged
merged 6 commits into from
Oct 24, 2021

Conversation

Seelengrab
Copy link
Contributor

@Seelengrab Seelengrab commented Oct 6, 2021

This allows for easier factoring of tests into functions, making it possible to include a test/runtests.jl file without running tests directly (should it be written with this in mind) and enabling running of specific testsets explicitly by calling the functions they're defined in instead.

For example, this PR makes the following work:

f(x) = @test isone(x)
function g(x)
    @test x > 0
    @testset f(x)
end
@testset g(1)

with output:

Test Summary: | Pass  Total
g             |    2      2
Test.DefaultTestSet("g", Any[Test.DefaultTestSet("f", Any[], 1, false, false)], 1, false, false)

If a description string is provided, it will be used instead of the function name.
In case of a failure (e.g. by running @testset g(0)):

g: Test Failed at REPL[11]:2
  Expression: x > 0
   Evaluated: 0 > 0
Stacktrace:
 [1] macro expansion
   @ ~/julia/usr/share/julia/stdlib/v1.8/Test/src/Test.jl:460 [inlined]
 [2] g(x::Int64)
   @ Main ./REPL[11]:2
f: Test Failed at REPL[10]:1
  Expression: isone(x)
Stacktrace:
 [1] macro expansion
   @ ~/julia/usr/share/julia/stdlib/v1.8/Test/src/Test.jl:460 [inlined]
 [2] f(x::Int64)
   @ Main ./REPL[10]:1
Test Summary: | Fail  Total
g             |    2      2
  f           |    1      1
ERROR: Some tests did not pass: 0 passed, 2 failed, 0 errored, 0 broken.

The change was surprisingly minimal and I think it just.. works? Rudimentary testing in the REPL as well as those written tests included here didn't provide any obvious footguns, so I'm inclined to say this is a good idea.

Nevertheless, this is still missing a NEWS.md entry, docs could/should be more extensive and I may have missed something, so I'm opening this as a draft.

desc, testsettype, options = parse_testset_args(args[1:end-1])
if desc === nothing
desc = "test set"
end
if tests.head === :call
desc = string(tests.args[1]) # use the function name as test name
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Unsure if this is the correct way to deal with getting a string of the function name here.

@Seelengrab
Copy link
Contributor Author

no idea what's going on with the buildbot/package_* failures, seems unrelated judging by the time they run?

@Seelengrab Seelengrab marked this pull request as ready for review October 12, 2021 13:55
@Seelengrab
Copy link
Contributor Author

I don't know what's going on with the failing package jobs, but I think this is ready for review now.

@Seelengrab
Copy link
Contributor Author

Small bump - would be nice to get some feedback here, whether this is desired for Base etc. I think it could improve modularization of testsuites & runnability of single tests quite a bit, without requiring something heavier.

@oscardssmith oscardssmith added the triage This should be discussed on a triage call label Oct 20, 2021
@JeffBezanson JeffBezanson added the testsystem The unit testing framework and Test stdlib label Oct 21, 2021
@JeffBezanson
Copy link
Sponsor Member

Triage approves! Just needs a rebase.

@JeffBezanson JeffBezanson removed the triage This should be discussed on a triage call label Oct 21, 2021
@Seelengrab
Copy link
Contributor Author

Rebased on current master! Let's see if we can get a green CI run.

@Seelengrab
Copy link
Contributor Author

Well FreeBSD didn't want to play nice, but everything else passed. I hope this can be merged before it needs another rebase :)

@oscardssmith oscardssmith merged commit a23aa79 into JuliaLang:master Oct 24, 2021
@oscardssmith
Copy link
Member

freebsd tests aren't your fault.

Seelengrab added a commit to Seelengrab/julia that referenced this pull request Oct 24, 2021
@Seelengrab Seelengrab deleted the testset_function_calls branch October 24, 2021 19:22
@Seelengrab
Copy link
Contributor Author

Slipped on the Revert button 🤦

@jonas-schulze
Copy link
Contributor

Just noted that this feature allows

@testset "foo" include("foo.jl")

which currently requires begin ... end and is a somewhat common pattern.

@Seelengrab
Copy link
Contributor Author

Yes, include is not special in that regard - it's treated like any other function. I'm frankly surprised a PR like this hasn't been made sooner 🤷 Maybe I've missed something and this breaks horribly down the road.

LilithHafner pushed a commit to LilithHafner/julia that referenced this pull request Feb 22, 2022
…ang#42518)

* Add the ability to use function calls in @testset

This allows for easier factoring of tests into functions, making it possible to `include` a `test/runtests.jl` file without running tests directly and enabling running of specific testsets explicitly.

* Fix doctest of @testset on functions

* Add note about naming of testset for called functions

* Tweak documentation, expand on intended use case

* Add NEWS.md entry

* Fix explicit description behavior, add documentation to docstring

Co-authored-by: Sukera <Seelengrab@users.noreply.github.com>
LilithHafner pushed a commit to LilithHafner/julia that referenced this pull request Feb 22, 2022
LilithHafner pushed a commit to LilithHafner/julia that referenced this pull request Mar 8, 2022
…ang#42518)

* Add the ability to use function calls in @testset

This allows for easier factoring of tests into functions, making it possible to `include` a `test/runtests.jl` file without running tests directly and enabling running of specific testsets explicitly.

* Fix doctest of @testset on functions

* Add note about naming of testset for called functions

* Tweak documentation, expand on intended use case

* Add NEWS.md entry

* Fix explicit description behavior, add documentation to docstring

Co-authored-by: Sukera <Seelengrab@users.noreply.github.com>
LilithHafner pushed a commit to LilithHafner/julia that referenced this pull request Mar 8, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
testsystem The unit testing framework and Test stdlib
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants