From b0700ba5f059c8aa6b60653b814ea18be8f3c3ae Mon Sep 17 00:00:00 2001 From: Joachim Jablon Date: Sat, 6 Jul 2024 12:11:17 +0200 Subject: [PATCH 1/4] app.replace_connector to ease tests --- docs/howto/django/tests.md | 30 ++++++++++++++-------------- docs/howto/production/testing.md | 31 +++++++++++++++++++++-------- procrastinate/app.py | 34 +++++++++++++++++++++++++++++++- 3 files changed, 71 insertions(+), 24 deletions(-) diff --git a/docs/howto/django/tests.md b/docs/howto/django/tests.md index 49e0a6f43..8d966ed84 100644 --- a/docs/howto/django/tests.md +++ b/docs/howto/django/tests.md @@ -14,18 +14,17 @@ from procrastinate.contrib.django import procrastinate_app @pytest.fixture def in_memory_app(monkeypatch): - app = procrastinate_app.current_app.with_connector( - testing.InMemoryConnector() - ) - monkeypatch.setattr(procrastinate_app, "current_app", app) - return app + in_memory = testing.InMemoryConnector() + with procrastinate_app.current_app.replace_connector(in_memory) as app: + monkeypatch.setattr(procrastinate_app, "current_app", app) + yield app ``` :::{note} `procrastinate.contrib.django.procrastinate_app.current_app` is not exactly _documented_ but whatever instance this variable points to is what will be returned as `procrastinate.contrib.django.app`. It's probably a bad -idea to use this outside of tests. +idea to manipulate this outside of tests. ::: ## Integration tests @@ -44,7 +43,7 @@ from procrastinate.contrib.django.models import ProcrastinateJob def test_my_task(app): # Run the task - app.defer("my_task", args=(1, 2)) + task.defer(a=1, b=2) # Check the job has been created assert ProcrastinateJob.objects.filter(task_name="my_task").count() == 1 @@ -59,11 +58,12 @@ additonal configuration. [`TransactionTestCase`]. With `TestCase`, you can't test code within a transaction with `select_for_update()`. If you use `pytest-django`, use the equivalent `@pytest.mark.django_db(transaction=True)`. -3. Lastly, setup the worker using `wait=False` and - `install_signal_handlers=False`. `wait=False` means that when all tasks are - executed, the call will return. `install_signal_handlers=False` is optional - and just here to keep the worker from changing the signal callbacks set by - your test runner. +3. Lastly, there are useful arguments to pass to `run_worker`: + - `wait=False` to exit the worker as soon as all jobs are done + - `install_signal_handlers=False` to avoid messing with signals in your + test runner + - `listen_notify=False` to avoid running the listen/notify coroutine which + is probably not needed in tests [`TransactionTestCase`]: https://docs.djangoproject.com/en/5.0/topics/testing/tools/#transactiontestcase @@ -78,7 +78,7 @@ class TestingTaskClass(TransactionTestCase): # Start worker app = app.with_connector(app.connector.get_worker_connector()) - app.run_worker(wait=False, install_signal_handlers=False, listen_notify=True) + app.run_worker(wait=False, install_signal_handlers=False, listen_notify=False) # Check task has been executed assert ProcrastinateJob.objects.filter(task_name="my_task").status == "succeeded" @@ -94,7 +94,7 @@ def test_task(): # Start worker app = app.with_connector(app.connector.get_worker_connector()) - app.run_worker(wait=False, install_signal_handlers=False, listen_notify=True) + app.run_worker(wait=False, install_signal_handlers=False, listen_notify=False) # Check task has been executed assert ProcrastinateJob.objects.filter(task_name="my_task").status == "succeeded" @@ -104,7 +104,7 @@ def test_task(): def worker(transactional_db): def _(): app = app.with_connector(app.connector.get_worker_connector()) - app.run_worker(wait=False, install_signal_handlers=False, listen_notify=True) + app.run_worker(wait=False, install_signal_handlers=False, listen_notify=False) return app return _ diff --git a/docs/howto/production/testing.md b/docs/howto/production/testing.md index caa2b9fe9..10486db33 100644 --- a/docs/howto/production/testing.md +++ b/docs/howto/production/testing.md @@ -6,15 +6,30 @@ controlled way. To use it, you can do: -``` -testing_app = normal_app.with_connector(procrastinate.testing.InMemoryConnector()) +```python +from mypackage.procrastinate import my_app, my_task + +@pytest.fixture +def app(): + in_memory = procrastinate.testing.InMemoryConnector() + + with my_app.replace_connector(in_memory) as app_with_connector: + yield app_with_connector + + +def test_my_code(app): + my_task.defer(...) -# Run the jobs your tests created, then stop the worker -app.run_worker(wait=False) + # Access all the existing jobs + jobs = app.connector.jobs + assert len(jobs) == 1 -# See the jobs created: -print(app.connector.jobs) + # Run the jobs + app.run_worker(wait=False) + assert task_side_effect() == "done" -# Reset the "in-memory pseudo-database" between tests: -app.connector.reset() + # Reset the in-memory pseudo-database. This usually isn't necessary if + # you make small scoped tests as you'll use a new app fixture for each test + # but it might come in handy. + app.connector.reset() ``` diff --git a/procrastinate/app.py b/procrastinate/app.py index cfe3bf956..13955a957 100644 --- a/procrastinate/app.py +++ b/procrastinate/app.py @@ -1,9 +1,10 @@ from __future__ import annotations import asyncio +import contextlib import functools import logging -from typing import TYPE_CHECKING, Any, Iterable +from typing import TYPE_CHECKING, Any, Iterable, Iterator from procrastinate import blueprints, exceptions, jobs, manager, schema, utils from procrastinate import connector as connector_module @@ -105,6 +106,11 @@ def with_connector(self, connector: connector_module.BaseConnector) -> App: (and its original connector) will be used, even when the new app's methods are used. + Parameters + ---------- + connector : + The new connector to use. + Returns ------- `App` @@ -119,6 +125,32 @@ def with_connector(self, connector: connector_module.BaseConnector) -> App: app.periodic_registry = self.periodic_registry return app + @contextlib.contextmanager + def replace_connector( + self, connector: connector_module.BaseConnector + ) -> Iterator[App]: + """ + Replace the connector of the app while in the context block, then restore it. + + Parameters + ---------- + connector : + The new connector to use. + + Returns + ------- + `App` + A new compatible app. + """ + old_connector = self.connector + self.connector = connector + self.job_manager.connector = connector + try: + yield self + finally: + self.connector = old_connector + self.job_manager.connector = old_connector + def _register_builtin_tasks(self) -> None: from procrastinate import builtin_tasks From 6d41030c29dddf9c36194efad4b2a08afe2d2868 Mon Sep 17 00:00:00 2001 From: Joachim Jablon Date: Sat, 6 Jul 2024 12:11:40 +0200 Subject: [PATCH 2/4] Small unrelated import fix --- procrastinate/signals.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/procrastinate/signals.py b/procrastinate/signals.py index 5e8e7cec9..a88bb5bb2 100644 --- a/procrastinate/signals.py +++ b/procrastinate/signals.py @@ -1,10 +1,10 @@ from __future__ import annotations import asyncio +import contextlib import logging import signal import threading -from contextlib import contextmanager from typing import Any, Callable logger = logging.getLogger(__name__) @@ -23,7 +23,7 @@ # one. And hope that there was no previously set async handler. -@contextmanager +@contextlib.contextmanager def on_stop(callback: Callable[[], None]): if threading.current_thread() is not threading.main_thread(): logger.warning( From eb523e27098cf0143b5a79a4c64fd93eee8f9403 Mon Sep 17 00:00:00 2001 From: Joachim Jablon Date: Sat, 6 Jul 2024 12:20:00 +0200 Subject: [PATCH 3/4] Test --- procrastinate/contrib/django/procrastinate_app.py | 1 + tests/unit/test_app.py | 15 +++++++++++++++ 2 files changed, 16 insertions(+) diff --git a/procrastinate/contrib/django/procrastinate_app.py b/procrastinate/contrib/django/procrastinate_app.py index 75dda403a..17108bdf4 100644 --- a/procrastinate/contrib/django/procrastinate_app.py +++ b/procrastinate/contrib/django/procrastinate_app.py @@ -45,6 +45,7 @@ class FutureApp(blueprints.Blueprint): "run_worker", "schema_manager", "with_connector", + "replace_connector", "will_configure_task", ] ) diff --git a/tests/unit/test_app.py b/tests/unit/test_app.py index 18f1cf9e6..8118abd02 100644 --- a/tests/unit/test_app.py +++ b/tests/unit/test_app.py @@ -284,3 +284,18 @@ def bar(): assert app.worker_defaults == new_app.worker_defaults assert app.import_paths == new_app.import_paths assert app.periodic_registry is new_app.periodic_registry + + +def test_replace_connector(app): + @app.task(name="foo") + def foo(): + pass + + foo.defer() + assert len(app.connector.jobs) == 1 + + new_connector = testing.InMemoryConnector() + with app.replace_connector(new_connector): + assert len(app.connector.jobs) == 0 + + assert len(app.connector.jobs) == 1 From dab00f013471c85536cd6d33eb14f6d34088f6f0 Mon Sep 17 00:00:00 2001 From: Joachim Jablon Date: Sat, 6 Jul 2024 17:17:33 +0200 Subject: [PATCH 4/4] Fixing code review remarks --- docs/howto/django/tests.md | 83 ++++++++++++++++++++++++++------ docs/howto/production/testing.md | 8 ++- 2 files changed, 73 insertions(+), 18 deletions(-) diff --git a/docs/howto/django/tests.md b/docs/howto/django/tests.md index 8d966ed84..dd642c366 100644 --- a/docs/howto/django/tests.md +++ b/docs/howto/django/tests.md @@ -2,8 +2,11 @@ ## Unit tests -If you want to be able to run your code that uses Procrastinate in a unit test, -you can use the {py:class}`procrastinate.testing.InMemoryConnector`. +There are different kind of tests you might want to do with Procrastinate in +a Django project. One such test is a unit test where you want fast tests that +don't hit the database. + +For this, you can use the {py:class}`procrastinate.testing.InMemoryConnector`. Here's an example pytest fixture you can write in your `conftest.py`: @@ -12,12 +15,34 @@ import pytest from procrastinate import testing from procrastinate.contrib.django import procrastinate_app +from mypackage.procrastinate import my_task + @pytest.fixture -def in_memory_app(monkeypatch): +def app(): in_memory = testing.InMemoryConnector() - with procrastinate_app.current_app.replace_connector(in_memory) as app: - monkeypatch.setattr(procrastinate_app, "current_app", app) - yield app + + # Replace the connector in the current app + # Note that this fixture gives you the app back for convenience, but it's + # the same instance as you'd get with `procrastinate.contrib.django.app`. + with procrastinate_app.current_app.replace_connector(in_memory) as app_with_connector: + yield app_with_connector + +def test_my_task(app): + # Run the task + my_task.defer(a=1, b=2) + + # Access all the existing jobs + jobs = app.connector.jobs + assert len(jobs) == 1 + + # Run the jobs + app.run_worker(wait=False) + assert task_side_effect() == "done" + + # Reset the in-memory pseudo-database. This usually isn't necessary if + # you make small scoped tests as you'll use a new app fixture for each test + # but it might come in handy. + app.connector.reset() ``` :::{note} @@ -29,11 +54,9 @@ idea to manipulate this outside of tests. ## Integration tests -You can use Procrastinate normally with `procrastinate.contrib.django.app` -in your tests, though registering new tasks or loading tasks from blueprints -within your tests might lead to test isolation issues. If you need to -do that, you may want to use the trick described above to have a dedicated -app for each relevant test. +Another kind of test, maybe more frequent within Django, would be an +integration test. In this kind of test you would use the database. You can use +Procrastinate normally with `procrastinate.contrib.django.app` in your tests. You can use the procrastinate models to check if the jobs have been created as expected. @@ -41,14 +64,38 @@ as expected. ```python from procrastinate.contrib.django.models import ProcrastinateJob -def test_my_task(app): +from mypackage.procrastinate import my_task + + +def test_my_task(): # Run the task - task.defer(a=1, b=2) + my_task.defer(a=1, b=2) # Check the job has been created assert ProcrastinateJob.objects.filter(task_name="my_task").count() == 1 ``` +:::{note} +Registering a new task on the django procrastinate app within a test will +make this task available even after the test has run. You probably want to +avoid this, for example by creating a new app within each of those tests: + +```python +from procrastinate.contrib.django import app + +def test_my_task(): + new_app = procrastinate_app.ProcrastinateApp(app.connector) + + @new_app.task + def my_task(a, b): + return a + b + + my_task.defer(a=1, b=2) + ... +``` + +::: + In addition, you can also run a worker in your integration tests. Whether you use `pytest-django` or Django's `TestCase` subclasses, this requires some additonal configuration. @@ -71,10 +118,12 @@ additonal configuration. from procrastinate.contrib.django import app from django.test import TransactionTestCase +from mypackage.procrastinate import my_task + class TestingTaskClass(TransactionTestCase): def test_task(self): # Run tasks - app.defer("my_task", args=(1, 2)) + my_task.defer(a=1, b=2) # Start worker app = app.with_connector(app.connector.get_worker_connector()) @@ -87,10 +136,12 @@ class TestingTaskClass(TransactionTestCase): ```python from procrastinate.contrib.django import app +from mypackage.procrastinate import my_task + @pytest.mark.django_db(transaction=True) def test_task(): # Run tasks - app.defer("my_task", args=(1, 2)) + my_task.defer(a=1, b=2) # Start worker app = app.with_connector(app.connector.get_worker_connector()) @@ -110,7 +161,7 @@ def worker(transactional_db): def test_task(worker): # Run tasks - app.defer("my_task", args=(1, 2)) + my_task.defer(a=1, b=2) # Start worker worker() diff --git a/docs/howto/production/testing.md b/docs/howto/production/testing.md index 10486db33..d9699ecbf 100644 --- a/docs/howto/production/testing.md +++ b/docs/howto/production/testing.md @@ -7,17 +7,21 @@ controlled way. To use it, you can do: ```python +from procrastinate import testing from mypackage.procrastinate import my_app, my_task @pytest.fixture def app(): - in_memory = procrastinate.testing.InMemoryConnector() + in_memory = testing.InMemoryConnector() + # Replace the connector in the current app + # Note that this fixture gives you the app back for covenience, + # but it's the same instance as `my_app`. with my_app.replace_connector(in_memory) as app_with_connector: yield app_with_connector -def test_my_code(app): +def test_my_task(app): my_task.defer(...) # Access all the existing jobs