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

finish forking: make sure most test pass w/o errors #3

Merged
merged 3 commits into from
Aug 12, 2021

Conversation

bassosimone
Copy link
Contributor

There is just a test that does not pass at the moment:

> go test ./...                                                   finish-forking
PASS
Too many goroutines running after net/http test(s).
1 instances of:
github.com/ooni/oohttp_test.interestingGoroutines(0x14015d9a1b0, 0x140156ea180, 0x140156f2017)
	/Users/sbs/src/github.com/ooni/oohttp/main_test.go:33 +0x5c
github.com/ooni/oohttp_test.goroutineLeaked(0x1400016a000)
	/Users/sbs/src/github.com/ooni/oohttp/main_test.go:72 +0x11c
github.com/ooni/oohttp_test.TestMain(0x1400016a000)
	/Users/sbs/src/github.com/ooni/oohttp/main_test.go:25 +0x4c
main.main()
	_testmain.go:1153 +0x150
FAIL	github.com/ooni/oohttp	52.947s
ok  	github.com/ooni/oohttp/cgi	(cached)
ok  	github.com/ooni/oohttp/cookiejar	(cached)
ok  	github.com/ooni/oohttp/fcgi	(cached)
ok  	github.com/ooni/oohttp/httptest	(cached)
ok  	github.com/ooni/oohttp/httptrace	(cached)
ok  	github.com/ooni/oohttp/httputil	(cached)
ok  	github.com/ooni/oohttp/internal	(cached)
FAIL

When I did another attempt where I just copied files, I did not see this leak. I suppose this is because here I am using go1.16 as base and there I was using go1.16.5. So, I am going to postpone fixing this issue to after I forward port the patches.

While I understand different prefixes, it is not fully clear to me
the `relevantCaller` part. Nonetheless, it seems safe to apply this
change in order to let the test pass (last famous words...)
Again, I do not fully understand why there are differences and maybe
one day I will, but this simplication seems safe.
@bassosimone bassosimone merged commit 84e4697 into main Aug 12, 2021
@bassosimone bassosimone deleted the finish-forking branch August 12, 2021 12:08
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.

1 participant