Add unit tests using Ginkgo #4
Conversation
Signed-off-by: Chetan Banavikalmutt <[email protected]>
Signed-off-by: Chetan Banavikalmutt <[email protected]>
There was a problem hiding this comment.
Looks great! There are some tweaks we might need to make to the original code, but we can look at those as part of a separate PR.
For now, let's get this one merged.
Some minor tweaks requested that I happen to notice while looking at the code outside the PR changes:
- Change
os.Exit()->Fail("")in test code - Couldn't hurt to add the weight value that is being updated to the log statement
slog.Info("updating route", slog.String("name", route)) Nginx Ingressreference in the old code
| return err | ||
| } | ||
|
|
||
| func (r *RpcPlugin) getRoute(ctx context.Context, namespace string, routeName string) (*routev1.Route, error) { |
There was a problem hiding this comment.
👍, Route client API is so much nicer than dynamic API.
| // Kill which should do nothing | ||
| c.Kill() |
There was a problem hiding this comment.
I see this was in the original test code, any idea what this is for? "which should do nothing" 🤔
There was a problem hiding this comment.
Looks like it got added from the rollouts-repo. I think it was included to verify if the plugin is configured in test mode. Since we are using it in a unit test it shouldn't do anything. But in production, it is expected to kill the plugin subprocess.
@ishitasequeira Please feel free to correct me.
There was a problem hiding this comment.
@chetan-rns you described it correctly. The kill basically kills the plugin process and makes sure that no pings are sent to it when the plugin is not running.
- Update logs and comments - Remove os.Exit Signed-off-by: Chetan Banavikalmutt <[email protected]>
chetan-rns
force-pushed
the
add-unit-tests
branch
from
9bc401a
to
73cfc7f
Compare
Fixes: #3