Fixes #45: Stabilize Redis-dependent tests when running in parallel

AGENTS.md

@@ -136,8 +136,9 @@ Check these integration details when changing the affected paths:
   15 and 14, and nothing else isolates it. A committing test otherwise enqueues
   jobs that a live worker would run with test primary keys. Logical database
   numbers do not make a shared server safe from `FLUSHALL`.
-- **Do not use `RQQueueTestMixin` to clear queues.** It calls server-wide
-  `flushall()`.
+- **`--parallel` workers share the test Redis database.** A test class that
+  reaches Redis lists NetBox's `RQQueueTestMixin` first. See the
+  [testing conventions](./docs/development/conventions.md#testing).
 - **Migration fixtures need real temporary source storage.** `ManagedFile.storage`
   and the built-in loader do not necessarily use the same backend instance.
 - **`has_perm(perm)` without an object is not a target-object check.** It

CONTRIBUTING.md

@@ -99,9 +99,10 @@ services matching that configuration. The database account must be able to creat
 the test database.
 
 **Use isolated development services, never production.** The test configuration
-uses Redis databases 15 for tasks and 14 for caching. No worker serving a different
-NetBox database may consume those test queues. Redis database numbers do not
-protect against commands that clear the whole server.
+uses Redis databases 15 for tasks and 14 for caching. The suite clears the tasks
+database during a run, whichever one the configuration names. No worker serving a
+different NetBox database may consume those test queues. Redis database numbers do
+not protect against commands that clear the whole server.
 
 These exports apply only inside the subshell:
 

docs/development/conventions.md

@@ -137,6 +137,14 @@ and APIs through NetBox's test client rather than mocking persistence. Use
 transactional tests for concurrency and transaction boundaries. A sequential
 `TestCase` alone does not demonstrate that an interleaving is safe.
 
+Parallel test workers get their own databases but share the test Redis
+database. A test class that reaches Redis lists NetBox's `RQQueueTestMixin`,
+from `utilities.testing.mixins`, as its first base and calls `super().setUp()`
+first. A class reaches Redis when it reads or clears a queue, writes an RQ job,
+or lets an unpatched enqueue run, either by committing or through
+`captureOnCommitCallbacks(execute=True)`. The mixin keeps such classes from
+running at the same time and clears the task database around each test.
+
 Keep tests in the same commit as the implementation they cover. Report the
 NetBox ref, command, configuration and result, including skipped or unrun checks.
 Distinguish actual integration tests from checks that replace a framework or

netbox_scripts/tests/api/test_locking.py

@@ -13,7 +13,6 @@
 
 from unittest import mock
 
-import django_rq
 from django.test import TransactionTestCase, override_settings
 from django.urls import reverse
 from rest_framework.test import APIClient
@@ -31,9 +30,6 @@ class ConditionalWriteLockOrderTestCase(ObjectPermissionTestMixin, TransactionTe
 
     def setUp(self):
         self.enterContext(override_settings(STORAGES=IN_MEMORY_STORAGES))
-        # This case commits, so a deletion really enqueues its cleanup. The queue is isolated by
-        # testing/configuration.py, and draining it leaves nothing behind even there.
-        self.addCleanup(django_rq.get_queue('default').empty)
         self.project = ScriptProject.objects.create(name='Deploy Devices', key='deploy-devices')
         self.user = User.objects.create_user(username='testuser')
         self.token = Token.objects.create(user=self.user)

netbox_scripts/tests/migration/test_cutover.py

@@ -25,6 +25,7 @@
 from netbox_scripts.tests.migration.test_staging import LEGACY_SCRIPT as SYNCED_SCRIPT
 from netbox_scripts.tests.migration.test_staging import LegacySourceMixin
 from users.models import ObjectPermission
+from utilities.testing.mixins import RQQueueTestMixin
 
 LEGACY_SCRIPT = b"""from extras.scripts import Script
 
@@ -35,7 +36,7 @@ def run(self, data, commit):
 """
 
 
-class CutoverTestCase(TestCase):
+class CutoverTestCase(RQQueueTestMixin, TestCase):
     """Crossing the fence: what it refuses, what it records, and what it closes."""
 
     @classmethod
@@ -58,6 +59,7 @@ def setUpClass(cls):
         )
 
     def setUp(self):
+        super().setUp()
         self.migration = MigrationRun.objects.create(state=MigrationStateChoices.STAGING)
         self.source = DataSource.objects.create(name='Automation', type='local', source_url='file:///tmp/a')
         self.data_file = DataFile.objects.create(
@@ -149,13 +151,8 @@ def rq_task(self, job, kwargs):
             timeout=600,
         )
         task.save()
-        self.addCleanup(self.discard_task, queue, str(job.job_id))
         return task
 
-    @staticmethod
-    def discard_task(queue, task_id):
-        queue.connection.delete(f'rq:job:{task_id}')
-
     def report_module(self, *, auto_sync_enabled=False):
         """Create a REPORTS-root module, which this migration does not cover."""
         data_file = DataFile.objects.create(

netbox_scripts/tests/migration/test_history.py

@@ -20,6 +20,7 @@
 from netbox_scripts.runtime.exceptions import ScriptFileImportError
 from netbox_scripts.tests.migration.test_references import ReferenceMigrationMixin
 from users.models import ObjectPermission
+from utilities.testing.mixins import RQQueueTestMixin
 
 # A legacy script with a variable, so the journal's primary keys have something to resolve back to.
 VARIABLE_SCRIPT = b"""from extras.scripts import ObjectVar, Script
@@ -49,7 +50,7 @@ def run(self, data, commit):
 """
 
 
-class LegacyJobMixin(ReferenceMigrationMixin):
+class LegacyJobMixin(RQQueueTestMixin, ReferenceMigrationMixin):
     """The built-in content plus a Job history and a queue to read schedules out of."""
 
     def setUp(self):
@@ -58,8 +59,6 @@ def setUp(self):
         self.variable_script = Script.objects.get(module=self.variable_module, name='Retire')
         self.site = Site.objects.create(name='HQ', slug='hq')
         self.queue = django_rq.get_queue('default')
-        self.queue.empty()
-        self.addCleanup(self.queue.empty)
 
     def legacy_job(self, *, script=None, status=JobStatusChoices.STATUS_COMPLETED, **fields):
         """One built-in Custom Script Job row, in whatever state the case needs."""
@@ -89,7 +88,6 @@ def legacy_schedule(self, *, script=None, data=None, commit=True, scheduled=None
             timeout=timeout,
         )
         task.save()
-        self.addCleanup(self.queue.connection.delete, f'rq:job:{job.job_id}')
         return job
 
     def plugin_script(self, class_name='Deploy'):

netbox_scripts/tests/migration/test_verification.py

@@ -13,9 +13,10 @@
 from netbox_scripts.models import MigrationRun, NetBoxScript, ScriptProject
 from netbox_scripts.tests.migration.test_cleanup import CleanupMixin
 from users.models import ObjectPermission
+from utilities.testing.mixins import RQQueueTestMixin
 
 
-class VerificationMixin(CleanupMixin):
+class VerificationMixin(RQQueueTestMixin, CleanupMixin):
     """A migration taken as far as each case needs, and a way to read one check out of the report."""
 
     @staticmethod
@@ -34,7 +35,6 @@ def rq_task(self, job):
             timeout=600,
         )
         task.save()
-        self.addCleanup(queue.connection.delete, f'rq:job:{job.job_id}')
         return task
 
     def waiting_schedule(self, run, *, name='nightly deploy', status=JobStatusChoices.STATUS_SCHEDULED):

netbox_scripts/tests/storage/test_concurrency.py

@@ -17,7 +17,6 @@
 import uuid
 from unittest import mock
 
-import django_rq
 from django.db.models.signals import post_save, pre_save
 from django.test import TransactionTestCase, override_settings
 
@@ -30,6 +29,7 @@
 from netbox_scripts.storage.manifest import compute_digest
 from netbox_scripts.storage.paths import revision_prefix
 from netbox_scripts.tests.plugin_testing import IN_MEMORY_STORAGES, SecondSession
+from utilities.testing.mixins import RQQueueTestMixin
 
 SOURCE = {'hello.py': b'print("hi")\n'}
 
@@ -42,15 +42,12 @@ def manifest_for(files):
     )
 
 
