Update get_version() tests based on discussion surrounding Issue #405

pyani/anib.py

@@ -124,14 +124,20 @@ def get_version(blast_exe: Path = pyani_config.BLASTN_DEFAULT) -> str:
 
     The following circumstances are explicitly reported as strings
 
+    - a value of None given for the executable
     - no executable at passed path
     - non-executable file at passed path (this includes cases where the user doesn't have execute permissions on the file)
     - no version info returned
     """
 
     try:
-        blastn_path = Path(shutil.which(blast_exe))  # type:ignore
-
+        # Returns a TypeError if `blast_exe` is None
+        try:
+            blastn_path = shutil.which(blast_exe)  # type:ignore
+        except TypeError:
+            return f"expected path to blastn executable; received {blast_exe}"
+        # Returns a TypeError if `blastn_path` is not on the PATH
+        blastn_path = Path(blastn_path)
     except TypeError:
         return f"{blast_exe} is not found in $PATH"
 
@@ -418,7 +424,7 @@ def generate_blastn_commands(
 
     :param filenames:  a list of paths to fragmented input FASTA files
     :param outdir:  path to output directory
-    :param blastn_exe:  path to BLASTN executable
+    :param blast_exe:  path to BLASTN executable
     :param mode:  str, analysis type (ANIb or ANIblastall)
 
     Assumes that the fragment sequence input filenames have the form
@@ -453,18 +459,18 @@ def construct_blastn_cmdline(
     fname1: Path,
     fname2: Path,
     outdir: Path,
-    blastn_exe: Path = pyani_config.BLASTN_DEFAULT,
+    blast_exe: Path = pyani_config.BLASTN_DEFAULT,
 ) -> str:
     """Return a single blastn command.
 
     :param fname1:
     :param fname2:
     :param outdir:
-    :param blastn_exe:  str, path to blastn executable
+    :param blast_exe:  str, path to blastn executable
     """
     prefix = outdir / f"{fname1.stem.replace('-fragments', '')}_vs_{fname2.stem}"
     return (
-        f"{blastn_exe} -out {prefix}.blast_tab -query {fname1} -db {fname2} "
+        f"{blast_exe} -out {prefix}.blast_tab -query {fname1} -db {fname2} "
         "-xdrop_gap_final 150 -dust no -evalue 1e-15 -max_target_seqs 1 -outfmt "
         "'6 qseqid sseqid length mismatch pident nident qlen slen "
         "qstart qend sstart send positive ppos gaps' "

pyani/aniblastall.py

@@ -70,6 +70,7 @@ def get_version(blast_exe: Path = pyani_config.BLASTALL_DEFAULT) -> str:
 
     The following circumstances are explicitly reported as strings
 
+    - a value of None given for the executable
     - no executable at passed path
     - non-executable file at passed path (this includes cases where the user doesn't have execute permissions on the file)
     - no version info returned
@@ -78,7 +79,13 @@ def get_version(blast_exe: Path = pyani_config.BLASTALL_DEFAULT) -> str:
     logger = logging.getLogger(__name__)
 
     try:
-        blastall_path = Path(shutil.which(blast_exe))  # type:ignore
+        # Returns a TypeError if `blast_exe` is None
+        try:
+            blastall_path = shutil.which(blast_exe)  # type:ignore
+        except TypeError:
+            return f"expected path to blastall executable; received {blast_exe}"
+        # Returns a TypeError if `blastall_path` is not on the PATH
+        blastall_path = Path(blastall_path)
     except TypeError:
         return f"{blast_exe} is not found in $PATH"
 

pyani/anim.py

@@ -110,14 +110,19 @@ def get_version(nucmer_exe: Path = pyani_config.NUCMER_DEFAULT) -> str:
     - non-executable file at passed path (this includes cases where the user doesn't have execute permissions on the file)
     - no version info returned
     """
-
     try:
-        nucmer_path = Path(shutil.which(nucmer_exe))  # type:ignore
+        # Returns a TypeError if `nucmer_exe` is None
+        try:
+            nucmer_path = shutil.which(nucmer_exe)  # type:ignore
+        except TypeError:
+            return f"expected path to nucmer executable; received {nucmer_exe}"
+        # Returns a TypeError if `nucmer_path` is not on the PATH
+        nucmer_path = Path(nucmer_path)
     except TypeError:
         return f"{nucmer_exe} is not found in $PATH"
 
     if not nucmer_path.is_file():  # no executable
-        return f"No nucmer at {nucmer_path}"
+        return f"No nucmer executable at {nucmer_path}"
 
     # This should catch cases when the file can't be executed by the user
     if not os.access(nucmer_path, os.X_OK):  # file exists but not executable

pyani/fastani.py

@@ -86,15 +86,25 @@ def get_version(fastani_exe: Path = pyani_config.FASTANI_DEFAULT) -> str:
 
     The following circumstances are explicitly reported as strings:
 
+    - a value of None given for the executable
     - no executable at passed path
     - non-executable file at passed path (this includes cases where the user doesn't have execute permissions on the file)
     - no version info returned
     """
     try:
-        fastani_path = Path(shutil.which(fastani_exe))  # type:ignore
+        # Returns a TypeError if `fastani_exe` is None
+        try:
+            fastani_path = shutil.which(fastani_exe)  # type:ignore
+        except TypeError:
+            return f"expected path to fastANI executable; received {fastani_exe}"
+        # Returns a TypeError if `fastani_path` is not on the PATH
+        fastani_path = Path(fastani_path)
     except TypeError:
         return f"{fastani_exe} is not found in $PATH"
 
+    # If a string that is not an executable is passed to
+    # shutil.which(), the return value will be None, so
+    # this check is still needed
     if fastani_path is None:
         return f"{fastani_exe} is not found in $PATH"
 

tests/test_anib.py

@@ -130,33 +130,44 @@ def test_get_version_nonetype():
     """Test behaviour when no location for the executable is given."""
     test_file_0 = None
 
-    assert anib.get_version(test_file_0) == f"{test_file_0} is not found in $PATH"
+    assert (
+        anib.get_version(test_file_0)
+        == f"expected path to blastn executable; received {test_file_0}"
+    )
+
+
+# Test case 1: no such file exists
+def test_get_version_random_string():
+    """Test behaviour when the given 'file' is not one."""
+    test_file_1 = "string"
 
+    assert anib.get_version(test_file_1) == f"{test_file_1} is not found in $PATH"
 
-# Test case 1: there is no executable
+
+# Test case 2: there is no executable
 def test_get_version_no_exe(executable_missing, monkeypatch):
     """Test behaviour when there is no file at the specified executable location."""
-    test_file_1 = Path("/non/existent/blastn")
-    assert anib.get_version(test_file_1) == f"No blastn executable at {test_file_1}"
+    test_file_2 = Path("/non/existent/blastn")
+    assert anib.get_version(test_file_2) == f"No blastn executable at {test_file_2}"
 
 
-# Test case 2: there is a file, but it is not executable
+# Test case 3: there is a file, but it is not executable
 def test_get_version_exe_not_executable(executable_not_executable, monkeypatch):
     """Test behaviour when the file at the executable location is not executable."""
-    test_file_2 = Path("/non/executable/blastn")
+    test_file_3 = Path("/non/executable/blastn")
     assert (
-        anib.get_version(test_file_2)
-        == f"blastn exists at {test_file_2} but not executable"
+        anib.get_version(test_file_3)
+        == f"blastn exists at {test_file_3} but not executable"
     )
 
 
-# Test case 3: there is an executable file, but the version can't be retrieved
+# Test case 4: there is an executable file, but the version can't be retrieved
 def test_get_version_exe_no_version(executable_without_version, monkeypatch):
     """Test behaviour when the version for the executable can not be retrieved."""
-    test_file_3 = Path("/missing/version/blastn")
+    test_file_4 = Path("/missing/version/blastn")
     assert (
-        anib.get_version(test_file_3)
-        == f"blastn exists at {test_file_3} but could not retrieve version"
+        anib.get_version(test_file_4)
+        == f"blastn exists at {test_file_4} but could not retrieve version"
     )
 
 

tests/test_aniblastall.py

@@ -57,34 +57,45 @@ def test_get_version_nonetype():
     test_file_0 = None
 
     assert (
-        aniblastall.get_version(test_file_0) == f"{test_file_0} is not found in $PATH"
+        aniblastall.get_version(test_file_0)
+        == f"expected path to blastall executable; received {test_file_0}"
     )
 
 
-# Test case 1: there is no executable
+# Test case 1: no such file exists
+def test_get_version_random_string():
+    """Test behaviour when the given 'file' is not one."""
+    test_file_1 = "string"
+
+    assert (
+        aniblastall.get_version(test_file_1) == f"{test_file_1} is not found in $PATH"
+    )
+
+
+# Test case 2: there is no executable
 def test_get_version_missing_exe(executable_missing):
     """Test behaviour when there is no file at the specified executable location."""
-    test_file_1 = Path("/non/existent/blastall")
-    assert aniblastall.get_version(test_file_1) == f"No blastall at {test_file_1}"
+    test_file_2 = Path("/non/existent/blastall")
+    assert aniblastall.get_version(test_file_2) == f"No blastall at {test_file_2}"
 
 
-# Test case 2: there is a file, but it is not executable
+# Test case 3: there is a file, but it is not executable
 def test_get_version_not_executable(executable_not_executable):
     """Test behaviour when the file at the executable location is not executable."""
-    test_file_2 = Path("/non/executable/blastall")
+    test_file_3 = Path("/non/executable/blastall")
     assert (
-        aniblastall.get_version(test_file_2)
-        == f"blastall exists at {test_file_2} but not executable"
+        aniblastall.get_version(test_file_3)
+        == f"blastall exists at {test_file_3} but not executable"
     )
 
 
-# Test case 3: there is an executable file, but the version can't be retrieved
+# Test case 4: there is an executable file, but the version can't be retrieved
 def test_get_version_no_version(executable_without_version):
     """Test behaviour when the version for the executable can not be retrieved."""
-    test_file_3 = Path("/missing/version/blastall")
+    test_file_4 = Path("/missing/version/blastall")
     assert (
-        aniblastall.get_version(test_file_3)
-        == f"blastall exists at {test_file_3} but could not retrieve version"
+        aniblastall.get_version(test_file_4)
+        == f"blastall exists at {test_file_4} but could not retrieve version"
     )
 
 

tests/test_anim.py

@@ -156,33 +156,44 @@ def test_get_version_nonetype():
     """Test behaviour when no location for the executable is given."""
     test_file_0 = None
 
-    assert anim.get_version(test_file_0) == f"{test_file_0} is not found in $PATH"
+    assert (
+        anim.get_version(test_file_0)
+        == f"expected path to nucmer executable; received {test_file_0}"
+    )
+
+
+# Test case 1: no such file exists
+def test_get_version_random_string():
+    """Test behaviour when the given 'file' is not one."""
+    test_file_1 = "string"
 
+    assert anim.get_version(test_file_1) == f"{test_file_1} is not found in $PATH"
 
-# Test case 1: there is no executable
+
+# Test case 2: there is no executable
 def test_get_version_no_exe(executable_missing):
     """Test behaviour when there is no file at the specified executable location."""
-    test_file_1 = Path("/non/existent/nucmer")
-    assert anim.get_version(test_file_1) == f"No nucmer at {test_file_1}"
+    test_file_2 = Path("/non/existent/nucmer")
+    assert anim.get_version(test_file_2) == f"No nucmer executable at {test_file_2}"
 
 
-# Test case 2: there is a file, but it is not executable
+# Test case 3: there is a file, but it is not executable
 def test_get_version_exe_not_executable(executable_not_executable):
     """Test behaviour when the file at the executable location is not executable."""
-    test_file_2 = Path("/non/executable/nucmer")
+    test_file_3 = Path("/non/executable/nucmer")
     assert (
-        anim.get_version(test_file_2)
-        == f"nucmer exists at {test_file_2} but not executable"
+        anim.get_version(test_file_3)
+        == f"nucmer exists at {test_file_3} but not executable"
     )
 
 
-# Test case 3: there is an executable file, but the version can't be retrieved
+# Test case 4: there is an executable file, but the version can't be retrieved
 def test_get_version_exe_no_version(executable_without_version):
     """Test behaviour when the version for the executable can not be retrieved."""
-    test_file_3 = Path("/missing/version/nucmer")
+    test_file_4 = Path("/missing/version/nucmer")
     assert (
-        anim.get_version(test_file_3)
-        == f"nucmer exists at {test_file_3} but could not retrieve version"
+        anim.get_version(test_file_4)
+        == f"nucmer exists at {test_file_4} but could not retrieve version"
     )