KEP template: enhance suggestions for integration and e2e tests #5077
Conversation
k8s-ci-robot
added
sig/testing
pohly
force-pushed
the
kep-template-tests
branch
from
927d48e to
f8851a6
Compare
The template gives the impression that both integration and e2e tests must be written. Let's be a bit more flexible and document that feature owners can opt out of that if it makes sense. Linking to the triage tool is not sufficient. We want an empty result there, but that could also have been caused by not running the tests at all. A link to the periodic job which included the tests is more important. To ensure more consistency, the suggested bullet list gets extended to be more realistic. Otherwise feature owners have to guess how they are meant to fill in the placeholders.
pohly
force-pushed
the
kep-template-tests
branch
from
f8851a6 to
9f589f6
Compare
| @@ -308,31 +308,43 @@ Integration tests are contained in k8s.io/kubernetes/test/integration. | |||
| Integration tests allow control of the configuration parameters used to start the binaries under test. | |||
| This is different from e2e tests which do not allow configuration of parameters. | |||
| Doing this allows testing non-default options and multiple different and potentially conflicting command line options. | |||
| For more details, see https://github.com/kubernetes/community/blob/master/contributors/devel/sig-testing/testing-strategy.md | |||
There was a problem hiding this comment.
Let's add a reference to the testing strategy doc in git.k8s.io/kubernetes/community ?
Added here because it already talked about integration and e2e testing.
|
Thanks @pohly for this enhancement! |
|
/lgtm |
k8s-ci-robot
added
the
lgtm
|
/assign @jeremyrickard For approval. Updating the template won't get mirrored in existing KEPs, so it will be useful to notify at least the leads of this update for consideration in future KEP graduations. Perhaps even all Kubernetes devs? |
kikisdeliveryservice
added
the
area/enhancements
|
Since enhancements freeze is next week, let's wait until that closes to merge a change to the template - that way we have a clear start release for the change and limit confusion.. We will also highlight it again for the next release. /hold |
k8s-ci-robot
added
the
do-not-merge/hold
|
LGTM |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: pohly, soltysh The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
The template gives the impression that both integration and e2e tests must be written. Let's be a bit more flexible and document that feature owners can opt out of that if it makes sense.
Linking to the triage tool is not sufficient. We want an empty result there, but that could also have been caused by not running the tests at all. A link to the periodic job which included the tests is more important.
To ensure more consistency, the suggested bullet list gets extended to be more realistic. Otherwise feature owners have to guess how they are meant to fill in the placeholders.
This was triggered by the discussion in #5035 (comment).
/sig testing
/cc @aojea @BenTheElder