From d20f498e7fdd3c1cf10c431a144b5beeb6aff03f Mon Sep 17 00:00:00 2001 From: Christopher Willis-Ford <7019101+cwillisf@users.noreply.github.com> Date: Wed, 13 Sep 2023 16:56:50 -0700 Subject: [PATCH 01/33] ci: first draft for GHA migration --- .circleci/config.yml | 42 ++-- .github/workflows/build-and-test.yml | 179 ++++++++++++++++++ .../{main.yml => prepare-release.yml} | 0 .nvmrc | 1 + 4 files changed, 206 insertions(+), 16 deletions(-) create mode 100644 .github/workflows/build-and-test.yml rename .github/workflows/{main.yml => prepare-release.yml} (100%) create mode 100644 .nvmrc diff --git a/.circleci/config.yml b/.circleci/config.yml index c51aa41b6..c634013a4 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -1,5 +1,9 @@ version: 2.1 +# WARNING: most of these jobs are disabled in preparation for GHA migration. +# They're retained here for reference during the migration work. +# This file should be deleted once the migration is complete. + aliases: - &defaults docker: @@ -127,10 +131,11 @@ workflows: filters: branches: only: - - develop - - beta - - /^hotfix\/.*/ - - /^release\/.*/ + - "" # disable +# - develop +# - beta +# - /^hotfix\/.*/ +# - /^release\/.*/ - integration-tests: requires: - build-and-deploy-staging @@ -141,10 +146,11 @@ workflows: filters: branches: only: - - develop - - beta - - /^hotfix\/.*/ - - /^release\/.*/ + - "" # disable +# - develop +# - beta +# - /^hotfix\/.*/ +# - /^release\/.*/ - build-and-deploy-production: context: - scratch-www-all @@ -153,7 +159,8 @@ workflows: filters: branches: only: - - master + - "" # disable +# - master - integration-tests: requires: - build-and-deploy-production @@ -164,7 +171,8 @@ workflows: filters: branches: only: - - master + - "" # disable +# - master Update-translations: triggers: - schedule: # every evening at 7pm EST (8pm EDT, Midnight UTC) @@ -189,9 +197,11 @@ workflows: - dockerhub-credentials filters: branches: - ignore: - - develop - - master - - beta - - /^hotfix\/.*/ - - /^release\/.*/ + only: + - "" # disable +# ignore: +# - develop +# - master +# - beta +# - /^hotfix\/.*/ +# - /^release\/.*/ diff --git a/.github/workflows/build-and-test.yml b/.github/workflows/build-and-test.yml new file mode 100644 index 000000000..1bf0a4c08 --- /dev/null +++ b/.github/workflows/build-and-test.yml @@ -0,0 +1,179 @@ +name: Build and Test and maybe Deploy + +on: + workflow_dispatch: # Allows you to run this workflow manually from the Actions tab + pull_request: # Runs whenever a pull request is created or updated + push: # Runs whenever a commit is pushed to the repository + +env: + CXX: g++-4.8 + FASTLY_ACTIVATE_CHANGES: true + FASTLY_PURGE_ALL: true + NODE_ENV: production + SKIP_CLEANUP: true + +jobs: + determine-environment: + runs-on: ubuntu-latest + outputs: + # map the output from the step with ID="set-scratch-environment" + # to the job output named "scratch_environment" + scratch_environment: ${{ steps.set-scratch-environment.outputs.scratch_environment }} + steps: + - id: set-scratch-environment + shell: bash + run: | + case "${{ github.ref }}" in + "refs/heads/master") + echo "scratch_environment=production" | tee -a $GITHUB_OUTPUT + ;; + "refs/heads/gha" | "refs/heads/develop" | "refs/heads/beta" | refs/heads/hotfix/* | refs/heads/release/*) + echo "scratch_environment=staging" | tee -a $GITHUB_OUTPUT + ;; + *) + echo "Leaving scratch_environment unset" + ;; + esac + build-and-test-and-maybe-deploy: + needs: determine-environment + runs-on: ubuntu-latest + environment: ${{ needs.determine-environment.outputs.scratch_environment }} + env: + SCRATCH_SHOULD_DEPLOY: ${{ vars.SCRATCH_ENV != '' }} + # env: # TODO: expose secrets only to those steps that need them + # # "all" environment + # AWS_ACCESS_KEY_ID: ${{ secrets.AWS_ACCESS_KEY_ID }} + # AWS_SECRET_ACCESS_KEY: ${{ secrets.AWS_SECRET_ACCESS_KEY }} + # COVERALLS_REPO_TOKEN: ${{ secrets.COVERALLS_REPO_TOKEN }} + # GTM_ID: ${{ secrets.GTM_ID }} + # S3_LOCAL_DIR: ${{ secrets.S3_LOCAL_DIR }} + # SAUCE_ACCESS_KEY: ${{ secrets.SAUCE_ACCESS_KEY }} + # SAUCE_USERNAME: ${{ secrets.SAUCE_USERNAME }} + # SENTRY_ORG: ${{ secrets.SENTRY_ORG }} + # SLACK_WEBHOOK_CIRCLECI_NOTIFICATIONS: ${{ secrets.SLACK_WEBHOOK_CIRCLECI_NOTIFICATIONS }} # TODO: rename or replace + # SLACK_WEBHOOK_ENGINEERING: ${{ secrets.SLACK_WEBHOOK_ENGINEERING }} + # SLACK_WEBHOOK_MODS: ${{ secrets.SLACK_WEBHOOK_MODS }} + # SMOKE_PASSWORD: ${{ secrets.SMOKE_PASSWORD }} + # SMOKE_USERNAME: ${{ secrets.SMOKE_USERNAME }} + + # # staging / production environments + # API_HOST: ${{ secrets.API_HOST }} + # ASSET_HOST: ${{ secrets.ASSET_HOST }} + # BACKPACK_HOST: ${{ secrets.BACKPACK_HOST }} + # CLOUDDATA_HOST: ${{ secrets.CLOUDDATA_HOST }} + # COMMENT_PROJECT_ID: ${{ secrets.COMMENT_PROJECT_ID }} + # COMMENT_STUDIO_ID: ${{ secrets.COMMENT_STUDIO_ID }} + # FASTLY_API_KEY: ${{ secrets.FASTLY_API_KEY }} + # FASTLY_SERVICE_ID: ${{ secrets.FASTLY_SERVICE_ID }} + # GA_TRACKER: ${{ secrets.GA_TRACKER }} + # GTM_ENV_AUTH: ${{ secrets.GTM_ENV_AUTH }} + # OWNED_SHARED_PROJECT_ID: ${{ secrets.OWNED_SHARED_PROJECT_ID }} + # OWNED_UNSHARED_PROJECT_ID: ${{ secrets.OWNED_UNSHARED_PROJECT_ID }} + # OWNED_UNSHARED_SCRATCH2_PROJECT_ID: ${{ secrets.OWNED_UNSHARED_SCRATCH2_PROJECT_ID }} + # PROJECT_HOST: ${{ secrets.PROJECT_HOST }} + # RATE_LIMIT_CHECK: ${{ secrets.RATE_LIMIT_CHECK }} + # RECAPTCHA_SITE_KEY: ${{ secrets.RECAPTCHA_SITE_KEY }} + # ROOT_URL: ${{ secrets.ROOT_URL }} + # S3_BUCKET_NAME: ${{ secrets.S3_BUCKET_NAME }} + # SCRATCH_ENV: ${{ vars.SCRATCH_ENV }} + # SENTRY_DSN: ${{ secrets.SENTRY_DSN }} + # SENTRY_PROJECT: ${{ secrets.SENTRY_PROJECT }} + # STATIC_HOST: ${{ secrets.STATIC_HOST }} + # TEST_PROJECT_ID: ${{ secrets.TEST_PROJECT_ID }} + # TEST_STUDIO_ID: ${{ secrets.TEST_STUDIO_ID }} + # UNOWNED_SHARED_PROJECT_ID: ${{ secrets.UNOWNED_SHARED_PROJECT_ID }} + # UNOWNED_UNSHARED_PROJECT_ID: ${{ secrets.UNOWNED_UNSHARED_PROJECT_ID }} + # UNOWNED_SHARED_SCRATCH2_PROJECT_ID: ${{ secrets.UNOWNED_SHARED_SCRATCH2_PROJECT_ID }} + steps: + - uses: actions/checkout@v3 + - uses: actions/setup-node@v3 + with: + cache: 'npm' + node-version-file: '.nvmrc' + - name: info + run: | + echo "GitHub environment: ${{ needs.determine-environment.outputs.scratch_environment }}" + echo "Scratch environment: ${{ env.SCRATCH_ENV }}" + echo "Node version: $(node --version)" + echo "NPM version: $(npm --version)" + - name: setup + run: | + npm --production=false ci + mkdir -p ./test/results + - name: lint + run: npm run test:lint:ci + - name: build + run: WWW_VERSION=${GITHUB_SHA:0:5} npm run build + env: + # webpack.config.js uses these with `DefinePlugin` + API_HOST: ${{ secrets.API_HOST }} + RECAPTCHA_SITE_KEY: ${{ secrets.RECAPTCHA_SITE_KEY }} + ASSET_HOST: ${{ secrets.ASSET_HOST }} + BACKPACK_HOST: ${{ secrets.BACKPACK_HOST }} + CLOUDDATA_HOST: ${{ secrets.CLOUDDATA_HOST }} + PROJECT_HOST: ${{ secrets.PROJECT_HOST }} + STATIC_HOST: ${{ secrets.STATIC_HOST }} + SCRATCH_ENV: ${{ vars.SCRATCH_ENV }} + + # used by src/template-config.js + GTM_ID: ${{ secrets.GTM_ID }} + GTM_ENV_AUTH: ${{ secrets.GTM_ENV_AUTH }} + - name: unit tests + run: | + JEST_JUNIT_OUTPUT_NAME=unit-jest-results.xml npm run test:unit:jest:unit -- --reporters=jest-junit + JEST_JUNIT_OUTPUT_NAME=localization-jest-results.xml npm run test:unit:jest:localization -- --reporters=jest-junit + npm run test:unit:tap -- --output-file ./test/results/unit-raw.tap + npm run test:unit:convertReportToXunit + - name: setup Python + if: ${{ env.SCRATCH_SHOULD_DEPLOY == 'true' }} + run: | + curl https://bootstrap.pypa.io/pip/3.5/get-pip.py -o get-pip.py + python3 get-pip.py pip==21.0.1 + pip install s3cmd==2.1.0 + - name: deploy + if: ${{ env.SCRATCH_SHOULD_DEPLOY == 'true' }} + run: echo npm run deploy + env: + S3_LOCAL_DIR: build + S3_BUCKET_NAME: ${{ secrets.S3_BUCKET_NAME }} + AWS_ACCESS_KEY_ID: ${{ secrets.AWS_ACCESS_KEY_ID }} + AWS_SECRET_ACCESS_KEY: ${{ secrets.AWS_SECRET_ACCESS_KEY }} + FASTLY_API_KEY: ${{ secrets.FASTLY_API_KEY }} + FASTLY_SERVICE_ID: ${{ secrets.FASTLY_SERVICE_ID }} + SLACK_WEBHOOK_CIRCLECI_NOTIFICATIONS: ${{ secrets.SLACK_WEBHOOK_CIRCLECI_NOTIFICATIONS }} # TODO: rename or replace + SLACK_WEBHOOK_ENGINEERING: ${{ secrets.SLACK_WEBHOOK_ENGINEERING }} + SLACK_WEBHOOK_MODS: ${{ secrets.SLACK_WEBHOOK_MODS }} + - name: integration tests + if: ${{ env.SCRATCH_SHOULD_DEPLOY == 'true' }} + run: JEST_JUNIT_OUTPUT_NAME=integration-jest-results.xml npm run test:integration:remote -- --reporters=jest-junit + env: + ROOT_URL: ${{ secrets.ROOT_URL }} + + # test/integration-legacy/selenium-helpers.js + CI: "true" + CIRCLECI: "true" # TODO + CIRCLE_BUILD_NUM: ${{ github.run_id }} # TODO + SAUCE_ACCESS_KEY: ${{ secrets.SAUCE_ACCESS_KEY }} + SAUCE_USERNAME: ${{ secrets.SAUCE_USERNAME }} + + # test/integration/* + SMOKE_USERNAME: ${{ secrets.SMOKE_USERNAME }} + SMOKE_PASSWORD: ${{ secrets.SMOKE_PASSWORD }} + COMMENT_PROJECT_ID: ${{ secrets.COMMENT_PROJECT_ID }} + COMMENT_STUDIO_ID: ${{ secrets.COMMENT_STUDIO_ID }} + UNOWNED_SHARED_PROJECT_ID: ${{ secrets.UNOWNED_SHARED_PROJECT_ID }} + OWNED_SHARED_PROJECT_ID: ${{ secrets.OWNED_SHARED_PROJECT_ID }} + OWNED_UNSHARED_PROJECT_ID: ${{ secrets.OWNED_UNSHARED_PROJECT_ID }} + UNOWNED_UNSHARED_PROJECT_ID: ${{ secrets.UNOWNED_UNSHARED_PROJECT_ID }} + UNOWNED_SHARED_SCRATCH2_PROJECT_ID: ${{ secrets.UNOWNED_SHARED_SCRATCH2_PROJECT_ID }} + OWNED_UNSHARED_SCRATCH2_PROJECT_ID: ${{ secrets.OWNED_UNSHARED_SCRATCH2_PROJECT_ID }} + TEST_STUDIO_ID: ${{ secrets.TEST_STUDIO_ID }} + RATE_LIMIT_CHECK: ${{ secrets.RATE_LIMIT_CHECK }} + - name: compress artifact + if: ${{ env.SCRATCH_SHOULD_DEPLOY == 'true' }} + run: tar -czvf build.tgz build + - name: upload artifact + if: ${{ env.SCRATCH_SHOULD_DEPLOY == 'true' }} + uses: actions/upload-artifact@v3 + with: + path: build.tgz diff --git a/.github/workflows/main.yml b/.github/workflows/prepare-release.yml similarity index 100% rename from .github/workflows/main.yml rename to .github/workflows/prepare-release.yml diff --git a/.nvmrc b/.nvmrc new file mode 100644 index 000000000..6f7f377bf --- /dev/null +++ b/.nvmrc @@ -0,0 +1 @@ +v16 From 75a430bb9215abd5e8378caa9e8556036af9ceca Mon Sep 17 00:00:00 2001 From: Christopher Willis-Ford <7019101+cwillisf@users.noreply.github.com> Date: Mon, 18 Sep 2023 11:14:15 -0700 Subject: [PATCH 02/33] ci: check server health before starting integration tests --- .github/workflows/build-and-test.yml | 7 +++- package.json | 2 +- test/health/server-health.test.js | 51 ++++++++++++++++++++++++++++ 3 files changed, 58 insertions(+), 2 deletions(-) create mode 100644 test/health/server-health.test.js diff --git a/.github/workflows/build-and-test.yml b/.github/workflows/build-and-test.yml index 1bf0a4c08..432648dc8 100644 --- a/.github/workflows/build-and-test.yml +++ b/.github/workflows/build-and-test.yml @@ -145,7 +145,11 @@ jobs: SLACK_WEBHOOK_MODS: ${{ secrets.SLACK_WEBHOOK_MODS }} - name: integration tests if: ${{ env.SCRATCH_SHOULD_DEPLOY == 'true' }} - run: JEST_JUNIT_OUTPUT_NAME=integration-jest-results.xml npm run test:integration:remote -- --reporters=jest-junit + run: | + # if the health test fails, there's no point in trying to run the integration tests + npm run test:health + # health test succeeded, so proceed with integration tests + JEST_JUNIT_OUTPUT_NAME=integration-jest-results.xml npm run test:integration -- --reporters=jest-junit env: ROOT_URL: ${{ secrets.ROOT_URL }} @@ -155,6 +159,7 @@ jobs: CIRCLE_BUILD_NUM: ${{ github.run_id }} # TODO SAUCE_ACCESS_KEY: ${{ secrets.SAUCE_ACCESS_KEY }} SAUCE_USERNAME: ${{ secrets.SAUCE_USERNAME }} + SMOKE_REMOTE: "true" # use Sauce Labs # test/integration/* SMOKE_USERNAME: ${{ secrets.SMOKE_USERNAME }} diff --git a/package.json b/package.json index ac4a9a47f..1aa6ec928 100644 --- a/package.json +++ b/package.json @@ -7,8 +7,8 @@ "test": "npm run test:lint && npm run build && npm run test:unit", "test:lint": "eslint . --ext .js,.jsx,.json", "test:lint:ci": "eslint . --ext .js,.jsx,.json --format junit -o ./test/results/lint-results.xml", + "test:health": "jest ./test/health/*.test.js", "test:integration": "jest ./test/integration/*.test.js --reporters=default --maxWorkers=5", - "test:integration:remote": "SMOKE_REMOTE=true jest ./test/integration/*.test.js --reporters=default --maxWorkers=5", "test:unit": "npm run test:unit:jest && npm run test:unit:tap", "test:unit:jest": "npm run test:unit:jest:unit && npm run test:unit:jest:localization", "test:unit:jest:unit": "jest ./test/unit/ --reporters=default", diff --git a/test/health/server-health.test.js b/test/health/server-health.test.js new file mode 100644 index 000000000..19233b01d --- /dev/null +++ b/test/health/server-health.test.js @@ -0,0 +1,51 @@ +/* eslint-disable no-console */ + +// this basic server health check is meant to be run before integration tests +// it should be run with the same environment variables as the integration tests +// and operate in the same way as the integration tests + +const SeleniumHelper = require('../integration/selenium-helpers.js'); + +const rootUrl = process.env.ROOT_URL || (() => { + const ROOT_URL_DEFAULT = 'https://scratch.ly'; + console.warn(`ROOT_URL not set, defaulting to ${ROOT_URL_DEFAULT}`); + return ROOT_URL_DEFAULT; +})(); + +jest.setTimeout(60000); + +describe('www server health check', () => { + /** @type {import('selenium-webdriver').ThenableWebDriver} */ + let driver; + + /** @type {SeleniumHelper} */ + let seleniumHelper; + + beforeAll(() => { + seleniumHelper = new SeleniumHelper(); + driver = seleniumHelper.buildDriver('www server health check'); + }); + + afterAll(() => driver.quit()); + + test('server is healthy', async () => { + const healthUrl = new URL('health/', rootUrl); + await driver.get(healthUrl.toString()); + + // Note: driver.getPageSource() will return the pretty HTML form of the JSON + const pageText = await driver.executeScript('return document.body.innerText'); + + let healthObject; + let serverReturnedValidJson = false; + + try { + healthObject = JSON.parse(pageText); + serverReturnedValidJson = true; + } catch (_e) { + // ignore + } + + expect(serverReturnedValidJson).toBe(true); + expect(healthObject).toHaveProperty('healthy', true); + }); +}); From b4428c4fd887bfb86e8996f7a356ecdb735d2439 Mon Sep 17 00:00:00 2001 From: Christopher Willis-Ford <7019101+cwillisf@users.noreply.github.com> Date: Tue, 19 Sep 2023 09:57:01 -0700 Subject: [PATCH 03/33] ci: specify concurrency group and cancel-in-progress --- .github/workflows/build-and-test.yml | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/.github/workflows/build-and-test.yml b/.github/workflows/build-and-test.yml index 432648dc8..007538593 100644 --- a/.github/workflows/build-and-test.yml +++ b/.github/workflows/build-and-test.yml @@ -5,6 +5,10 @@ on: pull_request: # Runs whenever a pull request is created or updated push: # Runs whenever a commit is pushed to the repository +concurrency: + group: '${{ github.workflow }} @ ${{ github.event.pull_request.head.label || github.head_ref || github.ref }}' + cancel-in-progress: true + env: CXX: g++-4.8 FASTLY_ACTIVATE_CHANGES: true From e01d3b1a00ed96b7d2d36c8c50c401e45c967ccd Mon Sep 17 00:00:00 2001 From: Christopher Willis-Ford <7019101+cwillisf@users.noreply.github.com> Date: Tue, 26 Sep 2023 09:01:37 -0700 Subject: [PATCH 04/33] ci: skip integration tests for now --- .github/workflows/build-and-test.yml | 49 +++------------------------- 1 file changed, 4 insertions(+), 45 deletions(-) diff --git a/.github/workflows/build-and-test.yml b/.github/workflows/build-and-test.yml index 007538593..2d69d58d8 100644 --- a/.github/workflows/build-and-test.yml +++ b/.github/workflows/build-and-test.yml @@ -44,50 +44,6 @@ jobs: environment: ${{ needs.determine-environment.outputs.scratch_environment }} env: SCRATCH_SHOULD_DEPLOY: ${{ vars.SCRATCH_ENV != '' }} - # env: # TODO: expose secrets only to those steps that need them - # # "all" environment - # AWS_ACCESS_KEY_ID: ${{ secrets.AWS_ACCESS_KEY_ID }} - # AWS_SECRET_ACCESS_KEY: ${{ secrets.AWS_SECRET_ACCESS_KEY }} - # COVERALLS_REPO_TOKEN: ${{ secrets.COVERALLS_REPO_TOKEN }} - # GTM_ID: ${{ secrets.GTM_ID }} - # S3_LOCAL_DIR: ${{ secrets.S3_LOCAL_DIR }} - # SAUCE_ACCESS_KEY: ${{ secrets.SAUCE_ACCESS_KEY }} - # SAUCE_USERNAME: ${{ secrets.SAUCE_USERNAME }} - # SENTRY_ORG: ${{ secrets.SENTRY_ORG }} - # SLACK_WEBHOOK_CIRCLECI_NOTIFICATIONS: ${{ secrets.SLACK_WEBHOOK_CIRCLECI_NOTIFICATIONS }} # TODO: rename or replace - # SLACK_WEBHOOK_ENGINEERING: ${{ secrets.SLACK_WEBHOOK_ENGINEERING }} - # SLACK_WEBHOOK_MODS: ${{ secrets.SLACK_WEBHOOK_MODS }} - # SMOKE_PASSWORD: ${{ secrets.SMOKE_PASSWORD }} - # SMOKE_USERNAME: ${{ secrets.SMOKE_USERNAME }} - - # # staging / production environments - # API_HOST: ${{ secrets.API_HOST }} - # ASSET_HOST: ${{ secrets.ASSET_HOST }} - # BACKPACK_HOST: ${{ secrets.BACKPACK_HOST }} - # CLOUDDATA_HOST: ${{ secrets.CLOUDDATA_HOST }} - # COMMENT_PROJECT_ID: ${{ secrets.COMMENT_PROJECT_ID }} - # COMMENT_STUDIO_ID: ${{ secrets.COMMENT_STUDIO_ID }} - # FASTLY_API_KEY: ${{ secrets.FASTLY_API_KEY }} - # FASTLY_SERVICE_ID: ${{ secrets.FASTLY_SERVICE_ID }} - # GA_TRACKER: ${{ secrets.GA_TRACKER }} - # GTM_ENV_AUTH: ${{ secrets.GTM_ENV_AUTH }} - # OWNED_SHARED_PROJECT_ID: ${{ secrets.OWNED_SHARED_PROJECT_ID }} - # OWNED_UNSHARED_PROJECT_ID: ${{ secrets.OWNED_UNSHARED_PROJECT_ID }} - # OWNED_UNSHARED_SCRATCH2_PROJECT_ID: ${{ secrets.OWNED_UNSHARED_SCRATCH2_PROJECT_ID }} - # PROJECT_HOST: ${{ secrets.PROJECT_HOST }} - # RATE_LIMIT_CHECK: ${{ secrets.RATE_LIMIT_CHECK }} - # RECAPTCHA_SITE_KEY: ${{ secrets.RECAPTCHA_SITE_KEY }} - # ROOT_URL: ${{ secrets.ROOT_URL }} - # S3_BUCKET_NAME: ${{ secrets.S3_BUCKET_NAME }} - # SCRATCH_ENV: ${{ vars.SCRATCH_ENV }} - # SENTRY_DSN: ${{ secrets.SENTRY_DSN }} - # SENTRY_PROJECT: ${{ secrets.SENTRY_PROJECT }} - # STATIC_HOST: ${{ secrets.STATIC_HOST }} - # TEST_PROJECT_ID: ${{ secrets.TEST_PROJECT_ID }} - # TEST_STUDIO_ID: ${{ secrets.TEST_STUDIO_ID }} - # UNOWNED_SHARED_PROJECT_ID: ${{ secrets.UNOWNED_SHARED_PROJECT_ID }} - # UNOWNED_UNSHARED_PROJECT_ID: ${{ secrets.UNOWNED_UNSHARED_PROJECT_ID }} - # UNOWNED_SHARED_SCRATCH2_PROJECT_ID: ${{ secrets.UNOWNED_SHARED_SCRATCH2_PROJECT_ID }} steps: - uses: actions/checkout@v3 - uses: actions/setup-node@v3 @@ -153,7 +109,10 @@ jobs: # if the health test fails, there's no point in trying to run the integration tests npm run test:health # health test succeeded, so proceed with integration tests - JEST_JUNIT_OUTPUT_NAME=integration-jest-results.xml npm run test:integration -- --reporters=jest-junit + echo "Skipping integration tests because they are not yet reliable on GHA." + echo "See branch 'exfoliate-tests' for more info." + echo "For now, you MUST run integration tests locally before deploying!" + #JEST_JUNIT_OUTPUT_NAME=integration-jest-results.xml npm run test:integration -- --reporters=jest-junit env: ROOT_URL: ${{ secrets.ROOT_URL }} From 70e2ecd702fcb7c927960ffc3c1a961495950da4 Mon Sep 17 00:00:00 2001 From: Christopher Willis-Ford <7019101+cwillisf@users.noreply.github.com> Date: Tue, 26 Sep 2023 09:02:54 -0700 Subject: [PATCH 05/33] ci: enable deploy from GHA --- .github/workflows/build-and-test.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/build-and-test.yml b/.github/workflows/build-and-test.yml index 2d69d58d8..f52ea3768 100644 --- a/.github/workflows/build-and-test.yml +++ b/.github/workflows/build-and-test.yml @@ -92,7 +92,7 @@ jobs: pip install s3cmd==2.1.0 - name: deploy if: ${{ env.SCRATCH_SHOULD_DEPLOY == 'true' }} - run: echo npm run deploy + run: npm run deploy env: S3_LOCAL_DIR: build S3_BUCKET_NAME: ${{ secrets.S3_BUCKET_NAME }} From 7b90c892b5c4beeb8ddc013bfe36f2ea298c29c1 Mon Sep 17 00:00:00 2001 From: Christopher Willis-Ford <7019101+cwillisf@users.noreply.github.com> Date: Tue, 26 Sep 2023 09:08:42 -0700 Subject: [PATCH 06/33] ci: fix variable context typo for SCRATCH_ENV --- .github/workflows/build-and-test.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/build-and-test.yml b/.github/workflows/build-and-test.yml index f52ea3768..c5bdef389 100644 --- a/.github/workflows/build-and-test.yml +++ b/.github/workflows/build-and-test.yml @@ -53,7 +53,7 @@ jobs: - name: info run: | echo "GitHub environment: ${{ needs.determine-environment.outputs.scratch_environment }}" - echo "Scratch environment: ${{ env.SCRATCH_ENV }}" + echo "Scratch environment: ${{ vars.SCRATCH_ENV }}" echo "Node version: $(node --version)" echo "NPM version: $(npm --version)" - name: setup From cf8e816151225de34a528449ba6e3681d4813355 Mon Sep 17 00:00:00 2001 From: Christopher Willis-Ford <7019101+cwillisf@users.noreply.github.com> Date: Tue, 26 Sep 2023 10:23:40 -0700 Subject: [PATCH 07/33] ci: temp --- .github/workflows/build-and-test.yml | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/.github/workflows/build-and-test.yml b/.github/workflows/build-and-test.yml index c5bdef389..477f24b15 100644 --- a/.github/workflows/build-and-test.yml +++ b/.github/workflows/build-and-test.yml @@ -92,7 +92,11 @@ jobs: pip install s3cmd==2.1.0 - name: deploy if: ${{ env.SCRATCH_SHOULD_DEPLOY == 'true' }} - run: npm run deploy + run: | + echo "${S3_BUCKET_NAME:0:2}...${S3_BUCKET_NAME:(-2):2}" + echo "${AWS_ACCESS_KEY_ID:0:2}...${AWS_ACCESS_KEY_ID:(-2):2}" + echo "${AWS_SECRET_ACCESS_KEY:0:2}...${AWS_SECRET_ACCESS_KEY:(-2):2}" + npm run deploy env: S3_LOCAL_DIR: build S3_BUCKET_NAME: ${{ secrets.S3_BUCKET_NAME }} From 41a80fe29db2c46f2c2f57659fc13cbad8d31e6f Mon Sep 17 00:00:00 2001 From: Christopher Willis-Ford <7019101+cwillisf@users.noreply.github.com> Date: Wed, 25 Oct 2023 15:46:39 -0700 Subject: [PATCH 08/33] ci: upgrade s3cmd for compat w/ Python 3.9+ --- .github/workflows/build-and-test.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/build-and-test.yml b/.github/workflows/build-and-test.yml index 477f24b15..bd572e491 100644 --- a/.github/workflows/build-and-test.yml +++ b/.github/workflows/build-and-test.yml @@ -89,7 +89,7 @@ jobs: run: | curl https://bootstrap.pypa.io/pip/3.5/get-pip.py -o get-pip.py python3 get-pip.py pip==21.0.1 - pip install s3cmd==2.1.0 + pip install s3cmd==2.3.0 - name: deploy if: ${{ env.SCRATCH_SHOULD_DEPLOY == 'true' }} run: | From fe3bfdd5e13e68ebf5ab639dace5149ac80a3570 Mon Sep 17 00:00:00 2001 From: Christopher Willis-Ford <7019101+cwillisf@users.noreply.github.com> Date: Wed, 25 Oct 2023 16:01:47 -0700 Subject: [PATCH 09/33] ci: re-enable integration tests --- .github/workflows/build-and-test.yml | 11 ++--------- 1 file changed, 2 insertions(+), 9 deletions(-) diff --git a/.github/workflows/build-and-test.yml b/.github/workflows/build-and-test.yml index bd572e491..ce8071d61 100644 --- a/.github/workflows/build-and-test.yml +++ b/.github/workflows/build-and-test.yml @@ -92,11 +92,7 @@ jobs: pip install s3cmd==2.3.0 - name: deploy if: ${{ env.SCRATCH_SHOULD_DEPLOY == 'true' }} - run: | - echo "${S3_BUCKET_NAME:0:2}...${S3_BUCKET_NAME:(-2):2}" - echo "${AWS_ACCESS_KEY_ID:0:2}...${AWS_ACCESS_KEY_ID:(-2):2}" - echo "${AWS_SECRET_ACCESS_KEY:0:2}...${AWS_SECRET_ACCESS_KEY:(-2):2}" - npm run deploy + run: npm run deploy env: S3_LOCAL_DIR: build S3_BUCKET_NAME: ${{ secrets.S3_BUCKET_NAME }} @@ -113,10 +109,7 @@ jobs: # if the health test fails, there's no point in trying to run the integration tests npm run test:health # health test succeeded, so proceed with integration tests - echo "Skipping integration tests because they are not yet reliable on GHA." - echo "See branch 'exfoliate-tests' for more info." - echo "For now, you MUST run integration tests locally before deploying!" - #JEST_JUNIT_OUTPUT_NAME=integration-jest-results.xml npm run test:integration -- --reporters=jest-junit + JEST_JUNIT_OUTPUT_NAME=integration-jest-results.xml npm run test:integration -- --reporters=jest-junit env: ROOT_URL: ${{ secrets.ROOT_URL }} From b856a2648ceaf8cdc2d2d0c8b8cb9d974f6a69c7 Mon Sep 17 00:00:00 2001 From: Christopher Willis-Ford <7019101+cwillisf@users.noreply.github.com> Date: Tue, 19 Sep 2023 10:16:24 -0700 Subject: [PATCH 10/33] test: reduce page loads in project-page.test.js --- test/integration/project-page.test.js | 14 ++++---------- 1 file changed, 4 insertions(+), 10 deletions(-) diff --git a/test/integration/project-page.test.js b/test/integration/project-page.test.js index d9dc82b89..eea1a3d47 100644 --- a/test/integration/project-page.test.js +++ b/test/integration/project-page.test.js @@ -123,10 +123,6 @@ describe('www-integration project-page signed in', () => { await findByXpath('//span[contains(@class, "profile-name")]'); }); - beforeEach(async () => { - await driver.get(rootUrl); - }); - afterAll(() => driver.quit()); // LOGGED in TESTS @@ -191,7 +187,6 @@ describe('www-integration project-creation signed in', () => { // expect(projectUrl).toBe(defined); driver = await buildDriver('www-integration project-creation signed in'); await driver.get(rootUrl); - await driver.sleep(1000); await signIn(username, password); await findByXpath('//span[contains(@class, "profile-name")]'); @@ -199,15 +194,11 @@ describe('www-integration project-creation signed in', () => { // https://support.saucelabs.com/hc/en-us/articles/115003685593-Uploading-Files-to-a-Sauce-Labs-Virtual-Machine-during-a-Test if (remote) { await driver.get('https://github.com/scratchfoundation/scratch-www/blob/develop/test/fixtures/project1.sb3'); - await clickXpath('//Button[@data-testid="download-raw-button"]'); + await clickXpath('//button[@data-testid="download-raw-button"]'); await driver.sleep(3000); } }); - beforeEach(async () => { - await driver.get(rootUrl); - }); - afterAll(() => driver.quit()); test('make a copy of a project', async () => { @@ -240,6 +231,9 @@ describe('www-integration project-creation signed in', () => { }); test('load project from file', async () => { + // load the editor without making a new project + await driver.get(unownedSharedUrl); + // if remote, projectPath is Saucelabs path to downloaded file const projectPath = remote ? '/Users/chef/Downloads/project1.sb3' : From aceac8237aac165b2e828fd50e6c0568b3fb0ec2 Mon Sep 17 00:00:00 2001 From: Christopher Willis-Ford <7019101+cwillisf@users.noreply.github.com> Date: Thu, 26 Oct 2023 14:52:50 -0700 Subject: [PATCH 11/33] test: make clickXpath wait until it can 'see' the element --- test/integration/project-page.test.js | 6 ---- test/integration/selenium-helpers.js | 46 +++++++++++++++++++++++++-- 2 files changed, 44 insertions(+), 8 deletions(-) diff --git a/test/integration/project-page.test.js b/test/integration/project-page.test.js index eea1a3d47..d07d171b0 100644 --- a/test/integration/project-page.test.js +++ b/test/integration/project-page.test.js @@ -118,9 +118,7 @@ describe('www-integration project-page signed in', () => { // expect(projectUrl).toBe(defined); driver = await buildDriver('www-integration project-page signed in'); await driver.get(rootUrl); - await driver.sleep(1000); await signIn(username, password); - await findByXpath('//span[contains(@class, "profile-name")]'); }); afterAll(() => driver.quit()); @@ -184,11 +182,9 @@ describe('www-integration project-page signed in', () => { describe('www-integration project-creation signed in', () => { beforeAll(async () => { - // expect(projectUrl).toBe(defined); driver = await buildDriver('www-integration project-creation signed in'); await driver.get(rootUrl); await signIn(username, password); - await findByXpath('//span[contains(@class, "profile-name")]'); // SauceLabs doesn't have access to the sb3 used in 'load project from file' test // https://support.saucelabs.com/hc/en-us/articles/115003685593-Uploading-Files-to-a-Sauce-Labs-Virtual-Machine-during-a-Test @@ -203,8 +199,6 @@ describe('www-integration project-creation signed in', () => { test('make a copy of a project', async () => { await driver.get(`${ownedUnsharedUrl}/editor`); - const gf = await findByXpath('//img[@class="green-flag_green-flag_1kiAo"]'); - await gf.isDisplayed(); await clickXpath(FILE_MENU_XPATH); await clickText('Save as a copy'); const successAlert = await findText('Project saved as a copy.'); diff --git a/test/integration/selenium-helpers.js b/test/integration/selenium-helpers.js index 707ce2f63..f8fb91aa4 100644 --- a/test/integration/selenium-helpers.js +++ b/test/integration/selenium-helpers.js @@ -276,6 +276,48 @@ class SeleniumHelper { } } + /** + * @param {string} xpath Wait until an element at the provided xpath is clickable. + * @returns {Promise} A promise that resolves to the clickable element. + */ + waitUntilClickable (xpath) { + // @ts-ignore - TS can't tell that `driver.wait()` called this way will never return `undefined`. + return this.driver.wait(async () => { + const elementAtPath = await this.findByXpath(xpath); + if (!elementAtPath) { + return; + } + const rect = await elementAtPath.getRect(); + const x = rect.x + (rect.width / 2); + const y = rect.y + (rect.height / 2); + const elementAtPoint = await this.driver.executeScript( + 'return document.elementFromPoint(arguments[0], arguments[1])', + x, + y + ); + if (!elementAtPoint) { + return; + } + // If we ask to click on a button and Selenium finds an image on the button, or vice versa, that's OK. + // It doesn't have to be an exact match. + const match = await this.driver.executeScript( + 'return arguments[0].contains(arguments[1]) || arguments[1].contains(arguments[0])', + elementAtPath, + elementAtPoint + ); + if (!match) { + return; + } + if (!await elementAtPath.isDisplayed()) { + return; + } + if (!await elementAtPath.isEnabled()) { + return; + } + return elementAtPath; + }); + } + /** * Wait until an element can be found by the provided xpath, then click on it. * @param {string} xpath The xpath to click. @@ -284,8 +326,8 @@ class SeleniumHelper { async clickXpath (xpath) { const outerError = new SeleniumHelperError('clickXpath failed', [{xpath}]); try { - const el = await this.findByXpath(xpath); - await el.click(); + const element = await this.waitUntilClickable(xpath); + element.click(); } catch (cause) { throw await outerError.chain(cause, this.driver); } From 95241fae6f1ccb756f130fd5e59afa7bbafd75dc Mon Sep 17 00:00:00 2001 From: Christopher Willis-Ford <7019101+cwillisf@users.noreply.github.com> Date: Wed, 20 Sep 2023 10:29:48 -0700 Subject: [PATCH 12/33] test: sign in again for each test if necessary --- test/integration/project-page.test.js | 19 +++++++-- test/integration/selenium-helpers.js | 55 ++++++++++++++++++++++++++- 2 files changed, 69 insertions(+), 5 deletions(-) diff --git a/test/integration/project-page.test.js b/test/integration/project-page.test.js index d07d171b0..761ed3185 100644 --- a/test/integration/project-page.test.js +++ b/test/integration/project-page.test.js @@ -11,6 +11,7 @@ const { clickXpath, findText, findByXpath, + isSignedIn, signIn, waitUntilVisible } = new SeleniumHelper(); @@ -117,8 +118,14 @@ describe('www-integration project-page signed in', () => { beforeAll(async () => { // expect(projectUrl).toBe(defined); driver = await buildDriver('www-integration project-page signed in'); + }); + + beforeEach(async () => { + // The browser may or may not retain cookies between tests, depending on configuration. await driver.get(rootUrl); - await signIn(username, password); + if (!await isSignedIn()) { + await signIn(username, password); + } }); afterAll(() => driver.quit()); @@ -183,8 +190,6 @@ describe('www-integration project-page signed in', () => { describe('www-integration project-creation signed in', () => { beforeAll(async () => { driver = await buildDriver('www-integration project-creation signed in'); - await driver.get(rootUrl); - await signIn(username, password); // SauceLabs doesn't have access to the sb3 used in 'load project from file' test // https://support.saucelabs.com/hc/en-us/articles/115003685593-Uploading-Files-to-a-Sauce-Labs-Virtual-Machine-during-a-Test @@ -195,6 +200,14 @@ describe('www-integration project-creation signed in', () => { } }); + beforeEach(async () => { + // The browser may or may not retain cookies between tests, depending on configuration. + await driver.get(rootUrl); + if (!await isSignedIn()) { + await signIn(username, password); + } + }); + afterAll(() => driver.quit()); test('make a copy of a project', async () => { diff --git a/test/integration/selenium-helpers.js b/test/integration/selenium-helpers.js index f8fb91aa4..d8ac01273 100644 --- a/test/integration/selenium-helpers.js +++ b/test/integration/selenium-helpers.js @@ -135,6 +135,7 @@ class SeleniumHelper { 'getDriver', 'getLogs', 'getSauceDriver', + 'isSignedIn', 'signIn', 'urlMatches', 'waitUntilGone' @@ -426,6 +427,56 @@ class SeleniumHelper { } } + /** + * @returns {string} The xpath to the login button, which is present only if signed out. + */ + getPathForLogin () { + return '//li[@class="link right login-item"]/a'; + } + + /** + * @returns {string} The xpath to the profile name, which is present only if signed in. + */ + getPathForProfileName () { + return '//span[contains(@class, "profile-name")]'; + } + + /** + * @returns {Promise} True if the user is signed in, false otherwise. + * @throws {SeleniumHelperError} If the user's sign-in state cannot be determined. + */ + async isSignedIn () { + const outerError = new SeleniumHelperError('isSignedIn failed'); + try { + const state = await this.driver.wait(() => + this.driver.executeScript( + ` + if (document.evaluate(arguments[0], document, null, XPathResult.FIRST_ORDERED_NODE_TYPE, null) + .singleNodeValue) { + return 'signed in'; + } + if (document.evaluate(arguments[1], document, null, XPathResult.FIRST_ORDERED_NODE_TYPE, null) + .singleNodeValue) { + return 'signed out'; + } + `, + this.getPathForProfileName(), + this.getPathForLogin() + ) + ); + switch (state) { + case 'signed in': + return true; + case 'signed out': + return false; + default: + throw new Error('Could not determine whether or not user is signed in'); + } + } catch (cause) { + throw await outerError.chain(cause, this.driver); + } + } + /** * Sign in on a `scratch-www` page. * @param {string} username The username to sign in with. @@ -438,12 +489,12 @@ class SeleniumHelper { {password: password ? 'provided' : 'absent'} ]); try { - await this.clickXpath('//li[@class="link right login-item"]/a'); + await this.clickXpath(this.getPathForLogin()); const name = await this.findByXpath('//input[@id="frc-username-1088"]'); await name.sendKeys(username); const word = await this.findByXpath('//input[@id="frc-password-1088"]'); await word.sendKeys(password + this.getKey('ENTER')); - await this.findByXpath('//span[contains(@class, "profile-name")]'); + await this.findByXpath(this.getPathForProfileName()); } catch (cause) { throw await outerError.chain(cause, this.driver); } From ca98ef0edec4252b5568d77689284a7943194ad5 Mon Sep 17 00:00:00 2001 From: Christopher Willis-Ford <7019101+cwillisf@users.noreply.github.com> Date: Wed, 20 Sep 2023 12:24:32 -0700 Subject: [PATCH 13/33] test: make 'load project from file' test more reliable --- test/integration/project-page.test.js | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/test/integration/project-page.test.js b/test/integration/project-page.test.js index 761ed3185..873f7f8de 100644 --- a/test/integration/project-page.test.js +++ b/test/integration/project-page.test.js @@ -3,6 +3,7 @@ // some tests use projects owned by user #2 const SeleniumHelper = require('./selenium-helpers.js'); +const {until} = require('selenium-webdriver'); import path from 'path'; const { @@ -238,25 +239,22 @@ describe('www-integration project-creation signed in', () => { }); test('load project from file', async () => { - // load the editor without making a new project - await driver.get(unownedSharedUrl); - // if remote, projectPath is Saucelabs path to downloaded file const projectPath = remote ? '/Users/chef/Downloads/project1.sb3' : path.resolve(__dirname, '../fixtures/project1.sb3'); - // upload file + // create a new project so there's unsaved content to trigger an alert await clickXpath('//li[@class="link create"]'); - const gf = await findByXpath('//img[@class="green-flag_green-flag_1kiAo"]'); - await gf.isDisplayed(); + + // upload file await clickXpath(FILE_MENU_XPATH); await clickText('Load from your computer'); - await driver.sleep(1000); const input = await findByXpath('//input[@accept=".sb,.sb2,.sb3"]'); await input.sendKeys(projectPath); // accept alert + await driver.wait(until.alertIsPresent()); const alert = await driver.switchTo().alert(); await alert.accept(); From 310fce63fed46acb8506c21dea679b07f0007dde Mon Sep 17 00:00:00 2001 From: Christopher Willis-Ford <7019101+cwillisf@users.noreply.github.com> Date: Wed, 20 Sep 2023 14:18:21 -0700 Subject: [PATCH 14/33] test: fix studios-page.test.js --- test/integration/studios-page.test.js | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/test/integration/studios-page.test.js b/test/integration/studios-page.test.js index deaaf14b0..b52e6e678 100644 --- a/test/integration/studios-page.test.js +++ b/test/integration/studios-page.test.js @@ -7,6 +7,7 @@ const { buildDriver, clickXpath, clickText, + isSignedIn, signIn } = new SeleniumHelper(); @@ -84,10 +85,10 @@ describe('studio management', () => { }); beforeEach(async () => { - await clickXpath('//a[contains(@class, "user-info")]'); - await clickText('Sign out'); - await driver.get(curatorTab); - await findByXpath('//div[@class="studio-tabs"]'); + if (await isSignedIn()) { + await clickXpath('//a[contains(@class, "user-info")]'); + await clickText('Sign out'); + } }); afterAll(() => driver.quit()); @@ -95,7 +96,7 @@ describe('studio management', () => { test('invite a curator', async () => { // sign in as user2 await signIn(username2, password); - await findByXpath('//span[contains(@class, "profile-name")]'); + await driver.get(curatorTab); // invite user3 to curate const inviteBox = await findByXpath('//div[@class="studio-adder-row"]/input'); @@ -110,7 +111,7 @@ describe('studio management', () => { test('accept curator invite', async () => { // Sign in user3 await signIn(username3, password); - await findByXpath('//span[contains(@class, "profile-name")]'); + await driver.get(curatorTab); // accept the curator invite await clickXpath('//button[@class="studio-invitation-button button"]'); From 876e2deb97820dca7a93463d67f8d5776f189cad Mon Sep 17 00:00:00 2001 From: Christopher Willis-Ford <7019101+cwillisf@users.noreply.github.com> Date: Thu, 21 Sep 2023 15:44:03 -0700 Subject: [PATCH 15/33] test: add scrolling support to clickXpath to fix comments test --- test/integration/comments.test.js | 3 +-- test/integration/selenium-helpers.js | 40 +++++++++++++++++++++++----- 2 files changed, 34 insertions(+), 9 deletions(-) diff --git a/test/integration/comments.test.js b/test/integration/comments.test.js index 6bf0e427d..10a02412d 100644 --- a/test/integration/comments.test.js +++ b/test/integration/comments.test.js @@ -256,8 +256,7 @@ describe('comment tests', () => { await composeBox.sendKeys(projectReply); // click post - const postButton = await findByXpath(`${replyRow}//button[@class = "button compose-post"]`); - await postButton.click(); + await clickXpath(`${replyRow}//button[@class = "button compose-post"]`); const postedReply = await findByXpath(`//span[contains(text(), "${projectReply}")]`); const commentVisible = await postedReply.isDisplayed(); diff --git a/test/integration/selenium-helpers.js b/test/integration/selenium-helpers.js index d8ac01273..0a8638e6d 100644 --- a/test/integration/selenium-helpers.js +++ b/test/integration/selenium-helpers.js @@ -279,22 +279,48 @@ class SeleniumHelper { /** * @param {string} xpath Wait until an element at the provided xpath is clickable. + * @param {boolean} [allowScrolling] Whether or not to allow page scrolling to reach the element. * @returns {Promise} A promise that resolves to the clickable element. */ - waitUntilClickable (xpath) { + waitUntilClickable (xpath, allowScrolling = true) { // @ts-ignore - TS can't tell that `driver.wait()` called this way will never return `undefined`. return this.driver.wait(async () => { const elementAtPath = await this.findByXpath(xpath); if (!elementAtPath) { return; } - const rect = await elementAtPath.getRect(); - const x = rect.x + (rect.width / 2); - const y = rect.y + (rect.height / 2); + + if (allowScrolling) { + await this.driver.executeScript( + ` + const element = arguments[0]; + const boundingRect = element.getBoundingClientRect(); + boundingRect.windowWidth = window.innerWidth; + boundingRect.windowHeight = window.innerHeight; + if (boundingRect.top < 0 || boundingRect.bottom > window.innerHeight || + boundingRect.left < 0 || boundingRect.right > window.innerWidth) + { + boundingRect.scrollIntoView = true; + element.scrollIntoView({ + behavior: 'instant', + block:'nearest', + inline: 'nearest' + }); + } + `, + elementAtPath + ); + } + const elementAtPoint = await this.driver.executeScript( - 'return document.elementFromPoint(arguments[0], arguments[1])', - x, - y + ` + const rect = arguments[0].getBoundingClientRect(); + return document.elementFromPoint( + rect.x + rect.width / 2, + rect.y + rect.height / 2 + ); + `, + elementAtPath ); if (!elementAtPoint) { return; From 7c49d1617c8af554585fc3c36a5ce673e95236b1 Mon Sep 17 00:00:00 2001 From: Christopher Willis-Ford <7019101+cwillisf@users.noreply.github.com> Date: Thu, 21 Sep 2023 15:44:50 -0700 Subject: [PATCH 16/33] test: wait for page load, fixing footer-links flakiness --- test/integration/footer-links.test.js | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/test/integration/footer-links.test.js b/test/integration/footer-links.test.js index be595682b..cc8aefc4f 100644 --- a/test/integration/footer-links.test.js +++ b/test/integration/footer-links.test.js @@ -28,8 +28,14 @@ describe('www-integration footer links', () => { // ==== About Scratch column ==== + const pageLoadComplete = () => + driver.wait(async () => { + return await driver.executeScript('return document.readyState;') === 'complete'; + }); + test('click About Scratch link', async () => { await clickText('About Scratch'); + await pageLoadComplete(); const url = await driver.getCurrentUrl(); const pathname = (new URL(url)).pathname; expect(pathname).toMatch(/^\/about\/?$/); @@ -37,6 +43,7 @@ describe('www-integration footer links', () => { test('click For Parents link', async () => { await clickText('For Parents'); + await pageLoadComplete(); const url = await driver.getCurrentUrl(); const pathname = (new URL(url)).pathname; expect(pathname).toMatch(/^\/parents\/?$/); @@ -44,6 +51,7 @@ describe('www-integration footer links', () => { test('click For Educators link', async () => { await clickText('For Educators'); + await pageLoadComplete(); const url = await driver.getCurrentUrl(); const pathname = (new URL(url)).pathname; expect(pathname).toMatch(/^\/educators\/?$/); @@ -51,6 +59,7 @@ describe('www-integration footer links', () => { test('click For Developers link', async () => { await clickText('For Developers'); + await pageLoadComplete(); const url = await driver.getCurrentUrl(); const pathname = (new URL(url)).pathname; expect(pathname).toMatch(/^\/developers\/?$/); @@ -60,6 +69,7 @@ describe('www-integration footer links', () => { test('click Community Guidelines link', async () => { await clickText('Community Guidelines'); + await pageLoadComplete(); const url = await driver.getCurrentUrl(); const pathname = (new URL(url)).pathname; expect(pathname).toMatch(/^\/community_guidelines\/?$/); @@ -67,6 +77,7 @@ describe('www-integration footer links', () => { test('click Discussion Forums link', async () => { await clickText('Discussion Forums'); + await pageLoadComplete(); const url = await driver.getCurrentUrl(); const pathname = (new URL(url)).pathname; expect(pathname).toMatch(/^\/discuss\/?$/); @@ -74,6 +85,7 @@ describe('www-integration footer links', () => { test('click Statistics link', async () => { await clickText('Statistics'); + await pageLoadComplete(); const url = await driver.getCurrentUrl(); const pathname = (new URL(url)).pathname; expect(pathname).toMatch(/^\/statistics\/?$/); @@ -83,6 +95,7 @@ describe('www-integration footer links', () => { test('click Ideas link', async () => { await clickText('Ideas'); + await pageLoadComplete(); const url = await driver.getCurrentUrl(); const pathname = (new URL(url)).pathname; expect(pathname).toMatch(/^\/ideas\/?$/); @@ -90,6 +103,7 @@ describe('www-integration footer links', () => { test('click FAQ link', async () => { await clickText('FAQ'); + await pageLoadComplete(); const url = await driver.getCurrentUrl(); const pathname = (new URL(url)).pathname; expect(pathname).toMatch(/^\/faq\/?$/); @@ -97,6 +111,7 @@ describe('www-integration footer links', () => { test('click Download link', async () => { await clickText('Download'); + await pageLoadComplete(); const url = await driver.getCurrentUrl(); const pathname = (new URL(url)).pathname; expect(pathname).toMatch(/^\/download\/?$/); @@ -104,6 +119,7 @@ describe('www-integration footer links', () => { test('click Contact Us link', async () => { await clickText('Contact Us'); + await pageLoadComplete(); const url = await driver.getCurrentUrl(); const pathname = (new URL(url)).pathname; expect(pathname).toMatch(/^\/contact-us\/?$/); @@ -113,6 +129,7 @@ describe('www-integration footer links', () => { test('click Terms of Use link', async () => { await clickText('Terms of Use'); + await pageLoadComplete(); const url = await driver.getCurrentUrl(); const pathname = (new URL(url)).pathname; expect(pathname).toMatch(/^\/terms_of_use\/?$/); @@ -120,6 +137,7 @@ describe('www-integration footer links', () => { test('click Privacy Policy link', async () => { await clickText('Privacy Policy'); + await pageLoadComplete(); const url = await driver.getCurrentUrl(); const pathname = (new URL(url)).pathname; expect(pathname).toMatch(/^\/privacy_policy\/?$/); @@ -127,6 +145,7 @@ describe('www-integration footer links', () => { test('click Cookies link', async () => { await clickText('Cookies'); + await pageLoadComplete(); const url = await driver.getCurrentUrl(); const pathname = (new URL(url)).pathname; expect(pathname).toMatch(/^\/cookies\/?$/); @@ -139,6 +158,7 @@ describe('www-integration footer links', () => { test('click DMCA link', async () => { await clickText('DMCA'); + await pageLoadComplete(); const url = await driver.getCurrentUrl(); const pathname = (new URL(url)).pathname; expect(pathname).toMatch(/^\/DMCA\/?$/); @@ -148,6 +168,7 @@ describe('www-integration footer links', () => { test('click Scratch Conference link', async () => { await clickText('Scratch Conference'); + await pageLoadComplete(); const url = await driver.getCurrentUrl(); const pathname = (new URL(url)).pathname; expect(pathname).toMatch(/^\/scratch-conference\/?$/); From bbb8618b876638ce68f37b78bfb57f05b25d6cb6 Mon Sep 17 00:00:00 2001 From: Christopher Willis-Ford <7019101+cwillisf@users.noreply.github.com> Date: Fri, 22 Sep 2023 08:43:45 -0700 Subject: [PATCH 17/33] test: fix join flow test flakiness - wait for navigation to complete so we don't accidentally grab an element on the page we're navigating away from, leading to a stale reference - wait for focus to move into a selected input field so we don't accidentally grab the `validation-message` attached to the old input box (the "username" box is selected by default on the join flow page) --- test/integration/footer-links.test.js | 40 ++++++++---------- test/integration/join.test.js | 25 +++++++---- test/integration/selenium-helpers.js | 60 ++++++++++++++++++++++----- 3 files changed, 83 insertions(+), 42 deletions(-) diff --git a/test/integration/footer-links.test.js b/test/integration/footer-links.test.js index cc8aefc4f..d55417f2d 100644 --- a/test/integration/footer-links.test.js +++ b/test/integration/footer-links.test.js @@ -5,7 +5,8 @@ const SeleniumHelper = require('./selenium-helpers.js'); const { clickText, buildDriver, - findText + findText, + waitUntilDocumentReady } = new SeleniumHelper(); const rootUrl = process.env.ROOT_URL || 'https://scratch.ly'; @@ -28,14 +29,9 @@ describe('www-integration footer links', () => { // ==== About Scratch column ==== - const pageLoadComplete = () => - driver.wait(async () => { - return await driver.executeScript('return document.readyState;') === 'complete'; - }); - test('click About Scratch link', async () => { await clickText('About Scratch'); - await pageLoadComplete(); + await waitUntilDocumentReady(); const url = await driver.getCurrentUrl(); const pathname = (new URL(url)).pathname; expect(pathname).toMatch(/^\/about\/?$/); @@ -43,7 +39,7 @@ describe('www-integration footer links', () => { test('click For Parents link', async () => { await clickText('For Parents'); - await pageLoadComplete(); + await waitUntilDocumentReady(); const url = await driver.getCurrentUrl(); const pathname = (new URL(url)).pathname; expect(pathname).toMatch(/^\/parents\/?$/); @@ -51,7 +47,7 @@ describe('www-integration footer links', () => { test('click For Educators link', async () => { await clickText('For Educators'); - await pageLoadComplete(); + await waitUntilDocumentReady(); const url = await driver.getCurrentUrl(); const pathname = (new URL(url)).pathname; expect(pathname).toMatch(/^\/educators\/?$/); @@ -59,7 +55,7 @@ describe('www-integration footer links', () => { test('click For Developers link', async () => { await clickText('For Developers'); - await pageLoadComplete(); + await waitUntilDocumentReady(); const url = await driver.getCurrentUrl(); const pathname = (new URL(url)).pathname; expect(pathname).toMatch(/^\/developers\/?$/); @@ -69,7 +65,7 @@ describe('www-integration footer links', () => { test('click Community Guidelines link', async () => { await clickText('Community Guidelines'); - await pageLoadComplete(); + await waitUntilDocumentReady(); const url = await driver.getCurrentUrl(); const pathname = (new URL(url)).pathname; expect(pathname).toMatch(/^\/community_guidelines\/?$/); @@ -77,7 +73,7 @@ describe('www-integration footer links', () => { test('click Discussion Forums link', async () => { await clickText('Discussion Forums'); - await pageLoadComplete(); + await waitUntilDocumentReady(); const url = await driver.getCurrentUrl(); const pathname = (new URL(url)).pathname; expect(pathname).toMatch(/^\/discuss\/?$/); @@ -85,7 +81,7 @@ describe('www-integration footer links', () => { test('click Statistics link', async () => { await clickText('Statistics'); - await pageLoadComplete(); + await waitUntilDocumentReady(); const url = await driver.getCurrentUrl(); const pathname = (new URL(url)).pathname; expect(pathname).toMatch(/^\/statistics\/?$/); @@ -95,7 +91,7 @@ describe('www-integration footer links', () => { test('click Ideas link', async () => { await clickText('Ideas'); - await pageLoadComplete(); + await waitUntilDocumentReady(); const url = await driver.getCurrentUrl(); const pathname = (new URL(url)).pathname; expect(pathname).toMatch(/^\/ideas\/?$/); @@ -103,7 +99,7 @@ describe('www-integration footer links', () => { test('click FAQ link', async () => { await clickText('FAQ'); - await pageLoadComplete(); + await waitUntilDocumentReady(); const url = await driver.getCurrentUrl(); const pathname = (new URL(url)).pathname; expect(pathname).toMatch(/^\/faq\/?$/); @@ -111,7 +107,7 @@ describe('www-integration footer links', () => { test('click Download link', async () => { await clickText('Download'); - await pageLoadComplete(); + await waitUntilDocumentReady(); const url = await driver.getCurrentUrl(); const pathname = (new URL(url)).pathname; expect(pathname).toMatch(/^\/download\/?$/); @@ -119,7 +115,7 @@ describe('www-integration footer links', () => { test('click Contact Us link', async () => { await clickText('Contact Us'); - await pageLoadComplete(); + await waitUntilDocumentReady(); const url = await driver.getCurrentUrl(); const pathname = (new URL(url)).pathname; expect(pathname).toMatch(/^\/contact-us\/?$/); @@ -129,7 +125,7 @@ describe('www-integration footer links', () => { test('click Terms of Use link', async () => { await clickText('Terms of Use'); - await pageLoadComplete(); + await waitUntilDocumentReady(); const url = await driver.getCurrentUrl(); const pathname = (new URL(url)).pathname; expect(pathname).toMatch(/^\/terms_of_use\/?$/); @@ -137,7 +133,7 @@ describe('www-integration footer links', () => { test('click Privacy Policy link', async () => { await clickText('Privacy Policy'); - await pageLoadComplete(); + await waitUntilDocumentReady(); const url = await driver.getCurrentUrl(); const pathname = (new URL(url)).pathname; expect(pathname).toMatch(/^\/privacy_policy\/?$/); @@ -145,7 +141,7 @@ describe('www-integration footer links', () => { test('click Cookies link', async () => { await clickText('Cookies'); - await pageLoadComplete(); + await waitUntilDocumentReady(); const url = await driver.getCurrentUrl(); const pathname = (new URL(url)).pathname; expect(pathname).toMatch(/^\/cookies\/?$/); @@ -158,7 +154,7 @@ describe('www-integration footer links', () => { test('click DMCA link', async () => { await clickText('DMCA'); - await pageLoadComplete(); + await waitUntilDocumentReady(); const url = await driver.getCurrentUrl(); const pathname = (new URL(url)).pathname; expect(pathname).toMatch(/^\/DMCA\/?$/); @@ -168,7 +164,7 @@ describe('www-integration footer links', () => { test('click Scratch Conference link', async () => { await clickText('Scratch Conference'); - await pageLoadComplete(); + await waitUntilDocumentReady(); const url = await driver.getCurrentUrl(); const pathname = (new URL(url)).pathname; expect(pathname).toMatch(/^\/scratch-conference\/?$/); diff --git a/test/integration/join.test.js b/test/integration/join.test.js index 9c3d58e73..83f8e56eb 100644 --- a/test/integration/join.test.js +++ b/test/integration/join.test.js @@ -3,9 +3,11 @@ const SeleniumHelper = require('./selenium-helpers.js'); const { - findByXpath, + buildDriver, clickXpath, - buildDriver + findByXpath, + navigate, + waitUntilDocumentReady } = new SeleniumHelper(); const rootUrl = process.env.ROOT_URL || 'https://scratch.ly'; @@ -18,14 +20,14 @@ let driver; describe('www-integration join flow', () => { beforeAll(async () => { driver = await buildDriver('www-integration join flow'); - await driver.get(rootUrl); }); afterAll(() => driver.quit()); beforeEach(async () => { - await driver.get(rootUrl); - await clickXpath('//a[@class="registrationLink"]'); + await navigate(rootUrl); // navigate to home page + await clickXpath('//a[@class="registrationLink"]'); // navigate to join page + await waitUntilDocumentReady(); }); test('click Join opens join modal', async () => { @@ -35,22 +37,24 @@ describe('www-integration join flow', () => { }); test('username validation message appears', async () => { - await clickXpath('//input[contains(@name, "username")]'); + const clickedInput = await clickXpath('//input[contains(@name, "username")]'); + await driver.wait(() => driver.executeScript('return document.activeElement == arguments[0]', clickedInput)); const message = await findByXpath('//div[contains(@class, "validation-message")]'); const messageText = await message.getText(); expect(messageText).toEqual('Don\'t use your real name'); - }); test('password validation message appears', async () => { - await clickXpath('//input[contains(@name, "password")]'); + const clickedInput = await clickXpath('//input[contains(@name, "password")]'); + await driver.wait(() => driver.executeScript('return document.activeElement == arguments[0]', clickedInput)); const message = await findByXpath('//div[contains(@class, "validation-message")]'); const messageText = await message.getText(); expect(messageText).toContain('Write it down so you remember.'); }); test('password confirmation validation message appears', async () => { - await clickXpath('//input[contains(@name, "passwordConfirm")]'); + const clickedInput = await clickXpath('//input[contains(@name, "passwordConfirm")]'); + await driver.wait(() => driver.executeScript('return document.activeElement == arguments[0]', clickedInput)); const message = await findByXpath('//div[contains(@class, "validation-message")]'); const messageText = await message.getText(); expect(messageText).toEqual('Type password again'); @@ -59,6 +63,7 @@ describe('www-integration join flow', () => { test('username validation: too short', async () => { const textInput = await findByXpath('//input[contains(@name, "username")]'); await textInput.click(); + await driver.wait(() => driver.executeScript('return document.activeElement == arguments[0]', textInput)); await textInput.sendKeys('ab'); await clickXpath('//div[@class = "join-flow-outer-content"]'); const message = await findByXpath('//div[contains(@class, "validation-error")]'); @@ -69,6 +74,7 @@ describe('www-integration join flow', () => { test('username validation: username taken', async () => { const textInput = await findByXpath('//input[contains(@name, "username")]'); await textInput.click(); + await driver.wait(() => driver.executeScript('return document.activeElement == arguments[0]', textInput)); await textInput.sendKeys(takenUsername); await clickXpath('//div[@class = "join-flow-outer-content"]'); const message = await findByXpath('//div[contains(@class, "validation-error")]'); @@ -79,6 +85,7 @@ describe('www-integration join flow', () => { test('username validation: bad word', async () => { const textInput = await findByXpath('//input[contains(@name, "username")]'); await textInput.click(); + await driver.wait(() => driver.executeScript('return document.activeElement == arguments[0]', textInput)); // Should be caught by the filter await textInput.sendKeys('xxxxxxxxx'); await clickXpath('//div[@class = "join-flow-outer-content"]'); diff --git a/test/integration/selenium-helpers.js b/test/integration/selenium-helpers.js index 0a8638e6d..888ce270b 100644 --- a/test/integration/selenium-helpers.js +++ b/test/integration/selenium-helpers.js @@ -136,8 +136,10 @@ class SeleniumHelper { 'getLogs', 'getSauceDriver', 'isSignedIn', + 'navigate', 'signIn', 'urlMatches', + 'waitUntilDocumentReady', 'waitUntilGone' ]); @@ -248,6 +250,36 @@ class SeleniumHelper { return Key[keyName]; } + /** + * Wait until the document is ready (i.e. the document.readyState is 'complete') + * @returns {Promise} A promise that resolves when the document is ready + */ + async waitUntilDocumentReady () { + const outerError = new SeleniumHelperError('waitUntilDocumentReady failed'); + try { + await this.driver.wait(async () => + await this.driver.executeScript('return document.readyState;') === 'complete' + ); + } catch (cause) { + throw await outerError.chain(cause, this.driver); + } + } + + /** + * Navigate to the given URL and wait until the document is ready + * @param {string} url The URL to navigate to. + * @returns {Promise} A promise that resolves when the document is ready + */ + async navigate (url) { + const outerError = new SeleniumHelperError('navigate failed', [{url}]); + try { + await this.driver.get(url); + await this.waitUntilDocumentReady(); + } catch (cause) { + throw await outerError.chain(cause, this.driver); + } + } + /** * Find an element by xpath. * @param {string} xpath The xpath to search for. @@ -287,29 +319,33 @@ class SeleniumHelper { return this.driver.wait(async () => { const elementAtPath = await this.findByXpath(xpath); if (!elementAtPath) { - return; + return null; } if (allowScrolling) { - await this.driver.executeScript( + const info = await this.driver.executeScript( ` + const info = {}; const element = arguments[0]; const boundingRect = element.getBoundingClientRect(); - boundingRect.windowWidth = window.innerWidth; - boundingRect.windowHeight = window.innerHeight; if (boundingRect.top < 0 || boundingRect.bottom > window.innerHeight || boundingRect.left < 0 || boundingRect.right > window.innerWidth) { - boundingRect.scrollIntoView = true; element.scrollIntoView({ behavior: 'instant', block:'nearest', inline: 'nearest' }); + info.didScroll = true; } + return info; `, elementAtPath ); + if (info.didScroll) { + // try again after the scroll completes + return null; + } } const elementAtPoint = await this.driver.executeScript( @@ -323,7 +359,7 @@ class SeleniumHelper { elementAtPath ); if (!elementAtPoint) { - return; + return null; } // If we ask to click on a button and Selenium finds an image on the button, or vice versa, that's OK. // It doesn't have to be an exact match. @@ -333,13 +369,13 @@ class SeleniumHelper { elementAtPoint ); if (!match) { - return; + return null; } if (!await elementAtPath.isDisplayed()) { - return; + return null; } if (!await elementAtPath.isEnabled()) { - return; + return null; } return elementAtPath; }); @@ -348,13 +384,15 @@ class SeleniumHelper { /** * Wait until an element can be found by the provided xpath, then click on it. * @param {string} xpath The xpath to click. + * @param {boolean} [allowScrolling] Whether or not to allow page scrolling to reach the element. * @returns {Promise} A promise that resolves when the element is clicked. */ - async clickXpath (xpath) { + async clickXpath (xpath, allowScrolling = true) { const outerError = new SeleniumHelperError('clickXpath failed', [{xpath}]); try { - const element = await this.waitUntilClickable(xpath); + const element = await this.waitUntilClickable(xpath, allowScrolling); element.click(); + return element; } catch (cause) { throw await outerError.chain(cause, this.driver); } From 88a7eb9c88398f6caed038c9e35781d3a4028b97 Mon Sep 17 00:00:00 2001 From: Christopher Willis-Ford <7019101+cwillisf@users.noreply.github.com> Date: Fri, 22 Sep 2023 09:13:34 -0700 Subject: [PATCH 18/33] test: make timeouts consistent, general cleanup --- test/integration/selenium-helpers.js | 107 +++++++++++---------------- 1 file changed, 45 insertions(+), 62 deletions(-) diff --git a/test/integration/selenium-helpers.js b/test/integration/selenium-helpers.js index 888ce270b..2b5e87235 100644 --- a/test/integration/selenium-helpers.js +++ b/test/integration/selenium-helpers.js @@ -233,8 +233,7 @@ class SeleniumHelper { accessKey: accessKey, name: name }) - .usingServer(`http://${username}:${accessKey - }@ondemand.saucelabs.com:80/wd/hub`) + .usingServer(`http://${username}:${accessKey}@ondemand.saucelabs.com:80/wd/hub`) .build(); return driver; } @@ -257,8 +256,9 @@ class SeleniumHelper { async waitUntilDocumentReady () { const outerError = new SeleniumHelperError('waitUntilDocumentReady failed'); try { - await this.driver.wait(async () => - await this.driver.executeScript('return document.readyState;') === 'complete' + await this.driver.wait( + async () => await this.driver.executeScript('return document.readyState;') === 'complete', + DEFAULT_TIMEOUT_MILLISECONDS ); } catch (cause) { throw await outerError.chain(cause, this.driver); @@ -315,70 +315,52 @@ class SeleniumHelper { * @returns {Promise} A promise that resolves to the clickable element. */ waitUntilClickable (xpath, allowScrolling = true) { - // @ts-ignore - TS can't tell that `driver.wait()` called this way will never return `undefined`. - return this.driver.wait(async () => { - const elementAtPath = await this.findByXpath(xpath); - if (!elementAtPath) { - return null; - } + return this.driver.wait(new webdriver.WebElementCondition( + 'for element to be clickable', + async () => { + const elementAtPath = await this.findByXpath(xpath); + if (!elementAtPath) { + return null; + } - if (allowScrolling) { - const info = await this.driver.executeScript( + if (allowScrolling) { + await this.driver.actions() + .move({origin: elementAtPath}) + .perform(); + } + + const elementAtPoint = await this.driver.executeScript( ` - const info = {}; - const element = arguments[0]; - const boundingRect = element.getBoundingClientRect(); - if (boundingRect.top < 0 || boundingRect.bottom > window.innerHeight || - boundingRect.left < 0 || boundingRect.right > window.innerWidth) - { - element.scrollIntoView({ - behavior: 'instant', - block:'nearest', - inline: 'nearest' - }); - info.didScroll = true; - } - return info; + const rect = arguments[0].getBoundingClientRect(); + return document.elementFromPoint( + rect.x + rect.width / 2, + rect.y + rect.height / 2 + ); `, elementAtPath ); - if (info.didScroll) { - // try again after the scroll completes + if (!elementAtPoint) { return null; } - } - - const elementAtPoint = await this.driver.executeScript( - ` - const rect = arguments[0].getBoundingClientRect(); - return document.elementFromPoint( - rect.x + rect.width / 2, - rect.y + rect.height / 2 + // If we ask to click on a button and Selenium finds an image on the button, or vice versa, that's OK. + // It doesn't have to be an exact match. + const match = await this.driver.executeScript( + 'return arguments[0].contains(arguments[1]) || arguments[1].contains(arguments[0])', + elementAtPath, + elementAtPoint ); - `, - elementAtPath - ); - if (!elementAtPoint) { - return null; + if (!match) { + return null; + } + if (!await elementAtPath.isDisplayed()) { + return null; + } + if (!await elementAtPath.isEnabled()) { + return null; + } + return elementAtPath; } - // If we ask to click on a button and Selenium finds an image on the button, or vice versa, that's OK. - // It doesn't have to be an exact match. - const match = await this.driver.executeScript( - 'return arguments[0].contains(arguments[1]) || arguments[1].contains(arguments[0])', - elementAtPath, - elementAtPoint - ); - if (!match) { - return null; - } - if (!await elementAtPath.isDisplayed()) { - return null; - } - if (!await elementAtPath.isEnabled()) { - return null; - } - return elementAtPath; - }); + ), DEFAULT_TIMEOUT_MILLISECONDS); } /** @@ -512,8 +494,8 @@ class SeleniumHelper { async isSignedIn () { const outerError = new SeleniumHelperError('isSignedIn failed'); try { - const state = await this.driver.wait(() => - this.driver.executeScript( + const state = await this.driver.wait( + () => this.driver.executeScript( ` if (document.evaluate(arguments[0], document, null, XPathResult.FIRST_ORDERED_NODE_TYPE, null) .singleNodeValue) { @@ -526,7 +508,8 @@ class SeleniumHelper { `, this.getPathForProfileName(), this.getPathForLogin() - ) + ), + DEFAULT_TIMEOUT_MILLISECONDS ); switch (state) { case 'signed in': From a68e3aef34a6c3ec862c9e7b8344015a45f5b3b3 Mon Sep 17 00:00:00 2001 From: Christopher Willis-Ford <7019101+cwillisf@users.noreply.github.com> Date: Fri, 22 Sep 2023 13:32:06 -0700 Subject: [PATCH 19/33] test: don't test external conference site --- test/integration/footer-links.test.js | 14 ++------------ 1 file changed, 2 insertions(+), 12 deletions(-) diff --git a/test/integration/footer-links.test.js b/test/integration/footer-links.test.js index d55417f2d..b001d8b9d 100644 --- a/test/integration/footer-links.test.js +++ b/test/integration/footer-links.test.js @@ -159,20 +159,9 @@ describe('www-integration footer links', () => { const pathname = (new URL(url)).pathname; expect(pathname).toMatch(/^\/DMCA\/?$/); }); - - // ==== SCRATCH FAMILY column ==== - - test('click Scratch Conference link', async () => { - await clickText('Scratch Conference'); - await waitUntilDocumentReady(); - const url = await driver.getCurrentUrl(); - const pathname = (new URL(url)).pathname; - expect(pathname).toMatch(/^\/scratch-conference\/?$/); - }); - }); -// The following links in are skipped because they are not on scratch.mit.edu +// The following links in the footer are skipped because they are not part of scratch-www // Jobs // Press @@ -183,3 +172,4 @@ describe('www-integration footer links', () => { // SCRATCH JR (SCRATCHJR) // SCRATCH DAY // SCRATCH FOUNDATION +// Scratch Conference From 621861ee403846b8f07eeb6fa32ed72cb360b41a Mon Sep 17 00:00:00 2001 From: Christopher Willis-Ford <7019101+cwillisf@users.noreply.github.com> Date: Fri, 22 Sep 2023 13:34:06 -0700 Subject: [PATCH 20/33] test: simplify clickXpath() by letting click() scroll Instead of trying to proactively scroll if necessary, just let Selenium handle scrolling. The only reason to handle scrolling manually was to tell the difference between an off-screen element and one that isn't visible due to being beneath another element (like the loading screen). Catching `ElementClickInterceptedError` is a simpler way to do that. --- test/integration/selenium-helpers.js | 81 ++++++++-------------------- 1 file changed, 22 insertions(+), 59 deletions(-) diff --git a/test/integration/selenium-helpers.js b/test/integration/selenium-helpers.js index 2b5e87235..a1371c75c 100644 --- a/test/integration/selenium-helpers.js +++ b/test/integration/selenium-helpers.js @@ -309,72 +309,35 @@ class SeleniumHelper { } } - /** - * @param {string} xpath Wait until an element at the provided xpath is clickable. - * @param {boolean} [allowScrolling] Whether or not to allow page scrolling to reach the element. - * @returns {Promise} A promise that resolves to the clickable element. - */ - waitUntilClickable (xpath, allowScrolling = true) { - return this.driver.wait(new webdriver.WebElementCondition( - 'for element to be clickable', - async () => { - const elementAtPath = await this.findByXpath(xpath); - if (!elementAtPath) { - return null; - } - - if (allowScrolling) { - await this.driver.actions() - .move({origin: elementAtPath}) - .perform(); - } - - const elementAtPoint = await this.driver.executeScript( - ` - const rect = arguments[0].getBoundingClientRect(); - return document.elementFromPoint( - rect.x + rect.width / 2, - rect.y + rect.height / 2 - ); - `, - elementAtPath - ); - if (!elementAtPoint) { - return null; - } - // If we ask to click on a button and Selenium finds an image on the button, or vice versa, that's OK. - // It doesn't have to be an exact match. - const match = await this.driver.executeScript( - 'return arguments[0].contains(arguments[1]) || arguments[1].contains(arguments[0])', - elementAtPath, - elementAtPoint - ); - if (!match) { - return null; - } - if (!await elementAtPath.isDisplayed()) { - return null; - } - if (!await elementAtPath.isEnabled()) { - return null; - } - return elementAtPath; - } - ), DEFAULT_TIMEOUT_MILLISECONDS); - } - /** * Wait until an element can be found by the provided xpath, then click on it. * @param {string} xpath The xpath to click. - * @param {boolean} [allowScrolling] Whether or not to allow page scrolling to reach the element. * @returns {Promise} A promise that resolves when the element is clicked. */ - async clickXpath (xpath, allowScrolling = true) { + async clickXpath (xpath) { const outerError = new SeleniumHelperError('clickXpath failed', [{xpath}]); try { - const element = await this.waitUntilClickable(xpath, allowScrolling); - element.click(); - return element; + return await this.driver.wait(new webdriver.WebElementCondition( + 'for element click to succeed', + async () => { + const element = await this.findByXpath(xpath); + if (!element) { + return null; + } + try { + await element.click(); + return element; + } catch (e) { + if (e instanceof webdriver.error.ElementClickInterceptedError) { + // something is in front of the element we want to click + // probably the loading screen + // this is the main reason for using wait() + return null; + } + throw e; + } + } + ), DEFAULT_TIMEOUT_MILLISECONDS); } catch (cause) { throw await outerError.chain(cause, this.driver); } From 103827ce19cc37d522a466e39b97da265f9842cf Mon Sep 17 00:00:00 2001 From: Christopher Willis-Ford <7019101+cwillisf@users.noreply.github.com> Date: Fri, 22 Sep 2023 18:20:51 -0700 Subject: [PATCH 21/33] test: fix homepage-rows.test.js flakiness --- test/integration/homepage-rows.test.js | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/test/integration/homepage-rows.test.js b/test/integration/homepage-rows.test.js index c180eda58..bc34f0bd5 100644 --- a/test/integration/homepage-rows.test.js +++ b/test/integration/homepage-rows.test.js @@ -3,9 +3,11 @@ const SeleniumHelper = require('./selenium-helpers.js'); const { + buildDriver, clickXpath, findByXpath, - buildDriver + navigate, + waitUntilDocumentReady } = new SeleniumHelper(); const rootUrl = process.env.ROOT_URL || 'https://scratch.ly'; @@ -21,7 +23,7 @@ describe('www-integration project rows', () => { }); beforeEach(async () => { - await driver.get(rootUrl); + await navigate(rootUrl); }); afterAll(() => driver.quit()); @@ -49,6 +51,7 @@ describe('www-integration project rows', () => { test('Featured Studios link', async () => { await clickXpath('//div[@class="box"][descendant::text()="Featured Studios"]' + '//div[contains(@class, "thumbnail")][1]/a[@class="thumbnail-image"]'); + await waitUntilDocumentReady(); const studioInfo = await findByXpath('//div[contains(@class, "studio-info")]'); const studioInfoDisplayed = await studioInfo.isDisplayed(); expect(studioInfoDisplayed).toBe(true); From c5bbbae71094dfee6ca29e8fae804dae2a0cdb7b Mon Sep 17 00:00:00 2001 From: Christopher Willis-Ford <7019101+cwillisf@users.noreply.github.com> Date: Fri, 22 Sep 2023 18:30:31 -0700 Subject: [PATCH 22/33] test: use navigate() in studios-page test, plus minor cleanup --- test/integration/studios-page.test.js | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/test/integration/studios-page.test.js b/test/integration/studios-page.test.js index b52e6e678..67aaf0835 100644 --- a/test/integration/studios-page.test.js +++ b/test/integration/studios-page.test.js @@ -3,11 +3,12 @@ import SeleniumHelper from './selenium-helpers.js'; const { - findByXpath, buildDriver, - clickXpath, clickText, + clickXpath, + findByXpath, isSignedIn, + navigate, signIn } = new SeleniumHelper(); @@ -34,11 +35,10 @@ describe('studio page while signed out', () => { beforeAll(async () => { // expect(projectUrl).toBe(defined); driver = await buildDriver('www-integration studio-page signed out'); - await driver.get(rootUrl); }); beforeEach(async () => { - await driver.get(studioUrl); + await navigate(studioUrl); const studioNav = await findByXpath('//div[@class="studio-tabs"]'); await studioNav.isDisplayed(); }); @@ -46,7 +46,7 @@ describe('studio page while signed out', () => { afterAll(() => driver.quit()); test('land on projects tab', async () => { - await driver.get(studioUrl); + await navigate(studioUrl); const projectGrid = await findByXpath('//div[@class="studio-projects-grid"]'); const projectGridDisplayed = await projectGrid.isDisplayed(); expect(projectGridDisplayed).toBe(true); @@ -71,13 +71,13 @@ describe('studio management', () => { beforeAll(async () => { driver = await buildDriver('www-integration studio management'); - await driver.get(rootUrl); + await navigate(rootUrl); // create a studio for tests await signIn(username2, password); await findByXpath('//span[contains(@class, "profile-name")]'); - await driver.get(rateLimitCheck); - await driver.get(myStuffURL); + await navigate(rateLimitCheck); + await navigate(myStuffURL); await clickXpath('//form[@id="new_studio"]/button[@type="submit"]'); await findByXpath('//div[@class="studio-tabs"]'); promoteStudioURL = await driver.getCurrentUrl(); @@ -96,7 +96,7 @@ describe('studio management', () => { test('invite a curator', async () => { // sign in as user2 await signIn(username2, password); - await driver.get(curatorTab); + await navigate(curatorTab); // invite user3 to curate const inviteBox = await findByXpath('//div[@class="studio-adder-row"]/input'); @@ -111,7 +111,7 @@ describe('studio management', () => { test('accept curator invite', async () => { // Sign in user3 await signIn(username3, password); - await driver.get(curatorTab); + await navigate(curatorTab); // accept the curator invite await clickXpath('//button[@class="studio-invitation-button button"]'); @@ -126,7 +126,7 @@ describe('studio management', () => { await findByXpath('//span[contains(@class, "profile-name")]'); // for some reason the user isn't showing up without waiting and reloading the page await driver.sleep(2000); - await driver.get(curatorTab); + await navigate(curatorTab); // promote user3 const user3href = `/users/${username3}`; @@ -149,7 +149,7 @@ describe('studio management', () => { await signIn(username2, password); await findByXpath('//span[contains(@class, "profile-name")]'); // for some reason the user isn't showing up without reloading the page - await driver.get(curatorTab); + await navigate(curatorTab); // open kebab menu const user2href = `/users/${username2}`; From a0d8129356e21473dba3a527b66cd594a4d56535 Mon Sep 17 00:00:00 2001 From: Christopher Willis-Ford <7019101+cwillisf@users.noreply.github.com> Date: Fri, 22 Sep 2023 22:36:37 -0700 Subject: [PATCH 23/33] test: make my-stuff test wait for page loads --- test/integration/my-stuff.test.js | 21 ++++++++++++--------- 1 file changed, 12 insertions(+), 9 deletions(-) diff --git a/test/integration/my-stuff.test.js b/test/integration/my-stuff.test.js index 92a895a19..01023f576 100644 --- a/test/integration/my-stuff.test.js +++ b/test/integration/my-stuff.test.js @@ -7,7 +7,9 @@ const { clickText, clickXpath, findByXpath, - signIn + signIn, + waitUntilDocumentReady, + urlMatches } = new SeleniumHelper(); const username = `${process.env.SMOKE_USERNAME}1`; @@ -25,7 +27,6 @@ describe('www-integration my_stuff', () => { beforeAll(async () => { driver = await buildDriver('www-integration my_stuff'); await driver.get(rootUrl); - await driver.sleep(1000); await signIn(username, password); await findByXpath('//span[contains(@class, "profile-name")]'); }); @@ -57,7 +58,7 @@ describe('www-integration my_stuff', () => { test('clicking a project title should take you to the project page', async () => { await driver.get(myStuffURL); await clickXpath('//span[@class="media-info-item title"]'); - await driver.sleep(6000); + await waitUntilDocumentReady(); const gui = await findByXpath('//div[@class="guiPlayer"]'); const guiVisible = await gui.isDisplayed(); expect(guiVisible).toBe(true); @@ -66,6 +67,7 @@ describe('www-integration my_stuff', () => { test('clicking "see inside" should take you to the editor', async () => { await driver.get(myStuffURL); await clickXpath('//a[@data-control="edit"]'); + await waitUntilDocumentReady(); const gf = await findByXpath('//img[@class="green-flag_green-flag_1kiAo"]'); const gfVisible = await gf.isDisplayed(); expect(gfVisible).toBe(true); @@ -83,6 +85,7 @@ describe('www-integration my_stuff', () => { test('+ New Project button should open the editor', async () => { await driver.get(myStuffURL); await clickText('+ New Project'); + await waitUntilDocumentReady(); const gf = await findByXpath('//img[@class="green-flag_green-flag_1kiAo"]'); const gfVisible = await gf.isDisplayed(); expect(gfVisible).toBe(true); @@ -92,6 +95,7 @@ describe('www-integration my_stuff', () => { await driver.get(rateLimitCheck); await driver.get(myStuffURL); await clickXpath('//form[@id="new_studio"]/button[@type="submit"]'); + await waitUntilDocumentReady(); const tabs = await findByXpath('//div[@class="studio-tabs"]'); const tabsVisible = await tabs.isDisplayed(); expect(tabsVisible).toBe(true); @@ -102,23 +106,23 @@ describe('www-integration my_stuff', () => { // 1st studio await driver.get(myStuffURL); await clickXpath('//form[@id="new_studio"]/button[@type="submit"]'); - await findByXpath('//div[@class="studio-tabs"]'); + await urlMatches(/\/studios\//); // 2nd studio await driver.get(myStuffURL); await clickXpath('//form[@id="new_studio"]/button[@type="submit"]'); - await findByXpath('//div[@class="studio-tabs"]'); + await urlMatches(/\/studios\//); // 3rd studio await driver.get(myStuffURL); await clickXpath('//form[@id="new_studio"]/button[@type="submit"]'); - await findByXpath('//div[@class="studio-tabs"]'); + await urlMatches(/\/studios\//); // 4th studio await driver.get(myStuffURL); await clickXpath('//form[@id="new_studio"]/button[@type="submit"]'); - await findByXpath('//div[@class="studio-tabs"]'); + await urlMatches(/\/studios\//); // 5th studio await driver.get(myStuffURL); await clickXpath('//form[@id="new_studio"]/button[@type="submit"]'); - await findByXpath('//div[@class="studio-tabs"]'); + await urlMatches(/\/studios\//); // 6th studio should fail await driver.get(myStuffURL); await clickXpath('//form[@id="new_studio"]/button[@type="submit"]'); @@ -128,5 +132,4 @@ describe('www-integration my_stuff', () => { await driver.get(rateLimitCheck); }); - }); From c9f67c06cf8ec8d29609ee6d42a3780a43293f93 Mon Sep 17 00:00:00 2001 From: Christopher Willis-Ford <7019101+cwillisf@users.noreply.github.com> Date: Mon, 25 Sep 2023 08:34:30 -0700 Subject: [PATCH 24/33] test: use navigate() instead of driver.get() --- test/integration/comments.test.js | 35 +++++++++++----------- test/integration/footer-links.test.js | 3 +- test/integration/homepage-rows.test.js | 2 +- test/integration/my-stuff.test.js | 37 ++++++++++++------------ test/integration/navbar.test.js | 5 ++-- test/integration/project-page.test.js | 29 ++++++++++--------- test/integration/search.test.js | 5 ++-- test/integration/selenium-helpers.js | 5 +++- test/integration/sign-in-and-out.test.js | 17 ++++++----- test/integration/statistics.test.js | 5 ++-- 10 files changed, 77 insertions(+), 66 deletions(-) diff --git a/test/integration/comments.test.js b/test/integration/comments.test.js index 10a02412d..191e3ddbb 100644 --- a/test/integration/comments.test.js +++ b/test/integration/comments.test.js @@ -8,6 +8,7 @@ const { clickXpath, containsClass, findByXpath, + navigate, signIn } = new SeleniumHelper(); @@ -48,7 +49,7 @@ let driver; describe('comment tests', () => { beforeAll(async () => { driver = await buildDriver('www-integration project comments'); - await driver.get(rootUrl); + await navigate(rootUrl); }); afterAll(() => driver.quit()); @@ -60,13 +61,13 @@ describe('comment tests', () => { }); afterAll(async () => { - await driver.get(rootUrl); + await navigate(rootUrl); await clickXpath('//a[contains(@class, "user-info")]'); await clickText('Sign out'); }); test('leave comment on project', async () => { - await driver.get(projectUrl); + await navigate(projectUrl); // leave the comment const commentBox = await findByXpath('//textArea[@name="compose-comment"]'); @@ -82,7 +83,7 @@ describe('comment tests', () => { }); test('leave comment on a profile', async () => { - await driver.get(profileUrl); + await navigate(profileUrl); // leave the comment const commentXpath = '//form[@id="main-post-form"]/div/textArea'; @@ -97,11 +98,11 @@ describe('comment tests', () => { expect(commentVisible).toBe(true); // return to homepage to sign out with www - await driver.get(rootUrl); + await navigate(rootUrl); }); test('leave comment on studio', async () => { - await driver.get(studioUrl); + await navigate(studioUrl); // leave the comment const commentBox = await findByXpath('//textArea[@name="compose-comment"]'); @@ -138,7 +139,7 @@ describe('comment tests', () => { }); test('project comment message visible', async () => { - await driver.get(`${rootUrl}/messages`); + await navigate(`${rootUrl}/messages`); const projectMessageXpath = '//p[@class="emoji-text mod-comment" ' + `and contains(text(), "${projectComment}")]`; @@ -148,7 +149,7 @@ describe('comment tests', () => { }); test('profile comment message visible', async () => { - await driver.get(`${rootUrl}/messages`); + await navigate(`${rootUrl}/messages`); const profileMessageXpath = '//p[@class="emoji-text mod-comment" ' + `and contains(text(), "${profileComment}")]`; @@ -163,7 +164,7 @@ describe('comment tests', () => { const projectLinkXpath = '//p[@class="emoji-text mod-comment" ' + `and contains(text(), "${projectComment}")]/../../../p[@class = "comment-message-info"]/span/a[2]`; - await driver.get(`${rootUrl}/messages`); + await navigate(`${rootUrl}/messages`); await clickXpath(projectLinkXpath); // find green flag overlay @@ -175,7 +176,7 @@ describe('comment tests', () => { const projectLinkXpath = '//p[@class="emoji-text mod-comment" ' + `and contains(text(), "${projectComment}")]/../../../p[@class = "comment-message-info"]/span/a[2]`; - await driver.get(`${rootUrl}/messages`); + await navigate(`${rootUrl}/messages`); await clickXpath(projectLinkXpath); const commentXpath = `//span[contains(text(), "${projectComment}")]`; @@ -189,7 +190,7 @@ describe('comment tests', () => { `and contains(text(), "${projectComment}")]/../../../p[@class = "comment-message-info"]/span/a[2]`; const containerXpath = `//span[contains(text(), "${projectComment}")]/../../../..`; - await driver.get(`${rootUrl}/messages`); + await navigate(`${rootUrl}/messages`); await clickXpath(projectLinkXpath); const commentContainer = await findByXpath(containerXpath); @@ -201,7 +202,7 @@ describe('comment tests', () => { const profileLinkXpath = '//p[@class="emoji-text mod-comment" ' + `and contains(text(), "${profileComment}")]/../../../` + 'p[@class = "comment-message-info"]/span/a[2]'; - await driver.get(`${rootUrl}/messages`); + await navigate(`${rootUrl}/messages`); await clickXpath(profileLinkXpath); // find profile data @@ -218,7 +219,7 @@ describe('comment tests', () => { const profileLinkXpath = '//p[@class="emoji-text mod-comment" ' + `and contains(text(), "${profileComment}")]/../../../` + 'p[@class = "comment-message-info"]/span/a[2]'; - await driver.get(`${rootUrl}/messages`); + await navigate(`${rootUrl}/messages`); await clickXpath(profileLinkXpath); // find comment @@ -233,7 +234,7 @@ describe('comment tests', () => { const profileLinkXpath = '//p[@class="emoji-text mod-comment" ' + `and contains(text(), "${profileComment}")]/../../../` + 'p[@class = "comment-message-info"]/span/a[2]'; - await driver.get(`${rootUrl}/messages`); + await navigate(`${rootUrl}/messages`); await clickXpath(profileLinkXpath); // comment highlighted? @@ -244,7 +245,7 @@ describe('comment tests', () => { }); test('project: reply to comment', async () => { - await driver.get(projectUrl); + await navigate(projectUrl); const commentXpath = `//span[contains(text(), "${projectComment}")]/../..`; const replyXpath = `${commentXpath}//span[@class = "comment-reply"]`; await clickXpath(replyXpath); @@ -264,7 +265,7 @@ describe('comment tests', () => { }); test('profile reply to comment', async () => { - await driver.get(profileUrl); + await navigate(profileUrl); // find the comment and click reply const commentXpath = `//div[contains(text(), "${profileComment}")]/..`; await clickXpath(`${commentXpath}//a[@class = "reply"]`); @@ -281,7 +282,7 @@ describe('comment tests', () => { }); test('studio: reply to comment', async () => { - await driver.get(studioUrl); + await navigate(studioUrl); // find the comment and click reply const commentXpath = `//span[contains(text(), "${studioComment}")]/../..`; diff --git a/test/integration/footer-links.test.js b/test/integration/footer-links.test.js index b001d8b9d..a5b6de28d 100644 --- a/test/integration/footer-links.test.js +++ b/test/integration/footer-links.test.js @@ -6,6 +6,7 @@ const { clickText, buildDriver, findText, + navigate, waitUntilDocumentReady } = new SeleniumHelper(); @@ -21,7 +22,7 @@ describe('www-integration footer links', () => { }); beforeEach(async () => { - await driver.get(rootUrl); + await navigate(rootUrl); await findText('Create stories, games, and animations'); }); diff --git a/test/integration/homepage-rows.test.js b/test/integration/homepage-rows.test.js index bc34f0bd5..d0cfcf0f6 100644 --- a/test/integration/homepage-rows.test.js +++ b/test/integration/homepage-rows.test.js @@ -19,7 +19,7 @@ let driver; describe('www-integration project rows', () => { beforeAll(async () => { driver = await buildDriver('www-integration project rows'); - // driver.get(rootUrl); + // navigate(rootUrl); }); beforeEach(async () => { diff --git a/test/integration/my-stuff.test.js b/test/integration/my-stuff.test.js index 01023f576..040246567 100644 --- a/test/integration/my-stuff.test.js +++ b/test/integration/my-stuff.test.js @@ -7,9 +7,10 @@ const { clickText, clickXpath, findByXpath, + navigate, signIn, - waitUntilDocumentReady, - urlMatches + urlMatches, + waitUntilDocumentReady } = new SeleniumHelper(); const username = `${process.env.SMOKE_USERNAME}1`; @@ -26,7 +27,7 @@ let driver; describe('www-integration my_stuff', () => { beforeAll(async () => { driver = await buildDriver('www-integration my_stuff'); - await driver.get(rootUrl); + await navigate(rootUrl); await signIn(username, password); await findByXpath('//span[contains(@class, "profile-name")]'); }); @@ -34,7 +35,7 @@ describe('www-integration my_stuff', () => { afterAll(() => driver.quit()); test('verify My Stuff structure (tabs, title)', async () => { - await driver.get(myStuffURL); + await navigate(myStuffURL); const header = await findByXpath('//div[@class="box-head"]/h2'); const headerVisible = await header.isDisplayed(); expect(headerVisible).toBe(true); @@ -56,7 +57,7 @@ describe('www-integration my_stuff', () => { }); test('clicking a project title should take you to the project page', async () => { - await driver.get(myStuffURL); + await navigate(myStuffURL); await clickXpath('//span[@class="media-info-item title"]'); await waitUntilDocumentReady(); const gui = await findByXpath('//div[@class="guiPlayer"]'); @@ -65,7 +66,7 @@ describe('www-integration my_stuff', () => { }); test('clicking "see inside" should take you to the editor', async () => { - await driver.get(myStuffURL); + await navigate(myStuffURL); await clickXpath('//a[@data-control="edit"]'); await waitUntilDocumentReady(); const gf = await findByXpath('//img[@class="green-flag_green-flag_1kiAo"]'); @@ -74,7 +75,7 @@ describe('www-integration my_stuff', () => { }); test('Add To button should bring up a list of studios', async () => { - await driver.get(myStuffURL); + await navigate(myStuffURL); await clickXpath('//div[@id="sidebar"]/ul/li[@data-tab="shared"]'); await clickXpath('//div[@data-control="add-to"]'); const dropDown = await findByXpath('//div[@class="dropdown-menu"]/ul/li'); @@ -83,7 +84,7 @@ describe('www-integration my_stuff', () => { }); test('+ New Project button should open the editor', async () => { - await driver.get(myStuffURL); + await navigate(myStuffURL); await clickText('+ New Project'); await waitUntilDocumentReady(); const gf = await findByXpath('//img[@class="green-flag_green-flag_1kiAo"]'); @@ -92,8 +93,8 @@ describe('www-integration my_stuff', () => { }); test('+ New Studio button should take you to the studio page', async () => { - await driver.get(rateLimitCheck); - await driver.get(myStuffURL); + await navigate(rateLimitCheck); + await navigate(myStuffURL); await clickXpath('//form[@id="new_studio"]/button[@type="submit"]'); await waitUntilDocumentReady(); const tabs = await findByXpath('//div[@class="studio-tabs"]'); @@ -102,34 +103,34 @@ describe('www-integration my_stuff', () => { }); test('New studio rate limited to five', async () => { - await driver.get(rateLimitCheck); + await navigate(rateLimitCheck); // 1st studio - await driver.get(myStuffURL); + await navigate(myStuffURL); await clickXpath('//form[@id="new_studio"]/button[@type="submit"]'); await urlMatches(/\/studios\//); // 2nd studio - await driver.get(myStuffURL); + await navigate(myStuffURL); await clickXpath('//form[@id="new_studio"]/button[@type="submit"]'); await urlMatches(/\/studios\//); // 3rd studio - await driver.get(myStuffURL); + await navigate(myStuffURL); await clickXpath('//form[@id="new_studio"]/button[@type="submit"]'); await urlMatches(/\/studios\//); // 4th studio - await driver.get(myStuffURL); + await navigate(myStuffURL); await clickXpath('//form[@id="new_studio"]/button[@type="submit"]'); await urlMatches(/\/studios\//); // 5th studio - await driver.get(myStuffURL); + await navigate(myStuffURL); await clickXpath('//form[@id="new_studio"]/button[@type="submit"]'); await urlMatches(/\/studios\//); // 6th studio should fail - await driver.get(myStuffURL); + await navigate(myStuffURL); await clickXpath('//form[@id="new_studio"]/button[@type="submit"]'); const alertMessage = await findByXpath('//div[contains(@class, "alert-error")]'); const errVisible = await alertMessage.isDisplayed(); expect(errVisible).toBe(true); - await driver.get(rateLimitCheck); + await navigate(rateLimitCheck); }); }); diff --git a/test/integration/navbar.test.js b/test/integration/navbar.test.js index 1f372f5ed..13f41d6d5 100644 --- a/test/integration/navbar.test.js +++ b/test/integration/navbar.test.js @@ -4,8 +4,9 @@ const SeleniumHelper = require('./selenium-helpers.js'); const { clickXpath, + buildDriver, findByXpath, - buildDriver + navigate } = new SeleniumHelper(); const rootUrl = process.env.ROOT_URL || 'https://scratch.ly'; @@ -20,7 +21,7 @@ describe('www-integration navbar links', () => { }); beforeEach(async () => { - await driver.get(rootUrl); + await navigate(rootUrl); }); afterAll(() => driver.quit()); diff --git a/test/integration/project-page.test.js b/test/integration/project-page.test.js index 873f7f8de..b63cb685d 100644 --- a/test/integration/project-page.test.js +++ b/test/integration/project-page.test.js @@ -14,6 +14,7 @@ const { findByXpath, isSignedIn, signIn, + navigate, waitUntilVisible } = new SeleniumHelper(); @@ -54,11 +55,11 @@ describe('www-integration project-page signed out', () => { beforeAll(async () => { // expect(projectUrl).toBe(defined); driver = await buildDriver('www-integration project-page signed out'); - await driver.get(rootUrl); + await navigate(rootUrl); }); beforeEach(async () => { - await driver.get(unownedSharedUrl); + await navigate(unownedSharedUrl); const gfOverlay = await findByXpath('//div[@class="stage-wrapper_stage-wrapper_2bejr box_box_2jjDp"]'); await waitUntilVisible(gfOverlay, driver); }); @@ -105,7 +106,7 @@ describe('www-integration project-page signed out', () => { // Load an unshared project while signed out, get error test('Load an ushared project you do not own (error)', async () => { - await driver.get(unownedUnsharedUrl); + await navigate(unownedUnsharedUrl); const unavailableImage = await findByXpath('//img[@class="not-available-image"]'); await waitUntilVisible(unavailableImage, driver); const unavailableVisible = await unavailableImage.isDisplayed(); @@ -123,7 +124,7 @@ describe('www-integration project-page signed in', () => { beforeEach(async () => { // The browser may or may not retain cookies between tests, depending on configuration. - await driver.get(rootUrl); + await navigate(rootUrl); if (!await isSignedIn()) { await signIn(username, password); } @@ -135,7 +136,7 @@ describe('www-integration project-page signed in', () => { // Load a shared project you own test('Load a shared project you own', async () => { - await driver.get(ownedSharedUrl); + await navigate(ownedSharedUrl); const gfOverlay = await findByXpath('//div[@class="stage-wrapper_stage-wrapper_2bejr box_box_2jjDp"]'); await waitUntilVisible(gfOverlay, driver); const gfVisible = await gfOverlay.isDisplayed(); @@ -144,7 +145,7 @@ describe('www-integration project-page signed in', () => { // Load a shared project you don't own test('Load a shared project you do not own', async () => { - await driver.get(unownedSharedUrl); + await navigate(unownedSharedUrl); const gfOverlay = await findByXpath('//div[@class="stage-wrapper_stage-wrapper_2bejr box_box_2jjDp"]'); await waitUntilVisible(gfOverlay, driver); const gfVisible = await gfOverlay.isDisplayed(); @@ -153,7 +154,7 @@ describe('www-integration project-page signed in', () => { // Load an unshared project you own test('Load an unshared project you own', async () => { - await driver.get(ownedUnsharedUrl); + await navigate(ownedUnsharedUrl); const gfOverlay = await findByXpath('//div[@class="stage-wrapper_stage-wrapper_2bejr box_box_2jjDp"]'); await waitUntilVisible(gfOverlay, driver); const gfVisible = await gfOverlay.isDisplayed(); @@ -162,7 +163,7 @@ describe('www-integration project-page signed in', () => { // Load an unshared project you don't own, get error test('Load an ushared project you do not own (error)', async () => { - await driver.get(unownedUnsharedUrl); + await navigate(unownedUnsharedUrl); const unavailableImage = await findByXpath('//img[@class="not-available-image"]'); await waitUntilVisible(unavailableImage, driver); const unavailableVisible = await unavailableImage.isDisplayed(); @@ -171,7 +172,7 @@ describe('www-integration project-page signed in', () => { // Load a shared scratch 2 project you don't own test('Load a shared scratch 2 project you do not own', async () => { - await driver.get(unownedSharedScratch2Url); + await navigate(unownedSharedScratch2Url); const gfOverlay = await findByXpath('//div[@class="stage-wrapper_stage-wrapper_2bejr box_box_2jjDp"]'); await waitUntilVisible(gfOverlay, driver); const gfVisible = await gfOverlay.isDisplayed(); @@ -180,7 +181,7 @@ describe('www-integration project-page signed in', () => { // Load an unshared scratch 2 project you own test('Load an unshared scratch 2 project you own', async () => { - await driver.get(ownedUnsharedScratch2Url); + await navigate(ownedUnsharedScratch2Url); const gfOverlay = await findByXpath('//div[@class="stage-wrapper_stage-wrapper_2bejr box_box_2jjDp"]'); await waitUntilVisible(gfOverlay, driver); const gfVisible = await gfOverlay.isDisplayed(); @@ -195,7 +196,7 @@ describe('www-integration project-creation signed in', () => { // SauceLabs doesn't have access to the sb3 used in 'load project from file' test // https://support.saucelabs.com/hc/en-us/articles/115003685593-Uploading-Files-to-a-Sauce-Labs-Virtual-Machine-during-a-Test if (remote) { - await driver.get('https://github.com/scratchfoundation/scratch-www/blob/develop/test/fixtures/project1.sb3'); + await navigate('https://github.com/scratchfoundation/scratch-www/blob/develop/test/fixtures/project1.sb3'); await clickXpath('//button[@data-testid="download-raw-button"]'); await driver.sleep(3000); } @@ -203,7 +204,7 @@ describe('www-integration project-creation signed in', () => { beforeEach(async () => { // The browser may or may not retain cookies between tests, depending on configuration. - await driver.get(rootUrl); + await navigate(rootUrl); if (!await isSignedIn()) { await signIn(username, password); } @@ -212,7 +213,7 @@ describe('www-integration project-creation signed in', () => { afterAll(() => driver.quit()); test('make a copy of a project', async () => { - await driver.get(`${ownedUnsharedUrl}/editor`); + await navigate(`${ownedUnsharedUrl}/editor`); await clickXpath(FILE_MENU_XPATH); await clickText('Save as a copy'); const successAlert = await findText('Project saved as a copy.'); @@ -225,7 +226,7 @@ describe('www-integration project-creation signed in', () => { }); test('remix a project', async () => { - await driver.get(unownedSharedUrl); + await navigate(unownedSharedUrl); const gfOverlay = await findByXpath('//div[@class="stage-wrapper_stage-wrapper_2bejr box_box_2jjDp"]'); await waitUntilVisible(gfOverlay, driver); await clickXpath('//button[@class="button remix-button"]'); diff --git a/test/integration/search.test.js b/test/integration/search.test.js index 7342ceeda..e73f6e197 100644 --- a/test/integration/search.test.js +++ b/test/integration/search.test.js @@ -6,7 +6,8 @@ const { buildDriver, clickXpath, findByXpath, - getKey + getKey, + navigate } = new SeleniumHelper(); const rootUrl = process.env.ROOT_URL || 'https://scratch.ly'; @@ -21,7 +22,7 @@ describe('www-integration search', () => { }); beforeEach(async () => { - await driver.get(rootUrl); + await navigate(rootUrl); }); afterAll(() => driver.quit()); diff --git a/test/integration/selenium-helpers.js b/test/integration/selenium-helpers.js index a1371c75c..2cbd2249c 100644 --- a/test/integration/selenium-helpers.js +++ b/test/integration/selenium-helpers.js @@ -266,7 +266,10 @@ class SeleniumHelper { } /** - * Navigate to the given URL and wait until the document is ready + * Navigate to the given URL and wait until the document is ready. + * The Selenium docs say the promise returned by `driver.get()` "will be resolved when the document has finished + * loading." In practice, that doesn't mean the page is ready for testing. I suspect it comes down to the + * difference between "interactive" and "complete" (or `DOMContentLoaded` and `load`). * @param {string} url The URL to navigate to. * @returns {Promise} A promise that resolves when the document is ready */ diff --git a/test/integration/sign-in-and-out.test.js b/test/integration/sign-in-and-out.test.js index f993164fb..a1c392129 100644 --- a/test/integration/sign-in-and-out.test.js +++ b/test/integration/sign-in-and-out.test.js @@ -9,6 +9,7 @@ const { clickXpath, findByXpath, getKey, + navigate, signIn, waitUntilVisible } = new SeleniumHelper(); @@ -34,13 +35,13 @@ describe('www-integration sign-in-and-out', () => { describe('sign in', () => { afterEach(async () => { - await driver.get(wwwURL); + await navigate(wwwURL); await clickXpath('//div[@class="account-nav"]'); await clickText('Sign out'); }); test('sign in on www', async () => { - await driver.get(wwwURL); + await navigate(wwwURL); await driver.sleep(1000); await clickXpath('//li[@class="link right login-item"]/a'); const name = await findByXpath('//input[@id="frc-username-1088"]'); @@ -57,7 +58,7 @@ describe('www-integration sign-in-and-out', () => { }); test('sign in on scratchr2', async () => { - await driver.get(scratchr2url); + await navigate(scratchr2url); await clickXpath('//li[@class="sign-in dropdown"]/span'); const name = await findByXpath('//input[@id="login_dropdown_username"]'); await name.sendKeys(username); @@ -72,7 +73,7 @@ describe('www-integration sign-in-and-out', () => { describe('sign out', () => { beforeEach(async () => { - await driver.get(wwwURL); + await navigate(wwwURL); await signIn(username, password); await driver.sleep(500); }); @@ -86,7 +87,7 @@ describe('www-integration sign-in-and-out', () => { }); test('sign out on scratchr2', async () => { - await driver.get(scratchr2url); + await navigate(scratchr2url); await clickXpath('//span[@class="user-name dropdown-toggle"]'); await clickXpath('//li[@id="logout"]'); const element = await findByXpath('//li[@class="link right login-item"]/a/span'); @@ -101,7 +102,7 @@ describe('www-integration sign-in-and-out', () => { const nonsenseUsername = Math.random().toString(36) .replace(/[^a-z]+/g, '') .substr(0, 5); - await driver.get(scratchr2url); + await navigate(scratchr2url); await clickXpath('//li[@class="sign-in dropdown"]/span'); const name = await findByXpath('//input[@id="login_dropdown_username"]'); await name.sendKeys(nonsenseUsername + getKey('ENTER')); @@ -117,7 +118,7 @@ describe('www-integration sign-in-and-out', () => { const nonsenseUsername = Math.random().toString(36) .replace(/[^a-z]+/g, '') .substr(0, 5); - await driver.get(scratchr2url); + await navigate(scratchr2url); await clickXpath('//li[@class="sign-in dropdown"]/span'); const name = await findByXpath('//input[@id="login_dropdown_username"]'); await name.sendKeys(nonsenseUsername); @@ -135,7 +136,7 @@ describe('www-integration sign-in-and-out', () => { const nonsensePassword = Math.random().toString(36) .replace(/[^a-z]+/g, '') .substr(0, 5); - await driver.get(scratchr2url); + await navigate(scratchr2url); await clickXpath('//li[@class="sign-in dropdown"]/span'); const name = await findByXpath('//input[@id="login_dropdown_username"]'); await name.sendKeys(username); diff --git a/test/integration/statistics.test.js b/test/integration/statistics.test.js index 96da0e5b8..3aa816a90 100644 --- a/test/integration/statistics.test.js +++ b/test/integration/statistics.test.js @@ -6,7 +6,8 @@ const { buildDriver, clickText, containsClass, - findByXpath + findByXpath, + navigate } = new SeleniumHelper(); const rootUrl = process.env.ROOT_URL || 'https://scratch.ly'; @@ -22,7 +23,7 @@ describe('www-integration statistics page', () => { }); beforeEach(async () => { - await driver.get(statisticsPage); + await navigate(statisticsPage); }); afterAll(() => driver.quit()); From 6182ba27040eaad9aa4ee8f9e37f73b05e69e304 Mon Sep 17 00:00:00 2001 From: Christopher Willis-Ford <7019101+cwillisf@users.noreply.github.com> Date: Fri, 27 Oct 2023 07:18:02 -0700 Subject: [PATCH 25/33] test: use ISO date to make comments unique, not build number --- test/integration/comments.test.js | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/test/integration/comments.test.js b/test/integration/comments.test.js index 191e3ddbb..b7a441627 100644 --- a/test/integration/comments.test.js +++ b/test/integration/comments.test.js @@ -30,13 +30,12 @@ const studioId = process.env.COMMENT_STUDIO_ID || 10005646; const studioUrl = `${rootUrl}/studios/${studioId}/comments`; // setup comments to leave +// make sure they are unique and will not be censored (avoid numbers that might look like phone numbers or other PII) const date = new Date(); -const dateString = `Y:${date.getFullYear()} - M:${date.getMonth() + 1} - D:${date.getDate()} ` + -`: ${date.getHours()}:${date.getMinutes()}:${date.getSeconds()}`; -const buildNumber = process.env.CIRCLE_BUILD_NUM || dateString; -const projectComment = `${buildNumber} project`; -const profileComment = `${buildNumber} profile`; -const studioComment = `${buildNumber} studio`; +const dateString = date.toISOString(); +const projectComment = `${dateString} project`; +const profileComment = `${dateString} profile`; +const studioComment = `${dateString} studio`; const projectReply = `${projectComment} reply`; const profileReply = `${profileComment} reply`; From 74f2d50e2d8da15951e7faf45b4e649551158473 Mon Sep 17 00:00:00 2001 From: Christopher Willis-Ford <7019101+cwillisf@users.noreply.github.com> Date: Fri, 27 Oct 2023 08:15:15 -0700 Subject: [PATCH 26/33] test: implement GHA-friendly Sauce labels --- test/integration/selenium-helpers.js | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/test/integration/selenium-helpers.js b/test/integration/selenium-helpers.js index 2cbd2249c..231492896 100644 --- a/test/integration/selenium-helpers.js +++ b/test/integration/selenium-helpers.js @@ -8,9 +8,9 @@ const chromedriverVersion = require('chromedriver').version; const headless = process.env.SMOKE_HEADLESS || false; const remote = process.env.SMOKE_REMOTE || false; -const ci = process.env.CI || false; -const usingCircle = process.env.CIRCLECI || false; -const buildID = process.env.CIRCLE_BUILD_NUM || '0000'; +const ciBuildPrefix = process.env.CI ? + `CI #${process.env.GITHUB_RUN_ID}/${process.env.GITHUB_RUN_ATTEMPT}` : + ''; // no prefix if not in CI const {SAUCE_USERNAME, SAUCE_ACCESS_KEY} = process.env; const {By, Key, until} = webdriver; @@ -160,9 +160,8 @@ class SeleniumHelper { buildDriver (name) { if (remote === 'true'){ let nameToUse; - if (ci === 'true'){ - const ciName = usingCircle ? 'circleCi ' : 'unknown '; - nameToUse = `${ciName + buildID} : ${name}`; + if (ciBuildPrefix){ + nameToUse = `${ciBuildPrefix}: ${name}`; } else { nameToUse = name; } From 7cb0baad643412dda4224a11a2ef175446420748 Mon Sep 17 00:00:00 2001 From: Christopher Willis-Ford <7019101+cwillisf@users.noreply.github.com> Date: Thu, 26 Oct 2023 13:32:45 -0700 Subject: [PATCH 27/33] ci: temporarily skip straight to integration tests --- .github/workflows/build-and-test.yml | 96 ++++++++++++++-------------- 1 file changed, 48 insertions(+), 48 deletions(-) diff --git a/.github/workflows/build-and-test.yml b/.github/workflows/build-and-test.yml index ce8071d61..d7026075b 100644 --- a/.github/workflows/build-and-test.yml +++ b/.github/workflows/build-and-test.yml @@ -62,47 +62,47 @@ jobs: mkdir -p ./test/results - name: lint run: npm run test:lint:ci - - name: build - run: WWW_VERSION=${GITHUB_SHA:0:5} npm run build - env: - # webpack.config.js uses these with `DefinePlugin` - API_HOST: ${{ secrets.API_HOST }} - RECAPTCHA_SITE_KEY: ${{ secrets.RECAPTCHA_SITE_KEY }} - ASSET_HOST: ${{ secrets.ASSET_HOST }} - BACKPACK_HOST: ${{ secrets.BACKPACK_HOST }} - CLOUDDATA_HOST: ${{ secrets.CLOUDDATA_HOST }} - PROJECT_HOST: ${{ secrets.PROJECT_HOST }} - STATIC_HOST: ${{ secrets.STATIC_HOST }} - SCRATCH_ENV: ${{ vars.SCRATCH_ENV }} + # - name: build + # run: WWW_VERSION=${GITHUB_SHA:0:5} npm run build + # env: + # # webpack.config.js uses these with `DefinePlugin` + # API_HOST: ${{ secrets.API_HOST }} + # RECAPTCHA_SITE_KEY: ${{ secrets.RECAPTCHA_SITE_KEY }} + # ASSET_HOST: ${{ secrets.ASSET_HOST }} + # BACKPACK_HOST: ${{ secrets.BACKPACK_HOST }} + # CLOUDDATA_HOST: ${{ secrets.CLOUDDATA_HOST }} + # PROJECT_HOST: ${{ secrets.PROJECT_HOST }} + # STATIC_HOST: ${{ secrets.STATIC_HOST }} + # SCRATCH_ENV: ${{ vars.SCRATCH_ENV }} - # used by src/template-config.js - GTM_ID: ${{ secrets.GTM_ID }} - GTM_ENV_AUTH: ${{ secrets.GTM_ENV_AUTH }} - - name: unit tests - run: | - JEST_JUNIT_OUTPUT_NAME=unit-jest-results.xml npm run test:unit:jest:unit -- --reporters=jest-junit - JEST_JUNIT_OUTPUT_NAME=localization-jest-results.xml npm run test:unit:jest:localization -- --reporters=jest-junit - npm run test:unit:tap -- --output-file ./test/results/unit-raw.tap - npm run test:unit:convertReportToXunit - - name: setup Python - if: ${{ env.SCRATCH_SHOULD_DEPLOY == 'true' }} - run: | - curl https://bootstrap.pypa.io/pip/3.5/get-pip.py -o get-pip.py - python3 get-pip.py pip==21.0.1 - pip install s3cmd==2.3.0 - - name: deploy - if: ${{ env.SCRATCH_SHOULD_DEPLOY == 'true' }} - run: npm run deploy - env: - S3_LOCAL_DIR: build - S3_BUCKET_NAME: ${{ secrets.S3_BUCKET_NAME }} - AWS_ACCESS_KEY_ID: ${{ secrets.AWS_ACCESS_KEY_ID }} - AWS_SECRET_ACCESS_KEY: ${{ secrets.AWS_SECRET_ACCESS_KEY }} - FASTLY_API_KEY: ${{ secrets.FASTLY_API_KEY }} - FASTLY_SERVICE_ID: ${{ secrets.FASTLY_SERVICE_ID }} - SLACK_WEBHOOK_CIRCLECI_NOTIFICATIONS: ${{ secrets.SLACK_WEBHOOK_CIRCLECI_NOTIFICATIONS }} # TODO: rename or replace - SLACK_WEBHOOK_ENGINEERING: ${{ secrets.SLACK_WEBHOOK_ENGINEERING }} - SLACK_WEBHOOK_MODS: ${{ secrets.SLACK_WEBHOOK_MODS }} + # # used by src/template-config.js + # GTM_ID: ${{ secrets.GTM_ID }} + # GTM_ENV_AUTH: ${{ secrets.GTM_ENV_AUTH }} + # - name: unit tests + # run: | + # JEST_JUNIT_OUTPUT_NAME=unit-jest-results.xml npm run test:unit:jest:unit -- --reporters=jest-junit + # JEST_JUNIT_OUTPUT_NAME=localization-jest-results.xml npm run test:unit:jest:localization -- --reporters=jest-junit + # npm run test:unit:tap -- --output-file ./test/results/unit-raw.tap + # npm run test:unit:convertReportToXunit + # - name: setup Python + # if: ${{ env.SCRATCH_SHOULD_DEPLOY == 'true' }} + # run: | + # curl https://bootstrap.pypa.io/pip/3.5/get-pip.py -o get-pip.py + # python3 get-pip.py pip==21.0.1 + # pip install s3cmd==2.3.0 + # - name: deploy + # if: ${{ env.SCRATCH_SHOULD_DEPLOY == 'true' }} + # run: npm run deploy + # env: + # S3_LOCAL_DIR: build + # S3_BUCKET_NAME: ${{ secrets.S3_BUCKET_NAME }} + # AWS_ACCESS_KEY_ID: ${{ secrets.AWS_ACCESS_KEY_ID }} + # AWS_SECRET_ACCESS_KEY: ${{ secrets.AWS_SECRET_ACCESS_KEY }} + # FASTLY_API_KEY: ${{ secrets.FASTLY_API_KEY }} + # FASTLY_SERVICE_ID: ${{ secrets.FASTLY_SERVICE_ID }} + # SLACK_WEBHOOK_CIRCLECI_NOTIFICATIONS: ${{ secrets.SLACK_WEBHOOK_CIRCLECI_NOTIFICATIONS }} # TODO: rename or replace + # SLACK_WEBHOOK_ENGINEERING: ${{ secrets.SLACK_WEBHOOK_ENGINEERING }} + # SLACK_WEBHOOK_MODS: ${{ secrets.SLACK_WEBHOOK_MODS }} - name: integration tests if: ${{ env.SCRATCH_SHOULD_DEPLOY == 'true' }} run: | @@ -134,11 +134,11 @@ jobs: OWNED_UNSHARED_SCRATCH2_PROJECT_ID: ${{ secrets.OWNED_UNSHARED_SCRATCH2_PROJECT_ID }} TEST_STUDIO_ID: ${{ secrets.TEST_STUDIO_ID }} RATE_LIMIT_CHECK: ${{ secrets.RATE_LIMIT_CHECK }} - - name: compress artifact - if: ${{ env.SCRATCH_SHOULD_DEPLOY == 'true' }} - run: tar -czvf build.tgz build - - name: upload artifact - if: ${{ env.SCRATCH_SHOULD_DEPLOY == 'true' }} - uses: actions/upload-artifact@v3 - with: - path: build.tgz + # - name: compress artifact + # if: ${{ env.SCRATCH_SHOULD_DEPLOY == 'true' }} + # run: tar -czvf build.tgz build + # - name: upload artifact + # if: ${{ env.SCRATCH_SHOULD_DEPLOY == 'true' }} + # uses: actions/upload-artifact@v3 + # with: + # path: build.tgz From 61d4ad8e70d838e8403f1b260bc6e6cd9f39b751 Mon Sep 17 00:00:00 2001 From: Christopher Willis-Ford <7019101+cwillisf@users.noreply.github.com> Date: Fri, 27 Oct 2023 10:00:14 -0700 Subject: [PATCH 28/33] test: try to make signing in more reliable --- test/integration/selenium-helpers.js | 1 + 1 file changed, 1 insertion(+) diff --git a/test/integration/selenium-helpers.js b/test/integration/selenium-helpers.js index 231492896..f039db360 100644 --- a/test/integration/selenium-helpers.js +++ b/test/integration/selenium-helpers.js @@ -506,6 +506,7 @@ class SeleniumHelper { await name.sendKeys(username); const word = await this.findByXpath('//input[@id="frc-password-1088"]'); await word.sendKeys(password + this.getKey('ENTER')); + await this.waitUntilDocumentReady(); await this.findByXpath(this.getPathForProfileName()); } catch (cause) { throw await outerError.chain(cause, this.driver); From 29b4362ea8385bb5dd680bef091c4710c3a4cd66 Mon Sep 17 00:00:00 2001 From: Christopher Willis-Ford <7019101+cwillisf@users.noreply.github.com> Date: Fri, 27 Oct 2023 09:08:45 -0700 Subject: [PATCH 29/33] test: remove redundant and unreliable visibility checks --- test/integration/my-stuff.test.js | 4 ++-- test/integration/studios-page.test.js | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/test/integration/my-stuff.test.js b/test/integration/my-stuff.test.js index 040246567..02ef120c0 100644 --- a/test/integration/my-stuff.test.js +++ b/test/integration/my-stuff.test.js @@ -127,9 +127,9 @@ describe('www-integration my_stuff', () => { // 6th studio should fail await navigate(myStuffURL); await clickXpath('//form[@id="new_studio"]/button[@type="submit"]'); + // findByXpath checks for both presence and visibility const alertMessage = await findByXpath('//div[contains(@class, "alert-error")]'); - const errVisible = await alertMessage.isDisplayed(); - expect(errVisible).toBe(true); + expect(alertMessage).toBeTruthy(); await navigate(rateLimitCheck); }); diff --git a/test/integration/studios-page.test.js b/test/integration/studios-page.test.js index 67aaf0835..4117caf9a 100644 --- a/test/integration/studios-page.test.js +++ b/test/integration/studios-page.test.js @@ -181,8 +181,8 @@ describe('studio management', () => { // click confirm // await clickXpath('//button[contains(@class, "confirm-transfer-button")]') await clickXpath('//span[contains(text(), "Confirm")]/..'); + // findByXpath checks for both presence and visibility const transferSuccess = await findByXpath('//div[contains(@class, "alert-success")]'); - const successVisible = await transferSuccess.isDisplayed(); - expect(successVisible).toBe(true); + expect(transferSuccess).toBeTruthy(); }); }); From 4312b17bfb7ce4bd2444b063abd9724740bd283f Mon Sep 17 00:00:00 2001 From: Christopher Willis-Ford <7019101+cwillisf@users.noreply.github.com> Date: Mon, 30 Oct 2023 08:18:21 -0700 Subject: [PATCH 30/33] ci: eschew unnecessary verbosity --- .github/workflows/{build-and-test.yml => ci-cd.yml} | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) rename .github/workflows/{build-and-test.yml => ci-cd.yml} (99%) diff --git a/.github/workflows/build-and-test.yml b/.github/workflows/ci-cd.yml similarity index 99% rename from .github/workflows/build-and-test.yml rename to .github/workflows/ci-cd.yml index d7026075b..5fa2bd1d9 100644 --- a/.github/workflows/build-and-test.yml +++ b/.github/workflows/ci-cd.yml @@ -1,4 +1,4 @@ -name: Build and Test and maybe Deploy +name: CI/CD on: workflow_dispatch: # Allows you to run this workflow manually from the Actions tab From 48bf6f883931e6572c553e64bb3f4e8fd073c34c Mon Sep 17 00:00:00 2001 From: Christopher Willis-Ford <7019101+cwillisf@users.noreply.github.com> Date: Mon, 30 Oct 2023 10:00:04 -0700 Subject: [PATCH 31/33] ci: undo temporary changes made for testing CI --- .circleci/config.yml | 207 ------------------------------------ .github/workflows/ci-cd.yml | 98 ++++++++--------- 2 files changed, 49 insertions(+), 256 deletions(-) delete mode 100644 .circleci/config.yml diff --git a/.circleci/config.yml b/.circleci/config.yml deleted file mode 100644 index c634013a4..000000000 --- a/.circleci/config.yml +++ /dev/null @@ -1,207 +0,0 @@ -version: 2.1 - -# WARNING: most of these jobs are disabled in preparation for GHA migration. -# They're retained here for reference during the migration work. -# This file should be deleted once the migration is complete. - -aliases: - - &defaults - docker: - - image: cimg/node:16.14.2-browsers - auth: - username: $DOCKERHUB_USERNAME - password: $DOCKERHUB_PASSWORD - working_directory: ~/repo - - &setup - name: "setup" - command: | - npm --production=false ci - mkdir ./test/results - - &lint - name: "run lint tests" - command: | - npm run test:lint:ci - - &build - name: "run npm build" - command: | - WWW_VERSION=${CIRCLE_SHA1:0:5} npm run build - - &unit - name: "Run unit tests" - command: | - JEST_JUNIT_OUTPUT_NAME=unit-jest-results.xml npm run test:unit:jest:unit -- --reporters=jest-junit - JEST_JUNIT_OUTPUT_NAME=localization-jest-results.xml npm run test:unit:jest:localization -- --reporters=jest-junit - npm run test:unit:tap -- --output-file ./test/results/unit-raw.tap - npm run test:unit:convertReportToXunit - - &setup_python - name: "setup python" - command: | - curl https://bootstrap.pypa.io/pip/3.5/get-pip.py -o get-pip.py - python3 get-pip.py pip==21.0.1 - pip install s3cmd==2.1.0 - - &deploy - name: "deploy" - command: | - npm run deploy - - &integration - name: "integration tests with Jest" - command: | - JEST_JUNIT_OUTPUT_NAME=integration-jest-results.xml npm run test:integration:remote -- --reporters=jest-junit - - &build_no_deploy - <<: *defaults - resource_class: large - steps: - - checkout - - run: - <<: *setup - - run: - <<: *lint - - run: - <<: *build - - run: - <<: *unit - - store_test_results: - path: test/results - - &build_and_deploy - <<: *defaults - resource_class: large - steps: - - checkout - - run: - <<: *setup - - run: - <<: *lint - - run: - <<: *build - - run: - <<: *unit - - run: - <<: *setup_python - - run: - <<: *deploy - - store_test_results: - path: test/results - - run: - name: Compress Artifacts - command: tar -cvzf build.tar build - - store_artifacts: - path: build.tar - - &integration_tests_and_store - <<: *defaults - resource_class: large - steps: - - checkout - - run: - <<: *setup - - run: - <<: *integration - - store_test_results: - path: test/results - - &update-translations - <<: *defaults - steps: - - checkout - - run: - name: "setup" - command: npm --production=false ci - - run: - name: "run i18n script" - command: npm run i18n:push - -# build-test-deploy requires two separately named jobs -jobs: - build-and-deploy-staging: - <<: *build_and_deploy - build-and-deploy-production: - <<: *build_and_deploy - integration-tests: - <<: *integration_tests_and_store - update-translations: - <<: *update-translations - build-no-deploy: - <<: *build_no_deploy - -workflows: - build-test-deploy: - jobs: - - build-and-deploy-staging: - context: - - scratch-www-all - - scratch-www-staging - - dockerhub-credentials - filters: - branches: - only: - - "" # disable -# - develop -# - beta -# - /^hotfix\/.*/ -# - /^release\/.*/ - - integration-tests: - requires: - - build-and-deploy-staging - context: - - scratch-www-all - - scratch-www-staging - - dockerhub-credentials - filters: - branches: - only: - - "" # disable -# - develop -# - beta -# - /^hotfix\/.*/ -# - /^release\/.*/ - - build-and-deploy-production: - context: - - scratch-www-all - - scratch-www-production - - dockerhub-credentials - filters: - branches: - only: - - "" # disable -# - master - - integration-tests: - requires: - - build-and-deploy-production - context: - - scratch-www-all - - scratch-www-production - - dockerhub-credentials - filters: - branches: - only: - - "" # disable -# - master - Update-translations: - triggers: - - schedule: # every evening at 7pm EST (8pm EDT, Midnight UTC) - cron: "0 0 * * *" - filters: - branches: - only: develop - jobs: - - update-translations: - context: - - scratch-www-all - - scratch-www-staging - - dockerhub-credentials - filters: - branches: - only: - - develop - build-test-no-deploy: - jobs: - - build-no-deploy: - context: - - dockerhub-credentials - filters: - branches: - only: - - "" # disable -# ignore: -# - develop -# - master -# - beta -# - /^hotfix\/.*/ -# - /^release\/.*/ diff --git a/.github/workflows/ci-cd.yml b/.github/workflows/ci-cd.yml index 5fa2bd1d9..48ab75e5e 100644 --- a/.github/workflows/ci-cd.yml +++ b/.github/workflows/ci-cd.yml @@ -31,7 +31,7 @@ jobs: "refs/heads/master") echo "scratch_environment=production" | tee -a $GITHUB_OUTPUT ;; - "refs/heads/gha" | "refs/heads/develop" | "refs/heads/beta" | refs/heads/hotfix/* | refs/heads/release/*) + "refs/heads/develop" | "refs/heads/beta" | refs/heads/hotfix/* | refs/heads/release/*) echo "scratch_environment=staging" | tee -a $GITHUB_OUTPUT ;; *) @@ -62,47 +62,47 @@ jobs: mkdir -p ./test/results - name: lint run: npm run test:lint:ci - # - name: build - # run: WWW_VERSION=${GITHUB_SHA:0:5} npm run build - # env: - # # webpack.config.js uses these with `DefinePlugin` - # API_HOST: ${{ secrets.API_HOST }} - # RECAPTCHA_SITE_KEY: ${{ secrets.RECAPTCHA_SITE_KEY }} - # ASSET_HOST: ${{ secrets.ASSET_HOST }} - # BACKPACK_HOST: ${{ secrets.BACKPACK_HOST }} - # CLOUDDATA_HOST: ${{ secrets.CLOUDDATA_HOST }} - # PROJECT_HOST: ${{ secrets.PROJECT_HOST }} - # STATIC_HOST: ${{ secrets.STATIC_HOST }} - # SCRATCH_ENV: ${{ vars.SCRATCH_ENV }} + - name: build + run: WWW_VERSION=${GITHUB_SHA:0:5} npm run build + env: + # webpack.config.js uses these with `DefinePlugin` + API_HOST: ${{ secrets.API_HOST }} + RECAPTCHA_SITE_KEY: ${{ secrets.RECAPTCHA_SITE_KEY }} + ASSET_HOST: ${{ secrets.ASSET_HOST }} + BACKPACK_HOST: ${{ secrets.BACKPACK_HOST }} + CLOUDDATA_HOST: ${{ secrets.CLOUDDATA_HOST }} + PROJECT_HOST: ${{ secrets.PROJECT_HOST }} + STATIC_HOST: ${{ secrets.STATIC_HOST }} + SCRATCH_ENV: ${{ vars.SCRATCH_ENV }} - # # used by src/template-config.js - # GTM_ID: ${{ secrets.GTM_ID }} - # GTM_ENV_AUTH: ${{ secrets.GTM_ENV_AUTH }} - # - name: unit tests - # run: | - # JEST_JUNIT_OUTPUT_NAME=unit-jest-results.xml npm run test:unit:jest:unit -- --reporters=jest-junit - # JEST_JUNIT_OUTPUT_NAME=localization-jest-results.xml npm run test:unit:jest:localization -- --reporters=jest-junit - # npm run test:unit:tap -- --output-file ./test/results/unit-raw.tap - # npm run test:unit:convertReportToXunit - # - name: setup Python - # if: ${{ env.SCRATCH_SHOULD_DEPLOY == 'true' }} - # run: | - # curl https://bootstrap.pypa.io/pip/3.5/get-pip.py -o get-pip.py - # python3 get-pip.py pip==21.0.1 - # pip install s3cmd==2.3.0 - # - name: deploy - # if: ${{ env.SCRATCH_SHOULD_DEPLOY == 'true' }} - # run: npm run deploy - # env: - # S3_LOCAL_DIR: build - # S3_BUCKET_NAME: ${{ secrets.S3_BUCKET_NAME }} - # AWS_ACCESS_KEY_ID: ${{ secrets.AWS_ACCESS_KEY_ID }} - # AWS_SECRET_ACCESS_KEY: ${{ secrets.AWS_SECRET_ACCESS_KEY }} - # FASTLY_API_KEY: ${{ secrets.FASTLY_API_KEY }} - # FASTLY_SERVICE_ID: ${{ secrets.FASTLY_SERVICE_ID }} - # SLACK_WEBHOOK_CIRCLECI_NOTIFICATIONS: ${{ secrets.SLACK_WEBHOOK_CIRCLECI_NOTIFICATIONS }} # TODO: rename or replace - # SLACK_WEBHOOK_ENGINEERING: ${{ secrets.SLACK_WEBHOOK_ENGINEERING }} - # SLACK_WEBHOOK_MODS: ${{ secrets.SLACK_WEBHOOK_MODS }} + # used by src/template-config.js + GTM_ID: ${{ secrets.GTM_ID }} + GTM_ENV_AUTH: ${{ secrets.GTM_ENV_AUTH }} + - name: unit tests + run: | + JEST_JUNIT_OUTPUT_NAME=unit-jest-results.xml npm run test:unit:jest:unit -- --reporters=jest-junit + JEST_JUNIT_OUTPUT_NAME=localization-jest-results.xml npm run test:unit:jest:localization -- --reporters=jest-junit + npm run test:unit:tap -- --output-file ./test/results/unit-raw.tap + npm run test:unit:convertReportToXunit + - name: setup Python + if: ${{ env.SCRATCH_SHOULD_DEPLOY == 'true' }} + run: | + curl https://bootstrap.pypa.io/pip/3.5/get-pip.py -o get-pip.py + python3 get-pip.py pip==21.0.1 + pip install s3cmd==2.3.0 + - name: deploy + if: ${{ env.SCRATCH_SHOULD_DEPLOY == 'true' }} + run: npm run deploy + env: + S3_LOCAL_DIR: build + S3_BUCKET_NAME: ${{ secrets.S3_BUCKET_NAME }} + AWS_ACCESS_KEY_ID: ${{ secrets.AWS_ACCESS_KEY_ID }} + AWS_SECRET_ACCESS_KEY: ${{ secrets.AWS_SECRET_ACCESS_KEY }} + FASTLY_API_KEY: ${{ secrets.FASTLY_API_KEY }} + FASTLY_SERVICE_ID: ${{ secrets.FASTLY_SERVICE_ID }} + SLACK_WEBHOOK_CIRCLECI_NOTIFICATIONS: ${{ secrets.SLACK_WEBHOOK_CIRCLECI_NOTIFICATIONS }} # TODO: rename or replace + SLACK_WEBHOOK_ENGINEERING: ${{ secrets.SLACK_WEBHOOK_ENGINEERING }} + SLACK_WEBHOOK_MODS: ${{ secrets.SLACK_WEBHOOK_MODS }} - name: integration tests if: ${{ env.SCRATCH_SHOULD_DEPLOY == 'true' }} run: | @@ -134,11 +134,11 @@ jobs: OWNED_UNSHARED_SCRATCH2_PROJECT_ID: ${{ secrets.OWNED_UNSHARED_SCRATCH2_PROJECT_ID }} TEST_STUDIO_ID: ${{ secrets.TEST_STUDIO_ID }} RATE_LIMIT_CHECK: ${{ secrets.RATE_LIMIT_CHECK }} - # - name: compress artifact - # if: ${{ env.SCRATCH_SHOULD_DEPLOY == 'true' }} - # run: tar -czvf build.tgz build - # - name: upload artifact - # if: ${{ env.SCRATCH_SHOULD_DEPLOY == 'true' }} - # uses: actions/upload-artifact@v3 - # with: - # path: build.tgz + - name: compress artifact + if: ${{ env.SCRATCH_SHOULD_DEPLOY == 'true' }} + run: tar -czvf build.tgz build + - name: upload artifact + if: ${{ env.SCRATCH_SHOULD_DEPLOY == 'true' }} + uses: actions/upload-artifact@v3 + with: + path: build.tgz From 76d8834ccd6bc4dc1e20427e1bbd9436e9621092 Mon Sep 17 00:00:00 2001 From: Christopher Willis-Ford <7019101+cwillisf@users.noreply.github.com> Date: Mon, 30 Oct 2023 10:26:40 -0700 Subject: [PATCH 32/33] ci: eliminate redundant PR+Push builds --- .github/workflows/ci-cd.yml | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/.github/workflows/ci-cd.yml b/.github/workflows/ci-cd.yml index 48ab75e5e..0fc558ecb 100644 --- a/.github/workflows/ci-cd.yml +++ b/.github/workflows/ci-cd.yml @@ -1,9 +1,10 @@ name: CI/CD on: - workflow_dispatch: # Allows you to run this workflow manually from the Actions tab pull_request: # Runs whenever a pull request is created or updated push: # Runs whenever a commit is pushed to the repository + branches: [master, develop, beta, hotfix/*] # ...on any of these branches + workflow_dispatch: # Allows you to run this workflow manually from the Actions tab concurrency: group: '${{ github.workflow }} @ ${{ github.event.pull_request.head.label || github.head_ref || github.ref }}' From b7ac4fdac00227e728ca5dc8fee8ee947a268b43 Mon Sep 17 00:00:00 2001 From: Christopher Willis-Ford <7019101+cwillisf@users.noreply.github.com> Date: Tue, 31 Oct 2023 07:34:27 -0700 Subject: [PATCH 33/33] ci: use Ron's method to determine environment --- .github/workflows/ci-cd.yml | 41 +++++++++++++++---------------------- 1 file changed, 17 insertions(+), 24 deletions(-) diff --git a/.github/workflows/ci-cd.yml b/.github/workflows/ci-cd.yml index 0fc558ecb..15e09d954 100644 --- a/.github/workflows/ci-cd.yml +++ b/.github/workflows/ci-cd.yml @@ -18,32 +18,26 @@ env: SKIP_CLEANUP: true jobs: - determine-environment: - runs-on: ubuntu-latest - outputs: - # map the output from the step with ID="set-scratch-environment" - # to the job output named "scratch_environment" - scratch_environment: ${{ steps.set-scratch-environment.outputs.scratch_environment }} - steps: - - id: set-scratch-environment - shell: bash - run: | - case "${{ github.ref }}" in - "refs/heads/master") - echo "scratch_environment=production" | tee -a $GITHUB_OUTPUT - ;; - "refs/heads/develop" | "refs/heads/beta" | refs/heads/hotfix/* | refs/heads/release/*) - echo "scratch_environment=staging" | tee -a $GITHUB_OUTPUT - ;; - *) - echo "Leaving scratch_environment unset" - ;; - esac build-and-test-and-maybe-deploy: - needs: determine-environment runs-on: ubuntu-latest - environment: ${{ needs.determine-environment.outputs.scratch_environment }} + environment: >- + ${{ + ( + (github.ref == 'refs/heads/master') && 'production' + ) || + ( + ( + (github.ref == 'refs/heads/develop') || + (github.ref == 'refs/heads/beta') || + startsWith(github.ref, 'refs/heads/hotfix/') || + startsWith(github.ref, 'refs/heads/release/') + ) && 'staging' + ) || + '' + }} env: + # SCRATCH_ENV comes from the GitHub Environment + # See https://github.com/scratchfoundation/scratch-www/settings/variables/actions SCRATCH_SHOULD_DEPLOY: ${{ vars.SCRATCH_ENV != '' }} steps: - uses: actions/checkout@v3 @@ -53,7 +47,6 @@ jobs: node-version-file: '.nvmrc' - name: info run: | - echo "GitHub environment: ${{ needs.determine-environment.outputs.scratch_environment }}" echo "Scratch environment: ${{ vars.SCRATCH_ENV }}" echo "Node version: $(node --version)" echo "NPM version: $(npm --version)"