-class SerializationTestCase(TransactionTestCase):
+class SerializationTestCase(RQQueueTestMixin, TransactionTestCase):
     """Shared fixture: one upload project whose content can be staged and reclaimed."""
 
     def setUp(self):
+        super().setUp()
         self.enterContext(override_settings(STORAGES=IN_MEMORY_STORAGES))
-        # This case commits, so the revision-deletion signal's on_commit callback really enqueues a
-        # cleanup job. The queue is isolated by testing/configuration.py, and draining it here means
-        # the suite leaves nothing behind even there.
-        self.addCleanup(django_rq.get_queue('default').empty)
         self.storage = config.get_storage()
         self.project = ScriptProject.objects.create(name='Deploy Devices', key='deploy-devices')
 

netbox_scripts/tests/test_event_rules.py

@@ -11,6 +11,7 @@
 from netbox_scripts.event_rules import RunNetBoxScriptAction
 from netbox_scripts.models import NetBoxScript
 from netbox_scripts.tests.test_execution import MAKES_A_TAG, ScriptJobTestMixin
+from utilities.testing.mixins import RQQueueTestMixin
 
 SLUG = 'netbox_scripts.run'
 
@@ -183,19 +184,15 @@ def test_a_script_in_another_project_does_not_resolve(self):
             self.action.resolve_import_object(f'other:{self.netbox_script.full_name}')
 
 
