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
Testing for NONE
log level
#42
Changes from 4 commits
0824dd4
5c20d87
c457118
72e2713
817e381
e866222
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -30,34 +30,43 @@ import io.klogging.events.LogEvent | |
* Logger interface for sending log events inside coroutines. | ||
*/ | ||
public interface Klogger : BaseLogger { | ||
|
||
public suspend fun emitEvent(level: Level, exception: Exception?, event: Any?) | ||
|
||
public suspend fun log(level: Level, exception: Exception, event: Any?): Unit = | ||
public suspend fun log(level: Level, exception: Exception, event: Any?) { | ||
if (!isLevelEnabled(level)) return | ||
emitEvent(level, exception, event) | ||
} | ||
|
||
public suspend fun log(level: Level, event: Any?): Unit = | ||
public suspend fun log(level: Level, event: Any?) { | ||
if (!isLevelEnabled(level)) return | ||
emitEvent(level, null, event) | ||
} | ||
|
||
public suspend fun log( | ||
level: Level, | ||
exception: Exception, | ||
template: String, | ||
vararg values: Any? | ||
): Unit = | ||
) { | ||
if (!isLevelEnabled(level)) return | ||
if (values.isEmpty()) emitEvent(level, exception, template) | ||
else emitEvent(level, exception, e(template, *values)) | ||
} | ||
|
||
public suspend fun log(level: Level, template: String, vararg values: Any?): Unit = | ||
public suspend fun log(level: Level, template: String, vararg values: Any?) { | ||
if (!isLevelEnabled(level)) return | ||
if (values.isEmpty()) emitEvent(level, null, template) | ||
else emitEvent(level, null, e(template, *values)) | ||
} | ||
|
||
public suspend fun log(level: Level, exception: Exception, event: suspend Klogger.() -> Any?) { | ||
if (isLevelEnabled(level)) emitEvent(level, exception, event()) | ||
if (!isLevelEnabled(level)) return | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think the original is clearer. An early return is only needed if what follows is more complex. But I won’t go to the barricades over this. I can see the consistency with the earlier, more complex functions. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah, two reasons:
|
||
emitEvent(level, exception, event()) | ||
} | ||
|
||
public suspend fun log(level: Level, event: suspend Klogger.() -> Any?) { | ||
if (isLevelEnabled(level)) emitEvent(level, null, event()) | ||
if (!isLevelEnabled(level)) return | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ditto. |
||
emitEvent(level, null, event()) | ||
} | ||
|
||
public suspend fun trace(event: Any?): Unit = log(TRACE, event) | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -31,9 +31,9 @@ import io.klogging.events.timestampNow | |
import io.klogging.template.templateItems | ||
import io.kotest.core.spec.style.DescribeSpec | ||
import io.kotest.matchers.maps.shouldContain | ||
import io.kotest.matchers.nulls.shouldBeNull | ||
import io.kotest.matchers.shouldBe | ||
import java.lang.Thread.currentThread | ||
import kotlin.time.Duration | ||
import kotlin.time.ExperimentalTime | ||
|
||
class TestLogger(private val minLevel: Level = TRACE) : Klogger { | ||
|
@@ -68,8 +68,38 @@ class TestException(message: String) : Exception(message) | |
class KloggerTest : DescribeSpec({ | ||
// Capture once rather than call several times: Avoid flaky tests if the OS sleeps our process | ||
val now = timestampNow() | ||
val thing = object { | ||
override fun toString() = "foo" | ||
} | ||
|
||
describe("KtLogger") { | ||
describe("does not log") { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Description is incomplete. It should be |
||
it("for a string message") { | ||
with(TestLogger()) { | ||
log(NONE, "foo") | ||
logged.shouldBeNull() | ||
} | ||
} | ||
it("for a string message with an exception") { | ||
with(TestLogger()) { | ||
log(NONE, TestException("low bar"), "foo") | ||
logged.shouldBeNull() | ||
} | ||
} | ||
it("for an object") { | ||
with(TestLogger()) { | ||
log(NONE, thing) | ||
logged.shouldBeNull() | ||
} | ||
} | ||
it("for an object with an exception") { | ||
with(TestLogger()) { | ||
log(NONE, TestException("low bar"), thing) | ||
logged.shouldBeNull() | ||
} | ||
} | ||
} | ||
|
||
describe("for different logging styles") { | ||
it("logs a string message") { | ||
val message = randomString() | ||
|
@@ -87,14 +117,14 @@ class KloggerTest : DescribeSpec({ | |
logged shouldBe message | ||
} | ||
} | ||
it("logs a string message in a lambda") { | ||
it("logs a string message in a code block") { | ||
val message = randomString() | ||
with(TestLogger()) { | ||
info { message } | ||
logged shouldBe message | ||
} | ||
} | ||
it("logs a string message in a lambda with an exception") { | ||
it("logs a string message in a code block with an exception") { | ||
val message = randomString() | ||
val exception = TestException(randomString()) | ||
with(TestLogger()) { | ||
|
@@ -105,28 +135,25 @@ class KloggerTest : DescribeSpec({ | |
} | ||
it("logs an object") { | ||
with(TestLogger()) { | ||
log(DEBUG, now) | ||
logged shouldBe now | ||
log(DEBUG, thing) | ||
logged shouldBe thing | ||
} | ||
} | ||
it("logs an object with an exception") { | ||
val thing = listOf(randomString(), randomString()) | ||
val exception = TestException(randomString()) | ||
with(TestLogger()) { | ||
log(WARN, exception, thing) | ||
except shouldBe exception | ||
logged shouldBe thing | ||
} | ||
} | ||
it("logs an object in a lambda") { | ||
val thing = randomString() to now | ||
it("logs an object in a code block") { | ||
with(TestLogger()) { | ||
info { thing } | ||
logged shouldBe thing | ||
} | ||
} | ||
it("logs an object in a lambda with an exception") { | ||
val thing = setOf(now - Duration.seconds(5), now) | ||
val exception = TestException(randomString()) | ||
with(TestLogger()) { | ||
error(exception) { thing } | ||
|
@@ -162,7 +189,7 @@ class KloggerTest : DescribeSpec({ | |
} | ||
} | ||
} | ||
it("logs a templated event using `e()` function in a lambda") { | ||
it("logs a templated event using `e()` function in a code block") { | ||
val template = "Id is {Id}" | ||
val id = randomString() | ||
with(TestLogger()) { | ||
|
@@ -174,7 +201,7 @@ class KloggerTest : DescribeSpec({ | |
} | ||
} | ||
} | ||
it("logs a templated event using `e()` function in a lambda with an exception") { | ||
it("logs a templated event using `e()` function in a code block with an exception") { | ||
val id = randomString() | ||
val exception = TestException(randomString()) | ||
with(TestLogger()) { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -20,9 +20,30 @@ package io.klogging | |
|
||
import io.klogging.events.LogEvent | ||
import io.kotest.core.spec.style.DescribeSpec | ||
import io.kotest.matchers.nulls.shouldBeNull | ||
import io.kotest.matchers.shouldBe | ||
|
||
class LevelsTestLogger(private val level: Level) : Klogger { | ||
internal class LevelsTest : DescribeSpec({ | ||
describe("at all logger levels") { | ||
it("`log()` calls `logMessage()` for all levels") { | ||
Level.values().forEach { loggerLevel -> | ||
val logger = LevelsTestLogger(loggerLevel) | ||
Level.values().forEach { eventLevel -> | ||
randomString().let { message -> | ||
logger.log(eventLevel, message) | ||
|
||
if (!logger.isLevelEnabled(eventLevel)) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why not put There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Artifact of refactoring. In Java, IntelliJ warns me about this. Not sure how to enable that for Kotlin support. I'm always of the same view as you just stated. (Another example of my preference for early return, to avoid this situation.) |
||
logger.loggedMessage.shouldBeNull() | ||
else | ||
logger.loggedMessage shouldBe message | ||
} | ||
} | ||
} | ||
} | ||
} | ||
}) | ||
|
||
private class LevelsTestLogger(private val level: Level) : Klogger { | ||
override val name = "LevelsTestLogger" | ||
|
||
override fun minLevel() = level | ||
|
@@ -37,19 +58,3 @@ class LevelsTestLogger(private val level: Level) : Klogger { | |
TODO("Not yet implemented") | ||
} | ||
} | ||
|
||
class LevelsTest : DescribeSpec({ | ||
describe("at all logger levels") { | ||
it("`log()` calls `logMessage()` for all levels") { | ||
Level.values().forEach { loggerLevel -> | ||
val logger = LevelsTestLogger(loggerLevel) | ||
Level.values().forEach { eventLevel -> | ||
randomString().let { msg -> | ||
logger.log(eventLevel, msg) | ||
logger.loggedMessage shouldBe msg | ||
} | ||
} | ||
} | ||
} | ||
} | ||
}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IMO this is prettier
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe. 😄
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
NONE
is used in SLF4J for example with dynamic changes in log level. I've actually used this in a prod system that was overflowing a downstream system with logging. We changed the log level in prod via JMX toNONE
.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you keep
NONE
, I'd update the above docs in the "Rules" section to call outNONE
as special.