diff --git a/apps/cli/executables/delivery/test/test_api.py b/apps/cli/executables/delivery/test/test_api.py
index a61565a2b4e0d346a1bb03f5ffcbb7a8995d84e9..e23c4c56e0a39cf7335db453d9c00f1efcda5ac7 100644
--- a/apps/cli/executables/delivery/test/test_api.py
+++ b/apps/cli/executables/delivery/test/test_api.py
@@ -9,6 +9,11 @@ from delivery.deliverer import LocalDestination
@pytest.fixture
def dest_dir(tmpdir) -> pathlib.Path:
+ """
+ Generate a destination directory.
+ :param tmpdir: the tmpdir fixture we depend on
+ :return: path to the destination directory
+ """
# generate the destination directory
dest_dir = pathlib.Path(tmpdir) / "after"
dest_dir.mkdir()
@@ -17,6 +22,11 @@ def dest_dir(tmpdir) -> pathlib.Path:
@pytest.fixture
def file_to_deliver(tmpdir) -> pathlib.Path:
+ """
+ Generate some files in a test directory for delivery.
+ :param tmpdir: pytest fixture for the temporary directory
+ :return: a file in the test directory
+ """
# rewrap the tmpdir as a pathlib.Path instance
tmpdir = pathlib.Path(tmpdir)
diff --git a/apps/cli/executables/delivery/test/test_cli.py b/apps/cli/executables/delivery/test/test_cli.py
index 24357edcfad1cfb1d9ae71ac422317df3984f363..c6ab65578fcc103470ca5263c69d63f16ee0951f 100644
--- a/apps/cli/executables/delivery/test/test_cli.py
+++ b/apps/cli/executables/delivery/test/test_cli.py
@@ -1,6 +1,5 @@
# Testing the CLI
import filecmp
-import os
import pathlib
import shutil
import tarfile
@@ -10,59 +9,88 @@ from delivery.context import DeliveryContext
from delivery.delivery import Delivery, main
-def test_local_rawdata_no_tar(tmpdir_factory):
+def verify_extracted_directory(
+ subdirectory: str,
+ tar_path: pathlib.Path,
+ extraction_target: pathlib.Path,
+ original_data_path: str,
+):
"""
- Test that a directory in the source is copied to a directory in the destination in the manner expected.
+ Verify that an extracted directory has the same contents as the supplied temporary directory.
+ Useful for testing tar-related functionality
+
+ :param subdirectory: subdirectory to look for inside extraction area
+ :param tar_path: path to the tarfile to examine
+ :param extraction_target: location to extract to
+ :param original_data_path: location of the original files to compare to
:return:
"""
+
+ # is it actually a tar?
+ assert tarfile.is_tarfile(tar_path)
+
+ # let's unpack it
+ shutil.unpack_archive(tar_path, extraction_target / "extracted")
+
+ # did it output what we expect?
+ assert (extraction_target / "extracted" / subdirectory).exists()
+
+ # compare the extracted results with the source
+ assert_directories_are_same(
+ extraction_target / "extracted" / subdirectory, (original_data_path + subdirectory)
+ )
+
+
+def assert_directories_are_same(left, right):
+ """
+ Check that the contents of two directories are the same as far as we care
+ :param left:
+ :param right:
+ :return:
+ """
+ compare_dirs = filecmp.dircmp(left, right)
+
+ # did the comparison report they are the same
+ assert len(compare_dirs.left_only) == 0
+ assert len(compare_dirs.right_only) == 0
+ assert len(compare_dirs.funny_files) == 0
+
+
+def test_local_rawdata_no_tar(tmpdir_factory):
+ """
+ Test that local delivery works without tar (the simplest case)
+ """
temp_directory = str(tmpdir_factory.mktemp("test_basic_rawdata_no_tar"))
test_data_path = "../../../../shared/workspaces/test/test_data/spool/724126739/"
eb_name = "17A-109.sb33151331.eb33786546.57892.65940042824"
main(["-r", "-l", temp_directory, test_data_path])
# compare the source and destination
- compare_dirs = filecmp.dircmp(temp_directory + "/" + eb_name, (test_data_path + eb_name))
- # did the comparison report they are the same
- assert len(compare_dirs.left_only) == 0
- assert len(compare_dirs.right_only) == 0
- assert len(compare_dirs.funny_files) == 0
+ assert_directories_are_same(temp_directory + "/" + eb_name, (test_data_path + eb_name))
def test_local_rawdata_with_tar(tmpdir_factory):
- temp_directory = str(tmpdir_factory.mktemp("test_basic_rawdata_with_tar"))
+ """
+ Test that local delivery works with tar
+ """
+ temp_directory = pathlib.Path(tmpdir_factory.mktemp("test_basic_rawdata_with_tar"))
test_data_path = "../../../../shared/workspaces/test/test_data/spool/724126739/"
- main(["-r", "-t", "-l", temp_directory, test_data_path])
+ main(["-r", "-t", "-l", str(temp_directory), test_data_path])
eb_name = "17A-109.sb33151331.eb33786546.57892.65940042824"
- tar_path = temp_directory + "/17A-109.sb33151331.eb33786546.57892.65940042824.tar"
+ tar_path = temp_directory / "17A-109.sb33151331.eb33786546.57892.65940042824.tar"
# does a tar exist where we think
- assert os.path.exists(tar_path)
+ assert tar_path.exists()
# do we only have it and the SHA1SUMS
- assert len(os.listdir(temp_directory)) == 2
-
- # is it actually a tar
- assert tarfile.is_tarfile(tar_path)
+ assert len(list(temp_directory.iterdir())) == 2
- # lets unpack it
- shutil.unpack_archive(tar_path, temp_directory + "/extracted")
-
- # did it output what we expect
- assert os.path.exists(temp_directory + "/extracted/" + eb_name)
-
- # compare the extracted results with the source
- compare_dirs = filecmp.dircmp(
- temp_directory + "/extracted/" + eb_name, (test_data_path + eb_name)
- )
- # is the source and extracted the same
- assert len(compare_dirs.left_only) == 0
- assert len(compare_dirs.right_only) == 0
- assert len(compare_dirs.funny_files) == 0
+ verify_extracted_directory(eb_name, tar_path, temp_directory, test_data_path)
# @pytest.mark.skip(reason="Test needs more dev time")
def test_web_rawdata_no_tar(tmpdir_factory):
"""
- Test that a directory in the source is copied to a directory in the destination in the manner expected.
+ Test that delivery works to a web destination without tar
"""
temp_directory = pathlib.Path(tmpdir_factory.mktemp("test_web_rawdata_no_tar"))
test_data_path = "../../../../shared/workspaces/test/test_data/spool/724126739/"
@@ -78,15 +106,13 @@ def test_web_rawdata_no_tar(tmpdir_factory):
actual_delivery_dir = temp_directory / destination_url.lstrip("http://testing")
# compare the source and destination
- compare_dirs = filecmp.dircmp(actual_delivery_dir / eb_name, f"{test_data_path}{eb_name}")
-
- # did the comparison report they are the same
- assert len(compare_dirs.left_only) == 0
- assert len(compare_dirs.right_only) == 0
- assert len(compare_dirs.funny_files) == 0
+ assert_directories_are_same(actual_delivery_dir / eb_name, f"{test_data_path}{eb_name}")
def test_web_rawdata_with_tar(tmpdir_factory):
+ """
+ Test that delivery works to a web destination with tar
+ """
temp_directory = pathlib.Path(tmpdir_factory.mktemp("test_web_rawdata_with_tar"))
test_data_path = "../../../../shared/workspaces/test/test_data/spool/724126739/"
test_context = DeliveryContext.parse_commandline(["-r", "-t", test_data_path])
@@ -108,21 +134,4 @@ def test_web_rawdata_with_tar(tmpdir_factory):
# is it the only thing there (did cleanup work)
assert len(list(actual_delivery_dir.iterdir())) == 3
- # is it actually a tar
- assert tarfile.is_tarfile(tar_path)
-
- # lets unpack it
- shutil.unpack_archive(tar_path, temp_directory / "extracted")
-
- # did it output what we expect
- assert (temp_directory / "extracted" / eb_name).exists()
-
- # compare the extracted results with the source
- compare_dirs = filecmp.dircmp(
- temp_directory / "extracted" / eb_name, (test_data_path + eb_name)
- )
-
- # is the source and extracted the same
- assert len(compare_dirs.left_only) == 0
- assert len(compare_dirs.right_only) == 0
- assert len(compare_dirs.funny_files) == 0
+ verify_extracted_directory(eb_name, tar_path, temp_directory, test_data_path)