From 5a6359516a3d592245931d4d0d285aa52742bb18 Mon Sep 17 00:00:00 2001
From: David Douard <david.douard@sdfa3.org>
Date: Thu, 19 Dec 2019 09:18:21 +0100
Subject: [PATCH] tests: make tests run with a proper scheduler

so the scheduler interaction code is executed.

Note that this does not test for correctness in these interactions yet.

also move tests/__init__.py content in tests/conftest.py and adapt test code
accordingly.

This also ensures retries_left is set otherwise tests may fail when using
the local sheduler.
---
 requirements-test.txt                         |  3 +-
 swh/deposit/api/private/deposit_check.py      |  3 +-
 swh/deposit/config.py                         |  2 +-
 swh/deposit/tests/__init__.py                 | 42 --------------
 swh/deposit/tests/api/test_deposit_binary.py  |  3 +-
 .../tests/api/test_deposit_private_check.py   | 45 ++++++++-------
 .../tests/api/test_service_document.py        |  3 +-
 swh/deposit/tests/conftest.py                 | 55 +++++++++++++++++++
 tox.ini                                       |  9 ++-
 9 files changed, 90 insertions(+), 75 deletions(-)

diff --git a/requirements-test.txt b/requirements-test.txt
index 2e64c384..cda155f8 100644
--- a/requirements-test.txt
+++ b/requirements-test.txt
@@ -2,6 +2,7 @@ pytest
 pytest-django
 pytest-mock
 swh.scheduler[testing]
+swh.loader.core[testing]
 pytest-postgresql >= 2.1.0
 requests_mock
-django-stubs < 1.3.0
+django-stubs
diff --git a/swh/deposit/api/private/deposit_check.py b/swh/deposit/api/private/deposit_check.py
index 7f0387fa..e17d4eab 100644
--- a/swh/deposit/api/private/deposit_check.py
+++ b/swh/deposit/api/private/deposit_check.py
@@ -219,7 +219,8 @@ class SWHChecksDeposit(SWHPrivateAPIView, SWHGetDepositAPI, DepositReadMixin):
             if not deposit.load_task_id and self.config['checks']:
                 url = deposit.origin_url
                 task = create_oneshot_task_dict(
-                    'load-deposit', url=url, deposit_id=deposit.id)
+                    'load-deposit', url=url, deposit_id=deposit.id,
+                    retries_left=3)
                 load_task_id = self.scheduler.create_tasks([task])[0]['id']
                 deposit.load_task_id = load_task_id
 
diff --git a/swh/deposit/config.py b/swh/deposit/config.py
index b7a1b6a0..912fb602 100644
--- a/swh/deposit/config.py
+++ b/swh/deposit/config.py
@@ -107,5 +107,5 @@ class SWHDefaultConfig(SWHConfig):
             additional_configs=[self.ADDITIONAL_CONFIG])
         self.config.update(config)
         self.log = logging.getLogger('swh.deposit')
-        if self.config['checks']:
+        if self.config.get('scheduler'):
             self.scheduler = get_scheduler(**self.config['scheduler'])
diff --git a/swh/deposit/tests/__init__.py b/swh/deposit/tests/__init__.py
index 2b34b26e..e69de29b 100644
--- a/swh/deposit/tests/__init__.py
+++ b/swh/deposit/tests/__init__.py
@@ -1,42 +0,0 @@
-# Copyright (C) 2017-2019  The Software Heritage developers
-# See the AUTHORS file at the top-level directory of this distribution
-# License: GNU General Public License version 3, or any later version
-# See top-level LICENSE file for more information
-
-from swh.deposit.config import setup_django_for
-from swh.deposit.config import SWHDefaultConfig  # noqa
-
-
-TEST_CONFIG = {
-    'max_upload_size': 500,
-    'extraction_dir': '/tmp/swh-deposit/test/extraction-dir',
-    'checks': False,
-    'provider': {
-        'provider_name': '',
-        'provider_type': 'deposit_client',
-        'provider_url': '',
-        'metadata': {
-        }
-    },
-    'tool': {
-        'name': 'swh-deposit',
-        'version': '0.0.1',
-        'configuration': {
-            'sword_version': '2'
-        }
-    }
-}
-
-
-def parse_deposit_config_file(base_filename=None, config_filename=None,
-                              additional_configs=None, global_config=True):
-    return TEST_CONFIG
-
-
-# monkey patch classes method permits to override, for tests purposes,
-# the default configuration without side-effect, i.e do not load the
-# configuration from disk
-SWHDefaultConfig.parse_config_file = parse_deposit_config_file  # type: ignore
-
-
-setup_django_for('testing')
diff --git a/swh/deposit/tests/api/test_deposit_binary.py b/swh/deposit/tests/api/test_deposit_binary.py
index 7d3eac5d..8f1cc763 100644
--- a/swh/deposit/tests/api/test_deposit_binary.py
+++ b/swh/deposit/tests/api/test_deposit_binary.py
@@ -11,7 +11,6 @@ from io import BytesIO
 
 from rest_framework import status
 
