[fdiff.diff] add handling for unified diff URL request exceptions, non-200 response code status

Author: chrissimpkins

lib/fdiff/diff.py

@@ -4,12 +4,13 @@
 
 from fontTools.ttLib import TTFont
 
+from fdiff.exceptions import AIOError
 from fdiff.remote import (
     _get_filepath_from_url,
     create_async_get_request_session_and_run,
 )
-from fdiff.utils import get_file_modtime
 from fdiff.thirdparty.fdifflib import unified_diff
+from fdiff.utils import get_file_modtime
 
 
 def u_diff(
@@ -18,8 +19,8 @@ def u_diff(
     """Performs a unified diff on a TTX serialized data format dump of font binary data using
     a modified version of the Python standard libary difflib module.
 
-    filepath_a: (string) pre-file path
-    filepath_b: (string) post-file path
+    filepath_a: (string) pre-file local file path or URL path
+    filepath_b: (string) post-file local file path or URL path
     context_lines: (int) number of context lines to include in the diff (default=3)
     include_tables: (list of str) Python list of OpenType tables to include in the diff
     exclude_tables: (list of str) Python list of OpentType tables to exclude from the diff
@@ -29,7 +30,10 @@ def u_diff(
 
     :returns: Generator of ordered diff line strings that include newline line endings
     :raises: KeyError if include_tables or exclude_tables includes a mis-specified table
-    that is not included in filepath_a OR filepath_b"""
+    that is not included in filepath_a OR filepath_b
+    :raises: fdiff.exceptions.AIOError if exception raised during execution of async I/O
+             GET request for URL or file write
+    :raises: fdiff.exceptions.AIOError if GET request to URL returned non-200 response status code"""
     with tempfile.TemporaryDirectory() as tmpdirname:
         # define the file paths with either local file requests
         # or pulls of remote files based on the command line request
@@ -62,12 +66,14 @@ def u_diff(
                 if task.exception():
                     # raise exception here to notify calling code that something
                     # did not work
-                    # TODO: handle exceptions
-                    pass
+                    raise AIOError(f"{task.exception()}")
                 elif task.result().http_status != 200:
-                    # TODO: handle non-200 HTTP response status codes + file write fails
-                    pass
+                    # handle non-200 HTTP response status codes + file write fails
+                    raise AIOError(
+                        f"failed to pull '{task.result().url}' with HTTP status code {task.result().http_status}"
+                    )
 
+        # instantiate left and right fontTools.ttLib.TTFont objects
         tt_left = TTFont(prepath)
         tt_right = TTFont(postpath)
 

tests/test_diff.py

@@ -3,6 +3,7 @@
 import pytest
 
 from fdiff.diff import u_diff
+from fdiff.exceptions import AIOError
 
 ROBOTO_BEFORE_PATH = os.path.join("tests", "testfiles", "Roboto-Regular.subset1.ttf")
 ROBOTO_AFTER_PATH = os.path.join("tests", "testfiles", "Roboto-Regular.subset2.ttf")
@@ -13,6 +14,11 @@
 ROBOTO_UDIFF_HEADPOSTONLY_EXPECTED_PATH = os.path.join("tests", "testfiles", "roboto_udiff_headpostonly_expected.txt")
 ROBOTO_UDIFF_EXCLUDE_HEADPOST_EXPECTED_PATH = os.path.join("tests", "testfiles", "roboto_udiff_ex_headpost_expected.txt")
 
+ROBOTO_BEFORE_URL = "https://github.com/source-foundry/fdiff/raw/master/tests/testfiles/Roboto-Regular.subset1.ttf"
+ROBOTO_AFTER_URL = "https://github.com/source-foundry/fdiff/raw/master/tests/testfiles/Roboto-Regular.subset2.ttf"
+
+URL_404 = "https://httpbin.org/status/404"
+
 # Setup: define the expected diff text for unified diff
 with open(ROBOTO_UDIFF_EXPECTED_PATH, "r") as robo_udiff:
     ROBOTO_UDIFF_EXPECTED = robo_udiff.read()
@@ -141,3 +147,67 @@ def test_unified_diff_include_with_bad_table_definition():
 def test_unified_diff_exclude_with_bad_table_definition():
     with pytest.raises(KeyError):
         u_diff(ROBOTO_BEFORE_PATH, ROBOTO_AFTER_PATH, exclude_tables=["bogus"])
+
+
+def test_unified_diff_remote_fonts():
+    res = u_diff(ROBOTO_BEFORE_URL, ROBOTO_AFTER_URL)
+    res_string = "".join(res)
+    res_string_list = res_string.split("\n")
+    expected_string_list = ROBOTO_UDIFF_EXPECTED.split("\n")
+
+    # have to handle the tests for the top two file path lines
+    # differently than the rest of the comparisons
+    for x, line in enumerate(res_string_list):
+        # treat top two lines of the diff as comparison of first 10 chars only
+        if x == 0:
+            assert line[0:9] == "--- https"
+        elif x == 1:
+            assert line[0:9] == "+++ https"
+        else:
+            assert line == expected_string_list[x]
+
+
+def test_unified_diff_remote_and_local_fonts():
+    res = u_diff(ROBOTO_BEFORE_URL, ROBOTO_AFTER_PATH)
+    res_string = "".join(res)
+    res_string_list = res_string.split("\n")
+    expected_string_list = ROBOTO_UDIFF_EXPECTED.split("\n")
+
+    # have to handle the tests for the top two file path lines
+    # differently than the rest of the comparisons
+    for x, line in enumerate(res_string_list):
+        # treat top two lines of the diff as comparison of first 10 chars only
+        if x == 0:
+            assert line[0:9] == "--- https"
+        elif x == 1:
+            assert line[0:9] == expected_string_list[x][0:9]
+        else:
+            assert line == expected_string_list[x]
+
+
+def test_unified_diff_local_and_remote_fonts():
+    res = u_diff(ROBOTO_BEFORE_PATH, ROBOTO_AFTER_URL)
+    res_string = "".join(res)
+    res_string_list = res_string.split("\n")
+    expected_string_list = ROBOTO_UDIFF_EXPECTED.split("\n")
+
+    # have to handle the tests for the top two file path lines
+    # differently than the rest of the comparisons
+    for x, line in enumerate(res_string_list):
+        # treat top two lines of the diff as comparison of first 10 chars only
+        if x == 0:
+            assert line[0:9] == expected_string_list[x][0:9]
+        elif x == 1:
+            assert line[0:9] == "+++ https"
+        else:
+            assert line == expected_string_list[x]
+
+
+def test_unified_diff_remote_404_first_file():
+    with pytest.raises(AIOError):
+        u_diff(URL_404, ROBOTO_AFTER_URL)
+
+
+def test_unified_diff_remote_404_second_file():
+    with pytest.raises(AIOError):
+        u_diff(ROBOTO_BEFORE_URL, URL_404)