-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Migrate e2e tests for RabbitMQ to Go. #3428
Conversation
Signed-off-by: Vighnesh Shenoy <vshenoy@microsoft.com>
/run-e2e rabbitmq |
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.
Looking good, just a nit: I would move all test .go files into one package (rabbitmq
) together with rabbitmq_helper.go.
I don't think we need to create a separate package for each one, or is there any reason?
Also, do you think that you can add activation
for this scaler (a separate PR is fine)?
Thanks
The only reason I have them in a separate package is because each test has variables like The solution is to have them use names like |
This is already being done in #2831. I didn't want to duplicate the work. |
Hm, so can we put all these packages under
|
Yeah, we can. Point to note, we haven't done this for other scalers having multiple tests (such as |
Signed-off-by: Vighnesh Shenoy <vshenoy@microsoft.com>
/run-e2e rabbitmq |
You are right. We've just discussed this with @JorTurFer and have agreed that we should group all relevant scaler families. So for example all AWS under |
Signed-off-by: Vighnesh Shenoy vshenoy@microsoft.com
Provide a description of what has been changed
Checklist
Fixes #3249
Relates to #2737