Unit Test Improvements (#5087)

* Disable migration testing

- Compare how long the unit tests take

* Change file

- To get unit tests to run

* Fix format

* Consolidate tasks.py

- Remove coverage task
- Add --coverage flag to test task

* Fix typo

* Run migration unit tests if migration files are updated

* Fix

* Touch migration file

- Should cause migration unit tests to be run

* Force migration checks for docker build

* Prevent default report creation in unit tests

- Should save some time

* Add simple profiling for plugin loading

- Display time taken to load each plugin

* Fix to invoke test

* Disable get_git_log (for testing)

* Disable get_git_path in CI

- Might remove this entirely?
- For now, bypass for unit testing

* Add debug for unit registry

- Display time taken to load registry

* Don't full-reload unit registry

* Adjust migration test workflow

- env var updates
- change paths-filter output

* Fix for migration_test.yaml

- Actually need to set the output

* env fix

* db name

* Prevent sleep if in test mode

* Reduce sleep time on wait_for_db

.github/workflows/docker.yaml

@@ -78,6 +78,7 @@ jobs:
         run: |
           echo "GITHUB_TOKEN=${{ secrets.GITHUB_TOKEN }}" >> docker.dev.env
           docker-compose run inventree-dev-server invoke test --disable-pty
+          docker-compose run inventree-dev-server invoke test --migrations --disable-pty
           docker-compose down
       - name: Set up QEMU
         if: github.event_name != 'pull_request'

.github/workflows/migration_test.yaml

@@ -16,11 +16,11 @@ on:
 env:
   python_version: 3.9
   GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
-  INVENTREE_DB_ENGINE: sqlite3
-  INVENTREE_DB_NAME: /home/runner/work/InvenTree/db.sqlite3
   INVENTREE_MEDIA_ROOT: ../test_inventree_media
   INVENTREE_STATIC_ROOT: ../test_inventree_static
   INVENTREE_BACKUP_DIR: ../test_inventree_backup
+  INVENTREE_DEBUG: info
+  INVENTREE_PLUGINS_ENABLED: false
 
 jobs:
   paths-filter:
@@ -28,7 +28,7 @@ jobs:
     runs-on: ubuntu-latest
 
     outputs:
-      server: ${{ steps.filter.outputs.server }}
+      migrations: ${{ steps.filter.outputs.migrations }}
 
     steps:
     - uses: actions/checkout@93ea575cb5d8a053eaa0ac8fa3b40d7e05a33cc8 # pin@v3.1.0
@@ -36,17 +36,54 @@ jobs:
       id: filter
       with:
         filters: |
-          server:
-            - 'InvenTree/**'
-            - 'requirements.txt'
-            - 'requirements-dev.txt'
-            - '.github/**'
+          migrations:
+            - '**/migrations/**'
+            - '.github/workflows**'
+
+  migration-tests:
+    name: Run Migration Unit Tests
+    runs-on: ubuntu-latest
+    needs: paths-filter
+    if: needs.paths-filter.outputs.migrations == 'true'
+
+    env:
+      INVENTREE_DB_ENGINE: django.db.backends.postgresql
+      INVENTREE_DB_NAME: inventree
+      INVENTREE_DB_USER: inventree
+      INVENTREE_DB_PASSWORD: password
+      INVENTREE_DB_HOST: '127.0.0.1'
+      INVENTREE_DB_PORT: 5432
+
+    services:
+      postgres:
+        image: postgres:14
+        env:
+          POSTGRES_USER: inventree
+          POSTGRES_PASSWORD: password
+        ports:
+          - 5432:5432
+
+    steps:
+      - uses: actions/checkout@93ea575cb5d8a053eaa0ac8fa3b40d7e05a33cc8 # pin@v3.1.0
+      - name: Environment Setup
+        uses: ./.github/actions/setup
+        with:
+          apt-dependency: gettext poppler-utils libpq-dev
+          pip-dependency: psycopg2
+          dev-install: true
+          update: true
+      - name: Run Tests
+        run: invoke test --migrations --report
 
   migrations-checks:
     name: Run Database Migrations
     runs-on: ubuntu-latest
     needs: paths-filter
-    if: needs.paths-filter.outputs.server == 'true'
+    if: needs.paths-filter.outputs.migrations == 'true'
+
+    env:
+      INVENTREE_DB_ENGINE: sqlite3
+      INVENTREE_DB_NAME: /home/runner/work/InvenTree/db.sqlite3
 
     steps:
       - uses: actions/checkout@v3

.github/workflows/qc_checks.yaml

@@ -204,7 +204,7 @@ jobs:
       - name: Check Migration Files
         run: python3 ci/check_migration_files.py
       - name: Coverage Tests
-        run: invoke coverage
+        run: invoke test --coverage
       - name: Upload Coverage Report
         uses: coverallsapp/github-action@v2
         with:

InvenTree/InvenTree/apps.py

@@ -48,7 +48,7 @@ class InvenTreeConfig(AppConfig):
         self.collect_notification_methods()
 
         # Ensure the unit registry is loaded
-        InvenTree.conversion.reload_unit_registry()
+        InvenTree.conversion.get_unit_registry()
 
         if canAppAccessDatabase() or settings.TESTING_ENV:
             self.add_user_on_startup()

InvenTree/InvenTree/conversion.py

@@ -1,5 +1,7 @@
 """Helper functions for converting between units."""
 
+import logging
+
 from django.core.exceptions import ValidationError
 from django.utils.translation import gettext_lazy as _
 
@@ -8,6 +10,9 @@ import pint
 _unit_registry = None
 
 
+logger = logging.getLogger('inventree')
+
+
 def get_unit_registry():
     """Return a custom instance of the Pint UnitRegistry."""
 
@@ -26,6 +31,9 @@ def reload_unit_registry():
     This function is called at startup, and whenever the database is updated.
     """
 
+    import time
+    t_start = time.time()
+
     global _unit_registry
 
     _unit_registry = pint.UnitRegistry()
@@ -39,6 +47,9 @@ def reload_unit_registry():
 
     # TODO: Allow for custom units to be defined in the database
 
+    dt = time.time() - t_start
+    logger.debug(f'Loaded unit registry in {dt:.3f}s')
+
 
 def convert_physical_value(value: str, unit: str = None):
     """Validate that the provided value is a valid physical quantity.

InvenTree/InvenTree/management/commands/wait_for_db.py

@@ -18,7 +18,7 @@ class Command(BaseCommand):
 
         while not connected:
 
-            time.sleep(5)
+            time.sleep(2)
 
             try:
                 connection.ensure_connection()

InvenTree/InvenTree/tasks.py

@@ -91,12 +91,14 @@ def check_daily_holdoff(task_name: str, n_days: int = 1) -> bool:
     """
 
     from common.models import InvenTreeSetting
+    from InvenTree.ready import isInTestMode
 
     if n_days <= 0:
         logger.info(f"Specified interval for task '{task_name}' < 1 - task will not run")
         return False
 
     # Sleep a random number of seconds to prevent worker conflict
+    if not isInTestMode():
         time.sleep(random.randint(1, 5))
 
     attempt_key = f'_{task_name}_ATTEMPT'

InvenTree/common/migrations/0017_notesimage.py

@@ -1,4 +1,4 @@
-# Generated by Django 3.2.18 on 2023-04-17 05:54
+# Generated by Django 3.2.18 on 2023-04-17 05:55
 
 from django.conf import settings
 from django.db import migrations, models

InvenTree/label/apps.py

@@ -35,7 +35,7 @@ class LabelConfig(AppConfig):
 
     def ready(self):
         """This function is called whenever the label app is loaded."""
-        if canAppAccessDatabase():
+        if canAppAccessDatabase(allow_test=False):
 
             try:
                 self.create_labels()  # pragma: no cover

InvenTree/plugin/helpers.py

@@ -1,6 +1,5 @@
 """Helpers for plugin app."""
 
-import datetime
 import inspect
 import logging
 import pathlib
@@ -14,8 +13,6 @@ from django.conf import settings
 from django.core.exceptions import AppRegistryNotReady
 from django.db.utils import IntegrityError
 
-from dulwich.repo import NotGitRepository, Repo
-
 logger = logging.getLogger('inventree')
 
 
@@ -112,9 +109,18 @@ def get_entrypoints():
 def get_git_log(path):
     """Get dict with info of the last commit to file named in path."""
 
+    import datetime
+
+    from dulwich.repo import NotGitRepository, Repo
+
+    from InvenTree.ready import isInTestMode
+
     output = None
     path = path.replace(str(settings.BASE_DIR.parent), '')[1:]
 
+    # only do this if we are not in test mode
+    if not isInTestMode():  # pragma: no cover
+
         try:
             walker = Repo.discover(path).get_walker(paths=[path.encode()], max_entries=1)
             try:

InvenTree/plugin/registry.py

@@ -9,6 +9,7 @@ import importlib
 import logging
 import os
 import subprocess
+import time
 from pathlib import Path
 from typing import Dict, List, OrderedDict
 
@@ -439,13 +440,16 @@ class PluginsRegistry:
                     continue  # continue -> the plugin is not loaded
 
                 # Initialize package - we can be sure that an admin has activated the plugin
-                logger.info(f'Loading plugin `{plg_name}`')
+                logger.debug(f'Loading plugin `{plg_name}`')
 
                 try:
+                    t_start = time.time()
                     plg_i: InvenTreePlugin = plg()
-                    logger.debug(f'Loaded plugin `{plg_name}`')
+                    dt = time.time() - t_start
+                    logger.info(f'Loaded plugin `{plg_name}` in {dt:.3f}s')
                 except Exception as error:
                     handle_error(error, log_name='init')  # log error and raise it -> disable plugin
+                    logger.warning(f"Plugin `{plg_name}` could not be loaded")
 
                 # Safe extra attributes
                 plg_i.is_package = getattr(plg_i, 'is_package', False)

InvenTree/plugin/test_api.py

@@ -10,7 +10,7 @@ from plugin.models import PluginConfig
 
 
 class PluginDetailAPITest(PluginMixin, InvenTreeAPITestCase):
-    """Tests the plugin API endpoints."""
+    """Tests the plugin API endpoints"""
 
     roles = [
         'admin.add',

InvenTree/report/apps.py

@@ -25,7 +25,7 @@ class ReportConfig(AppConfig):
         logging.getLogger('weasyprint').setLevel(logging.WARNING)
 
         # Create entries for default report templates
-        if canAppAccessDatabase(allow_test=True):
+        if canAppAccessDatabase(allow_test=False):
             self.create_default_test_reports()
             self.create_default_build_reports()
             self.create_default_bill_of_materials_reports()

tasks.py

@@ -565,9 +565,12 @@ def test_translations(c):
     help={
         'disable_pty': 'Disable PTY',
         'runtest': 'Specify which tests to run, in format <module>.<file>.<class>.<method>',
+        'migrations': 'Run migration unit tests',
+        'report': 'Display a report of slow tests',
+        'coverage': 'Run code coverage analysis (requires coverage package)',
     }
 )
-def test(c, disable_pty=False, runtest=''):
+def test(c, disable_pty=False, runtest='', migrations=False, report=False, coverage=False):
     """Run unit-tests for InvenTree codebase.
 
     To run only certain test, use the argument --runtest.
@@ -583,8 +586,32 @@ def test(c, disable_pty=False, runtest=''):
 
     pty = not disable_pty
 
-    # Run coverage tests
-    manage(c, f'test --slowreport {runtest}', pty=pty)
+    _apps = ' '.join(apps())
+
+    cmd = 'test'
+
+    if runtest:
+        # Specific tests to run
+        cmd += f' {runtest}'
+    else:
+        # Run all tests
+        cmd += f' {_apps}'
+
+    if report:
+        cmd += ' --slowreport'
+
+    if migrations:
+        cmd += ' --tag migration_test'
+    else:
+        cmd += ' --exclude-tag migration_test'
+
+    if coverage:
+        # Run tests within coverage environment, and generate report
+        c.run(f'coverage run {managePyPath()} {cmd}')
+        c.run('coverage html -i')
+    else:
+        # Run simple test runner, without coverage
+        manage(c, cmd, pty=pty)
 
 
 @task(help={'dev': 'Set up development environment at the end'})
@@ -629,25 +656,6 @@ def setup_test(c, ignore_update=False, dev=False, path="inventree-demo-dataset")
         setup_dev(c)
 
 
-@task
-def coverage(c):
-    """Run code-coverage of the InvenTree codebase, using the 'coverage' code-analysis tools.
-
-    Generates a code coverage report (available in the htmlcov directory)
-    """
-    # Run sanity check on the django install
-    manage(c, 'check')
-
-    # Run coverage tests
-    c.run('coverage run {manage} test {apps}'.format(
-        manage=managePyPath(),
-        apps=' '.join(apps())
-    ))
-
-    # Generate coverage report
-    c.run('coverage html -i')
-
-
 @task(help={
     'filename': "Output filename (default = 'schema.yml')",
     'overwrite': "Overwrite existing files without asking first (default = off/False)",