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

net/http/httputil: log spam in tests #25634

Closed
bradfitz opened this issue May 29, 2018 · 4 comments
Closed

net/http/httputil: log spam in tests #25634

bradfitz opened this issue May 29, 2018 · 4 comments
Labels
FrozenDueToAge help wanted NeedsFix The path to resolution is known, but the work has not been done. Testing An issue that has been verified to require only test changes, not just a test failure.
Milestone

Comments

@bradfitz
Copy link
Contributor

The net/http/httputil package has grown some log spam.

Fix.

$ go test -v 
...
=== RUN   TestClonesRequestHeaders
2018/05/29 22:05:49 http: proxy error: EOF
--- PASS: TestClonesRequestHeaders (0.00s)
...
=== RUN   TestReverseProxy_PanicBodyError
2018/05/29 22:05:49 httputil: ReverseProxy read error during body copy: unexpected EOF
--- PASS: TestReverseProxy_PanicBodyError (0.00s)
PASS
....
ok      net/http/httputil       0.034s
@bradfitz bradfitz added Testing An issue that has been verified to require only test changes, not just a test failure. help wanted NeedsFix The path to resolution is known, but the work has not been done. labels May 29, 2018
@bradfitz bradfitz added this to the Unplanned milestone May 29, 2018
@teaguecole
Copy link
Contributor

Hey Brad, new contributor here. Read through the Contribution guide and would like to work on this. I looked at the tests files and searched for "log" and will start from there and figure out how to adjust verbosity?

@bradfitz
Copy link
Contributor Author

See how other tests in net/http and net/http/httputil do it. Generally you change the default logger's output at the beginning of a test and defer a restore of it back to os.Stderr. Search for log.SetOutput. And make the test doesn't have t.Parallel in it.

@teaguecole
Copy link
Contributor

I found an example in ../serve_test.go and added the three lines of code to adjust verbosity to the 2 functions that are mentioned in the test snippet you posted. These edits were made in reverseproxy_test.go. I also searched for t.Parallel in reverseproxy_test.go and found no mentions of it. Are there any other tests that produce "spam". Could you clarify what you mean by log "spam" so when I run my test, I can look for any other instances that might not be showcased by your test snippet?

@gopherbot
Copy link

Change https://golang.org/cl/115296 mentions this issue: net/http/httputil: update log verbosity in reverseproxy_test.go

@golang golang locked and limited conversation to collaborators May 30, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge help wanted NeedsFix The path to resolution is known, but the work has not been done. Testing An issue that has been verified to require only test changes, not just a test failure.
Projects
None yet
Development

No branches or pull requests

3 participants