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

Make Test.TestLogger and Test.LogRecord part of the public API #44016

Closed
ericphanson opened this issue Feb 2, 2022 · 2 comments · Fixed by #44080
Closed

Make Test.TestLogger and Test.LogRecord part of the public API #44016

ericphanson opened this issue Feb 2, 2022 · 2 comments · Fixed by #44080
Labels
logging The logging framework

Comments

@ericphanson
Copy link
Contributor

ericphanson commented Feb 2, 2022

This can make writing tests that interact with functions that log a lot easier. The @test_logs macros are useful, but work better for small short functions that log one or two things. For end-to-end tests of larger applications, one might want to test things differently, and having access to the actual LogRecords to inspect and test against is very useful. Moreover, one can do something like

using Test, Logging

function relog(r)
    return Logging.handle_message(global_logger(), r.level, r.message, r._module, r.group,
                                  r.id, r.file, r.line; r.kwargs...)
end

test_logger = Test.TestLogger()
try
    with_logger(test_logger) do
        my_complicated_function_with_a_lot_of_logging()
    end
catch
    println("Caught exception, so dumping logs:")
    foreach(relog, test_logger.logs)
    rethrow()
end
# test against `test_logger.logs`...

The benefit of this is you can suppress logs except when an error occurs, in which case being able to see the logs might be useful in understanding the error.

This is all doable already, but it's not part of the public API. My proposal is to simply document and export Testlogger and LogRecord, to make the above usage not-involving-internals. We could also provide an accessor for the logs field if we don't want the field itself to be public API.

xref #43641 (comment)

@fredrikekre
Copy link
Member

Seems like a good idea. I think it would be nice if it caught all log messages so #43641 was probably not a good idea? It should be a logger for testing the events, not testing the logger itself.

@fredrikekre fredrikekre added the logging The logging framework label Feb 7, 2022
@ericphanson
Copy link
Contributor Author

Yeah, maybe we can revert that and do this instead, because this way gives more access to directly test keywords and things like that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
logging The logging framework
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants