-
Notifications
You must be signed in to change notification settings - Fork 34
Add a basic test for flavor validation #107
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
Conversation
c18e9c4 to
9c17150
Compare
mdbooth
left a comment
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.
Some nits, but as far as I'm concerned if the added tests pass and it passes make lint it's good to go 👌
test/apivalidations/common_test.go
Outdated
| ) | ||
|
|
||
| // Create a random string of length n | ||
| func generateResouceName(prefix string) string { |
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.
You shouldn't need this, btw, as each test runs in its own namespace. (Also typo in the name, btw)
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.
Agree
| obj.Name = generateResouceName(flavorPrefix) | ||
| obj.Namespace = namespace.Name | ||
| return obj | ||
| } |
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.
nit: missing blank line below. Surprised go fmt didn't complain about that 🤔
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
9c17150 to
ad210d0
Compare
|
go-fmt should succeed after a rebase |
Check that a minimal flavor pass Check that a flavor without RAM or VCPUs or with value 0 fails Add validations for RAM and VCPUs to have a minimum Check that a flavor with a description greater than 1024 fails Move testCredentials to common_test
ad210d0 to
73e3c1b
Compare
mdbooth
left a comment
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.
This looks great. Lets ship it!
mandre
left a comment
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.
LGTM
Check that a minimal flavor pass
Check that a flavor without RAM or VCPUs or with value 0 fails
Add validations for RAM and VCPUs to have a minimum
Check that a flavor with a description greater than 1024 fails
Move testCredentials to common_test