-from swh.deposit.tests import TEST_CONFIG
 from swh.deposit.config import (
     COL_IRI, EM_IRI, DEPOSIT_STATUS_DEPOSITED,
 )
@@ -224,7 +223,7 @@ def test_post_deposit_binary_upload_fail_if_upload_size_limit_exceeded(
 
     archive = create_arborescence_archive(
         tmp_path, 'archive2', 'file2', b'some content in file',
-        up_to_size=TEST_CONFIG['max_upload_size'])
+        up_to_size=500)
 
     external_id = 'some-external-id'
 
diff --git a/swh/deposit/tests/api/test_deposit_private_check.py b/swh/deposit/tests/api/test_deposit_private_check.py
index 50f0b1d0..a6bab004 100644
--- a/swh/deposit/tests/api/test_deposit_private_check.py
+++ b/swh/deposit/tests/api/test_deposit_private_check.py
@@ -4,6 +4,7 @@
 # See top-level LICENSE file for more information
 
 from django.urls import reverse
+import pytest
 from rest_framework import status
 
 from swh.deposit.config import (
@@ -33,8 +34,10 @@ def private_check_url_endpoints(collection, deposit):
     ]
 
 
+@pytest.mark.parametrize(
+    "extension", ['zip', 'tar', 'tar.gz', 'tar.bz2', 'tar.xz'])
 def test_deposit_ok(
-        authenticated_client, deposit_collection, ready_deposit_ok):
+        authenticated_client, deposit_collection, ready_deposit_ok, extension):
     """Proper deposit should succeed the checks (-> status ready)
 
     """
@@ -51,30 +54,30 @@ def test_deposit_ok(
         deposit.status = DEPOSIT_STATUS_DEPOSITED
         deposit.save()
 
-
+@pytest.mark.parametrize(
+    "extension", ['zip', 'tar', 'tar.gz', 'tar.bz2', 'tar.xz'])
 def test_deposit_invalid_tarball(
-        tmp_path, authenticated_client, deposit_collection):
+        tmp_path, authenticated_client, deposit_collection, extension):
     """Deposit with tarball (of 1 tarball) should fail the checks: rejected
 
     """
