From 3b4e20b54a486846eafce78a2a382c5d42d51cd2 Mon Sep 17 00:00:00 2001 From: Oliver Date: Fri, 23 Jun 2023 17:25:59 +1000 Subject: [PATCH] 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 | 1 + .github/workflows/migration_test.yaml | 55 ++++++++++++++++--- .github/workflows/qc_checks.yaml | 2 +- InvenTree/InvenTree/apps.py | 2 +- InvenTree/InvenTree/conversion.py | 11 ++++ .../management/commands/wait_for_db.py | 2 +- InvenTree/InvenTree/tasks.py | 4 +- .../common/migrations/0017_notesimage.py | 2 +- InvenTree/label/apps.py | 2 +- InvenTree/plugin/helpers.py | 40 ++++++++------ InvenTree/plugin/registry.py | 8 ++- InvenTree/plugin/test_api.py | 2 +- InvenTree/report/apps.py | 2 +- tasks.py | 52 ++++++++++-------- 14 files changed, 127 insertions(+), 58 deletions(-) diff --git a/.github/workflows/docker.yaml b/.github/workflows/docker.yaml index 4e814cbcca..34e319e048 100644 --- a/.github/workflows/docker.yaml +++ b/.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' diff --git a/.github/workflows/migration_test.yaml b/.github/workflows/migration_test.yaml index 1b25d0dd07..6fa9431f2c 100644 --- a/.github/workflows/migration_test.yaml +++ b/.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 diff --git a/.github/workflows/qc_checks.yaml b/.github/workflows/qc_checks.yaml index c89cf7a8f2..439fcf59c0 100644 --- a/.github/workflows/qc_checks.yaml +++ b/.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: diff --git a/InvenTree/InvenTree/apps.py b/InvenTree/InvenTree/apps.py index 4d2705be50..8a286a5982 100644 --- a/InvenTree/InvenTree/apps.py +++ b/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() diff --git a/InvenTree/InvenTree/conversion.py b/InvenTree/InvenTree/conversion.py index e2d114c728..8637ae382b 100644 --- a/InvenTree/InvenTree/conversion.py +++ b/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. diff --git a/InvenTree/InvenTree/management/commands/wait_for_db.py b/InvenTree/InvenTree/management/commands/wait_for_db.py index 00590ef602..3bcd2f5ffd 100644 --- a/InvenTree/InvenTree/management/commands/wait_for_db.py +++ b/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() diff --git a/InvenTree/InvenTree/tasks.py b/InvenTree/InvenTree/tasks.py index 6c813731f0..9c57a881c9 100644 --- a/InvenTree/InvenTree/tasks.py +++ b/InvenTree/InvenTree/tasks.py @@ -91,13 +91,15 @@ 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 - time.sleep(random.randint(1, 5)) + if not isInTestMode(): + time.sleep(random.randint(1, 5)) attempt_key = f'_{task_name}_ATTEMPT' success_key = f'_{task_name}_SUCCESS' diff --git a/InvenTree/common/migrations/0017_notesimage.py b/InvenTree/common/migrations/0017_notesimage.py index c9e08af461..8c9aff2f6d 100644 --- a/InvenTree/common/migrations/0017_notesimage.py +++ b/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 diff --git a/InvenTree/label/apps.py b/InvenTree/label/apps.py index 34dd190ad9..0d6aa0f8b0 100644 --- a/InvenTree/label/apps.py +++ b/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 diff --git a/InvenTree/plugin/helpers.py b/InvenTree/plugin/helpers.py index d616273d74..df00d1d37d 100644 --- a/InvenTree/plugin/helpers.py +++ b/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,25 +109,34 @@ 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:] - try: - walker = Repo.discover(path).get_walker(paths=[path.encode()], max_entries=1) + # only do this if we are not in test mode + if not isInTestMode(): # pragma: no cover + try: - commit = next(iter(walker)).commit - except StopIteration: + walker = Repo.discover(path).get_walker(paths=[path.encode()], max_entries=1) + try: + commit = next(iter(walker)).commit + except StopIteration: + pass + else: + output = [ + commit.sha().hexdigest(), + commit.author.decode().split('<')[0][:-1], + commit.author.decode().split('<')[1][:-1], + datetime.datetime.fromtimestamp(commit.author_time, ).isoformat(), + commit.message.decode().split('\n')[0], + ] + except NotGitRepository: pass - else: - output = [ - commit.sha().hexdigest(), - commit.author.decode().split('<')[0][:-1], - commit.author.decode().split('<')[1][:-1], - datetime.datetime.fromtimestamp(commit.author_time, ).isoformat(), - commit.message.decode().split('\n')[0], - ] - except NotGitRepository: - pass if not output: output = 5 * [''] # pragma: no cover diff --git a/InvenTree/plugin/registry.py b/InvenTree/plugin/registry.py index 18f22559eb..6f37c7d720 100644 --- a/InvenTree/plugin/registry.py +++ b/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) diff --git a/InvenTree/plugin/test_api.py b/InvenTree/plugin/test_api.py index bb4d5b3290..d181cb487d 100644 --- a/InvenTree/plugin/test_api.py +++ b/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', diff --git a/InvenTree/report/apps.py b/InvenTree/report/apps.py index 5118333922..e5eec5e736 100644 --- a/InvenTree/report/apps.py +++ b/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() diff --git a/tasks.py b/tasks.py index c2b9186fa1..204391428e 100644 --- a/tasks.py +++ b/tasks.py @@ -565,9 +565,12 @@ def test_translations(c): help={ 'disable_pty': 'Disable PTY', 'runtest': 'Specify which tests to run, in format ...', + '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)",