Rewrite fix_ogvjs_dist and its tests to remove tests redundancies + ensure proper coverage

Author: benoit74

src/zimscraperlib/fix_ogvjs_dist.py

@@ -7,7 +7,7 @@
 import logging
 import pathlib
 import sys
-from typing import Union
+from typing import List, Union
 
 logging.basicConfig(format="%(levelname)s:%(message)s", level=logging.DEBUG)
 logger = logging.getLogger(__name__)
@@ -33,16 +33,17 @@ def fix_source_dir(source_vendors_path: Union[pathlib.Path, str]):
     logger.info("all done.")
 
 
-def run():
-    if len(sys.argv) < 2:  # noqa: PLR2004 # pragma: no cover
-        print(f"Usage: {sys.argv[0]} <source_vendors_path>")  # noqa: T201
+def run(args: List[str] = sys.argv):
+    if len(args) < 2:  # noqa: PLR2004
+        print(f"Usage: {args[0]} <source_vendors_path>")  # noqa: T201
         print(  # noqa: T201
             "\t<source_vendors_path>\tpath to your folder containing "
             "ogvjs/videojs/videojs-ogvjs."
         )
-        sys.exit(1)
-    return sys.exit(fix_source_dir(sys.argv[1]))
+        return 1
+    fix_source_dir(args[1])
+    return 0
 
 
 if __name__ == "__main__":
-    run()
+    sys.exit(run())

tests/ogvjs/test_ogvjs.py

@@ -3,12 +3,12 @@
 
 import shutil
 import subprocess
-import sys
 import zipfile
 
 import pytest
 
 from zimscraperlib.download import save_large_file
+from zimscraperlib.fix_ogvjs_dist import run
 
 
 def prepare_ogvjs_folder(tmp_path, videojs_url, ogvjs_url, videojs_ogvjs_url):
@@ -44,43 +44,49 @@ def prepare_ogvjs_folder(tmp_path, videojs_url, ogvjs_url, videojs_ogvjs_url):
     tmp_path.joinpath(member).rename(tmp_path.joinpath("videojs-ogvjs.js"))
 
 
-def test_installed_script():
-    kwargs = {"universal_newlines": True, "stdout": subprocess.PIPE}
-    script = subprocess.run(["/usr/bin/env", "fix_ogvjs_dist"], **kwargs, check=False)
-    assert script.returncode == 1
-    assert script.stdout.strip().startswith("Usage: ")
-
-
-def test_missing_param():
+def test_ogvjs_installed_script_missing_param():
+    # run from installed script to check real conditions
     script = subprocess.run(
-        [
-            sys.executable,
-            "-m",
-            "zimscraperlib.fix_ogvjs_dist",
-        ],
+        ["/usr/bin/env", "fix_ogvjs_dist"],
+        text=True,
+        capture_output=True,
         check=False,
     )
     assert script.returncode == 1
+    assert script.stdout.strip().startswith("Usage: ")
+
+
+def test_ogvjs_from_code_missing_params():
+    # run from code to mesure coverage easily
+
+    assert run(["fix_ogvjs_dist"]) == 1
 
 
 @pytest.mark.slow
-def test_fix_ogvjs_dist(tmp_path, videojs_url, ogvjs_url, videojs_ogvjs_url):
+def test_ogvjs_installed_script_ok(tmp_path, videojs_url, ogvjs_url, videojs_ogvjs_url):
+    # run from installed script to check real conditions
+
     prepare_ogvjs_folder(tmp_path, videojs_url, ogvjs_url, videojs_ogvjs_url)
 
-    # run to fix it from source (using installed script name)
     script = subprocess.run(
-        [
-            sys.executable,
-            "-m",
-            "zimscraperlib.fix_ogvjs_dist",
-            str(tmp_path),
-        ],
+        ["/usr/bin/env", "fix_ogvjs_dist", str(tmp_path)],
         text=True,
-        stdout=subprocess.PIPE,
-        stderr=subprocess.STDOUT,
+        capture_output=True,
         check=False,
     )
     assert script.returncode == 0
 
     with open(tmp_path / "videojs-ogvjs.js") as fh:
         assert "webm" in fh.read()
+
+
+@pytest.mark.slow
+def test_ogvjs_from_code_ok(tmp_path, videojs_url, ogvjs_url, videojs_ogvjs_url):
+    # run from code to mesure coverage easily
+
+    prepare_ogvjs_folder(tmp_path, videojs_url, ogvjs_url, videojs_ogvjs_url)
+
+    assert run(["fix_ogvjs_dist", str(tmp_path)]) == 0
+
+    with open(tmp_path / "videojs-ogvjs.js") as fh:
+        assert "webm" in fh.read()