Split tests for faster execution (#22318)

* Split tests for faster execution * TMP: remove uneeded step * TMP: revert remove assertNumQueries * TMP: sort directories for logs * TMP: Missing } * TMP: unify default splits
2024-06-13 16:59:58 +02:00 · 2024-06-13 16:59:58 +02:00 · 8fc73b3a30
--- a/.github/actions/run-docker/action.yml
+++ b/.github/actions/run-docker/action.yml
@ -37,11 +37,6 @@ runs:
        DOCKER_SERVICES: ${{ inputs.services }}
        HOST_UID: ${{ steps.id.outputs.id }}
      run: |
-        if [[ -z "${{ inputs.run }}" ]]; then
-          echo "run input is required"
-          exit 1
-        fi
-
        # Start the specified services
        make up

--- a/.github/workflows/ci.yml
+++ b/.github/workflows/ci.yml
@ -8,14 +8,22 @@ on:
    branches:
      - master
  workflow_dispatch:
+    inputs:
+      splits:
+        description: 'The number of splits for test_main'
+        required: true
+        default: '14'

 concurrency:
  # different events on the same ref can run in parallel
  # different refs on the same event can run in parallel
-  group: ${{ github.workflow }}-${{ github.ref }}-${{ github.event_name}}
+  # different splits on the same ref + event can run in parallel
+  group: ${{ github.workflow }}-${{ github.ref }}-${{ github.event_name}}-${{ inputs.splits}}
  cancel-in-progress: true

 env:
+  log_artifact: test_main_logs
+  log_file: report.json
  docs_artifact: docs

 jobs:
@ -270,37 +278,6 @@ jobs:
            make push_locales ARGS="${args}"
          fi

-  test_test_addons_versions_files_ratings:
-    runs-on: ubuntu-latest
-    needs: build
-
-    steps:
-      - uses: actions/checkout@v4
-
-      - name: Test (test_addons_versions_files_ratings)
-        uses: ./.github/actions/run-docker
-        with:
-          services: ''
-          digest: ${{ needs.build.outputs.digest }}
-          version: ${{ needs.build.outputs.version }}
-          run: |
-            make test_addons_versions_files_ratings
-
-  test_amo_lib_locales_and_signing:
-    runs-on: ubuntu-latest
-    needs: build
-
-    steps:
-      - uses: actions/checkout@v4
-
-      - name: Test (test_amo_lib_locales_and_signing)
-        uses: ./.github/actions/run-docker
-        with:
-          services: ''
-          digest: ${{ needs.build.outputs.digest }}
-          version: ${{ needs.build.outputs.version }}
-          run: |
-            make test_amo_lib_locales_and_signing

  test_needs_locales_compilation:
    runs-on: ubuntu-latest
@ -338,54 +315,6 @@ jobs:
            make update_assets
            make test_static_assets

-  test_devhub:
-    runs-on: ubuntu-latest
-    needs: build
-
-    steps:
-      - uses: actions/checkout@v4
-
-      - name: Test (test_devhub)
-        uses: ./.github/actions/run-docker
-        with:
-          services: ''
-          digest: ${{ needs.build.outputs.digest }}
-          version: ${{ needs.build.outputs.version }}
-          run: |
-            make test_devhub
-
-  test_main:
-    runs-on: ubuntu-latest
-    needs: build
-
-    steps:
-      - uses: actions/checkout@v4
-
-      - name: Test (test_main)
-        uses: ./.github/actions/run-docker
-        with:
-          services: ''
-          digest: ${{ needs.build.outputs.digest }}
-          version: ${{ needs.build.outputs.version }}
-          run: |
-            make test_main
-
-  test_reviewers_and_zadmin:
-    runs-on: ubuntu-latest
-    needs: build
-
-    steps:
-      - uses: actions/checkout@v4
-
-      - name: Test (test_reviewers_and_zadmin)
-        uses: ./.github/actions/run-docker
-        with:
-          services: ''
-          digest: ${{ needs.build.outputs.digest }}
-          version: ${{ needs.build.outputs.version }}
-          run: |
-            make test_reviewers_and_zadmin
-
  test_internal_routes_allowed:
    runs-on: ubuntu-latest
    needs: build
@ -417,3 +346,87 @@ jobs:
          version: ${{ needs.build.outputs.version }}
          run: |
            make test_es_tests
