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

Speed up on go1.15 v2 #59

Merged
merged 4 commits into from
Sep 3, 2020
Merged

Conversation

irr123
Copy link
Contributor

@irr123 irr123 commented Sep 3, 2020

The same as #58, but with different methods to shutdown, which accepts context from outside

@irr123 irr123 mentioned this pull request Sep 3, 2020
Copy link
Contributor

@luza luza left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for your PR. This one looks good to me (I left a few comments though).

@@ -38,10 +38,16 @@ func (m *ServiceMock) StartServer() error {
return nil
}

// Not used: backward compatibility with public interface
func (m *ServiceMock) ShutdownServer() {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not a library-wide public method, so feel free to change its signature. Simply expand it with Context argument.

mocks/mocks.go Outdated
func (m *Mocks) Shutdown() {
_ = m.ShutdownContext(context.TODO())
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess here we should pre-cancel the context to defeat the slowing down issue.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I planned to switch my tests to context-related method, and do it in my code, but ok, lets leave "old" behaviour here.

_ = m.ShutdownServerContext(context.TODO())
}

func (m *ServiceMock) ShutdownServerContext(ctx context.Context) (err error) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's use unnamed return value, just like in all other methods around. Just to keep the coding style unified.

@irr123 irr123 requested a review from luza September 3, 2020 12:58
@fetinin fetinin merged commit 15b8a86 into lamoda:master Sep 3, 2020
@fetinin fetinin linked an issue Sep 3, 2020 that may be closed by this pull request
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.

Slow mocks shutdown in Go 1.15
3 participants