Low hanging CLI testing fruit

Leave primarily user-facing functions untested for the time being

.github/workflows/lint_test.yml

@@ -68,10 +68,11 @@ jobs:
         python-version: ${{ matrix.python-version }}
 
     - name: Set up (deadsnakes) Python ${{ matrix.python-version }}
-      uses: deadsnakes/action@v3.1.0
+      uses: sco1/action@test-me-tk
       if: endsWith(matrix.python-version, '-dev')
       with:
         python-version: ${{ matrix.python-version }}
+        tk: true
 
     - name: Restore uv cache
       uses: actions/cache@v4
@@ -125,12 +126,12 @@ jobs:
 
     - name: Report coverage
       run: |
-        echo '**Combined Coverage**' >> $GITHUB_STEP_SUMMARY
-        echo '```' >> $GITHUB_STEP_SUMMARY
-        coverage report -m --skip-covered >> $GITHUB_STEP_SUMMARY
-        echo '```' >> $GITHUB_STEP_SUMMARY
         coverage html
 
+        # Report a markdown version to the action summary
+        echo '**Combined Coverage**' >> $GITHUB_STEP_SUMMARY
+        coverage report --format=markdown >> $GITHUB_STEP_SUMMARY
+
     - name: Publish cov HTML
       uses: actions/upload-artifact@v4
       with:

README.md

@@ -31,7 +31,7 @@ Options:
 
 Commands:
   device       FlySight device utilities.