+
+  test_config:
+    runs-on: ubuntu-latest
+
+    outputs:
+      matrix: ${{ steps.result.outputs.matrix }}
+      splits: ${{ steps.result.outputs.splits }}
+
+    steps:
+      - uses: actions/checkout@v4
+
+      - name: Calculate splits
+        id: result
+        shell: bash
+        run: |
+          splits=${{ inputs.splits || 14 }}
+          echo "splits: $splits"
+          echo "splits=$splits" >> $GITHUB_OUTPUT
+
+          # Construct the matrix input for test_main using the groups count
+          # the matrix.group should be an array of numbers from 1 to $splits
+          matrix=[$(seq -s, 1 $splits)]
+          echo "matrix: $matrix"
+          echo "matrix=$matrix" >> $GITHUB_OUTPUT
+
+  test_main:
+    runs-on: ubuntu-latest
+    needs: [build, test_config]
+    strategy:
+      fail-fast: false
+      matrix:
+        group: ${{fromJson(needs.test_config.outputs.matrix)}}
+
+    steps:
+      - uses: actions/checkout@v4
+
+      - name: Test (test_matrix)
+        uses: ./.github/actions/run-docker
+        with:
+          services: ''
+          digest: ${{ needs.build.outputs.digest }}
+          version: ${{ needs.build.outputs.version }}
+          compose_file: docker-compose.yml
+          run: |
+            split="--splits ${{ needs.test_config.outputs.splits }}"
+            group="--group ${{ matrix.group }}"
+            report="--report-log ${{ env.log_file}}"
+            make test_main ARGS="${split} ${group} ${report}"
+
+      - name: Upload logs
+        uses: actions/upload-artifact@v4
+        with:
+          path: ${{ env.log_file }}
+          name: ${{ env.log_artifact }}-${{ matrix.group }}
+          retention-days: 1
+          overwrite: true
+
+  test_log:
+    runs-on: ubuntu-latest
+    if: always()
+    needs: [build, test_config, test_main]
+
+    steps:
+      - uses: actions/checkout@v4
+
+      - uses: actions/download-artifact@v4
+        with:
+          pattern: ${{ env.log_artifact }}*
+
+      - name: Cat logs
+        shell: bash
+        run: |
+          for dir in $(ls -d ${{ env.log_artifact }}* | sort -V); do
+            job=$(basename "$dir")
+            file="${dir}/${{ env.log_file }}"
+            if [ -f "$file" ]; then
+              cat "$file" | jq \
+                -r \
+                --arg job "$job" \
+                'select(has("when") and .when == "teardown") | "[\($job)] \(.outcome) \(.nodeid)"'
+            else
+              echo "$file: No such file or directory"
+            fi
+          done
--- a/94
+++ b/94
@ -242,62 +242,26 @@ reload: ## force django code reload

 reload-uwsgi: reload

-.PHONY: test_addons_versions_files_ratings
-test_addons_versions_files_ratings:
-	pytest \
-		-n 2	\
-		-m 'not es_tests and not needs_locales_compilation and not static_assets' \
-		-v src/olympia/addons/ src/olympia/versions/ src/olympia/files/ src/olympia/ratings/
-
-.PHONY: test_amo_lib_locales_and_signing
-test_amo_lib_locales_and_signing:
-	pytest \
-		-n 2 \
-		-m 'not es_tests and not needs_locales_compilation and not static_assets' \
-		-v src/olympia/amo/ src/olympia/lib/ src/olympia/signing
+PYTEST_SRC := src/olympia/

 .PHONY: test_needs_locales_compilation
 test_needs_locales_compilation:
-	pytest \
+	pytest $(PYTEST_SRC) \
 		-m 'needs_locales_compilation' \
-		-v src/olympia/
+		$(ARGS)

 .PHONY: test_static_assets
 test_static_assets: run_js_tests
-	pytest \
+	pytest $(PYTEST_SRC) \
 		-m 'static_assets' \
-		-v src/olympia/
-
-.PHONY: test_devhub
-test_devhub:
-	pytest \
-		-n 2 \
-		-m 'not es_tests and not needs_locales_compilation and not static_assets' \
-		-v src/olympia/devhub/
+		$(ARGS)

 .PHONY: test_main
 test_main:
-	pytest \
-	-n 2 \
-	-m 'not es_tests and not needs_locales_compilation and not static_assets and not internal_routes_allowed' \
-	-v src/olympia/ \
-	--ignore src/olympia/addons/ \
-	--ignore src/olympia/devhub/ \
-	--ignore src/olympia/files/ \
-	--ignore src/olympia/reviewers/ \
-	--ignore src/olympia/ratings/ \
-	--ignore src/olympia/amo/ \
-	--ignore src/olympia/lib/ \
-	--ignore src/olympia/signing \
-	--ignore src/olympia/versions/ \
-	--ignore src/olympia/zadmin
-
-.PHONY: test_reviewers_and_zadmin
-test_reviewers_and_zadmin:
-	pytest \
-		-n 2 \
-		-m 'not es_tests and not needs_locales_compilation and not static_assets' \
-		-v src/olympia/reviewers/ src/olympia/zadmin/
+	pytest $(PYTEST_SRC) \
+		-n auto \
+		-m 'not es_tests and not needs_locales_compilation and not static_assets and not internal_routes_allowed' \
+		$(ARGS)

 .PHONY: test_internal_routes_allowed
 test_internal_routes_allowed:
@ -305,38 +269,58 @@ test_internal_routes_allowed:
 # override an env variable here, and the next command requires
 # `INTERNAL_ROUTES_ALLOWED` to be set to `True`.
 	sed -i 's/^INTERNAL_ROUTES_ALLOWED.*/INTERNAL_ROUTES_ALLOWED=True/' settings_test.py
