-
Notifications
You must be signed in to change notification settings - Fork 32
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
MVP: Added End to End testing of Reckoner #88
Conversation
endzyme
commented
Apr 11, 2019
- Added a full end-to-end test with installing charts with reckoner on python 2 & 3
- Added a basic yaml course to test with
- Added a full end-to-end test with installing charts with reckoner on python 2 & 3 - Added a basic yaml course to test with
This is an MVP and isn't efficient or easily developed upon. I think there needs to be more thought put into the "tests" themselves. The MVP aspect is to assure it's possible to run reckoner against a kubernetes cluster with helm. The output results testing from there is the hard part. |
minimum_versions: #set minimum version requirements here | ||
helm: 0.0.0 | ||
reckoner: 0.0.0 | ||
charts: |
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.
It would be super simple to add a couple hooks here that will add testing for that path.
I know there's further expansion planned, but this would be a nice quick win.
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.
👍
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.
So I don't think reckoner returns a bad status code when hooks fail - it just skips the chart. I'll have to test to make sure the test would be valid - at least it would break even if it would cost some debugging :D
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 that relates a bit to #86. Definitely not a blocker for this PR.
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 did get an example running but it definitely isn't testing in enough isolation - MVP :D at least we've got something for a start. Still could have false positives and false negatives. We'll keep improving as we find bugs.
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 is super-duper awesome! Thanks Nick!