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

Allow @test_nothrow to catch 'no error' for simple smoke tests #18780

Open
djsegal opened this issue Oct 3, 2016 · 20 comments
Open

Allow @test_nothrow to catch 'no error' for simple smoke tests #18780

djsegal opened this issue Oct 3, 2016 · 20 comments
Labels
testsystem The unit testing framework and Test stdlib

Comments

@djsegal
Copy link

djsegal commented Oct 3, 2016

I want to be able to do a quick @test_throws nothing Foo(42) to run a simple smoke test.

The current documentation makes it look like this is not an option

@kshyatt kshyatt added the testsystem The unit testing framework and Test stdlib label Oct 3, 2016
@oxinabox
Copy link
Contributor

oxinabox commented Oct 4, 2016

I've spent far too much time thinking about this, a few weeks back.

You can readily approximate that by doing something like @test Foo(42) != π
which will always have a test result of Pass, (assuming that it does indeed not return pi),
unless there is a exception -- then it will instead have a test result of Error
This macro @test_throws nothing, would turn that Error result into a Fail result.

After a fair bit of thought on that, getting into the semantics of what is an Error vs what is a Fail,
is a rabbit hole that does not go anywhere useful. As far as it goes it is right now: Fails are almost always a fault in src, Errors can be either a fault in src or a fault in test or a fault in the enviroment (eg Out of Memory).

@test_throws nothing, would allow a Fail to be causes by anything that can cause a Error, like OOM.
Now, that is a loss of information.
But it doesn't matter, because Fail or Error. when your test cases do not Pass you investigate it and fix it.

I think preferable would be the syntax @test_doesnotthrow, or @test_noerror

One thing to note, is that this is not present in most testing frameworks.

The only framework I've fount that has it is nUnit, which as DoesNotThrow in classic mode, and ThrowsNothing in constraint mode.

@djsegal
Copy link
Author

djsegal commented Oct 4, 2016

@test_no_error ... would probably be good.

I know it's not the same, but rspec for ruby uses the syntax:

expect{Object.public_instance_methods}.to_not raise_error(NameError)

which came from:

@StefanKarpinski
Copy link
Member

I don't really get the application. Why do you need a testing construct for this at all? Every statement is implicitly @test_no_error; if an error is thrown, the test suite fails.

@djsegal
Copy link
Author

djsegal commented Oct 4, 2016

I see where you're coming from.

  • I just wanted it to be more explicit in test files that something failed at construction time
  • As opposed to just having a random line in your test file that some good samaritan might delete

Feel free to close issue at your own discretion

@kshyatt
Copy link
Contributor

kshyatt commented Oct 4, 2016

just having a random line in your test file that some good samaritan might delete

FWIW I feel like this problem can be addressed by commenting the tests more aggressively than we currently do. I often look at the base Julia tests and think "it sure would be nice to know what this is trying to show works!" Even a simple "test that X doesn't explode immediately" comment block would be good.

@djsegal
Copy link
Author

djsegal commented Oct 4, 2016

  • My only argument against comments is that they're out of date the second you write them
  • If anything, I'd prefer something like:
@test_no_error Foo(42) "Foo can be constructed with integer value"

@ararslan
Copy link
Member

ararslan commented Oct 4, 2016

I agree with the others that I don't think such a construct is that generally useful. Though for the sake of completeness, you can obtain what you're looking for relatively easily using existing machinery, it's just a little hacky and ugly:

using Base.Test
f(x::Int) = 1
@test !isa(try f(1) catch ex ex end, Exception)    # Passes
@test !isa(try f(1.0) catch ex ex end, Exception)  # Fails

Assuming you know the value of Foo(42), wouldn't @test Foo(42) == something be a sufficient proxy to testing that something isn't thrown? You end up getting more information, since if it passes you know both that nothing was thrown and that you get the expected value.

@StefanKarpinski
Copy link
Member

Ok, I see where this is coming from. You could do this: @test isa(ex, Any), although that's not very pretty. You could also write this macro:

macro no_error(ex)
    quote
        try
            $(esc(ex))
            true
        catch
            false
        end
    end
end

julia> @no_error 1 + 2
true

julia> @no_error error()
false

julia> @test @no_error 1 + 2
Test Passed
  Expression: @no_error 1 + 2

julia> @test @no_error error()
Test Failed
  Expression: @no_error error()
ERROR: There was an error during testing
 in record(::Base.Test.FallbackTestSet, ::Base.Test.Fail) at ./test.jl:397
 in do_test(::Base.Test.Returned, ::Expr) at ./test.jl:281

The main issue with using this is that no hint is given as to what error is thrown on failure. Hmm. I'm a bit ambivalent about whether this warrants a whole new @test_no_error macro.

@tpapp
Copy link
Contributor

tpapp commented Jan 28, 2017

I would find this construct useful for testing functions which (mainly) have side effects. Eg when I implement a method for Base.show, testing that it just runs is useful (of course the right way would be capturing the output in an IOBuffer and comparing it to something predefined, but it is nice to have something intermediate which is more robust to code changes).

@cossio
Copy link
Contributor

cossio commented Jan 30, 2017

Currently we have @test_warn, @test_nowarn, @test_throws. I think a @test_nothrow completes the picture.

Here is a possible use case. Suppose you want to test a method that returns no results. But you know it throws an exception if some condition is not met. If you know in a certain context the condition should hold, the only way to test it would be to make sure that the method doesn't throw the exception.

I don't agree with a generic @test_nothrow to test that the method does not throw any exception. Instead we should have a @test_nothrow except where except is a specific (or abstract) exception. A possible semantic would be:

  1. The test results in Error if an exception other than except is thrown.
  2. The test Fails if except is thrown.
  3. The test Passes if no exceptions are thrown.

