-
Notifications
You must be signed in to change notification settings - Fork 20
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
Add onidle chain element into cmd-nse-icmp-responder and cover it by docker tests #189
Add onidle chain element into cmd-nse-icmp-responder and cover it by docker tests #189
Conversation
suite_healthcheck_test.go
Outdated
f.Require().Eventually(func() bool { | ||
healthResponse, err = healthClient.Check(ctx, hcRequest) | ||
return err != nil | ||
}, f.idleTimeout+time.Second, time.Millisecond*50) | ||
f.Require().Nil(healthResponse) | ||
f.Require().Error(err) | ||
f.Require().Equal(codes.Unavailable, grpcutils.UnwrapCode(err)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it is not good to shut down the endpoint in the suite because it can break the next tests in the suite.
Can we create a separate test for onidle case?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I tried to make a separate test, and it had hanging-forever issues, probably because current suite setup code doesn't account for the possibility that it can be run several times.
Currently suite only has 1 test. I tried to think of which tests can be added to it, and didn't come up with anything, so I figured out I can just make this test terminating.
I'll try to modify the setup code we are expecting the we will add more tests to this suite in the future.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
Signed-off-by: Danil Uzlov <DanilUzlov@yandex.ru>
Signed-off-by: Danil Uzlov <DanilUzlov@yandex.ru>
Signed-off-by: Danil Uzlov <DanilUzlov@yandex.ru>
Signed-off-by: Danil Uzlov <DanilUzlov@yandex.ru>
Signed-off-by: Danil Uzlov <DanilUzlov@yandex.ru>
Signed-off-by: Danil Uzlov <DanilUzlov@yandex.ru>
Signed-off-by: Danil Uzlov <DanilUzlov@yandex.ru>
Signed-off-by: Danil Uzlov <DanilUzlov@yandex.ru>
Signed-off-by: Danil Uzlov <DanilUzlov@yandex.ru>
Signed-off-by: Danil Uzlov <DanilUzlov@yandex.ru>
Signed-off-by: Danil Uzlov <DanilUzlov@yandex.ru>
Signed-off-by: Danil Uzlov <DanilUzlov@yandex.ru>
LGTM |
fixes #188