pghoard: improve DownloadResultsProcessor tests

test/test_webserver.py

@@ -10,6 +10,7 @@
 import socket
 import threading
 import time
+from collections import deque
 from distutils.version import LooseVersion
 from http.client import HTTPConnection
 from queue import Queue
@@ -774,10 +775,15 @@ def test_uncontrolled_target_path(self, pghoard):
         assert status == 400
 
 
+@pytest.fixture(name="download_results_processor")
+def fixture_download_results_processor() -> DownloadResultsProcessor:
+    return DownloadResultsProcessor(threading.RLock(), logging.getLogger("WebServer"), Queue(), {}, deque())
+
+
 class TestDownloadResultsProcessor:
     wal_name = "000000060000000000000001"
 
-    def save_wal_and_dowload_callback(self, pg_wal_dir, drp, wal_name=None, is_valid_wal=True):
+    def save_wal_and_download_callback(self, pg_wal_dir, download_results_processor, wal_name=None, is_valid_wal=True):
         if wal_name is None:
             wal_name = self.wal_name
         tmp_path = os.path.join(pg_wal_dir, f"{wal_name}.pghoard.tmp")
@@ -799,49 +805,30 @@ def save_wal_and_dowload_callback(self, pg_wal_dir, drp, wal_name=None, is_valid
         pending_op = PendingDownloadOp(
             started_at=time.monotonic(), target_path=target_path, filetype="xlog", filename=wal_name
         )
-        drp.pending_download_ops[wal_name] = pending_op
-        drp.download_results.put(download_result)
-        return tmp_path, target_path
-
-    def init_download_results_processor(self):
-        return DownloadResultsProcessor(threading.RLock(), logging.getLogger("WebServer"), Queue(), {}, [])
-
-    def test_rename_valid_wal(self, tmpdir):
-        drp = self.init_download_results_processor()
-        tmp_path, target_path = self.save_wal_and_dowload_callback(tmpdir, drp)
-        download_result_item = drp.download_results.get()
-        drp.process_queue_item(download_result_item)
-        assert os.path.exists(target_path)
-        assert not os.path.exists(tmp_path)
-
-    def test_dont_save_invalid_wal(self, tmpdir):
-        drp = self.init_download_results_processor()
-        tmp_path, target_path = self.save_wal_and_dowload_callback(tmpdir, drp, is_valid_wal=False)
-        download_result_item = drp.download_results.get()
-        drp.process_queue_item(download_result_item)
-        assert not os.path.exists(target_path)
-        assert not os.path.exists(tmp_path)
-
-    def test_skip_not_pending_op(self, tmpdir):
-        drp = self.init_download_results_processor()
-        tmp_path, target_path = self.save_wal_and_dowload_callback(tmpdir, drp)
-        download_result_item = drp.download_results.get()
-        drp.pending_download_ops = {}
-        drp.process_queue_item(download_result_item)
-        assert not os.path.exists(target_path)
+        download_results_processor.pending_download_ops[wal_name] = pending_op
+        return tmp_path, target_path, download_result
+
+    @pytest.mark.parametrize("empty_pending_download_ops", [True, False])
+    @pytest.mark.parametrize("is_valid_wal", [True, False])
+    def test_rename_wal(self, download_results_processor, tmpdir, is_valid_wal, empty_pending_download_ops):
+        tmp_path, target_path, download_result_item = self.save_wal_and_download_callback(
+            tmpdir, download_results_processor, is_valid_wal=is_valid_wal
+        )
+        if empty_pending_download_ops:
+            download_results_processor.pending_download_ops = {}
+        download_results_processor.process_queue_item(download_result_item)
+        assert os.path.exists(target_path) is (is_valid_wal and not empty_pending_download_ops)
         assert not os.path.exists(tmp_path)
 
-    def test_dont_overwrite_existing_target_file(self, tmpdir):
-        drp = self.init_download_results_processor()
-        tmp_path, target_path = self.save_wal_and_dowload_callback(tmpdir, drp)
+    def test_dont_overwrite_existing_target_file(self, download_results_processor, tmpdir):
+        tmp_path, target_path, download_result_item = self.save_wal_and_download_callback(tmpdir, download_results_processor)
         existing_file_data = b"-"
         with open(target_path, "wb") as out_file:
             out_file.write(existing_file_data)
         assert os.path.exists(target_path)
         assert os.path.exists(tmp_path)
 
-        download_result_item = drp.download_results.get()
-        drp.process_queue_item(download_result_item)
+        download_results_processor.process_queue_item(download_result_item)
         assert os.path.exists(target_path)
         assert open(target_path, "rb").read() == existing_file_data
         assert os.path.exists(tmp_path)