-
-
Notifications
You must be signed in to change notification settings - Fork 289
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
test type annotations with mypy #668
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -39,3 +39,24 @@ jobs: | |
- name: Run Test | ||
run: | | ||
`which django-admin` test django_rq --settings=django_rq.tests.settings --pythonpath=. | ||
|
||
mypy: | ||
runs-on: ubuntu-latest | ||
name: Type check | ||
|
||
steps: | ||
- uses: actions/checkout@v3 | ||
|
||
- name: Set up Python 3.8 | ||
uses: actions/[email protected] | ||
with: | ||
python-version: "3.8" | ||
|
||
- name: Install dependencies | ||
run: | | ||
python -m pip install --upgrade pip | ||
pip install django-stubs[compatible-mypy] rq types-redis | ||
|
||
- name: Run Test | ||
run: | | ||
mypy django_rq |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,9 +1,13 @@ | ||
from rq.decorators import job as _rq_job | ||
from typing import TYPE_CHECKING, Union | ||
|
||
from django.conf import settings | ||
|
||
from .queues import get_queue | ||
|
||
if TYPE_CHECKING: | ||
from rq import Queue | ||
|
||
|
||
def job(func_or_queue, connection=None, *args, **kwargs): | ||
""" | ||
|
@@ -18,7 +22,7 @@ def job(func_or_queue, connection=None, *args, **kwargs): | |
""" | ||
if callable(func_or_queue): | ||
func = func_or_queue | ||
queue = 'default' | ||
queue: Union['Queue', str] = 'default' | ||
else: | ||
func = None | ||
queue = func_or_queue | ||
|
@@ -30,13 +34,17 @@ def job(func_or_queue, connection=None, *args, **kwargs): | |
connection = queue.connection | ||
except KeyError: | ||
pass | ||
else: | ||
if connection is None: | ||
connection = queue.connection | ||
Comment on lines
+37
to
+39
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think this was an oversight in the previous code? |
||
|
||
RQ = getattr(settings, 'RQ', {}) | ||
default_result_ttl = RQ.get('DEFAULT_RESULT_TTL') | ||
if default_result_ttl is not None: | ||
kwargs.setdefault('result_ttl', default_result_ttl) | ||
|
||
decorator = _rq_job(queue, connection=connection, *args, **kwargs) | ||
kwargs['connection'] = connection | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is just for mypy (python/mypy#6799) and rather than adding a type ignore I figured I'd update the kwargs, since it would never contain the connection based on the function's definition |
||
decorator = _rq_job(queue, *args, **kwargs) | ||
if func: | ||
return decorator(func) | ||
return decorator |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -22,9 +22,7 @@ def handle(self, *args, **options): | |
Queues the function given with the first argument with the | ||
parameters given with the rest of the argument list. | ||
""" | ||
verbosity = int(options.get('verbosity', 1)) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. These will all exist, and using indexed access will tell mypy they are not |
||
timeout = options.get('timeout') | ||
queue = get_queue(options.get('queue')) | ||
job = queue.enqueue_call(args[0], args=args[1:], timeout=timeout) | ||
if verbosity: | ||
queue = get_queue(options['queue']) | ||
job = queue.enqueue_call(args[0], args=args[1:], timeout=options['timeout']) | ||
if options['verbosity']: | ||
print('Job %s created' % job.id) |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -20,13 +20,6 @@ | |
except ImportError: | ||
REDIS_CACHE_TYPE = 'none' | ||
|
||
try: | ||
from django.utils.log import NullHandler | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This was removed from Django way before any of the supported versions |
||
|
||
nullhandler = 'django.utils.log.NullHandler' | ||
except: | ||
nullhandler = 'logging.NullHandler' | ||
|
||
INSTALLED_APPS = [ | ||
'django.contrib.contenttypes', | ||
'django.contrib.admin', | ||
|
@@ -86,7 +79,7 @@ | |
}, | ||
'null': { | ||
'level': 'DEBUG', | ||
'class': nullhandler, | ||
'class': 'logging.NullHandler', | ||
}, | ||
}, | ||
'loggers': { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -69,6 +69,7 @@ def test_job_details_with_results(self): | |
result = job.results()[0] | ||
url = reverse('rq_job_detail', args=[queue_index, job.id]) | ||
response = self.client.get(url) | ||
assert result.id | ||
self.assertContains(response, result.id) | ||
|
||
def test_job_details_on_deleted_dependency(self): | ||
|
@@ -185,6 +186,7 @@ def test_enqueue_jobs(self): | |
|
||
# Check that job is updated correctly | ||
last_job = queue.fetch_job(last_job.id) | ||
assert last_job | ||
self.assertEqual(last_job.get_status(), JobStatus.QUEUED) | ||
self.assertIsNotNone(last_job.enqueued_at) | ||
|
||
|
@@ -407,7 +409,7 @@ def test_action_stop_jobs(self): | |
self.assertEqual(len(canceled_job_registry), len(job_ids)) | ||
|
||
for job_id in job_ids: | ||
self.assertIn(job_id, canceled_job_registry) | ||
self.assertIn(job_id, canceled_job_registry) # type: ignore[arg-type] | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. bug in |
||
|
||
def test_scheduler_jobs(self): | ||
# Override testing RQ_QUEUES | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,6 +1,7 @@ | ||
import sys | ||
import datetime | ||
import time | ||
from typing import Any, cast, Dict, List | ||
from unittest import skipIf, mock | ||
from unittest.mock import patch, PropertyMock, MagicMock | ||
from uuid import uuid4 | ||
|
@@ -34,7 +35,7 @@ | |
) | ||
from django_rq import thread_queue | ||
from django_rq.templatetags.django_rq import force_escape, to_localtime | ||
from django_rq.tests.fixtures import DummyJob, DummyQueue, DummyWorker | ||
from django_rq.tests.fixtures import access_self, DummyJob, DummyQueue, DummyWorker | ||
from django_rq.utils import get_jobs, get_statistics, get_scheduler_pid | ||
from django_rq.workers import get_worker, get_worker_class | ||
|
||
|
@@ -50,10 +51,6 @@ | |
QUEUES = settings.RQ_QUEUES | ||
|
||
|
||
def access_self(): | ||
return get_current_job().id | ||
|
||
|
||
def divide(a, b): | ||
return a / b | ||
|
||
|
@@ -271,7 +268,7 @@ def test_pass_queue_via_commandline_args(self): | |
Checks that passing queues via commandline arguments works | ||
""" | ||
queue_names = ['django_rq_test', 'django_rq_test2'] | ||
jobs = [] | ||
jobs: List[Any] = [] | ||
for queue_name in queue_names: | ||
queue = get_queue(queue_name) | ||
jobs.append( | ||
|
@@ -288,7 +285,7 @@ def test_pass_queue_via_commandline_args(self): | |
self.assertIn(job['job'].id, job['finished_job_registry'].get_job_ids()) | ||
|
||
# Test with rqworker-pool command | ||
jobs = [] | ||
jobs: List[Any] = [] | ||
for queue_name in queue_names: | ||
queue = get_queue(queue_name) | ||
jobs.append( | ||
|
@@ -441,8 +438,7 @@ def test_async(self): | |
# Make sure old keyword argument 'async' works for backwards | ||
# compatibility with code expecting older versions of rq or django-rq. | ||
# Note 'async' is a reserved keyword in Python >= 3.7. | ||
kwargs = {'async': False} | ||
default_queue_async = get_queue('default', **kwargs) | ||
default_queue_async = get_queue('default', **cast(Dict[str, Any], {'async': False})) | ||
self.assertFalse(default_queue_async._is_async) | ||
|
||
# Make sure is_async setting works | ||
|
@@ -722,7 +718,7 @@ def test_commandline_verbosity_affects_logging_level(self, setup_loghandlers_moc | |
setup_loghandlers_mock.assert_called_once_with(expected_level[verbosity]) | ||
|
||
@override_settings(RQ={'SCHEDULER_CLASS': 'django_rq.tests.fixtures.DummyScheduler'}) | ||
def test_scheduler_default_timeout(self): | ||
def test_scheduler_default(self): | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This test was overridden by the one that comes after it (thanks mypy!) |
||
""" | ||
Scheduler class customization. | ||
""" | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,13 +1,13 @@ | ||
from django.contrib import admin | ||
from django.urls import path | ||
|
||
from django_rq.urls import urlpatterns | ||
from django_rq.urls import urlpatterns as django_rq_urlpatterns | ||
|
||
from . import views | ||
|
||
urlpatterns = [ | ||
path('admin/', admin.site.urls), | ||
path('success/', views.success, name='success'), | ||
path('error/', views.error, name='error'), | ||
path('django-rq/', (urlpatterns, '', 'django_rq')), | ||
path('django-rq/', (django_rq_urlpatterns, '', 'django_rq')), | ||
] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Picking an old supported Python version since it doesn't it's tested otherwise, and this will make sure the type annotations don't break the older Pythons