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

Doesn't compose well with testsets #103

Open
staticfloat opened this issue Apr 5, 2021 · 3 comments
Open

Doesn't compose well with testsets #103

staticfloat opened this issue Apr 5, 2021 · 3 comments

Comments

@staticfloat
Copy link
Member

staticfloat commented Apr 5, 2021

I have found a few usages in the wild of @require enabling test sets dynamically, but it has some surprising interactions in that a failing test within the nested @require-@testset gets printed, but does not impact Julia's return value. Example:

# test.jl
using Test, Requires                                                                                                                                               

@require Test = "8dfed614-e22c-5e08-85e1-65c5234f0b40" begin
    @testset "Linux-specific Tests" begin
        @test false
    end
end
$ julia test.jl && echo yes
Linux-specific Tests: Test Failed at /home/sabae/src/test.jl:5
  Expression: false
Stacktrace:
 [1] macro expansion
   @ ~/src/test.jl:5 [inlined]
 [2] macro expansion
   @ /buildworker/worker/package_linux64/build/usr/share/julia/stdlib/v1.6/Test/src/Test.jl:1151 [inlined]
 [3] top-level scope
   @ ~/src/test.jl:5
Test Summary:        | Fail  Total
Linux-specific Tests |    1      1
┌ Warning: Error requiring `Test` from `Main`
│   exception = Some tests did not pass: 0 passed, 1 failed, 0 errored, 0 broken.
└ @ Requires ~/.julia/packages/Requires/7Ncym/src/require.jl:49
yes

It looks to me like a failing test set might get caught by some generic error handling routine.

@KristofferC
Copy link
Member

KristofferC commented Apr 6, 2021

function err(@nospecialize(f), listener::Module, modname::String)
try
f()
catch exc
@warn "Error requiring `$modname` from `$listener`" exception=(exc,catch_backtrace())
end
end
I guess. I don't really understand why you wouldn't just error there, something is cleary broken.

@staticfloat
Copy link
Member Author

Seems to me that we should rethrow(exc)?

@KristofferC
Copy link
Member

KristofferC commented Apr 7, 2021

Imo yes. But maybe there is some good reason. I guess it can be argued that if the @requires part is an optional addition of functionality, failing to load that shouldn't be a hard error. But it is unclear what state the system is in after that part errors so in my opinion error is probably the right thing to do.

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

No branches or pull requests

2 participants