-
Notifications
You must be signed in to change notification settings - Fork 520
test: update kubernetes e2e to use GINKGO_FAIL_FAST parameter value #3660
test: update kubernetes e2e to use GINKGO_FAIL_FAST parameter value #3660
Conversation
Codecov Report
@@ Coverage Diff @@
## master #3660 +/- ##
=======================================
Coverage 73.17% 73.17%
=======================================
Files 147 147
Lines 25322 25322
=======================================
Hits 18529 18529
Misses 5655 5655
Partials 1138 1138 Continue to review full report at Codecov.
|
@@ -114,7 +114,7 @@ docker run --rm \ | |||
-e CUSTOM_KUBE_PROXY_IMAGE="${CUSTOM_KUBE_PROXY_IMAGE}" \ | |||
-e IS_JENKINS="${IS_JENKINS}" \ | |||
-e SKIP_TEST="${SKIP_TESTS}" \ | |||
-e GINKGO_FAIL_FAST=true \ | |||
-e GINKGO_FAIL_FAST="${GINKGO_FAIL_FAST}" \ |
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.
The problem with using any other value than true is that the "focus" and "skip" options stop working. So if you want to use fail fase = false, beware that you won't be able to use focus/skip.
Also, if we make this change, let's make sure to set the default to true at the top of the script, e.g.:
GINKGO_FAIL_FAST="${GINKGO_FAIL_FAST:-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.
Thanks Jack, I am a little confused here. I see on the GINKGO doc it says "When using the command line flags you can specify one or both of --focus and --skip. If both are specified the constraints will be ANDed together." But I am not sure why the GINKGO_FAIL_FAST is also affecting the other two.
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.
Feel free to test and see if my recollection is mistaken!
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 definitely think you are right, because for now we are suffering the other problem you mentioned, which is GINKGO_SKIP not working (Sorry I failed to see your comments earlier). But it's just confusing me why the design of GINKGO features is like this and it's not documented.
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.
Relevant fix for the conflict between "--failFast" and "--focus/--skip" is in #3738
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.
/lgtm
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: haofan-ms, jackfrancis The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Reason for Change:
Update kubernetes e2e to use GINKGO_FAIL_FAST parameter value
Issue Fixed:
Requirements:
Notes: