Skip to content
This repository has been archived by the owner on Oct 24, 2023. It is now read-only.

test: update kubernetes e2e to use GINKGO_FAIL_FAST parameter value #3660

Merged
merged 4 commits into from Aug 24, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
11 changes: 6 additions & 5 deletions test/e2e/cluster.sh
Expand Up @@ -10,6 +10,7 @@ MASTER_VM_UPGRADE_SKU="${MASTER_VM_UPGRADE_SKU:-Standard_D4_v3}"
AZURE_ENV="${AZURE_ENV:-AzurePublicCloud}"
IDENTITY_SYSTEM="${IDENTITY_SYSTEM:-azure_ad}"
ARC_LOCATION="eastus"
GINKGO_FAIL_FAST="${GINKGO_FAIL_FAST:-false}"
mkdir -p _output || exit 1

# Assumes we're running from the git root of aks-engine
Expand Down Expand Up @@ -115,7 +116,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}" \
Copy link
Member

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}"

Copy link
Contributor Author

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.

Copy link
Member

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!

Copy link
Contributor Author

@haofan-ms haofan-ms Aug 7, 2020

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.

Copy link
Contributor Author

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

-e GINKGO_FOCUS="${GINKGO_FOCUS}" \
-e GINKGO_SKIP="${GINKGO_SKIP}" \
-e API_PROFILE="${API_PROFILE}" \
Expand Down Expand Up @@ -243,7 +244,7 @@ if [ -n "$ADD_NODE_POOL_INPUT" ]; then
-e REGIONS=$REGION \
-e IS_JENKINS=${IS_JENKINS} \
-e SKIP_LOGS_COLLECTION=true \
-e GINKGO_FAIL_FAST=true \
-e GINKGO_FAIL_FAST="${GINKGO_FAIL_FAST}" \
-e GINKGO_SKIP="${SKIP_AFTER_SCALE_DOWN}" \
-e GINKGO_FOCUS="${GINKGO_FOCUS}" \
-e SKIP_TEST=${SKIP_TESTS_AFTER_ADD_POOL} \
Expand Down Expand Up @@ -317,7 +318,7 @@ if [ "${SCALE_CLUSTER}" = "true" ]; then
-e REGIONS=$REGION \
-e IS_JENKINS=${IS_JENKINS} \
-e SKIP_LOGS_COLLECTION=true \
-e GINKGO_FAIL_FAST=true \
-e GINKGO_FAIL_FAST="${GINKGO_FAIL_FAST}" \
-e GINKGO_SKIP="${SKIP_AFTER_SCALE_DOWN}" \
-e GINKGO_FOCUS="${GINKGO_FOCUS}" \
-e SKIP_TEST=${SKIP_TESTS_AFTER_SCALE_DOWN} \
Expand Down Expand Up @@ -403,7 +404,7 @@ if [ "${UPGRADE_CLUSTER}" = "true" ]; then
-e REGIONS=$REGION \
-e IS_JENKINS=${IS_JENKINS} \
-e SKIP_LOGS_COLLECTION=${SKIP_LOGS_COLLECTION} \
-e GINKGO_FAIL_FAST=true \
-e GINKGO_FAIL_FAST="${GINKGO_FAIL_FAST}" \
-e GINKGO_SKIP="${SKIP_AFTER_UPGRADE}" \
-e GINKGO_FOCUS="${GINKGO_FOCUS}" \
-e SKIP_TEST=${SKIP_TESTS_AFTER_UPGRADE} \
Expand Down Expand Up @@ -478,7 +479,7 @@ if [ "${SCALE_CLUSTER}" = "true" ]; then
-e REGIONS=$REGION \
-e IS_JENKINS=${IS_JENKINS} \
-e SKIP_LOGS_COLLECTION=${SKIP_LOGS_COLLECTION} \
-e GINKGO_FAIL_FAST=true \
-e GINKGO_FAIL_FAST="${GINKGO_FAIL_FAST}" \
-e GINKGO_SKIP="${SKIP_AFTER_SCALE_UP}" \
-e GINKGO_FOCUS="${GINKGO_FOCUS}" \
-e SKIP_TEST=${SKIP_TESTS_AFTER_SCALE_UP} \
Expand Down
2 changes: 1 addition & 1 deletion test/e2e/config/config.go
Expand Up @@ -47,7 +47,7 @@ type Config struct {
UseDeployCommand bool `envconfig:"USE_DEPLOY_COMMAND" default:"false"`
GinkgoFocus string `envconfig:"GINKGO_FOCUS" default:""`
GinkgoSkip string `envconfig:"GINKGO_SKIP" default:""`
GinkgoFailFast bool `envconfig:"GINKGO_FAIL_FAST" default:"true"`
GinkgoFailFast bool `envconfig:"GINKGO_FAIL_FAST" default:"false"`
DebugAfterSuite bool `envconfig:"DEBUG_AFTERSUITE" default:"false"`
BlockSSHPort bool `envconfig:"BLOCK_SSH" default:"false"`
AddNodePoolInput string `envconfig:"ADD_NODE_POOL_INPUT" default:""`
Expand Down