forked from simendsjo/mysqln
-
Notifications
You must be signed in to change notification settings - Fork 28
#113 use std.experimental.logger #187
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
Merged
Merged
Changes from all commits
Commits
Show all changes
2 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,123 @@ | ||
module mysql.logger; | ||
|
||
/* | ||
The aliased log functions in this module map to equivelant functions in either vibe.core.log or std.experimental.logger. | ||
For this reason, only log levels common to both are used. The exception to this is logDebug which is uses trace when | ||
using std.experimental.logger, only because it's commonly used and trace/debug/verbose are all similar in use. | ||
Also, I've chosen not to support fatal errors as std.experimental.logger will create an exception if you choose to | ||
log at this level, which is an unhelpful side effect. | ||
|
||
See the following table for how they are mapped: | ||
|
||
| Our logger | vibe.core.log | LogLevel (std.experimental.logger) | | ||
| --------------- | ------------- | ---------------------------------- | | ||
| logTrace | logTrace | LogLevel.trace | | ||
| N/A | logDebugV | N/A | | ||
| logDebug | logDebug | LogLevel.trace | | ||
| N/A | logDiagnostic | N/A | | ||
| logInfo | logInfo | LogLevel.info | | ||
| logWarn | logWarn | LogLevel.warning | | ||
| logError | logError | LogLevel.error | | ||
| logCritical | logCritical | LogLevel.critical | | ||
| N/A | logFatal | LogLevel.fatal | | ||
|
||
*/ | ||
version(Have_vibe_core) { | ||
import vibe.core.log; | ||
|
||
alias logTrace = vibe.core.log.logTrace; | ||
alias logDebug = vibe.core.log.logDebug; | ||
alias logInfo = vibe.core.log.logInfo; | ||
alias logWarn = vibe.core.log.logWarn; | ||
alias logError = vibe.core.log.logError; | ||
alias logCritical = vibe.core.log.logCritical; | ||
//alias logFatal = vibe.core.log.logFatal; | ||
} else static if(__traits(compiles, (){ import std.experimental.logger; } )) { | ||
import std.experimental.logger; | ||
|
||
alias logTrace = std.experimental.logger.tracef; | ||
alias logDebug = std.experimental.logger.tracef; // no debug level in std.experimental.logger but arguably trace/debug/verbose all mean the same | ||
alias logInfo = std.experimental.logger.infof; | ||
alias logWarn = std.experimental.logger.warningf; | ||
alias logError = std.experimental.logger.errorf; | ||
alias logCritical = std.experimental.logger.criticalf; | ||
//alias logFatal = std.experimental.logger.fatalf; | ||
} else static assert(false); | ||
|
||
unittest { | ||
version(Have_vibe_core) { | ||
import std.stdio : writeln; | ||
writeln("Running the logger tests using (vibe.core.log)"); | ||
// Althouth there are no asserts here, this confirms that the alias compiles ok also the output | ||
// is shown in the terminal when running 'dub test' and the levels logged using different colours. | ||
logTrace("Test that a call to mysql.logger.logTrace maps to vibe.core.log.logTrace"); | ||
logDebug("Test that a call to mysql.logger.logDebug maps to vibe.core.log.logDebug"); | ||
logInfo("Test that a call to mysql.logger.logInfo maps to vibe.core.log.logInfo"); | ||
logWarn("Test that a call to mysql.logger.logWarn maps to vibe.core.log.logWarn"); | ||
logError("Test that a call to mysql.logger.logError maps to vibe.core.log.logError"); | ||
logCritical("Test that a call to mysql.logger.logCritical maps to vibe.core.log.logCritical"); | ||
//logFatal("Test that a call to mysql.logger.logFatal maps to vibe.core.log.logFatal"); | ||
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 can imagine that vibe logger also has customizable logging? |
||
} else static if(__traits(compiles, (){ import std.experimental.logger; } )) { | ||
// Checks that when using std.experimental.logger the log entry is correct. | ||
// This test kicks in when commenting out the 'vibe-core' dependency and running 'dub test', although | ||
// not ideal if vibe-core is availble the logging goes through vibe anyway. | ||
// Output can be seen in terminal when running 'dub test'. | ||
import std.experimental.logger : Logger, LogLevel, sharedLog; | ||
import std.stdio : writeln, writefln; | ||
import std.conv : to; | ||
|
||
writeln("Running the logger tests using (std.experimental.logger)"); | ||
|
||
class TestLogger : Logger { | ||
LogLevel logLevel; | ||
string file; | ||
string moduleName; | ||
string msg; | ||
|
||
this(LogLevel lv) @safe { | ||
super(lv); | ||
} | ||
|
||
override void writeLogMsg(ref LogEntry payload) @trusted { | ||
this.logLevel = payload.logLevel; | ||
this.file = payload.file; | ||
this.moduleName = payload.moduleName; | ||
this.msg = payload.msg; | ||
// now output it to stdio so it can be seen in terminal when testing | ||
writefln(" - testing [%s] %s(%s) : %s", payload.logLevel, payload.file, to!string(payload.line), payload.msg); | ||
} | ||
} | ||
|
||
auto logger = new TestLogger(LogLevel.all); | ||
sharedLog = logger; | ||
|
||
// check that the various log alias functions get the expected results | ||
logDebug("This is a TRACE message"); | ||
assert(logger.logLevel == LogLevel.trace, "expected 'LogLevel.trace' got: " ~ logger.logLevel); | ||
assert(logger.msg == "This is a TRACE message", "The logger should have logged 'This is a TRACE message' but instead was: " ~ logger.msg); | ||
assert(logger.file == "source/mysql/logger.d", "expected 'source/mysql/logger.d' got: " ~ logger.file); | ||
assert(logger.moduleName == "mysql.logger", "expected 'mysql.logger' got: " ~ logger.moduleName); | ||
|
||
logDebug("This is a DEBUG message (maps to trace)"); | ||
assert(logger.logLevel == LogLevel.trace, "expected 'LogLevel.trace' got: " ~ logger.logLevel); | ||
assert(logger.msg == "This is a DEBUG message (maps to trace)", "The logger should have logged 'This is a DEBUG message (maps to trace)' but instead was: " ~ logger.msg); | ||
assert(logger.file == "source/mysql/logger.d", "expected 'source/mysql/logger.d' got: " ~ logger.file); | ||
assert(logger.moduleName == "mysql.logger", "expected 'mysql.logger' got: " ~ logger.moduleName); | ||
|
||
logInfo("This is an INFO message"); | ||
assert(logger.logLevel == LogLevel.info, "expected 'LogLevel.info' got: " ~ logger.logLevel); | ||
assert(logger.msg == "This is an INFO message", "The logger should have logged 'This is an INFO message' but instead was: " ~ logger.msg); | ||
|
||
logWarn("This is a WARNING message"); | ||
assert(logger.logLevel == LogLevel.warning, "expected 'LogLevel.warning' got: " ~ logger.logLevel); | ||
assert(logger.msg == "This is a WARNING message", "The logger should have logged 'This is a WARNING message' but instead was: " ~ logger.msg); | ||
|
||
logError("This is a ERROR message"); | ||
assert(logger.logLevel == LogLevel.error, "expected 'LogLevel.error' got: " ~ logger.logLevel); | ||
assert(logger.msg == "This is a ERROR message", "The logger should have logged 'This is a ERROR message' but instead was: " ~ logger.msg); | ||
|
||
logCritical("This is a CRITICAL message"); | ||
assert(logger.logLevel == LogLevel.critical, "expected 'LogLevel.critical' got: " ~ logger.logLevel); | ||
assert(logger.msg == "This is a CRITICAL message", "The logger should have logged 'This is a CRITICAL message' but instead was: " ~ logger.msg); | ||
} | ||
} |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
For some reason this is using std.experimental.logger even when vibe is used: https://github.com/mysql-d/mysql-native/runs/4796611671?check_suite_focus=true#step:6:991
I'm trying to think of a reason, is the build for one unittest file being done without vibe?
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.
it probably is that. It's within the function for opening a socket with phobos which should only be done when vibe-d is not present. the route to opening a socket using the alternative vibe based function requires having gone through a
version(Have_vibe_core)
check on the constructor.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.
But isn't the vibe test compiled with vibe? How is it that part of the code is built with the vibe version, but part is not? Indeed, most of the logging is vibe, just a few parts aren't, and I'm not sure why.
Phobos sockets can still be used even if you are linking in vibe-core. It's a parameter to the constructor. I would still expect vibe logging if I am using vibe.
Now, technically, I'd like to see the log type be configurable. And really, as said above, it would be best if we used one logging facility and it wrapped the other. But at this point, I just want it to be consistent.
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 the solution isn't immediately obvious, then we can just merge this, and I can take a deeper look later. This is after all an issue with unittests, run in the integration subproject, which may not affect the main library.
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.
FYI, I found out why this is happening. One of the integration tests builds the example project, which doesn't use vibe. All the output from running dub is suppressed, which is why it just inexplicably shows up.
See the test here despite what the comment says, it's not using vibe. I'm unsure why we run it this way, I might switch to using github actions to run it.