Locations V3: add import performance test and autocorrect counts (#14501)

valentijnscholten · github-actions[bot] · web-flow · commit 5dc55a329041 · 2026-03-16T08:48:46.000-06:00
* Locations V3: add import performance test * feat: run performance tests in separate CI job with auto-update - Tag performance test classes with @tag('performance') - Exclude performance tests from main unit test suite - Add keep-alive compose override for performance test CI - Add performance-tests.yml workflow that auto-updates counts and commits - Wire performance tests job into unit-tests.yml * fix: correct artifact name for docker image download * fix: remove manual migration step, update script handles it via --keepdb * fix: set explicit celery broker URL in unit test compose overrides Replace individual DD_CELERY_BROKER_* vars with a single explicit DD_CELERY_BROKER_URL in both unit test compose overrides. This makes the SQLite broker self-contained in the compose file rather than relying on the entrypoint script to unset DD_CELERY_BROKER_URL. Also update performance-tests.yml to reference docker-compose.override.unit_tests_cicd.yml explicitly instead of the docker-compose.override.yml symlink. * perf: run full test class at once in update script instead of per-method * fix: push to head branch by name in detached HEAD state * fix: fetch and rebase before and after performance tests to handle remote ahead * remove rebase * remove rebase * checkout correct ref * chore: update performance test counts [skip ci] * test with wrong counts * chore: update performance test counts [skip ci] * chore: update performance test counts [skip ci] * fix(ci): remove [skip ci] so checks remain visible after auto-update commit * fix(ci): pass head_ref to performance-tests, expand cancel workflow list - Add head_ref input to performance-tests (workflow_call context bug fix) - Pass head_ref from unit-tests so checkout/push use correct PR branch - Fix cancel workflow typo: k8s-testing.yml -> k8s-tests.yml - Add validate_docs_build, test-helm-chart, ruff, shellcheck to cancel list * test wrong counts * fix * chore: update performance test counts * fix(ci): re-trigger unit-tests after auto-update commit, guard against loop GITHUB_TOKEN pushes do not trigger new workflow runs, so after auto-updating performance test counts, dispatch unit-tests.yml manually to ensure the PR gets fresh CI checks on the updated commit. Add a loop guard: if the remote branch's last commit was already an auto-update and counts still differ, abort with an error instead of looping indefinitely. * text * reduce timeout * test incorrect count * chore: update performance test counts * fix(ci): use PAT for auto-update push to trigger pull_request:synchronize GITHUB_TOKEN pushes are silently ignored by GitHub's workflow trigger system, so CI checks never run on the auto-updated commit. Pushing via a PAT (secrets.GH_TOKEN) fires a pull_request:synchronize event which re-triggers unit-tests.yml naturally, without needing workflow_dispatch. * test wrong count * chore: update performance test counts * fix(ci): fail on wrong performance test counts instead of auto-committing Remove the auto-commit logic from the performance tests workflow. If counts are out of date, fail the job and print the diff along with the command to fix it locally. This avoids all the complexity around GITHUB_TOKEN not triggering CI on bot pushes. * fix counts --------- Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
diff --git a/.github/workflows/cancel-outdated-workflow-runs.yml b/.github/workflows/cancel-outdated-workflow-runs.yml
@@ -15,5 +15,5 @@ jobs:
     steps:
       - uses: styfle/cancel-workflow-action@3155a141048f8f89c06b4cdae32e7853e97536bc # 0.13.0
         with:
-          workflow_id: 'integration-tests.yml,k8s-testing.yml,unit-tests.yml'
+          workflow_id: 'integration-tests.yml,k8s-tests.yml,unit-tests.yml,validate_docs_build.yml,test-helm-chart.yml,ruff.yml,shellcheck.yml'
           access_token: ${{ github.token }}
diff --git a/.github/workflows/performance-tests.yml b/.github/workflows/performance-tests.yml
@@ -0,0 +1,74 @@
+name: Performance Tests
+
+on:
+  workflow_call:
+
+jobs:
+  performance-tests:
+    name: Performance Tests
+    runs-on: ubuntu-latest
+    needs: []
+    permissions:
+      contents: read
+
+    steps:
+      - name: Checkout
+        uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2
+
+      - name: Set-platform
+        run: |
+          echo "PLATFORM=linux-amd64" >> $GITHUB_ENV
+
+      - name: Load images from artifacts
+        uses: actions/download-artifact@70fc10c6e5e1ce46ad2ea6f2b72d43f7d47b13c3 # v8.0.0
+        with:
+          path: built-docker-image
+          pattern: built-docker-image-django-alpine-linux-amd64
+          merge-multiple: true
+
+      - name: Load docker images
+        timeout-minutes: 10
+        run: |
+          docker load -i built-docker-image/django-alpine-linux-amd64_img
+          docker images
+
+      - name: Set unit-test mode
+        run: docker/setEnv.sh unit_tests_cicd
+
+      - name: Start Postgres and webhook.endpoint
+        run: docker compose up --no-deps -d postgres webhook.endpoint
+
+      - name: Start uwsgi (idle)
+        timeout-minutes: 5
+        run: |
+          docker compose -f docker-compose.yml -f docker-compose.override.unit_tests_cicd.yml \
+            -f docker/docker-compose.override.performance_tests_cicd.yml \
+            up -d --no-deps uwsgi
+        env:
+          DJANGO_VERSION: alpine
+
+      - name: Run performance tests (auto-update counts)
+        timeout-minutes: 15
+        run: python3 scripts/update_performance_test_counts.py
+
+      - name: Check counts are up to date
+        run: |
+          if ! git diff --quiet unittests/test_importers_performance.py; then
+            echo "Performance test counts are out of date. Fix them by running locally:"
+            echo ""
+            echo "  python3 scripts/update_performance_test_counts.py"
+            echo ""
+            echo "Diff:"
+            git diff unittests/test_importers_performance.py
+            exit 1
+          else
+            echo "Performance test counts are up to date."
+          fi
+
+      - name: Logs
+        if: failure()
+        run: docker compose logs --tail="2500" uwsgi
+
+      - name: Shutdown
+        if: always()
+        run: docker compose down
diff --git a/.github/workflows/unit-tests.yml b/.github/workflows/unit-tests.yml
@@ -21,6 +21,11 @@ jobs:
     with:
       platform: ${{ matrix.platform }}
 
+  test-performance:
+    needs: build-docker-containers
+    uses: ./.github/workflows/performance-tests.yml
+    secrets: inherit
+
   test-rest-framework:
     strategy:
       matrix:
@@ -45,3 +50,4 @@ jobs:
     needs: build-docker-containers
     uses: ./.github/workflows/k8s-tests.yml
     secrets: inherit
+
diff --git a/docker-compose.override.unit_tests.yml b/docker-compose.override.unit_tests.yml
@@ -21,13 +21,7 @@ services:
       DD_DATABASE_PORT: ${DD_DATABASE_PORT:-5432}
       DD_DATABASE_USER: ${DD_DATABASE_USER:-defectdojo}
       DD_DATABASE_PASSWORD: ${DD_DATABASE_PASSWORD:-defectdojo}
-      DD_CELERY_BROKER_SCHEME: 'sqla+sqlite'
-      DD_CELERY_BROKER_USER: ''
-      DD_CELERY_BROKER_PASSWORD: ''
-      DD_CELERY_BROKER_HOST: ''
-      DD_CELERY_BROKER_PORT: "-1"
-      DD_CELERY_BROKER_PATH: '/dojo.celerydb.sqlite'
-      DD_CELERY_BROKER_PARAMS: ''
+      DD_CELERY_BROKER_URL: 'sqla+sqlite:///dojo.celerydb.sqlite'
       DD_JIRA_EXTRA_ISSUE_TYPES: 'Vulnerability' # Shouldn't trigger a migration error
   celerybeat: !reset
   celeryworker: !reset
diff --git a/docker-compose.override.unit_tests_cicd.yml b/docker-compose.override.unit_tests_cicd.yml
@@ -20,13 +20,7 @@ services:
       DD_DATABASE_ENGINE: ${DD_DATABASE_ENGINE:-django.db.backends.postgresql}
       DD_DATABASE_HOST: ${DD_DATABASE_HOST:-postgres}
       DD_DATABASE_PORT: ${DD_DATABASE_PORT:-5432}
-      DD_CELERY_BROKER_SCHEME: 'sqla+sqlite'
-      DD_CELERY_BROKER_USER: ''
-      DD_CELERY_BROKER_PASSWORD: ''
-      DD_CELERY_BROKER_HOST: ''
-      DD_CELERY_BROKER_PORT: "-1"
-      DD_CELERY_BROKER_PATH: '/dojo.celerydb.sqlite'
-      DD_CELERY_BROKER_PARAMS: ''
+      DD_CELERY_BROKER_URL: 'sqla+sqlite:///dojo.celerydb.sqlite'
       DD_JIRA_EXTRA_ISSUE_TYPES: 'Vulnerability' # Shouldn't trigger a migration error
       DD_V3_FEATURE_LOCATIONS: ${DD_V3_FEATURE_LOCATIONS:-False}
   celerybeat: !reset
diff --git a/docker/docker-compose.override.performance_tests_cicd.yml b/docker/docker-compose.override.performance_tests_cicd.yml
@@ -0,0 +1,6 @@
+---
+# uwsgi container with no running daemon, used to run performance tests via run-unittest.sh
+services:
+  uwsgi:
+    entrypoint: ["tail", "-f", "/dev/null"]
+    command: []
diff --git a/docker/entrypoint-unit-tests-devDocker.sh b/docker/entrypoint-unit-tests-devDocker.sh
@@ -73,10 +73,10 @@ echo "Unit Tests"
 echo "------------------------------------------------------------"
 
 # Removing parallel and shuffle for now to maintain stability
-python3 manage.py test unittests -v 3 --keepdb --no-input --exclude-tag="non-parallel" || {
+python3 manage.py test unittests -v 3 --keepdb --no-input --exclude-tag="non-parallel" --exclude-tag="performance" || {
     exit 1;
 }
-python3 manage.py test unittests -v 3 --keepdb --no-input --tag="non-parallel" || {
+python3 manage.py test unittests -v 3 --keepdb --no-input --tag="non-parallel" --exclude-tag="performance" || {
     exit 1;
 }
 
diff --git a/docker/entrypoint-unit-tests.sh b/docker/entrypoint-unit-tests.sh
@@ -80,16 +80,16 @@ echo "Unit Tests"
 echo "------------------------------------------------------------"
 
 # Removing parallel and shuffle for now to maintain stability
-python3 manage.py test unittests --keepdb --no-input --exclude-tag="non-parallel" --exclude-tag="transactional" || {
+python3 manage.py test unittests --keepdb --no-input --exclude-tag="non-parallel" --exclude-tag="transactional" --exclude-tag="performance" || {
     exit 1;
 }
-python3 manage.py test unittests --keepdb --no-input --tag="non-parallel" || {
+python3 manage.py test unittests --keepdb --no-input --tag="non-parallel" --exclude-tag="performance" || {
     exit 1;
 }
 # Running one unit tests that inherits from TransactionTestCase somehow changes the behaviour of how Django loads fixtures into the database.
 # Meaning any test after this one would fail to load our dojo_testdata.json fixture. In a way this makes sense as it contains some data integrity problems.
 # I tried to fix these in https://github.com/DefectDojo/django-DefectDojo/pull/13217.
 # For now here we run the only TranscationTestCase at the end to avoid the problem.
-python3 manage.py test unittests --keepdb --no-input --tag="transactional" || {
+python3 manage.py test unittests --keepdb --no-input --tag="transactional" --exclude-tag="performance" || {
     exit 1;
 }
diff --git a/dojo/base_models/base.py b/dojo/base_models/base.py
@@ -42,7 +42,7 @@ class Meta:
 
         abstract = True
 
-    def save(self, *args: list, skip_validation: bool = not settings.V3_FEATURE_LOCATIONS, **kwargs: dict) -> None:
+    def save(self, *args: list, skip_validation: bool | None = None, **kwargs: dict) -> None:
         """
         Override save method to call the `full_clean()` validation function each save.
 
@@ -53,6 +53,9 @@ def save(self, *args: list, skip_validation: bool = not settings.V3_FEATURE_LOCA
         - Validate the field uniqueness - `validate_unique()`
         All three steps are performed when you call a model's full_clean() method in the order above
         """
+        # make sure this is evaluated at runtime, not at class definition time which would be the case if we used it in the parameter default value
+        if skip_validation is None:
+            skip_validation = not settings.V3_FEATURE_LOCATIONS
         # Run the pre save logic, if enabled
         self.pre_save_logic()
         # Call the validations
diff --git a/readme-docs/CONTRIBUTING.md b/readme-docs/CONTRIBUTING.md
@@ -55,6 +55,24 @@ Please use [these test scripts](../tests) to test your changes. These are the sc
 
 For changes that require additional settings, you can now use local_settings.py file. See the logging section below for more information.
 
+## Updating Performance Test Query Counts
+
+The importer performance tests in `unittests/test_importers_performance.py` assert on expected database query and async task counts. If your changes affect import behavior (e.g., adding queries or changing celery task usage), these counts may need to be updated.
+
+Run the update script to refresh expected counts:
+
+```bash
+python3 scripts/update_performance_test_counts.py
+```
+
+The script runs both `TestDojoImporterPerformanceSmall` (v2 endpoints) and `TestDojoImporterPerformanceSmallLocations` (v3 locations), captures actual counts, and updates the test file when they differ from expectations.
+
+To verify all tests pass after updating:
+
+```bash
+python3 scripts/update_performance_test_counts.py --verify
+```
+
 ## Python3 Version
 For compatibility reasons, the code in dev branch should be python3.13 compliant.
 
diff --git a/scripts/update_performance_test_counts.py b/scripts/update_performance_test_counts.py
diff --git a/unittests/test_importers_performance.py b/unittests/test_importers_performance.py

Original file line number	Diff line number	Diff line change
`@@ -73,10 +73,10 @@ echo "Unit Tests"`
`73`	`73`	`echo "------------------------------------------------------------"`
`74`	`74`
`75`	`75`	`# Removing parallel and shuffle for now to maintain stability`
`76`		`-python3 manage.py test unittests -v 3 --keepdb --no-input --exclude-tag="non-parallel" \|\| {`
	`76`	`+python3 manage.py test unittests -v 3 --keepdb --no-input --exclude-tag="non-parallel" --exclude-tag="performance" \|\| {`
`77`	`77`	`exit 1;`
`78`	`78`	`}`
`79`		`-python3 manage.py test unittests -v 3 --keepdb --no-input --tag="non-parallel" \|\| {`
	`79`	`+python3 manage.py test unittests -v 3 --keepdb --no-input --tag="non-parallel" --exclude-tag="performance" \|\| {`
`80`	`80`	`exit 1;`
`81`	`81`	`}`
`82`	`82`
Original file line number	Diff line number	Diff line change
`@@ -80,16 +80,16 @@ echo "Unit Tests"`
`80`	`80`	`echo "------------------------------------------------------------"`
`81`	`81`
`82`	`82`	`# Removing parallel and shuffle for now to maintain stability`
`83`		`-python3 manage.py test unittests --keepdb --no-input --exclude-tag="non-parallel" --exclude-tag="transactional" \|\| {`
	`83`	`+python3 manage.py test unittests --keepdb --no-input --exclude-tag="non-parallel" --exclude-tag="transactional" --exclude-tag="performance" \|\| {`
`84`	`84`	`exit 1;`
`85`	`85`	`}`
`86`		`-python3 manage.py test unittests --keepdb --no-input --tag="non-parallel" \|\| {`
	`86`	`+python3 manage.py test unittests --keepdb --no-input --tag="non-parallel" --exclude-tag="performance" \|\| {`
`87`	`87`	`exit 1;`
`88`	`88`	`}`
`89`	`89`	`# Running one unit tests that inherits from TransactionTestCase somehow changes the behaviour of how Django loads fixtures into the database.`
`90`	`90`	`# Meaning any test after this one would fail to load our dojo_testdata.json fixture. In a way this makes sense as it contains some data integrity problems.`
`91`	`91`	`# I tried to fix these in https://github.com/DefectDojo/django-DefectDojo/pull/13217.`
`92`	`92`	`# For now here we run the only TranscationTestCase at the end to avoid the problem.`
`93`		`-python3 manage.py test unittests --keepdb --no-input --tag="transactional" \|\| {`
	`93`	`+python3 manage.py test unittests --keepdb --no-input --tag="transactional" --exclude-tag="performance" \|\| {`
`94`	`94`	`exit 1;`
`95`	`95`	`}`