SIGBlockchain / project_aurum

SIG Blockchain blockchain project in Go
https://acm.cs.uic.edu/sigblockchain
MIT License
7 stars 0 forks source link

Go routine in TestContractRequestHandler #325

Closed kastolars closed 5 years ago

kastolars commented 5 years ago

Why is there a need for a go routine here? Can we make this test pass without it?

The line in question: https://github.com/SIGBlockchain/project_aurum/blob/master/internal/handlers/handlers_test.go#L295

HarryL5004 commented 5 years ago

I think the go routine is there is because there's a contract channel in the test and in HandleContractRequest that waits for each other before channeling a contract.

kastolars commented 5 years ago

@HarryL5004 I just noticed I completely linked the wrong thing

kastolars commented 5 years ago

I think the go routine is there is because there's a contract channel in the test and in HandleContractRequest that waits for each other before channeling a contract.

I see what you mean now. Is there a way to make this test pass without the sleep call?

HarryL5004 commented 5 years ago

It seems the handler needs at least a nanosecond to process the request before it can return the correct HTTP status code and response. Perhaps it's due to all of the processes leading up to contract validation in pendingMap.Add()?

A nanosecond sleep call seems to work just fine.

kastolars commented 5 years ago

The problem with that solution is it might be a nanosecond on your machine but on travis or a VM or someone else's machine it might not be enough. It's not a huge deal, but if this is to be a learning point, we need to think about code that runs as fast as the machine allows it. I think instead of sleeping, it might be best to leverage a WaitGroup and add the goroutine to it, then wait for it to finish before proceeding:

https://golang.org/pkg/sync/#WaitGroup

What do you think?

HarryL5004 commented 5 years ago

The problem with that solution is it might be a nanosecond on your machine but on travis or a VM or someone else's machine it might not be enough. It's not a huge deal, but if this is to be a learning point, we need to think about code that runs as fast as the machine allows it. I think instead of sleeping, it might be best to leverage a WaitGroup and add the goroutine to it, then wait for it to finish before proceeding:

https://golang.org/pkg/sync/#WaitGroup

What do you think?

Right, for some reason I wasn't thinking about other environments/machines.

I was thinking about adding WaitGroup as well. I will try to do that.

HarryL5004 commented 5 years ago

Completed