Skip to content
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

proposal: testing: Add LogWriter() method to testing.T for usage with log.Logger #39519

Open
nhooyr opened this issue Jun 11, 2020 · 0 comments
Open
Labels
Milestone

Comments

@nhooyr
Copy link
Contributor

@nhooyr nhooyr commented Jun 11, 2020

Problem

Presently there is no quick way to direct output from a log.Logger into a testing.T. In my experience, this is an annoying aspect of using log.Logger as all my logs become useless in parallel tests as they are not associated with their test. I suspect this is a frustration shared by numerous other projects that use both log and testing.

Your only option is to write a io.Writer wrapping testing.T yourself. See https://godoc.org/code.soquee.net/testlog for example

Proposal

I think we should make this easier as it is a common use case.

See #22513 for some previous discussion on a similar proposal. That proposal was rejected as the proposed API would make testing depended on log.

My proposal is that we add a func (t *testing.T) LogWriter() io.Writer method to testing.T that returns an io.Writer where each Write() is a t.Log() call with the byte slice converted to a string.

Following the closure of the previous proposal, @SamWhited suggested to make testing.T an io.Writer but I think this could be confusing as it's not clear what a Write() on testing.T directly would do. Whereas having an explicit LogWriter method ensures that the code is clear about what's going on.

Line Numbers

One issue with my proposal and redirecting log.Logger to testing.T in general is that you get incorrect line numbers as there are no t.Helper() calls inside the log package. Every log indicates that it was logged from inside the log package. I'm not sure what we should do about this if we don't want log to depend on testing. I think it'd be worth the dependency for correct line numbers. Or perhaps we could hard code testing to ignore any lines within the log package.

@toothrot toothrot added this to the Proposal milestone Jun 12, 2020
@toothrot toothrot added the Proposal label Jun 12, 2020
@toothrot toothrot changed the title testing: Add LogWriter() method to testing.T for usage with log.Logger proposal: testing: Add LogWriter() method to testing.T for usage with log.Logger Jun 12, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
2 participants
You can’t perform that action at this time.