Speedup mocks start/stop #58
Conversation
|
You've patched 3 different pieces of code:
How much each of them give speed gain? Are all three really necessary? |
runner/runner.go
Outdated
| @@ -148,6 +148,7 @@ func (r *Runner) executeTest(v models.TestInterface, client *http.Client) (*mode | |||
|
|
|||
| body, err := ioutil.ReadAll(resp.Body) | |||
| if err != nil { | |||
| _ = resp.Body.Close() | |||
There was a problem hiding this comment.
Can we have only one _ = resp.Body.Close() call here? If you sure that body needs to be closed despite the error, let's call Close() before the error check.
There was a problem hiding this comment.
You're right, we can.
And yep, needs to close body despite ReadAll err, it may be err about too large body or something else
I checked how many impact it changes separately do and you right, concurrent start/stop is needless |
| @@ -39,7 +39,9 @@ func (m *ServiceMock) StartServer() error { | |||
| } | |||
|
|
|||
| func (m *ServiceMock) ShutdownServer() { | |||
| m.server.Shutdown(context.TODO()) | |||
| ctx, cancel := context.WithCancel(context.TODO()) | |||
There was a problem hiding this comment.
This also looks a bit hacky. At least here should be a comment describing why we create a context and then immediately cancel it.
Did you manage to find out why we still have active connections to mock server? Was it caused by not closed body in runner.go?
There was a problem hiding this comment.
Was it caused by not closed body in runner.go
Nope, it's rare edge case, I found it when investigating where connection may leak, but dont find
This also looks a bit hacky
There are exists another way, we can add new method:
func (m *ServiceMock) ShutdownServerContext(ctx context.Context) error
and than reuse it in ShutdownServer, like in others libraries, which migrated to context
There was a problem hiding this comment.
I added comment here and made another PR with ShutdownServerContext method (#59), both of this solutions ok for me, choose what you like best
|
Closing in favour of #59 |
Tests speed up from 10min+ to 30sec on proj with 30+ mocks with go1.15