-    for archive_extension in ['zip', 'tar', 'tar.gz', 'tar.bz2', 'tar.xz']:
-        deposit = create_deposit_archive_with_archive(
-            tmp_path, archive_extension,
-            authenticated_client,
-            deposit_collection.name)
-        for url in private_check_url_endpoints(deposit_collection, deposit):
-            response = authenticated_client.get(url)
-            assert response.status_code == status.HTTP_200_OK
-            data = response.json()
-            assert data['status'] == DEPOSIT_STATUS_REJECTED
-            details = data['details']
-            # archive checks failure
-            assert len(details['archive']) == 1
-            assert details['archive'][0]['summary'] == \
-                MANDATORY_ARCHIVE_INVALID
-
-            deposit = Deposit.objects.get(pk=deposit.id)
-            assert deposit.status == DEPOSIT_STATUS_REJECTED
+    deposit = create_deposit_archive_with_archive(
+        tmp_path, extension,
+        authenticated_client,
+        deposit_collection.name)
+    for url in private_check_url_endpoints(deposit_collection, deposit):
+        response = authenticated_client.get(url)
+        assert response.status_code == status.HTTP_200_OK
+        data = response.json()
+        assert data['status'] == DEPOSIT_STATUS_REJECTED
+        details = data['details']
+        # archive checks failure
+        assert len(details['archive']) == 1
+        assert details['archive'][0]['summary'] == \
+            MANDATORY_ARCHIVE_INVALID
+
+        deposit = Deposit.objects.get(pk=deposit.id)
+        assert deposit.status == DEPOSIT_STATUS_REJECTED
 
 
 def test_deposit_ko_missing_tarball(
diff --git a/swh/deposit/tests/api/test_service_document.py b/swh/deposit/tests/api/test_service_document.py
index 558c7598..dda59a88 100644
--- a/swh/deposit/tests/api/test_service_document.py
+++ b/swh/deposit/tests/api/test_service_document.py
@@ -6,7 +6,6 @@
 from django.urls import reverse
 from rest_framework import status
 
-from swh.deposit.tests import TEST_CONFIG
 from swh.deposit.config import SD_IRI
 
 
@@ -80,7 +79,7 @@ def check_response(response, username):
         </collection>
     </workspace>
 </service>
-''' % (TEST_CONFIG['max_upload_size'],
+''' % (500,
        username,
        username,
        username,
diff --git a/swh/deposit/tests/conftest.py b/swh/deposit/tests/conftest.py
index 79ac88ad..6346896d 100644
--- a/swh/deposit/tests/conftest.py
+++ b/swh/deposit/tests/conftest.py
@@ -18,8 +18,11 @@ from rest_framework import status
 from rest_framework.test import APIClient
 from typing import Mapping
 
+from swh.scheduler import get_scheduler
 from swh.scheduler.tests.conftest import *  # noqa
+from swh.deposit.config import setup_django_for
 from swh.deposit.parsers import parse_xml
+from swh.deposit.config import SWHDefaultConfig
 from swh.deposit.config import (
     COL_IRI, EDIT_SE_IRI, DEPOSIT_STATUS_DEPOSITED, DEPOSIT_STATUS_REJECTED,
     DEPOSIT_STATUS_PARTIAL, DEPOSIT_STATUS_LOAD_SUCCESS,
@@ -40,6 +43,58 @@ TEST_USER = {
 }
 
 
+TEST_CONFIG = {
+    'max_upload_size': 500,
+    'extraction_dir': '/tmp/swh-deposit/test/extraction-dir',
+    'checks': False,
+    'provider': {
+        'provider_name': '',
+        'provider_type': 'deposit_client',
+        'provider_url': '',
+        'metadata': {
+        }
+    },
+    'tool': {
+        'name': 'swh-deposit',
+        'version': '0.0.1',
+        'configuration': {
+            'sword_version': '2'
+        }
+    },
+}
+
+
+def pytest_configure():
+    setup_django_for('testing')
+
+
+@pytest.fixture()
+def deposit_config():
+    return TEST_CONFIG
+
+
+@pytest.fixture(autouse=True)
+def deposit_autoconfig(monkeypatch, deposit_config, swh_scheduler_config):
+    """Enforce config for deposit classes inherited from SWHDefaultConfig."""
+    def mock_parse_config(*args, **kw):
+        config = deposit_config.copy()
+        config['scheduler'] = {
+            'cls': 'local',
+            'args': swh_scheduler_config,
+        }
+        return config
+    monkeypatch.setattr(
+        SWHDefaultConfig, "parse_config_file",
+        mock_parse_config)
+
+    scheduler = get_scheduler('local', swh_scheduler_config)
+    task_type = {
+        'type': 'load-deposit',
+        'backend_name': 'swh.loader.packages.deposit.tasks.LoadDeposit',
+        'description': 'why does this have not-null constraint?'}
+    scheduler.create_task_type(task_type)
+
+
 @pytest.fixture(scope='session')
 def django_db_setup(
         request,
diff --git a/tox.ini b/tox.ini
index 74556aad..aa8038a0 100644
--- a/tox.ini
+++ b/tox.ini
@@ -10,10 +10,8 @@ deps =
   swh.core[http] >= 0.0.75
   dev: ipdb
   pytest-cov
-  pifpaf
 commands =
-  pifpaf run postgresql -- \
-    pytest \
+  pytest \
   !dev: --cov {envsitepackagesdir}/swh/deposit --cov-branch \
         {envsitepackagesdir}/swh/deposit \
         {posargs}
@@ -27,10 +25,11 @@ commands =
     --exclude=.tox,.git,__pycache__,.tox,.eggs,*.egg,swh/deposit/migrations
 
 [testenv:mypy]
-setenv = DJANGO_SETTINGS_MODULE = swh.deposit.settings.testing
+setenv = DJANGO_SETTINGS_MODULE=swh.deposit.settings.testing
 extras =
   testing
 deps =
-  mypy < 0.750
+  mypy
+  django-stubs
 commands =
   mypy swh
-- 
GitLab