work on storage test coverage

Author: alimanfoo

zarr/storage.py

@@ -274,11 +274,9 @@ def init_group(store, overwrite=False, path=None):
 def ensure_bytes(s):
     if isinstance(s, binary_type):
         return s
-    if hasattr(s, 'encode'):
-        return s.encode()
     if hasattr(s, 'tobytes'):
         return s.tobytes()
-    if PY2 and hasattr(s, 'tostring'):
+    if PY2 and hasattr(s, 'tostring'):  # pragma: no cover
         return s.tostring()
     return memoryview(s).tobytes()
 
@@ -295,7 +293,7 @@ def _dict_store_keys(d, prefix='', cls=dict):
 
 def buffersize(v):
     from array import array as _stdlib_array
-    if PY2 and isinstance(v, _stdlib_array):
+    if PY2 and isinstance(v, _stdlib_array):  # pragma: no cover
         # special case array.array because does not support buffer
         # interface in PY2
         return v.buffer_info()[1] * v.itemsize
@@ -533,8 +531,13 @@ def __setitem__(self, key, value):
 
         # ensure containing directory exists
         dir_path, file_name = os.path.split(file_path)
+        if os.path.isfile(dir_path):
+            raise KeyError(key)
         if not os.path.exists(dir_path):
-            os.makedirs(dir_path)
+            try:
+                os.makedirs(dir_path)
+            except NotADirectoryError:
+                raise KeyError(key)
 
         # write to temporary file
         with tempfile.NamedTemporaryFile(mode='wb', delete=False,

zarr/tests/test_storage.py

@@ -146,12 +146,22 @@ def test_hierarchy(self):
         assert 'c/' not in store
         assert 'c/e' not in store
         assert 'c/e/' not in store
+        assert 'c/d/x' not in store
+
+        # check __getitem__
+        with assert_raises(KeyError):
+            store['c']
+        with assert_raises(KeyError):
+            store['c/e']
+        with assert_raises(KeyError):
+            store['c/d/x']
 
         # test listdir (optional)
         if hasattr(store, 'listdir'):
             eq({'a', 'b', 'c'}, set(store.listdir()))
             eq({'d', 'e'}, set(store.listdir('c')))
             eq({'f', 'g'}, set(store.listdir('c/e')))
+        # TODO further listdir tests
 
         # test getsize (optional)
         if hasattr(store, 'getsize'):
@@ -163,6 +173,7 @@ def test_hierarchy(self):
             eq(6, store.getsize('c/e'))
             eq(3, store.getsize('c/e/f'))
             eq(3, store.getsize('c/e/g'))
+        # TODO further getsize tests
 
         # test rmdir (optional)
         if hasattr(store, 'rmdir'):
@@ -339,7 +350,7 @@ def test_init_group_overwrite(self):
                  order='F')
         )
 
-        # don't overwrite (default)
+        # don't overwrite array (default)
         with assert_raises(ValueError):
             init_group(store)
 
@@ -354,6 +365,10 @@ def test_init_group_overwrite(self):
             meta = decode_group_metadata(store[group_meta_key])
             eq(ZARR_FORMAT, meta['zarr_format'])
 
+        # don't overwrite group
+        with assert_raises(ValueError):
+            init_group(store)
+
     def test_init_group_overwrite_path(self):
         # setup
         path = 'foo/bar'
@@ -398,11 +413,52 @@ def create_store(self):
         return dict()
 
 
+def setdel_hierarchy_checks(store):
+    # these tests are for stores that are aware of hierarchy levels; this
+    # behaviour is not stricly required by Zarr but these tests are included
+    # to define behaviour of DictStore and DirectoryStore classes
+
+    # check __setitem__ and __delitem__ blocked by leaf
+
+    store['a/b'] = b'aaa'
+    with assert_raises(KeyError):
+        store['a/b/c'] = b'xxx'
+    with assert_raises(KeyError):
+        del store['a/b/c']
+
+    store['d'] = b'ddd'
+    with assert_raises(KeyError):
+        store['d/e/f'] = b'xxx'
+    with assert_raises(KeyError):
+        del store['d/e/f']
+
+    # test __setitem__ overwrite level
+    store['x/y/z'] = b'xxx'
+    store['x/y'] = b'yyy'
+    eq(b'yyy', store['x/y'])
+    assert 'x/y/z' not in store
+    store['x'] = b'zzz'
+    eq(b'zzz', store['x'])
+    assert 'x/y' not in store
+
+    # test __delitem__ overwrite level
+    store['r/s/t'] = b'xxx'
+    del store['r/s']
+    assert 'r/s/t' not in store
+    store['r/s'] = b'xxx'
+    del store['r']
+    assert 'r/s' not in store
+
+
 class TestDictStore(StoreTests, unittest.TestCase):
 
     def create_store(self):
         return DictStore()
 
+    def test_setdel(self):
+        store = self.create_store()
+        setdel_hierarchy_checks(store)
+
 
 def rmtree_if_exists(path, rmtree=shutil.rmtree, isdir=os.path.isdir):
     if isdir(path):
@@ -446,6 +502,10 @@ def test_pickle_ext(self):
         store2['xxx'] = b'yyy'
         eq(b'yyy', store['xxx'])
 
+    def test_setdel(self):
+        store = self.create_store()
+        setdel_hierarchy_checks(store)
+
 
 class TestZipStore(StoreTests, unittest.TestCase):