invenia / Memento.jl

A flexible logging library for Julia
https://invenia.github.io/Memento.jl/latest
Other
88 stars 14 forks source link

`@test_warn` doesn't (always) work with Memento.jl #48

Closed spurll closed 6 years ago

spurll commented 6 years ago

@test_warn appears to work when no logger is specified:

julia> @test_warn "Help!" Memento.warn("Help!")

julia> 

But @test_warn doesn't appear to work when a logger is specified:

julia> @test_warn "Help!" Memento.warn(Memento.get_logger(), "Help!")
Test Failed
  Expression: (Base.Test.ismatch_warn)("Help!", (Base.Test.readstring)(#297#fname))
ERROR: There was an error during testing
rofinn commented 6 years ago

It looks like the reason the former works is because the Memento.warn call is dispatching to Base.warn. Having a more general test macro like @test_log(msg, level, expr) might be useful for testing log messages at various log levels.

rofinn commented 6 years ago

This is my prototype of a general @test_log(logger, level, msg, expr) macro https://github.com/invenia/Memento.jl/blob/rf/test_log/src/test.jl.

julia> using Memento, Memento.Test; l = Memento.config("info")
INFO: Recompiling stale cache file /Users/rory/repos/Memento.jl/.playground/packages/lib/v0.6/Memento.ji for module Memento.
Logger(root)

julia> @test_log(l, "warn", "Hello!", warn(l, "Hello!"))
[warn | root]: Hello!

julia> @test_log(l, "warn", "Hello!", warn(l, "Hello"))
[warn | root]: Hello
Test Failed
  Expression: #11#handler.found
ERROR: There was an error during testing
spurll commented 6 years ago

That looks good to me. Is there a way to get @test_warn and @test_error to work as well? Seems like it might be tough, what with the logger argument.

spurll commented 6 years ago

@test_log is definitely sufficient for my purposes, but because @test_error and @test_warn exist in Base.Test I can see some people expecting them to work.

rofinn commented 6 years ago

@spurll Could you review #53?

spurll commented 6 years ago

Done! Looks good to me.

spurll commented 6 years ago

Sorry to do this to you, @rofinn, but I ran into an issue. The @test_warn and @test_error macros in Base.Test actually have one bit of functionality that we kinda need:

Test whether evaluating expr results in output that contains the msg string or matches the msg regular expression

(They're also a little more flexible in other ways, allowing us to pass a function to be evaluated on the message, for example; we don't need a dispatch like that at this point.)

This partial-matching is important because we often won't know exactly what message we're going to get. (The message often contains some sort of UUID or something that's impossible to predict.)

Matching the functionality of the Base.Test macros would be great, but if @test_log could support testing whether the handler's message contains msg (rather than equals msg) at a minimum that would be really helpful. (It would eliminate a few dozen lines of code from several tests.)

omus commented 6 years ago

I was talking to @rofinn about this an I think the solution is to break apart complicated messages. Usually you can use string interpolation to fill in the UUIDs to make exact matches.

spurll commented 6 years ago

Yep, I am using 0.5.1.

spurll commented 6 years ago

Curt has mentioned that Sam Morrison is currently working on fixing the specific case where this is an issue in S3DB, but I think that partial matches are still a useful thing to support, especially since they're working in base. Feel free to close or fix as you see fit.

rofinn commented 6 years ago

@spurll Please open a new issue for the specific behaviour you'd like. The documentation for @test_warn is unclear and doesn't provide any examples.

I had to go looking in base julia for some example tests. https://github.com/JuliaLang/julia/pull/19903/files#diff-cac25e1e95e0a8d859cacad2a2e69677R285 and https://github.com/JuliaLang/julia/pull/19903/files#diff-cac25e1e95e0a8d859cacad2a2e69677R295