Merge pull request #1233 from dandi/morepath

Use pathlib more in tests

dandi/cli/tests/test_cmd_ls.py

@@ -1,5 +1,8 @@
+from __future__ import annotations
+
 import json
-import os
+from pathlib import Path
+from typing import Any
 from unittest.mock import ANY
 
 from click.testing import CliRunner
@@ -15,25 +18,30 @@
 @pytest.mark.parametrize(
     "format", ("auto", "json", "json_pp", "json_lines", "yaml", "pyout")
 )
-def test_smoke(simple1_nwb_metadata, simple1_nwb, format):
+def test_smoke(
+    simple1_nwb_metadata: dict[str, Any], simple1_nwb: Path, format: str
+) -> None:
     runner = CliRunner()
-    r = runner.invoke(ls, ["-f", format, simple1_nwb])
+    r = runner.invoke(ls, ["-f", format, str(simple1_nwb)])
     assert r.exit_code == 0, f"Exited abnormally. out={r.stdout}"
     # we would need to redirect pyout for its analysis
     out = r.stdout
 
     if format == "json_lines":
-        load = json.loads
+
+        def load(s: str) -> Any:
+            return json.loads(s)
+
     elif format.startswith("json"):
 
-        def load(s):
+        def load(s: str) -> Any:
             obj = json.loads(s)
             assert len(obj) == 1  # will be a list with a single elem
             return obj[0]
 
     elif format == "yaml":
 
-        def load(s):
+        def load(s: str) -> Any:
             obj = yaml_load(s, typ="base")
             assert len(obj) == 1  # will be a list with a single elem
             return obj[0]
@@ -49,41 +57,45 @@ def load(s):
         assert metadata[f] == simple1_nwb_metadata[f]
 
 
-def test_ls_nwb_file(simple2_nwb):
-    bids_file_path = "simple2.nwb"
-    bids_file_path = os.path.join(simple2_nwb, bids_file_path)
-    r = CliRunner().invoke(ls, ["-f", "yaml", bids_file_path])
+def test_ls_nwb_file(simple2_nwb: Path) -> None:
+    bids_file_path = simple2_nwb / "simple2.nwb"
+    r = CliRunner().invoke(ls, ["-f", "yaml", str(bids_file_path)])
     assert r.exit_code == 0, r.output
     data = yaml_load(r.stdout, "safe")
     assert len(data) == 1
 
 
 @mark.skipif_no_network
-def test_ls_bids_file(bids_examples):
-    bids_file_path = "asl003/sub-Sub1/anat/sub-Sub1_T1w.nii.gz"
-    bids_file_path = os.path.join(bids_examples, bids_file_path)
-    r = CliRunner().invoke(ls, ["-f", "yaml", bids_file_path])
+def test_ls_bids_file(bids_examples: Path) -> None:
+    bids_file_path = (
+        bids_examples / "asl003" / "sub-Sub1" / "anat" / "sub-Sub1_T1w.nii.gz"
+    )
+    r = CliRunner().invoke(ls, ["-f", "yaml", str(bids_file_path)])
     assert r.exit_code == 0, r.output
     data = yaml_load(r.stdout, "safe")
     assert len(data) == 1
     assert data[0]["identifier"] == "Sub1"
 
 
 @mark.skipif_no_network
-def test_ls_zarrbids_file(bids_examples):
+def test_ls_zarrbids_file(bids_examples: Path) -> None:
     bids_file_path = (
-        "micr_SEMzarr/sub-01/ses-01/micr/sub-01_ses-01_sample-A_SPIM.ome.zarr"
+        bids_examples
+        / "micr_SEMzarr"
+        / "sub-01"
+        / "ses-01"
+        / "micr"
+        / "sub-01_ses-01_sample-A_SPIM.ome.zarr"
     )
-    bids_file_path = os.path.join(bids_examples, bids_file_path)
-    r = CliRunner().invoke(ls, ["-f", "yaml", bids_file_path])
+    r = CliRunner().invoke(ls, ["-f", "yaml", str(bids_file_path)])
     assert r.exit_code == 0, r.output
     data = yaml_load(r.stdout, "safe")
     assert len(data) == 1
     assert data[0]["identifier"] == "01"
 
 
 @mark.skipif_no_network
-def test_ls_dandiset_url():
+def test_ls_dandiset_url() -> None:
     r = CliRunner().invoke(
         ls, ["-f", "yaml", "https://api.dandiarchive.org/api/dandisets/000027"]
     )
@@ -94,7 +106,7 @@ def test_ls_dandiset_url():
 
 
 @mark.skipif_no_network
