[Git][reproducible-builds/diffoscope][master] 2 commits: Use assert_diff in test_zip over get_data and separate assert.

[Chris Lamb (@lamby)]

Chris Lamb pushed to branch master at Reproducible Builds / diffoscope


Commits:
decb8f9d by Chris Lamb at 2022-05-17T10:00:15-07:00
Use assert_diff in test_zip over get_data and separate assert.

- - - - -
e4ae650e by Chris Lamb at 2022-05-17T10:04:29-07:00
Don't mask differences in .zip/.jar central directory extra fields.

Re. reproducible-builds/strip-nondeterminism#19

- - - - -


2 changed files:

- diffoscope/comparators/zip.py
- tests/comparators/test_zip.py


Changes:

=====================================
diffoscope/comparators/zip.py
=====================================
@@ -120,23 +120,33 @@ class BsdtarVerbose(Command):
 def zipinfo_differences(file, other):
     """
     Run all our zipinfo variants.
-
-    We only return the first kind of metadata command that returns some output.
-    The compromise here is that "zipinfo -v" returns more info and, if the .zip
-    files contain both extended and simple kinds differences, then this logic
-    will only show the simpler differences.
-
-    In practice, this can mean that the values of .zip "extra fields" can be
-    hidden (see, for example, reproducible-builds/strip-nondeterminism#19).
-    This is considered less bad than the inverse (ie. prefering to show the
-    output of "zipinfo -v" over merely "zipinfo") as this will result in
-    virtually unreadable diffs in the case that differences when the
-    differences in the .zip are conventional/common/simple.
     """
-    for x in (Zipinfo, ZipinfoVerbose, BsdtarVerbose):
-        result = Difference.from_operation(x, file.path, other.path)
-        if result is not None:
-            return [result]
+
+    # We need to run both "zipinfo" and "zipinfo -v" in all code paths, so just
+    # run them both first.
+    zipinfo = Difference.from_operation(Zipinfo, file.path, other.path)
+    verbose = Difference.from_operation(ZipinfoVerbose, file.path, other.path)
+
+    # First try and prefer "zipinfo"...
+    if zipinfo is not None:
+        # ... but if "zipinfo -v" indicates we have differences in .zip
+        # directory "extra fields", add a comment and prefer that output.
+        if re.search(r"[-+]  The central-directory extra field contains:", verbose.unified_diff):
+            verbose.add_comment(
+                "Differences in extra fields detected; using output from zipinfo -v"
+            )
+            return [verbose]
+
+        return [zipinfo]
+
+    # If none of this was detected, fallback to "zipinfo -v"...
+    if verbose is not None:
+        return [verbose]
+
+    # ... and, finally, "bsdtar -tvz"
+    bsdtar = Difference.from_operation(BsdtarVerbose, file.path, other.path)
+    if bsdtar is not None:
+        return [bsdtar]
 
     return []
 


=====================================
tests/comparators/test_zip.py
=====================================
@@ -21,7 +21,7 @@ import pytest
 
 from diffoscope.comparators.zip import ZipFile, MozillaZipFile, JmodJavaModule
 
-from ..utils.data import load_fixture, get_data
+from ..utils.data import load_fixture, assert_diff
 from ..utils.tools import skip_unless_tools_exist
 from ..utils.nonexisting import assert_non_existing
 
@@ -60,22 +60,19 @@ def differences2(zip1, zip3):
 
 @skip_unless_tools_exist("zipinfo")
 def test_metadata(differences):
-    expected_diff = get_data("zip_zipinfo_expected_diff")
-    assert differences[0].unified_diff == expected_diff
+    assert_diff(differences[0], "zip_zipinfo_expected_diff")
 
 
 @skip_unless_tools_exist("zipinfo")
 def test_compressed_files(differences):
     assert differences[1].source1 == "dir/text"
     assert differences[1].source2 == "dir/text"
-    expected_diff = get_data("text_ascii_expected_diff")
-    assert differences[1].unified_diff == expected_diff
+    assert_diff(differences[1], "text_ascii_expected_diff")
 
 
 @skip_unless_tools_exist("zipinfo", "bsdtar")
 def test_extra_fields(differences2):
-    expected_diff = get_data("zip_bsdtar_expected_diff")
-    assert differences2[0].unified_diff == expected_diff
+    assert_diff(differences2[0], "zip_bsdtar_expected_diff")
 
 
 @skip_unless_tools_exist("zipinfo")
@@ -99,16 +96,15 @@ def mozzip_differences(mozzip1, mozzip2):
 
 @skip_unless_tools_exist("zipinfo")
 def test_mozzip_metadata(mozzip_differences, mozzip1, mozzip2):
-    expected_diff = get_data("mozzip_zipinfo_expected_diff")
-    assert mozzip_differences[0].unified_diff == expected_diff
+    assert_diff(mozzip_differences[0], "mozzip_zipinfo_expected_diff")
+
 
 
 @skip_unless_tools_exist("zipinfo")
 def test_mozzip_compressed_files(mozzip_differences):
     assert mozzip_differences[-1].source1 == "dir/text"
     assert mozzip_differences[-1].source2 == "dir/text"
-    expected_diff = get_data("text_ascii_expected_diff")
-    assert mozzip_differences[-1].unified_diff == expected_diff
+    assert_diff(mozzip_differences[-1], "text_ascii_expected_diff")
 
 
 @skip_unless_tools_exist("zipinfo")
@@ -132,8 +128,7 @@ def jmod_differences(jmod1, jmod2):
 
 @skip_unless_tools_exist("zipinfo")
 def test_jmod_metadata(jmod_differences, jmod1, jmod2):
-    expected_diff = get_data("jmod_zipinfo_expected_diff")
-    assert jmod_differences[0].unified_diff == expected_diff
+    assert_diff(jmod_differences[0], "jmod_zipinfo_expected_diff")
 
 
 def test_encrypted(encrypted_zip1, encrypted_zip2):
@@ -148,5 +143,4 @@ def comment_differences(test_comment1, test_comment2):
 
 @skip_unless_tools_exist("zipnote")
 def test_commented(comment_differences):
-    expected_diff = get_data("comment_zipinfo_expected_diff")
-    assert comment_differences[1].unified_diff == expected_diff
+    assert_diff(comment_differences[1], "comment_zipinfo_expected_diff")



View it on GitLab: https://salsa.debian.org/reproducible-builds/diffoscope/-/compare/3f01ab7cd6da03c720053404ee528a91ce9f71dd...e4ae650e4334754f0468ca6639744708c95ecab9

-- 
View it on GitLab: https://salsa.debian.org/reproducible-builds/diffoscope/-/compare/3f01ab7cd6da03c720053404ee528a91ce9f71dd...e4ae650e4334754f0468ca6639744708c95ecab9
You're receiving this email because of your account on salsa.debian.org.


-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.reproducible-builds.org/pipermail/rb-commits/attachments/20220517/fa2b5d6c/attachment.htm>