-	pytest -m 'internal_routes_allowed' -v src/olympia/
+	pytest \
+		$(PYTEST_SRC) \
+		-m 'internal_routes_allowed' \
+		$(ARGS)

 .PHONY: test_es_tests
 test_es_tests:
 	pytest \
-		-m "es_tests and not needs_locales_compilation and not static_assets" \
-		-v src/olympia/
-
+		$(PYTEST_SRC) \
+		-m 'es_tests and not needs_locales_compilation and not static_assets' \
+		$(ARGS)

 .PHONY: test
 test: ## run the entire test suite
-	pytest $(APP) $(ARGS)
+	pytest \
+		$(PYTEST_SRC) \
+		$(ARGS)

 .PHONY: test_es
 test_es: ## run the ES tests
-	pytest -m es_tests $(APP) $(ARGS)
+	pytest \
+		$(PYTEST_SRC) \
+		-m es_tests \
+		$(ARGS)

 .PHONY: test_no_es
 test_no_es: ## run all but the ES tests
-	pytest -m "not es_tests" $(APP) $(ARGS)
+	pytest \
+		$(PYTEST_SRC) \
+		-m "not es_tests" \
+		$(ARGS)

 .PHONY: test_force_db
 test_force_db: ## run the entire test suite with a new database
-	pytest --create-db $(APP) $(ARGS)
+	pytest \
+		$(PYTEST_SRC) \
+		--create-db \
+		$(ARGS)

 .PHONY: tdd
 tdd: ## run the entire test suite, but stop on the first error
-	pytest -x --pdb $(ARGS) $(APP)
+	pytest \
+		$(PYTEST_SRC) \
+		-x --pdb \
+		$(ARGS)

 .PHONY: test_failed
 test_failed: ## rerun the failed tests from the previous run
-	pytest --lf $(ARGS) $(APP)
+	pytest \
+	$(PYTEST_SRC) \
+	--lf \
+	$(ARGS)

 .PHONY: run_js_tests
 run_js_tests: ## Run the JavaScript test suite (requires compiled/compressed assets).
--- a/requirements/dev.txt
+++ b/requirements/dev.txt
@ -338,3 +338,9 @@ polib==1.2.0 \
 click==8.1.7 \
    --hash=sha256:ae74fb96c20a0277a1d615f1e4d73c8414f5a98db8b799a7931d1582f3390c28 \
    --hash=sha256:ca9853ad459e787e2192211578cc907e7594e294c7ccc834310722b41b9ca6de
+pytest-split==0.8.2 \
+    --hash=sha256:446f330e3607572027f3861058c27d9b3eaa80d83dc86675abe2978bbf50c02f \
+    --hash=sha256:b7fa704659cb224b9f7f5c24536bc04eff351f42d852bf0312e03774fd9c0972
+pytest-reportlog==0.4.0 \
+    --hash=sha256:5db4d00586546d8c6b95c66466629f1e913440c36d97795a673d2e19c5cedd5c \
+    --hash=sha256:c9f2079504ee51f776d3118dcf5e4730f163d3dcf26ebc8f600c1fa307bf638c
--- a/settings_test.py
+++ b/settings_test.py
@ -90,6 +90,7 @@ CELERY_TASK_ROUTES.update(
        'olympia.amo.tests.test_celery.fake_task_with_result': {'queue': 'amo'},
        'olympia.amo.tests.test_celery.sleeping_task': {'queue': 'amo'},
        'olympia.search.tests.test_commands.dummy_task': {'queue': 'amo'},
+        'olympia.devhub.tests.test_tasks.fake_task': {'queue': 'amo'},
    }
 )

--- a/src/olympia/users/tests/test_admin.py
+++ b/src/olympia/users/tests/test_admin.py
@ -48,7 +48,7 @@ class TestUserAdmin(TestCase):
        )

    def test_search_by_email_simple(self):
-        user = user_factory(email='someone@mozilla.com')
+        user = user_factory(email='someone@mozilla.com', username='test')
        self.grant_permission(user, 'Users:Edit')
        self.client.force_login(user)
        another_user = user_factory()
@ -81,7 +81,7 @@ class TestUserAdmin(TestCase):
        user = user_factory(email='someone@mozilla.com')
        self.grant_permission(user, 'Users:Edit')
        self.client.force_login(user)
-        another_user = user_factory(email='someone@notzilla.org')
+        another_user = user_factory(email='someone@notzilla.org', username='test')
        response = self.client.get(
            self.list_url,
            {'q': 'some*@notzilla.org'},
@ -110,7 +110,7 @@ class TestUserAdmin(TestCase):
        assert str(user.pk) not in doc('#result_list').text()

    def test_search_by_email_multiple_like(self):
-        user = user_factory(email='someone@mozilla.com')
+        user = user_factory(email='someone@mozilla.com', username='test')
        self.grant_permission(user, 'Users:Edit')
        self.client.force_login(user)
        another_user = user_factory(email='someone@notzilla.com')