better context handling + timeouts for QGB E2E network #459
Conversation
| @@ -5,4 +5,4 @@ PACKAGES=$(shell go list ./...) | |||
| all: test | |||
|
|
|||
| test: | |||
| @QGB_INTEGRATION_TEST=true go test -mod=readonly -test.timeout 30m -v $(PACKAGES) | |||
| @QGB_INTEGRATION_TEST=true go test -mod=readonly -failfast -test.timeout 30m -v $(PACKAGES) | |||
There was a problem hiding this comment.
I reintroduced this as it makes it easier to just everything when ctrl+c
| @@ -86,6 +118,17 @@ func (network QGBNetwork) DeleteAll() error { | |||
| return nil | |||
| } | |||
|
|
|||
| // KillAll kills all the containers. | |||
| func (network QGBNetwork) KillAll() error { | |||
There was a problem hiding this comment.
not currently used, but might be useful in the future
| func (network QGBNetwork) WaitForNodeToStart(rpcAddr string) error { | ||
| timeoutChan := time.After(5 * time.Minute) | ||
| func (network QGBNetwork) WaitForNodeToStart(_ctx context.Context, rpcAddr string) error { | ||
| ctx, _ := context.WithTimeout(_ctx, 5*time.Minute) //nolint:govet |
There was a problem hiding this comment.
no need for a cancel function, we only need the timeout. If the happy path executes, then it will just return.
Codecov Report
@@ Coverage Diff @@
## qgb-integration #459 +/- ##
===================================================
- Coverage 11.44% 11.39% -0.06%
===================================================
Files 60 60
Lines 11518 11569 +51
===================================================
Hits 1318 1318
- Misses 10113 10164 +51
Partials 87 87
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
iirc, the orignal comment was just to use context.Context as a timeout and not actually pass times to all of the functions. This way ctx.Done() combines both the timeout and the cancels. However I could definitely see this working too! I'm curious to hear more about what you had in mind, and we can discuss it in a sync sometime tmrw or whenever you have time
Closes #438