CLD-7841 Optimize E2E testing costs (mattermost#27883)

* Utilize free runners for E2E tests, report back to PR on run completion * Increase test stability on smaller instances * Merge worker reports * merge start+prepare steps * upgrade keycloak * increase test timeouts * Add video recording to report artifacts Co-authored-by: Saturnino Abril <[email protected]> --------- Co-authored-by: Saturnino Abril <[email protected]>
devinbinnie · Aug 14, 2024 · 948f894 · 948f894
1 parent 5f19d85
commit 948f894
Show file tree

Hide file tree

Showing 37 changed files with 261 additions and 151 deletions.
diff --git a/.github/workflows/e2e-fulltests-ci.yml b/.github/workflows/e2e-fulltests-ci.yml
@@ -62,12 +62,15 @@ jobs:
       REPORT_TYPE: "${{ steps.generate.outputs.REPORT_TYPE }}"
       ROLLING_RELEASE_commit_sha: "${{ steps.generate.outputs.ROLLING_RELEASE_commit_sha }}"
       ROLLING_RELEASE_SERVER_IMAGE: "${{ steps.generate.outputs.ROLLING_RELEASE_SERVER_IMAGE }}"
+      WORKFLOW_RUN_URL: "${{steps.generate.outputs.WORKFLOW_RUN_URL}}"
+      CYCLE_URL: "${{steps.generate.outputs.CYCLE_URL}}"
     env:
       GH_TOKEN: "${{ github.token }}"
       REF: "${{ inputs.ref || github.sha }}"
       PR_NUMBER: "${{ inputs.PR_NUMBER || '' }}"
       REPORT_TYPE: "${{ inputs.REPORT_TYPE }}"
       ROLLING_RELEASE_FROM_TAG: "${{ inputs.ROLLING_RELEASE_FROM_TAG }}"
+      AUTOMATION_DASHBOARD_URL: "${{ secrets.MM_E2E_AUTOMATION_DASHBOARD_URL }}"
       # We could exclude the @smoke group for PRs, but then we wouldn't have it in the report
       TEST_FILTER_PR: >-
         --stage="@prod"
@@ -161,9 +164,13 @@ jobs:
             ROLLING_RELEASE_SERVER_IMAGE="mattermost/mattermost-enterprise-edition:${ROLLING_RELEASE_SERVER_IMAGE_TAG}"
             BUILD_ID_SUFFIX="rolling${ROLLING_RELEASE_FROM_TAG/-/_}-$BUILD_ID_SUFFIX"
             BUILD_ID_SUFFIX_IN_STATUS_CHECK=true
+            WORKERS_NUMBER=10   # Rolling release tests are particularly impacted by increased parallelism. It's more efficient to run on fewer workers
             ### Run sanity assertions after variable generations
             git show-ref --verify "refs/tags/${ROLLING_RELEASE_FROM_TAG}" # 'inputs.ROLLING_RELEASE_FROM_TAG' must be a tag, for release report types
           fi
+          # BUILD_ID format: $pipelineID-$imageTag-$testType-$serverType-$serverEdition
+          # Reference on BUILD_ID parsing: https://github.com/saturninoabril/automation-dashboard/blob/175891781bf1072c162c58c6ec0abfc5bcb3520e/lib/common_utils.ts#L3-L23
+          BUILD_ID="${{ github.run_id }}_${{ github.run_attempt }}-${SERVER_IMAGE_TAG}-${BUILD_ID_SUFFIX}"
           echo "commit_sha=${COMMIT_SHA}" >> $GITHUB_OUTPUT
           echo "BRANCH=${BRANCH}" >> $GITHUB_OUTPUT
           echo "SERVER_IMAGE=${SERVER_IMAGE_ORG}/mattermost-enterprise-edition:${SERVER_IMAGE_TAG}" >> $GITHUB_OUTPUT
@@ -177,18 +184,22 @@ jobs:
           echo "REPORT_TYPE=${COMPUTED_REPORT_TYPE}" >> $GITHUB_OUTPUT
           echo "ROLLING_RELEASE_commit_sha=${ROLLING_RELEASE_COMMIT_SHA}" >> $GITHUB_OUTPUT
           echo "ROLLING_RELEASE_SERVER_IMAGE=${ROLLING_RELEASE_SERVER_IMAGE}" >> $GITHUB_OUTPUT
-          # BUILD_ID format: $pipelineID-$imageTag-$testType-$serverType-$serverEdition
-          # Reference on BUILD_ID parsing: https://github.com/saturninoabril/automation-dashboard/blob/175891781bf1072c162c58c6ec0abfc5bcb3520e/lib/common_utils.ts#L3-L23
-          echo "BUILD_ID=${{ github.run_id }}_${{ github.run_attempt }}-${SERVER_IMAGE_TAG}-${BUILD_ID_SUFFIX}" >> $GITHUB_OUTPUT
+          echo "BUILD_ID=${BUILD_ID}" >> $GITHUB_OUTPUT
+          # User notification variables
+          echo "WORKFLOW_RUN_URL=${{ github.server_url }}/${{ github.repository }}/actions/runs/${{github.run_id}}" >> $GITHUB_OUTPUT
+          echo "CYCLE_URL=${AUTOMATION_DASHBOARD_URL%%/api}/cycle/${BUILD_ID}" >> $GITHUB_OUTPUT
       - name: ci/notify-user
+        env:
+          COMMIT_SHA: "${{steps.generate.outputs.commit_sha}}"
+          STATUS_CHECK_CONTEXT: "${{steps.generate.outputs.status_check_context}}"
+          WORKFLOW_RUN_URL: "${{steps.generate.outputs.WORKFLOW_RUN_URL}}"
+          CYCLE_URL: "${{steps.generate.outputs.CYCLE_URL}}"
         run: |
           if [ -n "$PR_NUMBER" ]; then
-            WORKFLOW_RUN_URL="${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }}"
-            CYCLE_URL="https://automation-dashboard.vercel.app/cycle/${{ github.run_id }}"
             gh issue -R "${{ github.repository }}" comment "$PR_NUMBER" --body-file - <<EOF
-          E2E test run is starting for commit \`${{ steps.generate.outputs.commit_sha }}\`${MM_ENV:+, with \`MM_ENV=$MM_ENV\`}.
+          E2E test run is starting for commit \`${COMMIT_SHA}\`${MM_ENV:+, with \`MM_ENV=$MM_ENV\`}.
           You can check its progress by either:
-          - Looking at the corresponding commit status, which will be available in a few moments: \`${{ steps.generate.outputs.status_check_context }}\`.
+          - Looking at the corresponding commit status, which will be available in a few moments: \`${STATUS_CHECK_CONTEXT}\`.
           - Looking at the [E2E test's Workflow Run]($WORKFLOW_RUN_URL), with Run ID \`${{ github.run_id }}\`.
           - Looking at the [E2E test's Cycle on the Automation Dashboard]($CYCLE_URL).
           EOF
@@ -229,3 +240,34 @@ jobs:
       REPORT_TM4J_TEST_CYCLE_LINK_PREFIX: "${{ secrets.MM_E2E_TEST_CYCLE_LINK_PREFIX }}"
       CWS_URL: "${{ needs.generate-test-variables.outputs.SERVER == 'cloud' && secrets.MM_E2E_CWS_URL || '' }}"
       CWS_EXTRA_HTTP_HEADERS: "${{ needs.generate-test-variables.outputs.SERVER == 'cloud' && secrets.MM_E2E_CWS_EXTRA_HTTP_HEADERS || '' }}"
+
+  notify-user:
+    runs-on: ubuntu-latest
+    if: always()
+    needs:
+      - generate-test-variables
+      - e2e-fulltest
+    permissions:
+      issues: write
+      pull-requests: write
+    defaults:
+      run:
+        shell: bash
+    env:
+      GH_TOKEN: "${{ github.token }}"
+      PR_NUMBER: "${{ inputs.PR_NUMBER || '' }}"
+      MM_ENV: "${{ inputs.MM_ENV || '' }}"
+      COMMIT_SHA: "${{ needs.generate-test-variables.outputs.commit_sha }}"
+      STATUS_CHECK_CONTEXT: "${{ needs.generate-test-variables.outputs.status_check_context }}"
+      WORKFLOW_RUN_URL: "${{ needs.generate-test-variables.outputs.WORKFLOW_RUN_URL }}"
+      CYCLE_URL: "${{ needs.generate-test-variables.outputs.CYCLE_URL }}"
+    steps:
+      - name: ci/notify-user-test-completion
+        run: |
+          if [ -n "$PR_NUMBER" ]; then
+            gh issue -R "${{ github.repository }}" comment "$PR_NUMBER" --body-file - <<EOF
+          E2E test has completed for commit \`${COMMIT_SHA}\`${MM_ENV:+, with \`MM_ENV=$MM_ENV\`}.
+          You can check its result on the [Automation Dashboard]($CYCLE_URL), or by checking out the status check named \`${STATUS_CHECK_CONTEXT}\` in this PR.
+          The run summary artifact is available in the corresponding [Workflow Run]($WORKFLOW_RUN_URL).
+          EOF
+          fi
diff --git a/.github/workflows/e2e-tests-ci-template.yml b/.github/workflows/e2e-tests-ci-template.yml
@@ -240,11 +240,11 @@ jobs:
         # - For MacOS: works on developer machines, but uses too many resources to be able to run on Github Actions
         # - for Windows: cannot currently run on Github Actions, since the runners do not support running linux containers, at the moment
         #
-        #os: [ubuntu-latest-8-cores, windows-2022, macos-12-xl]
-        os: [ubuntu-latest-8-cores]
+        #os: [ubuntu-latest, windows-2022, macos-12-xl]
+        os: [ubuntu-latest]
         worker_index: ${{ fromJSON(needs.generate-build-variables.outputs.workers) }} # https://docs.github.com/en/actions/learn-github-actions/expressions#example-returning-a-json-object
     runs-on: "${{ matrix.os }}"
-    timeout-minutes: 90
+    timeout-minutes: 120
     needs:
       - cypress-check
       - playwright-check
@@ -265,7 +265,7 @@ jobs:
       MM_ENV: "${{ inputs.MM_ENV }}"
       BRANCH: "${{ inputs.BRANCH }}"
       BUILD_ID: "${{ inputs.BUILD_ID }}"
-      CI_BASE_URL: "http://localhost:8065/?worker_index=${{ matrix.worker_index }}"
+      CI_BASE_URL: "${{ matrix.os }}-${{ matrix.worker_index }}"
       CYPRESS_pushNotificationServer: "${{ secrets.PUSH_NOTIFICATION_SERVER }}"
       CWS_URL: "${{ secrets.CWS_URL }}"
       CWS_EXTRA_HTTP_HEADERS: "${{ secrets.CWS_EXTRA_HTTP_HEADERS }}"
@@ -326,6 +326,7 @@ jobs:
           path: |
             e2e-tests/cypress/logs/
             e2e-tests/cypress/results/
+          retention-days: 1
 
   report:
     runs-on: ubuntu-latest
@@ -349,6 +350,13 @@ jobs:
         with:
           path: e2e-tests/cypress/
           merge-multiple: true
+      - name: ci/upload-report-global
+        uses: actions/upload-artifact@5d5d22a31266ced268874388b861e4b58bb5c2f3 # v4.3.1
+        with:
+          name: e2e-test-results
+          path: |
+            e2e-tests/cypress/logs/
+            e2e-tests/cypress/results/
       - name: ci/report-calculate-failures
         id: calculate-failures
         run: |

diff --git a/e2e-tests/.ci/.e2erc b/e2e-tests/.ci/.e2erc
@@ -93,6 +93,7 @@ export ENABLED_DOCKER_SERVICES_DEFAULT="postgres inbucket"
 export TEST_FILTER_DEFAULT='--stage=@prod --group=@smoke'
 export BRANCH_DEFAULT=$(git branch --show-current || echo -n "unknown")
 export BUILD_ID_DEFAULT=$(date +%s)
+export CI_BASE_URL_DEFAULT="localhost"
 # OS specific defaults overrides
 case $MME2E_OSTYPE in
   darwin )
@@ -111,3 +112,4 @@ export BRANCH_UNSANITIZED=${BRANCH:-$BRANCH_DEFAULT}
 export BRANCH=${BRANCH_UNSANITIZED::50} # The automation dashboard only accepts branch names up to 50 characters
 export BUILD_ID_UNSANITIZED=${BUILD_ID:-$BUILD_ID_DEFAULT}
 export BUILD_ID=${BUILD_ID_UNSANITIZED::64} # The automation dashboard only accepts build IDs up to 64 characters
+export CI_BASE_URL=${CI_BASE_URL:-$CI_BASE_URL_DEFAULT}
diff --git a/e2e-tests/.ci/server.generate.sh b/e2e-tests/.ci/server.generate.sh
@@ -276,7 +276,6 @@ generate_env_files() {
 
   # Generating TEST-specific env files
   # Some are defaulted in .e2erc due to being needed to other scripts as well
-  export CI_BASE_URL="${CI_BASE_URL:-http://localhost:8065}"
   export REPO=mattermost # Static, but declared here for making generate_test_cycle.js easier to run
   export HEADLESS=true   # Static, but declared here for making generate_test_cycle.js easier to run
   case "$TEST" in

diff --git a/e2e-tests/.ci/server.run_cypress.sh b/e2e-tests/.ci/server.run_cypress.sh
@@ -29,15 +29,16 @@ EOF
 
 # Run cypress test
 # No need to collect its exit status: if it's nonzero, this script will terminate since we use '-e'
+LOGFILE_SUFFIX="${CI_BASE_URL//\//_}" # Remove slashes from CI_BASE_URL to produce a usable filename
 # shellcheck disable=SC2016
 if ${MME2E_DC_SERVER} exec -T -u "$MME2E_UID" -- cypress bash -c '[ -n "${AUTOMATION_DASHBOARD_URL}" ]'; then
   mme2e_log "AUTOMATION_DASHBOARD_URL is set. Using run_test_cycle.js for the cypress run"
-  ${MME2E_DC_SERVER} exec -T -u "$MME2E_UID" -- cypress node run_test_cycle.js | tee ../cypress/logs/cypress.log
+  ${MME2E_DC_SERVER} exec -T -u "$MME2E_UID" -- cypress node run_test_cycle.js | tee ../cypress/logs/${LOGFILE_SUFFIX}_cypress.log
 else
   mme2e_log "AUTOMATION_DASHBOARD_URL is unset. Using run_tests.js for the cypress run"
   # shellcheck disable=SC2086
-  ${MME2E_DC_SERVER} exec -T -u "$MME2E_UID" -- cypress node run_tests.js $TEST_FILTER | tee ../cypress/logs/cypress.log
+  ${MME2E_DC_SERVER} exec -T -u "$MME2E_UID" -- cypress node run_tests.js $TEST_FILTER | tee ../cypress/logs/${LOGFILE_SUFFIX}_cypress.log
 fi
 
 # Collect server logs
-${MME2E_DC_SERVER} logs --no-log-prefix -- server >../cypress/logs/mattermost.log 2>&1
+${MME2E_DC_SERVER} logs --no-log-prefix -- server >../cypress/logs/${LOGFILE_SUFFIX}_mattermost.log 2>&1
diff --git a/e2e-tests/Makefile b/e2e-tests/Makefile
@@ -2,18 +2,17 @@ SHELL := /bin/bash
 
 .PHONY: all run stop clean
 all: run
-run: generate-server start-server prepare-server run-test
+run: generate-server start-server run-test
 stop: stop-server stop-dashboard clean
 clean:
 	rm -fv .ci/server.yml
 	rm -fv .ci/.env.{server,dashboard,cypress,playwright}
 
-.PHONY: generate-server start-server prepare-server run-test stop-server restart-server
+.PHONY: generate-server start-server run-test stop-server restart-server
 generate-server:
 	bash ./.ci/server.generate.sh
 start-server: generate-server
 	bash ./.ci/server.start.sh
-prepare-server:
 	bash ./.ci/server.prepare.sh
 run-test:
 	bash ./.ci/server.run_test.sh

diff --git a/e2e-tests/README.md b/e2e-tests/README.md
@@ -55,7 +55,7 @@ Notes:
   * If you need to introduce variables that you want to control from `.ci/env`: you need to update the scripts under the `.ci/` dir and configure them to write the new variables' values over to the appropriate `.env.*` file. In particular, avoid defining variables that depend on other variables within the docker-compose override files: this is to ensure uniformity in their availability and simplifies the question of what container has access to which variable considerably.
   * Exceptions are of course accepted wherever it makes sense (e.g. if you need to group variables based on some common functionality)
 - The `publish-report` Make target is meant for internal usage. Usage and variables are documented in the respective scripts.
-- `make start-server` won't cleanup containers that don't change across runs. This means that you can use it to emulate a Mattermost server upgrade while retaining your database data by simply changing the `SERVER_IMAGE` variable on your machine, and then re-runing `make start-server` (and `make prepare-server` if you intend to run cypress again, since its container is also recreated). But this also means that if you want to run a clean local environment, you may have to manually run `make stop` to cleanup any running containers and their volumes, which include e.g. the database.
+- `make start-server` won't cleanup containers that don't change across runs. This means that you can use it to emulate a Mattermost server upgrade while retaining your database data by simply changing the `SERVER_IMAGE` variable on your machine, and then re-running `make start-server`. But this also means that if you want to run a clean local environment, you may have to manually run `make stop` to cleanup any running containers and their volumes, which include e.g. the database.
 
 ##### For code changes:
 * `make fmt-ci` to format and check yaml files and shell scripts.
diff --git a/e2e-tests/cypress/cypress.config.ts b/e2e-tests/cypress/cypress.config.ts
@@ -5,13 +5,15 @@ import {defineConfig} from 'cypress';
 
 export default defineConfig({
     chromeWebSecurity: false,
-    defaultCommandTimeout: 20000,
+    defaultCommandTimeout: 30000,
     downloadsFolder: 'tests/downloads',
     fixturesFolder: 'tests/fixtures',
     numTestsKeptInMemory: 0,
     screenshotsFolder: 'tests/screenshots',
     taskTimeout: 20000,
-    video: false,
+    video: true,
+    videoCompression: true,
+    videosFolder: 'tests/videos',
     viewportWidth: 1300,
     env: {
         adminEmail: '[email protected]',

diff --git a/e2e-tests/cypress/package-lock.json b/e2e-tests/cypress/package-lock.json
diff --git a/e2e-tests/cypress/package.json b/e2e-tests/cypress/package.json
@@ -33,7 +33,7 @@
     "chalk": "4.1.2",
     "client-oauth2": "github:larkox/js-client-oauth2#e24e2eb5dfcbbbb3a59d095e831dbe0012b0ac49",
     "cross-env": "7.0.3",
-    "cypress": "13.7.3",
+    "cypress": "13.13.2",
     "cypress-file-upload": "5.0.8",
     "cypress-multi-reporters": "1.6.4",
     "cypress-plugin-tab": "1.0.5",

diff --git a/e2e-tests/cypress/run_test_cycle.js b/e2e-tests/cypress/run_test_cycle.js
@@ -63,6 +63,7 @@ async function runCypressTest(specExecution) {
         spec: specExecution.file,
         config: {
             screenshotsFolder: `${MOCHAWESOME_REPORT_DIR}/screenshots`,
+            videosFolder: `${MOCHAWESOME_REPORT_DIR}/videos`,
             trashAssetsBeforeRuns: false,
         },
         reporter: 'cypress-multi-reporters',

diff --git a/e2e-tests/cypress/run_tests.js b/e2e-tests/cypress/run_tests.js
@@ -109,6 +109,7 @@ async function runTests() {
             spec: testFile,
             config: {
                 screenshotsFolder: `${MOCHAWESOME_REPORT_DIR}/screenshots`,
+                videosFolder: `${MOCHAWESOME_REPORT_DIR}/videos`,
                 trashAssetsBeforeRuns: false,
             },
             env: {