-class ActionInputTestCase(ScriptJobTestMixin, TestCase):
+class ActionInputTestCase(RQQueueTestMixin, ScriptJobTestMixin, TestCase):
     """That the rule's action_data reaches the script as its input, verbatim."""
 
     # enqueue_run keeps input off the Job row on purpose, because a variable resolves to a model
     # instance or an uploaded file. So the queued task is the only place it can be read.
 
     def setUp(self):
         super().setUp()
-        # The queue is isolated by testing/configuration.py, so this only separates one test from
-        # the next. Never clear with NetBox's RQQueueTestMixin, which uses a server-wide flushall().
         self.queue = django_rq.get_queue('default')
-        self.queue.empty()
-        self.addCleanup(self.queue.empty)
         self.publish({'deploy.py': MAKES_A_TAG})
         self.netbox_script = self.script()
         self.action = get_event_rule_action(SLUG)

netbox_scripts/tests/test_event_sources.py

@@ -13,6 +13,7 @@
     ScriptProject,
     ScriptProjectRevision,
 )
+from utilities.testing.mixins import RQQueueTestMixin
 
 EVERY_MODEL = (ScriptProject, ScriptProjectRevision, ScriptFile, NetBoxScript)
 
@@ -83,16 +84,12 @@ def test_every_model_serializes(self):
                 self.assertEqual(serialize_for_event(instance)['id'], instance.pk)
 
 
-class EventDispatchTestCase(TestCase):
+class EventDispatchTestCase(RQQueueTestMixin, TestCase):
     """That a rule matching a plugin object reaches the queue."""
 
     def setUp(self):
         super().setUp()
-        # The queue is isolated by testing/configuration.py, so this only separates one test from
-        # the next. Never clear with NetBox's RQQueueTestMixin, which uses a server-wide flushall().
         self.queue = django_rq.get_queue('default')
-        self.queue.empty()
-        self.addCleanup(self.queue.empty)
         self.project = ScriptProject.objects.create(name='Dispatched', key='dispatched')
         self.object_type = ObjectType.objects.get_for_model(ScriptProject)
         webhook = Webhook.objects.create(name='Receiver', payload_url='http://localhost/hook/')

netbox_scripts/tests/test_execution.py

@@ -39,6 +39,7 @@
 from utilities.datetime import local_now
 from utilities.exceptions import AbortScript as LegacyAbortScript
 from utilities.request import NetBoxFakeRequest
+from utilities.testing.mixins import RQQueueTestMixin
 
 
 class Creating(Script):
@@ -477,7 +478,7 @@ def test_relative_imports_remain_available_during_construction_and_form_building
             self.assertIsNotNone(instance.as_form())
 
 
-class EnqueueRunTestCase(ScriptJobTestMixin, TestCase):
+class EnqueueRunTestCase(RQQueueTestMixin, ScriptJobTestMixin, TestCase):
     def test_enqueue_pins_the_active_revision_on_the_job(self):
         revision = self.publish({'deploy.py': MAKES_A_TAG})
         script = self.script()
@@ -554,8 +555,6 @@ def test_a_run_no_event_drove_records_none(self):
     def test_an_immediate_run_never_reaches_the_queue(self):
         self.publish({'deploy.py': MAKES_A_TAG})
         queue = django_rq.get_queue('default')
-        queue.empty()
-        self.addCleanup(queue.empty)
 
         # Core queues every non-immediate job from a transaction.on_commit hook, so an
         # implementation reaching that branch would hand the finished run to a worker as well.