@StefanKarpinski
Copy link
Member

Sure, I guess if someone wants to put together a @test_nothrow PR we could add this. Those semantics seem sensible enough.

@JeffBezanson JeffBezanson changed the title Allow @test_throws to catch 'no error' for simple smoke tests Allow @test_nothrow to catch 'no error' for simple smoke tests Mar 28, 2017
@jondeuce
Copy link
Contributor

FWIW, I found myself searching for this functionality, too. I ended up hacking together this solution (amusingly, with the same macro name):

struct NoException <: Exception end

macro test_nothrow(ex)
    esc(:(@test_throws NoException ($(ex); throw(NoException()))))
end

Of course, I'm not suggesting this implementation. But I agree with @tpapp that it would be nice to have an explicit, self-documenting test that just says "this code should run without issues" as a kind of sanity check.

@vtjnash
Copy link
Member

vtjnash commented Jun 17, 2021

Why not @test (worked(); true), @test worked() === nothing, or just worked()?

@jondeuce
Copy link
Contributor

Yes those would surely work for my case, though I do feel that @test_nothrow makes your intentions more clear.

But more generally I think the proposal of @cossio above would be useful. My example would then be a special case: @test_nothrow Exception <expr>.

@StefanKarpinski
Copy link
Member

That proposal does seem reasonable. The fail/error distinction makes sense to me.

@tpapp
Copy link
Contributor

tpapp commented Jun 18, 2021

I wonder if we could shove this functionality into @test_throws, by special-casing eg Nothing (which is <: Exception, so there is no possibility of confusion). Eg

@test_throws Union{Nothing,DomainError} maybe_domain_error(a, b)
@test_throws Nothing this_should_just_run(c, d)

@schlichtanders
Copy link

I just had the same which for a @test_nothrow because I wanted to have a clean interface check which checks for MethodErrors explicitly.

I came up with the following definition, which is more or less just the code by @test_throws with different return objects.

module TestNoThrows
export @test_nothrows

using Test: Returned, Threw, Fail, Error, Pass, record, get_testset, ExecutionResult

"""
@test_nothrows exception expr

Tests that the expression `expr` does not throw `exception`.
"""
macro test_nothrows(extype, ex)
    orig_ex = Expr(:inert, ex)
    result = quote
        try
            Returned($(esc(ex)), nothing, $(QuoteNode(__source__)))
        catch _e
            if $(esc(extype)) != InterruptException && _e isa InterruptException
                rethrow()
            end
            Threw(_e, Base.current_exceptions(), $(QuoteNode(__source__)))
        end
    end
    Base.remove_linenums!(result)
    :(do_test_nothrows($result, $orig_ex, $(esc(extype))))
end

# An internal function, called by the code generated by @test_throws
# to evaluate and catch the thrown exception - if it exists
function do_test_nothrows(result::ExecutionResult, orig_expr, extype)
    if isa(result, Threw)
        # Check that the right type of exception was thrown
        success = false
        message_only = false
        exc = result.exception
        if isa(extype, Type)
            success = isa(exc, extype)
        elseif isa(extype, Exception) || !isa(exc, Exception)
            if extype isa LoadError && !(exc isa LoadError) && typeof(extype.error) == typeof(exc)
                extype = extype.error # deprecated
            end
            if isa(exc, typeof(extype))
                success = true
                for fld in 1:nfields(extype)
                    if !isequal(getfield(extype, fld), getfield(exc, fld))
                        success = false
                        break
                    end
                end
            end
        else
            message_only = true
            exc = sprint(showerror, exc)
            success = contains_warn(exc, extype)
            exc = repr(exc)
            if isa(extype, AbstractString)
                extype = repr(extype)
            elseif isa(extype, Function)
                extype = "< match function >"
            end
        end
        if success
            # the semantics is exactly the opposite of test_throws
            testres = Fail(:test_throws, orig_expr, extype, exc, result.source, message_only)
        else
            testres = Error(:test_error, orig_expr, exc, result.backtrace::Vector{Any}, result.source)
        end
    else
        testres = Pass(:test_throws_nothing, orig_expr, extype, nothing, result.source)
    end
    record(get_testset(), testres)
end


end # module TestNoThrows

I never did an addition to Julia Test or any core Julia package.
If this little helper could be added, I am happy to create my first pullrequest. Please let me know (whoever you are).

@KeithWM
Copy link

KeithWM commented Mar 15, 2023

I've now visited this thread for the second time and still can't really figure out what the argument is against @test_nothrow, albeit with a different name. It's obviously something there's a desire for and any kind of workaround either adds a lot of boilerplate or, worse still, adds utter confusion about what you are trying to achieve with a test.

In my case: I'm trying to remove some method ambiguities and all I want to confirm is that a particular function can be called with particular arguments without throwing a MethodError. I don't want to test anything about the output because is simply besides the point of the test.

@emstoudenmire
Copy link
Contributor

Here's a real-life example for wanting @test_nothrow that just happened to me.

I was looking at a test in our codebase which was written as

@test isnothing(f(x))

for some particular f. I spent half an hour thinking about why we wanted f to return nothing and discussing with our lead developer, only to learn that the test was actually intended to test that f doesn't throw for the input x, and it just happened that nothing is the expected output for that input. (I might have even been the one who wrote that test two years ago and just forgot!)

So the test, as written, communicated the wrong thing to me about what the test was really for, whereas if it had been

@test_nothrow f(x)

I would have understood its purpose immediately, saved half an hour of my morning, and understood our code base better.

@tpapp
Copy link
Contributor

tpapp commented Oct 6, 2023

I think that this issue is just waiting for someone to create a PR.

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

No branches or pull requests