From 93ff284b5b4a34d1c2aaff80bc6060a78fc01771 Mon Sep 17 00:00:00 2001 From: Mark Boyd Date: Wed, 22 Nov 2023 14:35:51 -0500 Subject: [PATCH] Refactor tests (#87) * add shellcheckrc * update gitignore * update docs * refactor scripts and env vars * refactor helper script * refactor tests to avoid arbitrary timeouts and rely on element assertisons * update tests * increase test timeout * refactor CI pipeline * update pipeline * update locator for home page title --- .gitignore | 1 + .shellcheckrc | 1 + DEVELOPMENT.md | 8 ---- README.md | 18 ++++++++ ci/init-config.sh | 2 +- ci/pipeline.yml | 26 ++++++------ ci/provision-cf-access.sh | 69 ++++++++++++++----------------- ci/seed-es-data.sh | 41 ++++++++++++------ dev | 87 ++++++++++++++++++++++++++++++++++++--- e2e/__init__.py | 1 + e2e/utils.py | 38 +++++++++-------- 11 files changed, 195 insertions(+), 97 deletions(-) create mode 100644 .shellcheckrc diff --git a/.gitignore b/.gitignore index ec9f7bd..c0e5ddd 100644 --- a/.gitignore +++ b/.gitignore @@ -14,3 +14,4 @@ test-results *.webm .DS_STORE *.vars +*.log diff --git a/.shellcheckrc b/.shellcheckrc new file mode 100644 index 0000000..8226afb --- /dev/null +++ b/.shellcheckrc @@ -0,0 +1 @@ +external-sources=true diff --git a/DEVELOPMENT.md b/DEVELOPMENT.md index fa18d6b..5759dbe 100644 --- a/DEVELOPMENT.md +++ b/DEVELOPMENT.md @@ -8,14 +8,6 @@ outside of this repository. To see all of the available commands, run `./dev -h`. -## Running e2e tests locally - -1. Update your `.env` file and uncomment/set values for the e2e test variables. You can find the necessary -variables in `.env-sample`. You can get the values used in the pipeline from the credentials file stored -on S3. -1. [Make sure your local development stack is up and running](./README.md#running-locally) -1. Run `./dev e2e-local` - ## Code style Code is styled with `black`, which is configured in `pyproject.toml`. This means you can (and diff --git a/README.md b/README.md index e41dd0b..e44efa8 100644 --- a/README.md +++ b/README.md @@ -47,6 +47,24 @@ After starting up the auth-proxy using the above steps, run: ./dev e2e-local ``` +To debug the e2e tests (see ): + +```shell +PWDEBUG=1 ./dev e2e-local +``` + +To target specific e2e test(s), you can supply an `E2E_TEST_FILTER` environment variable: + +```shell +E2E_TEST_FILTER="discover_user" ./dev e2e-local +``` + +To retain video records of failed tests: + +```shell +./dev e2e-local --video retain-on-failure +``` + ### Adding client In order to run the app locally, you will need to create a UAA client application. diff --git a/ci/init-config.sh b/ci/init-config.sh index e42f4bf..926b3d4 100755 --- a/ci/init-config.sh +++ b/ci/init-config.sh @@ -20,4 +20,4 @@ ssh_pid=$! echo "Waiting for tunnel to come up ..." sleep 10 -bash "${dir}/seed-es-data.sh" +bash "${dir}/../dev seed-es-data" diff --git a/ci/pipeline.yml b/ci/pipeline.yml index d8551b6..c961028 100644 --- a/ci/pipeline.yml +++ b/ci/pipeline.yml @@ -122,14 +122,13 @@ jobs: params: <<: *dev-cf-auth-params - CF_ORG_ID_1: ((dev-test-org-1-id)) - CF_SPACE_ID_1: ((dev-test-org-1-space-1-id)) + CF_ORG_1_NAME: ((dev-test-org-1-name)) + CF_ORG_2_NAME: ((dev-test-org-2-name)) - CF_ORG_ID_2: ((dev-test-org-2-id)) - CF_SPACE_ID_2: ((dev-test-org-2-space-2-id)) - - CF_ORG_ID_1_BOTH_ORGS_SPACE: ((dev-test-org-1-both-orgs-space-id)) - CF_ORG_ID_2_BOTH_ORGS_SPACE: ((dev-test-org-2-both-orgs-space-id)) + CF_ORG_1_SPACE_1_NAME: ((dev-test-org-1-space-1-name)) + CF_ORG_2_SPACE_2_NAME: ((dev-test-org-2-space-2-name)) + + BOTH_ORGS_SPACE_NAME: ((dev-test-both-orgs-space-name)) TEST_USER_1_USERNAME: ((dev-test-user-1-username)) TEST_USER_1_PASSWORD: ((dev-test-user-1-password)) @@ -161,14 +160,13 @@ jobs: ES_USER: ((es-admin-username)) ES_PASSWORD: ((es-admin-password)) - CF_ORG_ID_1: ((dev-test-org-1-id)) - CF_SPACE_ID_1: ((dev-test-org-1-space-1-id)) + CF_ORG_1_NAME: ((dev-test-org-1-name)) + CF_ORG_2_NAME: ((dev-test-org-2-name)) - CF_ORG_ID_2: ((dev-test-org-2-id)) - CF_SPACE_ID_2: ((dev-test-org-2-space-2-id)) - - CF_ORG_ID_1_BOTH_ORGS_SPACE: ((dev-test-org-1-both-orgs-space-id)) - CF_ORG_ID_2_BOTH_ORGS_SPACE: ((dev-test-org-2-both-orgs-space-id)) + CF_ORG_1_SPACE_1_NAME: ((dev-test-org-1-space-1-name)) + CF_ORG_2_SPACE_2_NAME: ((dev-test-org-1-space-1-name)) + + BOTH_ORGS_SPACE_NAME: ((dev-test-both-orgs-space-name)) DASHBOARDS_APP_NAME: ((dev-test-opensearch-dashboards-app-name)) diff --git a/ci/provision-cf-access.sh b/ci/provision-cf-access.sh index 3e76556..29dcedd 100755 --- a/ci/provision-cf-access.sh +++ b/ci/provision-cf-access.sh @@ -6,50 +6,41 @@ if ! cf target > /dev/null; then cf t -o "${CF_ORGANIZATION}" -s "${CF_SPACE}" fi -if [[ -z "$TEST_USER_1_USERNAME" ]]; then - echo "TEST_USER_1_USERNAME environment variable is required" - exit 1 -fi - -if [[ -z "$TEST_USER_2_USERNAME" ]]; then - echo "TEST_USER_2_USERNAME environment variable is required" - exit 1 -fi - -if [[ -z "$TEST_USER_3_USERNAME" ]]; then - echo "TEST_USER_3_USERNAME environment variable is required" - exit 1 -fi - -if [[ -z "$TEST_USER_4_USERNAME" ]]; then - echo "TEST_USER_4_USERNAME environment variable is required" - exit 1 -fi - -ORG_1="kibana-test-org-1" -ORG_2="kibana-test-org-2" - -ORG_1_SPACE_1="test-kibana-space-1" -ORG_2_SPACE_2="test-kibana-space-2" -BOTH_ORGS_SPACE="both-orgs-space" - -cf create-org "$ORG_1" -cf create-org "$ORG_2" - -cf create-space "$ORG_1_SPACE_1" -o "$ORG_1" -cf create-space "$BOTH_ORGS_SPACE" -o "$ORG_1" -cf create-space "$ORG_2_SPACE_2" -o "$ORG_2" -cf create-space "$BOTH_ORGS_SPACE" -o "$ORG_2" +required_env_vars=( + TEST_USER_1_USERNAME + TEST_USER_2_USERNAME + TEST_USER_3_USERNAME + TEST_USER_4_USERNAME + CF_ORG_1_NAME + CF_ORG_2_NAME + CF_ORG_1_SPACE_1_NAME + CF_ORG_2_SPACE_2_NAME + BOTH_ORGS_SPACE_NAME +) +for var in "${required_env_vars[@]}"; do + if [ -z "${!var+x}" ]; then + echo "$var is a required environment variable" + exit 1 + fi +done + +cf create-org "$CF_ORG_1_NAME" +cf create-org "$CF_ORG_2_NAME" + +cf create-space "$CF_ORG_1_SPACE_1_NAME" -o "$CF_ORG_1_NAME" +cf create-space "$BOTH_ORGS_SPACE_NAME" -o "$CF_ORG_1_NAME" +cf create-space "$CF_ORG_2_SPACE_2_NAME" -o "$CF_ORG_2_NAME" +cf create-space "$BOTH_ORGS_SPACE_NAME" -o "$CF_ORG_2_NAME" # User 1 is a space developer in space 1, with no org-level role -cf set-space-role "$TEST_USER_1_USERNAME" "$ORG_1" "$ORG_1_SPACE_1" SpaceDeveloper +cf set-space-role "$TEST_USER_1_USERNAME" "$CF_ORG_1_NAME" "$CF_ORG_1_SPACE_1_NAME" SpaceDeveloper # User 2 is an org manager in org 2, with no space-level role -cf set-org-role "$TEST_USER_2_USERNAME" "$ORG_2" OrgManager +cf set-org-role "$TEST_USER_2_USERNAME" "$CF_ORG_2_NAME" OrgManager # User 3 is a space developer in space 1 and space 2, with no org-level roles -cf set-space-role "$TEST_USER_3_USERNAME" "$ORG_1" "$ORG_1_SPACE_1" SpaceDeveloper -cf set-space-role "$TEST_USER_3_USERNAME" "$ORG_2" "$ORG_2_SPACE_2" SpaceDeveloper +cf set-space-role "$TEST_USER_3_USERNAME" "$CF_ORG_1_NAME" "$CF_ORG_1_SPACE_1_NAME" SpaceDeveloper +cf set-space-role "$TEST_USER_3_USERNAME" "$CF_ORG_2_NAME" "$CF_ORG_2_SPACE_2_NAME" SpaceDeveloper # User 4 is a space developer in space both-orgs-space in org 1, with no org-level roles -cf set-space-role "$TEST_USER_4_USERNAME" "$ORG_1" "$BOTH_ORGS_SPACE" SpaceDeveloper +cf set-space-role "$TEST_USER_4_USERNAME" "$CF_ORG_1_NAME" "$BOTH_ORGS_SPACE_NAME" SpaceDeveloper diff --git a/ci/seed-es-data.sh b/ci/seed-es-data.sh index aa7f001..db3d2aa 100644 --- a/ci/seed-es-data.sh +++ b/ci/seed-es-data.sh @@ -10,6 +10,23 @@ trap cleanup exit cookie_jar=$(mktemp) +required_env_vars=( + ES_USER + ES_PASSWORD + CF_ORG_1_ID + CF_ORG_1_SPACE_1_ID + CF_ORG_1_BOTH_ORGS_SPACE_ID + CF_ORG_2_ID + CF_ORG_2_SPACE_2_ID + CF_ORG_2_BOTH_ORGS_SPACE_ID +) +for var in "${required_env_vars[@]}"; do + if [ -z "${!var+x}" ]; then + echo "$var is a required environment variable" + exit 1 + fi +done + # we have to create index and component templates # to work around the baked-in stream templates echo "creating component template" @@ -89,8 +106,8 @@ curl --silent --show-error -u "${ES_USER}":"${ES_PASSWORD}" -k \ # - user should not be able to see logs without a space id # We should have this set up ahead of time: -# - org 1 has space space 1 with id ${CF_SPACE_ID_1} -# - org 2 has space space 2 with id ${CF_SPACE_ID_2} +# - org 1 has space space 1 with id ${CF_ORG_1_SPACE_1_ID} +# - org 2 has space space 2 with id ${CF_ORG_2_SPACE_2_ID} # - user 1 is a space developer in space 1, with no org-level role # - user 2 is an org manager in org 2, with no space-level role # - user 3 is a space developer in space 1 and space 2, with no org-level roles @@ -128,8 +145,8 @@ curl --fail --silent --show-error -u "${ES_USER}":"${ES_PASSWORD}" -k \ -d '{ "@timestamp": "'${time}'", "@cf": { - "org_id": "'${CF_ORG_ID_1}'", - "space_id":"'${CF_SPACE_ID_1}'" + "org_id": "'${CF_ORG_1_ID}'", + "space_id":"'${CF_ORG_1_SPACE_1_ID}'" }, "message": "space_id_1" }' | jq @@ -146,8 +163,8 @@ curl --fail --silent --show-error -u "${ES_USER}":"${ES_PASSWORD}" -k \ -d '{ "@timestamp": "'${time}'", "@cf": { - "org_id": "'${CF_ORG_ID_2}'", - "space_id":"'${CF_SPACE_ID_2}'" + "org_id": "'${CF_ORG_2_ID}'", + "space_id":"'${CF_ORG_2_SPACE_2_ID}'" }, "message": "space_id_2" }' | jq @@ -174,7 +191,7 @@ curl --fail --silent --show-error -u "${ES_USER}":"${ES_PASSWORD}" -k \ https://localhost:9200/logs-app-now/_doc?refresh=true \ -d '{ "@timestamp": "'${time}'", - "@cf":{ "org_id":"'${CF_ORG_ID_1}'"}, + "@cf":{ "org_id":"'${CF_ORG_1_ID}'"}, "message": "org_id_1" }' | jq @@ -189,7 +206,7 @@ curl --fail --silent --show-error -u "${ES_USER}":"${ES_PASSWORD}" -k \ https://localhost:9200/logs-app-now/_doc?refresh=true \ -d '{ "@timestamp": "'${time}'", - "@cf": {"org_id":"'${CF_ORG_ID_2}'"}, + "@cf": {"org_id":"'${CF_ORG_2_ID}'"}, "message": "org_id_2" }' | jq @@ -205,8 +222,8 @@ curl --fail --silent --show-error -u "${ES_USER}":"${ES_PASSWORD}" -k \ -d '{ "@timestamp": "'${time}'", "@cf": { - "org_id": "'${CF_ORG_ID_1}'", - "space_id":"'${CF_ORG_ID_1_BOTH_ORGS_SPACE}'" + "org_id": "'${CF_ORG_1_ID}'", + "space_id":"'${CF_ORG_1_BOTH_ORGS_SPACE_ID}'" }, "message": "org_1_both_orgs_space" }' | jq @@ -223,8 +240,8 @@ curl --fail --silent --show-error -u "${ES_USER}":"${ES_PASSWORD}" -k \ -d '{ "@timestamp": "'${time}'", "@cf": { - "org_id": "'${CF_ORG_ID_2}'", - "space_id":"'${CF_ORG_ID_2_BOTH_ORGS_SPACE}'" + "org_id": "'${CF_ORG_2_ID}'", + "space_id":"'${CF_ORG_2_BOTH_ORGS_SPACE_ID}'" }, "message": "org_2_both_orgs_space" }' | jq diff --git a/dev b/dev index cb56c0d..29de931 100755 --- a/dev +++ b/dev @@ -135,6 +135,72 @@ cf_network() { cf add-network-policy "$PROXY_APP_NAME" "$DASHBOARDS_APP_NAME" --protocol tcp --port 5601 } +source_env_vars() { + set -o allexport + + ENVIRONMENT=${ENVIRONMENT:-} + + pushd "${dir}" + case $ENVIRONMENT in + prod|production) + source "prod.env" + ;; + + *) + source ".env" + ;; + esac + popd + + set +o allexport +} + +set_cf_default_vars() { + CF_ORG_1_NAME=${CF_ORG_1_NAME:-kibana-test-org-1} + CF_ORG_2_NAME=${CF_ORG_2_NAME:-kibana-test-org-2} + + export CF_ORG_1_NAME + export CF_ORG_2_NAME + + CF_ORG_1_SPACE_1_NAME=${CF_ORG_1_SPACE_1_NAME:-test-kibana-space-1} + CF_ORG_2_SPACE_2_NAME=${CF_ORG_2_SPACE_2_NAME:-test-kibana-space-2} + BOTH_ORGS_SPACE_NAME=${BOTH_ORGS_SPACE_NAME:-both-orgs-space} + + export CF_ORG_1_SPACE_1_NAME + export CF_ORG_2_SPACE_2_NAME + export BOTH_ORGS_SPACE_NAME +} + +set_cf_env_vars() { + CF_ORG_1_ID=$(cf org "$CF_ORG_1_NAME" --guid) + CF_ORG_2_ID=$(cf org "$CF_ORG_2_NAME" --guid) + + export CF_ORG_1_ID + export CF_ORG_2_ID + + cf target -o "$CF_ORG_1_NAME" + CF_ORG_1_SPACE_1_ID=$(cf space "$CF_ORG_1_SPACE_1_NAME" --guid) + CF_ORG_1_BOTH_ORGS_SPACE_ID=$(cf space "$BOTH_ORGS_SPACE_NAME" --guid) + + export CF_ORG_1_SPACE_1_ID + export CF_ORG_1_BOTH_ORGS_SPACE_ID + + cf target -o "$CF_ORG_2_NAME" + CF_ORG_2_SPACE_2_ID=$(cf space "$CF_ORG_2_SPACE_2_NAME" --guid) + CF_ORG_2_BOTH_ORGS_SPACE_ID=$(cf space "$BOTH_ORGS_SPACE_NAME" --guid) + + export CF_ORG_2_SPACE_2_ID + export CF_ORG_2_BOTH_ORGS_SPACE_ID +} + +seed_es_data() { + bash ./ci/seed-es-data.sh +} + +provision_cf_access() { + bash ./ci/provision-cf-access.sh +} + main() { pushd "${dir}" trap popd exit @@ -162,11 +228,12 @@ main() { ${python} -m pytest tests ;; provision-cf-access) - set -o allexport; source "${dir}/.env"; set +o allexport - ./ci/provision-cf-access.sh + source_env_vars + set_cf_default_vars + provision_cf_access ;; serve) - set -o allexport; source "${dir}/.env"; set +o allexport + source_env_vars export FLASK_APP="opensearch_dashboards_cf_auth_proxy.app:create_app()" ${python} -m flask run -p "${PORT}" @@ -197,13 +264,21 @@ main() { ${python} -m pytest e2e --browser firefox "$@" ;; e2e-local) - set -o allexport; source "${dir}/.env"; set +o allexport - bash ./ci/seed-es-data.sh - ${python} -m pytest e2e --browser firefox "$@" + E2E_TEST_FILTER=${E2E_TEST_FILTER:-*} + source_env_vars + set_cf_default_vars + set_cf_env_vars + seed_es_data + ${python} -m pytest e2e -k "$E2E_TEST_FILTER" --browser firefox "$@" ;; format) ${python} -m black . ;; + seed-es-data) + set_cf_default_vars + set_cf_env_vars + seed_es_data + ;; *) usage exit 1 diff --git a/e2e/__init__.py b/e2e/__init__.py index bd0d49e..40fdd51 100644 --- a/e2e/__init__.py +++ b/e2e/__init__.py @@ -1,3 +1,4 @@ from os import getenv AUTH_PROXY_URL = getenv("AUTH_PROXY_URL") +UAA_AUTH_URL = getenv("UAA_AUTH_URL") diff --git a/e2e/utils.py b/e2e/utils.py index 528907c..198dc72 100644 --- a/e2e/utils.py +++ b/e2e/utils.py @@ -1,7 +1,6 @@ import re -from . import AUTH_PROXY_URL - +from . import AUTH_PROXY_URL, UAA_AUTH_URL def log_in(user, page, start_at=None): page.set_default_timeout(60000) @@ -9,7 +8,7 @@ def log_in(user, page, start_at=None): if start_at is None: start_at = AUTH_PROXY_URL - # go to opensearch dashboard + # go to auth proxy page.goto(start_at) # accept the monitoring notice @@ -41,26 +40,22 @@ def log_in(user, page, start_at=None): login_button.wait_for() login_button.click() - # lots of redirects and stuff happen here, so just, like, chill, ok? - page.wait_for_load_state("networkidle") + # wait for OAuth authorize page or auth proxy page + page.wait_for_url(re.compile(f"({AUTH_PROXY_URL}|{UAA_AUTH_URL})")) + # if OAuth authorize page, then authorize the application if "/authorize?" in page.url: # first time using this app with this user authorize_button = page.get_by_text("Authorize") authorize_button.wait_for() authorize_button.click() - page.wait_for_load_state("networkidle") + # wait for redirect to auth proxy from OAuth URL page.wait_for_url(f"{AUTH_PROXY_URL}*") - # handle first-login stuff when it's here - page.wait_for_timeout(5000) - - if page.get_by_text("Start by adding your data").is_visible(): - explore_button = page.get_by_text("Explore on my own") - explore_button.wait_for() - explore_button.click() - + # wait for dashboard to finish loading + home_title = page.get_by_text("Home") + home_title.wait_for() def switch_tenants(page, tenant="Global"): """ @@ -68,6 +63,12 @@ def switch_tenants(page, tenant="Global"): """ tenant_option = page.get_by_text(re.compile(f"^{tenant}.*$")) tenant_option.wait_for() + + if page.get_by_text("Start by adding your data").is_visible(): + explore_button = page.get_by_text("Explore on my own") + explore_button.wait_for() + explore_button.click() + tenant_option.click() # submit @@ -75,10 +76,13 @@ def switch_tenants(page, tenant="Global"): submit_button.wait_for() submit_button.click() - # todo: there is a page refresh that happens after submitting the tenant option. - # we should wait on an element instead of arbitrary timeout - page.wait_for_timeout(2000) + # wait for loading screen + loading_text = page.get_by_text("Loading Opensearch Dashboards") + loading_text.wait_for() + # wait for dashboard to finish loading + home_title = page.get_by_role("heading", name="Home") + home_title.wait_for() def go_to_discover_page(page): # open the hamburger menu