From f11302b98ec40a27f73b0c87a9b88fff808568ff Mon Sep 17 00:00:00 2001
From: Dima Gerasimov <karlicoss@gmail.com>
Date: Thu, 14 Apr 2022 09:31:05 +0100
Subject: [PATCH] more fixes

---
 my/core/kompress.py         | 14 ++++++++++++--
 tests/core/test_kompress.py | 22 ++++++++++++----------
 2 files changed, 24 insertions(+), 12 deletions(-)

diff --git a/my/core/kompress.py b/my/core/kompress.py
index c56de73..3037c9d 100644
--- a/my/core/kompress.py
+++ b/my/core/kompress.py
@@ -5,6 +5,7 @@ from __future__ import annotations
 
 import pathlib
 from pathlib import Path
+import sys
 from typing import Union, IO, Sequence
 import io
 
@@ -120,7 +121,16 @@ def kexists(path: PathIsh, subpath: str) -> bool:
 
 
 import zipfile
-class ZipPath(zipfile.Path):
+if sys.version_info[:2] >= (3, 8):
+    ZipPathBase = zipfile.Path
+else:
+    # meh... it's not available on 3.7
+    ZipPathBase = object
+
+
+class ZipPath(ZipPathBase):
+    # NOTE: is_dir/is_file might not behave as expected, the base class checks it only based on the slash in path
+
     # seems that at/root are not exposed in the docs, so might be an implementation detail
     at: str
     root: zipfile.ZipFile
@@ -159,4 +169,4 @@ class ZipPath(zipfile.Path):
         # hmm, super class doesn't seem to treat as equals unless they are the same object
         if not isinstance(other, ZipPath):
             return False
-        return self.filename == other.filename and self.at == other.at
+        return self.filename == other.filename and Path(self.at) == Path(other.at)
diff --git a/tests/core/test_kompress.py b/tests/core/test_kompress.py
index 9819f1d..3561444 100644
--- a/tests/core/test_kompress.py
+++ b/tests/core/test_kompress.py
@@ -1,5 +1,6 @@
-from pathlib import Path
 import lzma
+from pathlib import Path
+import sys
 import zipfile
 
 from my.core.kompress import kopen, kexists, CPath
@@ -49,6 +50,10 @@ def prepare(tmp_path: Path):
         pass
 
 
+@pytest.mark.skipif(
+    sys.version_info[:2] < (3, 8),
+    reason=f"ZipFile.Path is only available since 3.8",
+)
 def test_zippath() -> None:
     from my.core.kompress import ZipPath
     target = structure_data / 'gdpr_export.zip'
@@ -58,7 +63,7 @@ def test_zippath() -> None:
 
     # magic! convenient to make third party libraries agnostic of ZipPath
     assert isinstance(zp, Path)
-    # FIXME maybe change str? since it's a bit misleading...
+    # TODO maybe change __str__/__repr__? since it's a bit misleading:
     # Path('/code/hpi/tests/core/structure_data/gdpr_export.zip', 'gdpr_export/')
 
     assert ZipPath(target) == ZipPath(target)
@@ -68,6 +73,7 @@ def test_zippath() -> None:
     assert (zp / 'gdpr_export/comments').exists()
     # check str constructor just in case
     assert (ZipPath(str(target)) / 'gdpr_export/comments').exists()
+    assert not (ZipPath(str(target)) / 'whatever').exists()
 
     matched = list(zp.rglob('*'))
     assert len(matched) > 0
@@ -85,15 +91,13 @@ def test_zippath() -> None:
     ], rpaths
 
 
-    # TODO hmm this doesn't work atm, although Path does
+    # TODO hmm this doesn't work atm, wheras Path does
     # not sure if it should be defensive or something...
     # ZipPath('doesnotexist')
     # same for this one
     # assert ZipPath(Path('test'), 'whatever').absolute() == ZipPath(Path('test').absolute(), 'whatever')
-    #
-    # FIXME vvv this should really work...
-    # assert (ZipPath(target) / 'gdpr_export/comments').exists()
-    # assert ZipPath(target, 'gdpr_export/comments').exists()
+
+    assert (ZipPath(target) / 'gdpr_export/comments').exists()
 
     jsons = [str(p.relative_to(zp / 'gdpr_export')) for p in zp.rglob('*.json')]
     assert jsons == [
@@ -101,6 +105,4 @@ def test_zippath() -> None:
         'profile/settings.json',
     ]
 
-    # FIXME uhh.. this doesn't work? without slash probably should...
-    # assert list(zp.rglob('mes*')) == [ZipPath(target, 'gdpr_export/messages')]
-    assert list(zp.rglob('mes*')) == [ZipPath(target, 'gdpr_export/messages/')]
+    assert list(zp.rglob('mes*')) == [ZipPath(target, 'gdpr_export/messages')]