-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
Don't run the E2E tests on installer documentation #1191
Conversation
@@ -35,6 +35,7 @@ presubmits: | |||
- dockerless-build-temporary | |||
rerun_command: "/test e2e-aws" | |||
always_run: true | |||
run_if_changed: "^((?!Documentation).)*$" |
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.
Maybe just:
run_if_changed: '^(Documentation/)'
? [edit: err, never mind. For some reason I saw ^
and thought "inverse" (like [^...]
) instead of "anchor at the start of the string" :p]
@@ -35,6 +35,8 @@ presubmits: | |||
- dockerless-build-temporary | |||
rerun_command: "/test e2e-aws" | |||
always_run: true |
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.
You need to drop this to avoid:
# test that the prow config is parseable
time="2018-08-14T23:10:51Z" level=fatal msg="Error loading config." error="job pull-ci-origin-installer-e2e-aws is set to always run but also declares run_if_changed targets, which are mutually exclusive"
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 was wondering about that. The documentation for prow features an example that does exactly this.
@@ -34,7 +34,7 @@ presubmits: | |||
- master | |||
- dockerless-build-temporary | |||
rerun_command: "/test e2e-aws" | |||
always_run: true | |||
run_if_changed: "^([^D]|D(D|oD|ocD|ocuD|ocum(D|e(D|n(D|t(D|aD|atD|atiD|atioD)))))*([^Do]|o[^Dc]|oc[^Du]|ocu[^Dm]|ocum([^De]|e([^Dn]|n([^Dt]|t([^Da]|a[^Dt]|at[^Di]|ati[^Do]|atio[^Dn]))))))*(D(D|oD|ocD|ocuD|ocum(D|e(D|n(D|t(D|aD|atD|atiD|atioD)))))*(o|oc|ocu|ocum(e(n(t(a|at|ati|atio)?)?)?)?)?)?$" |
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.
nit: Is it possible to add a comment of this?
This was generated using the following tool: http://www.formauri.es/personal/pgimeno/misc/non-match-regex
/lgtm |
@crawford: Updated the
In response to this: Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
always_run: true | ||
# The abomination below is equivilent to `^((?!Documentation).)*$`. Since | ||
# Go doesn't support negative lookaheads, we are stuck with the following. | ||
run_if_changed: "^([^D]|D(D|oD|ocD|ocuD|ocum(D|e(D|n(D|t(D|aD|atD|atiD|atioD)))))*([^Do]|o[^Dc]|oc[^Du]|ocu[^Dm]|ocum([^De]|e([^Dn]|n([^Dt]|t([^Da]|a[^Dt]|at[^Di]|ati[^Do]|atio[^Dn]))))))*(D(D|oD|ocD|ocuD|ocum(D|e(D|n(D|t(D|aD|atD|atiD|atioD)))))*(o|oc|ocu|ocum(e(n(t(a|at|ati|atio)?)?)?)?)?)?$" |
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.
How often does your repository change top-level directories? Seems like you could have called those out in a much more simple manner.
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.
How often does your repository change top-level directories?
We have a top-level change in flight now with openshift/installer#120 ;). But we'll need CI changes in other places to adapt to that anyway, so I wouldn't mind switching to a whitelist approach here too. I'm also fine with the blacklist approach.
* Support lease expiry option This was added to metal3-dev-env in [0]. Expose the setting for dev-scripts too. 0: metal3-io/metal3-dev-env#599 * Bump to current metal3-dev-env
No description provided.