-  log_convert  FlySight device log utilities.
+  log_convert  FlySight V2 log conversion.
   logs         FlySight device log utilities.
   trim         FlySight log trimming.
 ```

pyflysight/cli.py

@@ -55,7 +55,7 @@ def _print_connected_drives(flysight_drives: t.Iterable[Path]) -> None:
         print(md_str)
 
 
-def _ask_select_flysight() -> Path:
+def _ask_select_flysight() -> Path:  # pragma: no cover
     """List connected FlySight drives and prompt user for selection."""
     flysight_drives = get_flysight_drives()
     if not flysight_drives:
@@ -75,7 +75,7 @@ def _ask_select_flysight() -> Path:
 
 
 @device_app.command()
-def list(wait_for: int = typer.Option(0, min=0)) -> None:
+def list(wait_for: int = typer.Option(0, min=0)) -> None:  # pragma: no cover
     """
     List connected FlySight devices.
 
@@ -110,7 +110,7 @@ def _try_write_config(device_root: Path, config: FlysightConfig, backup_existing
 def write_default(
     flysight_root: Path = typer.Option(None, exists=True, file_okay=False, dir_okay=True),
     backup_existing: bool = typer.Option(True),
-) -> None:
+) -> None:  # pragma: no cover
     """
     Write default configuration.
 
@@ -136,7 +136,7 @@ def write_from_json(
     flysight_root: Path = typer.Option(None, exists=True, file_okay=False, dir_okay=True),
     config_source: Path = typer.Option(None, exists=True, file_okay=True, dir_okay=False),
     backup_existing: bool = typer.Option(True),
-) -> None:
+) -> None:  # pragma: no cover
     """
     Write previously serialized paramters.
 
@@ -171,7 +171,7 @@ def write_from_other(
     flysight_root: Path = typer.Option(None, exists=True, file_okay=False, dir_okay=True),
     config_source: Path = typer.Option(None, exists=True, file_okay=True, dir_okay=False),
     backup_existing: bool = typer.Option(True),
-) -> None:
+) -> None:  # pragma: no cover
     """
     Copy configuration from file.
 
@@ -207,15 +207,14 @@ def copy(
     flysight_root: Path = typer.Option(None, exists=True, file_okay=False, dir_okay=True),
     dest: Path = typer.Option(None, exists=True, file_okay=False, dir_okay=True),
     exist_ok: bool = typer.Option(True),
-) -> None:
+) -> None:  # pragma: no cover
     """Copy all logs on device to the specified destination."""
     if flysight_root is None:
         flysight_root = _ask_select_flysight()
 
     drive_metadata = get_device_metadata(flysight_root)
     if drive_metadata.n_logs == 0:
-        print("No logs on device to copy.")
-        return
+        _abort_with_message("No logs on device to copy.")
 
     if dest is None:
         dest = prompt_for_dir(title="Select Log Destination")
@@ -230,7 +229,7 @@ def copy(
 def clear(
     flysight_root: Path = typer.Option(None, exists=True, file_okay=False, dir_okay=True),
     force: bool = typer.Option(False),
-) -> None:
+) -> None:  # pragma: no cover
     """
     Clear all logs on device.
 
@@ -243,8 +242,7 @@ def clear(
 
     drive_metadata = get_device_metadata(flysight_root)
     if drive_metadata.n_logs == 0:
-        print("No logs on device to erase.")
-        return
+        _abort_with_message("No logs on device to erase.")
 
     if not force:
         confirm_clear = typer.confirm(f"This will erase {drive_metadata.n_logs} logs. Confirm?")
@@ -257,37 +255,37 @@ def clear(
         _abort_with_message("Error: FlySight device is write protected. ")
 
 
-def _check_log_dir(log_dir: Path, verbose: bool) -> None:
-    """Check log directory parameters & exit CLI if issues are encountered."""
+def _check_log_dir(log_dir: Path, v2_only: bool = False) -> None:
+    """
+    Check log directory parameters & exit CLI if issues are encountered.
+
+    The CLI is aborted if one of the following conditions is met:
+        * No log files in the provided directory
+        * If `v2_only` is `True`, a FlySight V1 flight log directory is provided
+    """
     try:
         flysight_type = classify_log_dir(log_dir)
     except ValueError:
-        if verbose:
-            print("Error: No log files found in provided log directory.")
-        return
+        _abort_with_message("Error: No log files found in provided log directory.")
 
-    if flysight_type == FlysightType.VERSION_1:
-        if verbose:
-            print("Error: Functionality is currently not implemented for FlySight V1 hardware.")
-        return
+    if v2_only:
+        if flysight_type == FlysightType.VERSION_1:
+            _abort_with_message("Error: Currently not implemented for FlySight V1 hardware.")
 
 
-def _trim_pipeline(log_dir: Path, verbose: bool, normalize_gps: bool) -> None:
-    if verbose:
-        print(f"Trimming: {log_dir}...", end="")
+def _trim_pipeline(log_dir: Path, normalize_gps: bool) -> None:
+    print(f"Trimming: {log_dir}...", end="")
 
     windowtrim_flight_log(log_dir, write_csv=True, normalize_gps=normalize_gps)
 
-    if verbose:
-        print("Done!")
+    print("Done!")
 
 
 @trim_app.command()
 def single(
     log_dir: Path = typer.Option(None, exists=True, file_okay=False, dir_okay=True),
     normalize_gps: bool = typer.Option(False),
-    verbose: bool = typer.Option(True),
-) -> None:
+) -> None:  # pragma: no cover
     """
     Trim single flight log.
 
@@ -297,16 +295,15 @@ def single(
     if log_dir is None:
         log_dir = prompt_for_dir(title="Select Log Directory For Processing")
 
-    _check_log_dir(log_dir, verbose=verbose)
-    _trim_pipeline(log_dir, normalize_gps=normalize_gps, verbose=verbose)
+    _check_log_dir(log_dir, v2_only=True)
+    _trim_pipeline(log_dir, normalize_gps=normalize_gps)
 
 
 @trim_app.command()
 def batch(
     log_dir: Path = typer.Option(None, exists=True, file_okay=False, dir_okay=True),
     normalize_gps: bool = typer.Option(False),
-    verbose: bool = typer.Option(True),
-) -> None:
+) -> None:  # pragma: no cover
     """
     Batch trim a directory of flight logs.
 
@@ -317,27 +314,24 @@ def batch(
         log_dir = prompt_for_dir(title="Select Directory For Batch Processing")
 
     for ld in iter_log_dirs(log_dir, flysight_type=FlysightType.VERSION_2):
-        _check_log_dir(ld.log_dir, verbose=verbose)
-        _trim_pipeline(ld.log_dir, normalize_gps=normalize_gps, verbose=verbose)
+        _check_log_dir(log_dir, v2_only=True)
+        _trim_pipeline(ld.log_dir, normalize_gps=normalize_gps)
 
 
-def _v2_log_parse2csv_pipeline(log_dir: Path, verbose: bool, normalize_gps: bool) -> None:
-    if verbose:
-        print(f"Converting: {log_dir}...", end="")
+def _v2_log_parse2csv_pipeline(log_dir: Path, normalize_gps: bool) -> None:
+    print(f"Converting: {log_dir}...", end="")
 
     fl = parse_v2_log_directory(log_dir, normalize_gps=normalize_gps)
     fl.to_csv(log_dir)  # No need to pass normalize_gps a second time
 
-    if verbose:
-        print("Done!")
+    print("Done!")
 
 
 @v2_log_convert_app.command(name="single")
 def single_convert(
     log_dir: Path = typer.Option(None, exists=True, file_okay=False, dir_okay=True),
     normalize_gps: bool = typer.Option(False),
-    verbose: bool = typer.Option(True),
-) -> None:
+) -> None:  # pragma: no cover
     """
     Parse a single FlySight V2 flight log into a user-friendly CSV collection.
 
@@ -347,16 +341,15 @@ def single_convert(
     if log_dir is None:
         log_dir = prompt_for_dir(title="Select Log Directory For Processing")
 
-    _check_log_dir(log_dir, verbose=verbose)
-    _v2_log_parse2csv_pipeline(log_dir, normalize_gps=normalize_gps, verbose=verbose)
+    _check_log_dir(log_dir, v2_only=True)
+    _v2_log_parse2csv_pipeline(log_dir, normalize_gps=normalize_gps)
 
 
 @v2_log_convert_app.command(name="batch")
 def batch_convert(
     log_dir: Path = typer.Option(None, exists=True, file_okay=False, dir_okay=True),
     normalize_gps: bool = typer.Option(False),
-    verbose: bool = typer.Option(True),
-) -> None:
+) -> None:  # pragma: no cover
     """
     Batch parse a directory of FlySight V2 flight logs into a user-friendly CSV collection.
 
@@ -367,8 +360,8 @@ def batch_convert(
         log_dir = prompt_for_dir(title="Select Directory For Batch Processing")
 
     for ld in iter_log_dirs(log_dir, flysight_type=FlysightType.VERSION_2):
-        _check_log_dir(ld.log_dir, verbose=verbose)
-        _v2_log_parse2csv_pipeline(ld.log_dir, normalize_gps=normalize_gps, verbose=verbose)
+        _check_log_dir(log_dir, v2_only=True)
+        _v2_log_parse2csv_pipeline(ld.log_dir, normalize_gps=normalize_gps)
 
 
 # For mkdocs-click, this must be after all the commands have been defined

pyproject.toml

@@ -57,6 +57,7 @@ dev-dependencies = [
     "pymdown-extensions~=10.9",
     "pytest-check~=2.4",
     "pytest-cov~=5.0",
+    "pytest-mock~=3.14",
     "pytest-randomly~=3.15",
     "pytest~=8.3",
     "ruff~=0.6",

tests/test_cli.py

@@ -0,0 +1,101 @@
+from pathlib import Path
+
+import pytest
+import typer
+from pytest_mock import MockerFixture
+
+from pyflysight import FlysightType
+from pyflysight.cli import (
+    _abort_with_message,
+    _check_log_dir,
+    _print_connected_drives,
+    _try_write_config,
+)
+from pyflysight.config_utils import FlysightV2Config
+from pyflysight.flysight_utils import FlysightMetadata
+
+
+def test_abort_with_message(capsys: pytest.CaptureFixture) -> None:
+    with pytest.raises(typer.Abort):
+        _abort_with_message("Hello")
+
+    captured = capsys.readouterr()
+    assert captured.out == "Hello"
+
+
+DRIVE_METADATA = (
+    FlysightMetadata(flysight_type=FlysightType.VERSION_1, serial="ab", firmware="1", n_logs=2),
+    FlysightMetadata(flysight_type=FlysightType.VERSION_2, serial="cd", firmware="2", n_logs=1),
+)
+
+TRUTH_PRINTED_METADATA = """\
+0. A: - FlySight V1, Logs Available: 2
+    Serial: ab
+    Firmware: 1
+1. B: - FlySight V2, Logs Available: 1
+    Serial: cd
+    Firmware: 2
+"""
+
+
+def test_print_connected_drives(mocker: MockerFixture, capsys: pytest.CaptureFixture) -> None:
+    mocker.patch("pyflysight.cli.get_device_metadata", side_effect=DRIVE_METADATA)
+    _print_connected_drives((Path("A:"), Path("B:")))
+
+    captured = capsys.readouterr()
+    assert captured.out == TRUTH_PRINTED_METADATA
+
+
+def test_try_write_config(tmp_path: Path) -> None:
+    cfg = FlysightV2Config()
+    _try_write_config(tmp_path, config=cfg, backup_existing=False)
+
+    assert len(list(tmp_path.glob("CONFIG.TXT"))) == 1
+
+
+def test_try_write_config_no_permission_raises(tmp_path: Path, mocker: MockerFixture) -> None:
+    # I don't understand if permission modes work or not on Windows so will mock instead
+    mocker.patch("pyflysight.cli.write_config", side_effect=PermissionError())
+
+    cfg = FlysightV2Config()
+    with pytest.raises(typer.Abort):
+        _try_write_config(tmp_path, config=cfg, backup_existing=False)
+
+
+@pytest.mark.parametrize(("hw_type",), ((FlysightType.VERSION_1,), (FlysightType.VERSION_2,)))
+def test_check_log_dir_noop(
+    hw_type: FlysightType, capsys: pytest.CaptureFixture, mocker: MockerFixture
+) -> None:
+    mocker.patch("pyflysight.cli.classify_log_dir", return_value=hw_type)
+    _check_log_dir(Path())  # Since we're mocking the return, path doesn't matter
+
+    captured = capsys.readouterr()
+    assert not captured.out
+
+
+def test_check_log_dir_fsv2_noop(capsys: pytest.CaptureFixture, mocker: MockerFixture) -> None:
+    mocker.patch("pyflysight.cli.classify_log_dir", return_value=FlysightType.VERSION_2)
+    _check_log_dir(Path(), v2_only=True)  # Since we're mocking the return, path doesn't matter
+
+    captured = capsys.readouterr()
+    assert not captured.out
+
+
+def test_check_log_dir_no_log_errors(capsys: pytest.CaptureFixture, mocker: MockerFixture) -> None:
+    mocker.patch("pyflysight.cli.classify_log_dir", side_effect=ValueError())
+
+    with pytest.raises(typer.Abort):
+        _check_log_dir(Path(), v2_only=False)  # Since we're mocking the return, path doesn't matter
+
+    captured = capsys.readouterr()
+    assert "No log files" in captured.out
+
+
+def test_check_log_dir_fsv1_errors(capsys: pytest.CaptureFixture, mocker: MockerFixture) -> None:
+    mocker.patch("pyflysight.cli.classify_log_dir", return_value=FlysightType.VERSION_1)
+
+    with pytest.raises(typer.Abort):
+        _check_log_dir(Path(), v2_only=True)  # Since we're mocking the return, path doesn't matter
+
+    captured = capsys.readouterr()
+    assert "FlySight V1 hardware" in captured.out

tox.ini

@@ -8,9 +8,7 @@ addopts =
     --cov-report term-missing:skip-covered
 
 [coverage:run]
-# Omit functionality that requires user input
 omit =
-    pyflysight/cli.py
     pyflysight/trim_app.py
 
 [coverage:report]
@@ -32,6 +30,8 @@ deps =
     pytest
     pytest-check
     pytest-cov
+    pytest-mock
+    pytest-randomly
 
 [testenv:clean]
 deps = coverage