Skip to content

Conversation

@stevenroose
Copy link

When unit testing a subpackage, it's often useful to see log messages.

For this, it would be possible to add something like this in a _test.go file:

func init() {
    log = btclog.TestLogger
}

@jrick
Copy link
Member

jrick commented Dec 5, 2017

I would prefer that log messages are written to the testing.T logger, so they only appear when the test fails or the verbose flag is used. I don't think there's a great way to handle that from the btclog package though, since it can't import testing to implement the required interface.

var Disabled Logger = &slog{lvl: LevelOff, b: NewBackend(ioutil.Discard)}

func init() {
Disabled = &slog{lvl: LevelOff, b: NewBackend(ioutil.Discard)}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fyi, this was done just to remove some ugly from the godoc page which would otherwise show the variable initialized to an unexported type, which isn't super helpful

@stevenroose
Copy link
Author

stevenroose commented Dec 5, 2017 via email

@jrick
Copy link
Member

jrick commented Dec 5, 2017

also, this should already be possible from a test file with something like:

func init() {
    SetLogger(btclog.NewBackend(os.Stdout).Logger("TEST"))
}

which is the same number of lines as what it would require with this patch.

After considering what I really want though, which is to write to the testing.T logger, I don't believe this is currently possible, and definitely not if using parallel tests, because the testing.T is different for each test function while the log variable is a package global and can't be modified by several tests concurrently. We would need some refactoring to allow the logger to be a dependency of any created object, which is something I've advocated for before with little to no success :(

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants