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

Epic: Only use *testing.T in the testing goroutine #2

Closed
keks opened this issue Dec 27, 2018 · 5 comments
Closed

Epic: Only use *testing.T in the testing goroutine #2

keks opened this issue Dec 27, 2018 · 5 comments

Comments

@keks
Copy link
Contributor

keks commented Dec 27, 2018

yea sounds good but let's make this a meta-todo for all the repos then.

Ahh, I see. Yeah I think we should use an chan error for that, but I see why you did it how you did. Not sure what the best way is - maybe let everything run in goroutines in the test goroutine just read the error chans that all the goroutines write to?

#1 (comment)

@cryptix
Copy link
Member

cryptix commented Jan 12, 2019

I tried to do this on the testRefactor branch but got intangled in the weird functions we return from the setup code...

https://github.com/cryptoscope/ssb/tree/testRefactor

@cryptix
Copy link
Member

cryptix commented Jan 16, 2019

pretty sure I checked muxrpc for this and change the tests.

Should also check shs, luigi and margaret, though!

@keks keks changed the title Meta: Make sure we cleanly and properly handle goroutines in tests Epic: Only use *testing.T in the testing goroutine Jan 21, 2019
@cryptix
Copy link
Member

cryptix commented Oct 8, 2019

I just found and fixed a pretty old instance of this in secretstream.

I really hope golang/go#28135 will get some traction.

cryptix added a commit to ssbc/go-secretstream that referenced this issue Oct 8, 2019
whoops... I thought i had all instances of
ssbc/go-ssb#2
@freight-stack
Copy link
Contributor

Hi @keks @cryptix,

This issue looks old. Is it active?

@cryptix
Copy link
Member

cryptix commented Jun 5, 2020

Nope, not active. All instances are resolved but has to be kept in mind.

@cryptix cryptix closed this as completed Jun 5, 2020
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

No branches or pull requests

3 participants