-
Notifications
You must be signed in to change notification settings - Fork 12
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
Specify a default LoggerProvider for tests. #39
Comments
Hey @jamesross3 , sorry for the late reply on this. I'm against this for a few reasons:
It is a bit unfortunate that the "default" for tests will print the warning and also print output to stdout rather than not printing at all, but I think that's the best trade-off to make at the moment. Also, although not ideal, the |
Given that logging is intended to assist debugging, I think having log output available for test failures is actually quite important. |
I think this is pretty context-dependent and kind of gets into the spectrum of unit vs. integration testing etc. Ideally, most "unit" tests should be quick to run and verify, and if they fail it should make the reason quite clear. If a failure could use more context from debugging, then I would model "turning on logging" as similar to adding There may certainly be tests that are an exception to this (a particularly flaky test that is hard to track down, an integration test where you want the full regular lifecycle including logging, etc.), but I do believe that these should generally be exceptions rather than the rule. |
Log output can be helpful in diagnosing test failures, but witchcraft-go-logging requires a LoggerProvider to be set to actually emit useful logs. One workaround to this is including either of the methods specified below to set a LoggerProvider within a test package:
This is cumbersome, since every test package would require setting this. Furthermore, the go community seems to have inconsistencies regarding test package naming - for a package
foo
, tests can be written either infoo
orfoo_test
. I believe this means that the init/TestMain method approach above could actually clobber another init method's SetDefaultLoggerProvider call (not sure what guarantees go makes about ordering init methods across packages).Proposed solution:
Add an init method to witchcraft-go-logging that checks if we're running in a test and, if this is the case, set some LoggerProvider:
@bmoylan @nmiyake thoughts? I am happy to contribute this as well as a stdlog LoggerProvider implementation.
The text was updated successfully, but these errors were encountered: