Conversation
hamzahrmalik
force-pushed
the
in_memory_log_handler
branch
2 times, most recently
from
bd5f9c0 to
15c267c
Compare
hamzahrmalik
force-pushed
the
in_memory_log_handler
branch
from
15c267c to
a57256c
Compare
| level: level, | ||
| message: message, | ||
| metadata: metadata | ||
| ) |
There was a problem hiding this comment.
for completeness, wouldn't we want to also store the source/line? Maybe bad idea to assert on them but perhaps can be useful in debugging in some weird situations, not up to us to judge that I think 🤔
There was a problem hiding this comment.
We could do. But then equating Entry's becomes hard, unelss you know what source to expect...
There was a problem hiding this comment.
We'd skip it from equatable and hashable impls -- lets see if anyone cares about it during review; in any case this is LGTM as well as is
There was a problem hiding this comment.
I'm happy to not store file/line for now in this handler.
ktoso
left a comment
ktoso
left a comment
There was a problem hiding this comment.
That's a nice non-controversial addition and follows suit what we did in tracing https://github.com/apple/swift-distributed-tracing/tree/main/Sources/InMemoryTracing -- LGTM 👍
I also agree with keeping any assertions out of this, it's just an in memory handler, not specifically for testing.
FranzBusch
left a comment
FranzBusch
left a comment
There was a problem hiding this comment.
Should we do a small proposal for this since we have a proposal process setup now? Otherwise LGTM!
| level: level, | ||
| message: message, | ||
| metadata: metadata | ||
| ) |
There was a problem hiding this comment.
I'm happy to not store file/line for now in this handler.
proposals/0002-in-memory-handler.md
Outdated
There was a problem hiding this comment.
If we're not doing the proposal, no need to add the proposal .md either.
| import Logging | ||
| import Synchronization | ||
|
|
||
| /// A custom log handler which doesn't actually emit logs, but just collects them into memory. |
There was a problem hiding this comment.
| /// A custom log handler which doesn't actually emit logs, but just collects them into memory. | |
| /// A custom log handler which collects them into memory. |
| /// You can then retrieve a list of what was logged and run assertions on it. | ||
| /// This handler is intended to be used in tests. |
There was a problem hiding this comment.
I think this log handler has more applications than just tests. Can we make this doc comment a bit more general purpose?
Co-authored-by: Franz Busch <[email protected]>
|
Doesn’t this close #242? |
5 participants
Add an InMemoryLogHandler
Motivation:
Library maintainers should be able to test that their libraries log what they expect
Modifications:
Create a new InMemoryLogging product, which contains an InMemoryLogHandler
This log handler can be used to make a logger, pass the logger into some function, and then assert that logs were emitted.
See the tests for example usage
Future:
For now i have left this very bare-bones. We could in future add convenience functions. For example