-def test_ls_dandiset_url_recursive():
+def test_ls_dandiset_url_recursive() -> None:
     r = CliRunner().invoke(
         ls, ["-f", "yaml", "-r", "https://api.dandiarchive.org/api/dandisets/000027"]
     )
@@ -106,7 +118,7 @@ def test_ls_dandiset_url_recursive():
 
 
 @mark.skipif_no_network
-def test_ls_path_url():
+def test_ls_path_url() -> None:
     r = CliRunner().invoke(
         ls,
         [
@@ -124,7 +136,7 @@ def test_ls_path_url():
     assert data[0]["path"] == "sub-RAT123/sub-RAT123.nwb"
 
 
-def test_smoke_local_schema(simple1_nwb):
+def test_smoke_local_schema(simple1_nwb: Path) -> None:
     runner = CliRunner()
     r = runner.invoke(
         ls,
@@ -133,7 +145,7 @@ def test_smoke_local_schema(simple1_nwb):
             "json",
             "--schema",
             DANDI_SCHEMA_VERSION,
-            simple1_nwb,
+            str(simple1_nwb),
         ],
     )
     assert r.exit_code == 0, f"Exited abnormally. out={r.stdout}"

dandi/cli/tests/test_cmd_validate.py

@@ -1,5 +1,4 @@
 import json
-import os
 from pathlib import Path
 
 from click.testing import CliRunner
@@ -10,44 +9,40 @@
 
 
 @pytest.mark.parametrize("dataset", BIDS_ERROR_TESTDATA_SELECTION)
-def test_validate_bids_error(bids_error_examples, dataset):
-
-    broken_dataset = os.path.join(bids_error_examples, dataset)
-    with open(os.path.join(broken_dataset, ".ERRORS.json")) as f:
+def test_validate_bids_error(bids_error_examples: Path, dataset: str) -> None:
+    broken_dataset = bids_error_examples / dataset
+    with (broken_dataset / ".ERRORS.json").open() as f:
         expected_errors = json.load(f)
-    r = CliRunner().invoke(validate, [broken_dataset])
+    r = CliRunner().invoke(validate, [str(broken_dataset)])
     # Does it break?
     assert r.exit_code == 1
-
     # Does it detect all errors?
     for key in expected_errors:
         assert key in r.output
 
 
-def test_validate_nwb_error(simple3_nwb):
-    """
-    Do we fail on critical NWB validation errors?
-    """
-
-    r = CliRunner().invoke(validate, [simple3_nwb])
+def test_validate_nwb_error(simple3_nwb: Path) -> None:
+    """Do we fail on critical NWB validation errors?"""
+    r = CliRunner().invoke(validate, [str(simple3_nwb)])
     # does it fail? as per:
     # https://github.com/dandi/dandi-cli/pull/1157#issuecomment-1312546812
     assert r.exit_code != 0
 
 
-def test_validate_bids_grouping_error(bids_error_examples, dataset="invalid_asl003"):
+def test_validate_bids_grouping_error(
+    bids_error_examples: Path, dataset: str = "invalid_asl003"
+) -> None:
     """
-    This is currently a placeholder test, and should be updated once we have paths with
-    multiple errors for which grouping functionality can actually be tested.
+    This is currently a placeholder test, and should be updated once we have
+    paths with multiple errors for which grouping functionality can actually be
+    tested.
     """
-
-    dataset = os.path.join(bids_error_examples, dataset)
-    r = CliRunner().invoke(validate, ["--grouping=path", dataset])
+    bids_dataset = bids_error_examples / dataset
+    r = CliRunner().invoke(validate, ["--grouping=path", str(bids_dataset)])
     # Does it break?
     assert r.exit_code == 1
-
     # Does it detect all errors?
-    assert dataset in r.output
+    assert str(bids_dataset) in r.output
 
 
 def test_validate_nwb_path_grouping(organized_nwb_dir3: Path) -> None:
@@ -64,15 +59,13 @@ def test_validate_nwb_path_grouping(organized_nwb_dir3: Path) -> None:
 
 
 def test_validate_bids_error_grouping_notification(
-    bids_error_examples, dataset="invalid_asl003"
-):
+    bids_error_examples: Path, dataset: str = "invalid_asl003"
+) -> None:
     """Test user notification for unimplemented parameter value."""
-
-    broken_dataset = os.path.join(bids_error_examples, dataset)
-    r = CliRunner().invoke(validate, ["--grouping=error", broken_dataset])
+    broken_dataset = bids_error_examples / dataset
+    r = CliRunner().invoke(validate, ["--grouping=error", str(broken_dataset)])
     # Does it break?
     assert r.exit_code == 2
-
     # Does it notify the user correctly?
     notification_substring = "Invalid value for '--grouping'"
     assert notification_substring in r.output