Skip to content

Commit e687f0a

Browse files
committed
testing strategy: add policy for presubmits
This was motivated in part by kubernetes/test-infra#33463 (comment) and is part of an effort to document best practices. The part about blocking presubmits and running them always is https://kubernetes.slack.com/archives/C2C40FMNF/p1734418617113169?thread_ts=1734417601.687079&cid=C2C40FMNF
1 parent ade28d6 commit e687f0a

File tree

1 file changed

+49
-0
lines changed

1 file changed

+49
-0
lines changed

contributors/devel/sig-testing/testing-strategy.md

+49
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,55 @@ The Kubernetes job uses [prow](https://prow.k8s.io) to implement the CI system.
2626
- **Postsubmit:** Runs after code is merged. Useful for building artifacts.
2727
- **Periodic:** Runs at scheduled intervals. Ideal for monitoring trends and catching regressions.
2828

29+
#### Presubmit jobs
30+
31+
Blocking pre-submit jobs must always run. If they didn't, it could happen that
32+
a pull request where they didn't run gets merged and introduces a regression
33+
which breaks such a job. Then when it runs in a different pull request, that
34+
pull request cannot be merged, even if that was desirable, until the regression
35+
is fixed.
36+
37+
Usually, non-blocking jobs don't run by default. The `/test` command has to be
38+
used explicitly for such informing jobs. It is possible to configure them so that they
39+
[run automatically when certain paths are modified](https://github.com/kubernetes/test-infra/blob/ee70308f09c10f7cd933c26c98acc7ebf785d436/config/jobs/kubernetes/sig-node/sig-node-presubmit.yaml#L3201-L3202).
40+
41+
Non-blocking jobs cannot detect all regressions. A test flake might succeed
42+
when tested only once during presubmit. When defining the path trigger, it's
43+
impossible to list everything that might cause a need to run tests
44+
(e.g. tool changes, updates in packages that a feature depends on). Therefore
45+
it is required to have a periodic job which runs the same tests regularly.
46+
47+
The advantage of also having a non-blocking job is that problems get discovered
48+
sooner. Without it, maintainers are forced to diagnose regressions in a
49+
periodic job and then have to ping the contributor who caused the problem. If
50+
that contributor is unresponsive, maintainers may have to fix the problem
51+
themselves. Instead, the burden is on the contributor whose pull request fails
52+
the tests. If they are unresponsive, their change doesn't get merged and
53+
there's no regression.
54+
55+
The advantage of running a non-blocking job automatically for some changes is
56+
that reviewers don't need to remember which jobs currently should be tested
57+
before merging and that the results are available immediately when a reviewer
58+
looks at a pull request for the first time, assuming that the contributor is a
59+
community member and tests run automatically.
60+
61+
> :warning: **Warning:** A non-blocking job that fails confuses other
62+
> contributors who are not familiar with the job or the failures. If it runs
63+
> too often, it wastes CI resources.
64+
65+
To avoid those negative consequences for the project, the guidelines for
66+
setting up such a job are:
67+
68+
* The job owners are responsive and react to problems with the job.
69+
* The job must have a low failure rate to avoid confusion in drive-by pull requests.
70+
* The importance of the feature must justify the extra CI resources (depends
71+
on how often it gets triggered).
72+
* The `run_if_changed` regular expression must be narrow enough that
73+
the job doesn't run for unrelated changes. A good practice is to
74+
limit it to code changes, for example with:
75+
76+
/(directory_a|directory_b)/.*\.go
77+
2978
#### SIG Release Blocking and Informing jobs
3079

3180
SIG Release maintains two sets of jobs that decide whether the release is

0 commit comments

Comments
 (0)