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 · 2 comments

Comments

@nhooyr
Copy link
Contributor

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 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
@ianlancetaylor ianlancetaylor added this to Incoming in Proposals (old) Jan 6, 2021
@jpap
Copy link
Contributor

jpap commented Oct 3, 2021

I found this issue because I discovered there was no such method on testing.T when I wanted to reach for it.

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.

It would be nice if the returned io.Writer buffers writes so that the log output is split into newlines that appear in the byte stream written to it. Any written content is going to be human-readable text (it doesn't make sense to emit non-readable bytes to t.Log), so you might as well delineate newlines properly. This would be very helpful in my current use-case.

In this case, perhaps an io.WriteCloser is more appropriate, so that any last line of text without newline terminator is emitted on Close. Alternatively, one last t.Log might need to be issued when the Test func returns.

@CantankerousBullMoose
Copy link

Regarding the effort to get the line numbers/callers right, is there a reason t.Helper() can't be sticky or inherited by children? Or have a separate implementation that is? If you had that, it would be trivial to wrap an existing Logger and just implement your own Log() function that's marked as a Helper, right? I could see other cases where that would be useful, too.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: Incoming
Development

No branches or pull requests

4 participants