-
Notifications
You must be signed in to change notification settings - Fork 101
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
[KEP 0004] Add Testing Infrastructure #126
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.
Some minor changes, and had a question on future testing.
|
||
|
||
|
||
## Motivation |
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.
drop section if unnecessary
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.
Motivation is the higher level item that contains Goals and Non-Goals as members
|
||
## Motivation | ||
|
||
### Goals |
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.
Additionally, do we want to create a definition of "mergeable" for other stories to set the baseline amount of testing required to merge future features once we pull ourselves out of the hole we've given ourselves? Is "tests must be included for new functionality" enough? Is it "overall code coverage can not go down in new PRs without entire group approval (for hard to test things)"?
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 it would be good to have a definition/requirement like this. The downside of "code coverage" is now that's something we need to be able to track and compare between commits.
Depending on our CI tool, that could in theory be a warning that says "the code coverage went down" to give reviewers insight.
* First commit for testing kep * Spelling fixes * Set KEP to implementable
* First commit for testing kep * Spelling fixes * Set KEP to implementable
authors: | ||
- "@runyontr" | ||
owners: | ||
- @runyontr |
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.
s/@runyontr/"@runyontr"
needs name in double quotes
## Summary | ||
|
||
As the complexity and scope of KUDO grows, it becomes impossible to manually validate | ||
exisiting frameworks and capabilities still function as expected. As part of a robust |
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.
s/exisiting/existing
|
||
* Ensure validation of API objects is functioning correctly | ||
* Ensure controllers execute known process flows correctly | ||
* Validate Framework and FrameworkVersions in kudobuilder/frameworks adhere to the API spec defined by Kudo. Provide common |
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.
s/Kudo/KUDO
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.
s/Provide/\n* Provide
misconfigurations and validate the testing framework notifies the users/developer of failure | ||
* Reduce review time for code changes by not requiring reviewers to validate functionality of test cases | ||
* Reduce developer time for code changes by providing tools to validate functionality | ||
* Provide developers clear tooling for addition additional tests to infrastructure to validate bug fixes and new features |
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.
s/ addition/
No description provided.