-
Notifications
You must be signed in to change notification settings - Fork 8
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 compute capacity unit tests #2820
Conversation
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.
Really good progress
I shouldn't be able to construct a class in invalid state, so what I suggest is we maybe introduce a constructor that enforces our invariants.
That being said, I should be expecting the following
1- valid state scenarios to construct e.g a for loop to create computecapacity with numbers (or use a library to generate these)
2- checks for invariants e.g in negative, zeros, decimals, non-ints, boundary values, overflow values, and special numbers like min/max for ram and cpu and ensuring you can't initialize less or higher than them
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.
The code looks good to me. However, when I tried to run all of the tests, including yours, nothing happened, and my laptop crashed. Please have someone else run the tests instead of me.
Does this happen every time you run these tests? |
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.
Because the validation doesn't work in all workloads. There's an issue opened for it. The tests are implemented correctly though. |
Description
Add compute capacity unit tests
Related Issues
Checklist