Repository: zarr-developers/zarr-python

Issue:
Cannot create FSStore for unlistable http URLs

#### Minimal, reproducible code sample, a copy-pastable example if possible

```python
import zarr
import fsspec

# public
url = 'https://g-c70289.0da32.08cc.data.globus.org/pyqg_ensemble/001.zarr'

# this errors
store = zarr.storage.FSStore(url)

# because of this
fs = fsspec.implementations.http.HTTPFileSystem()
fs.isdir(url) # -> False

# however, there is consolidated metadata
data = fs.get_mapper(url)['.zmetadata']
```

#### Problem description

We often store Zarr on HTTP servers that don't support directory listing. This works because Zarr consolidated metadata tells us everything we need to know about the data available, so no listing is required.

However, Zarr won't let me create such a Store, as shown in the example above.

There needs to be a workaround to allow unlistable http stores with consolidated metadata. 

#### Version and installation information

Please provide the following:

* zarr 2.11.0
* fsspec 2022.02.0

This came up in https://github.com/pangeo-forge/pangeo-forge-recipes/issues/222


Discussion:
codecov[bot]: # [Codecov](https://codecov.io/gh/zarr-developers/zarr-python/pull/911?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None) Report
> Merging [#911](https://codecov.io/gh/zarr-developers/zarr-python/pull/911?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None) (ded2d60) into [master](https://codecov.io/gh/zarr-developers/zarr-python/commit/de7858ab9d70b868cf58eaf14a9c4eab16ecb263?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None) (de7858a) will **increase** coverage by `0.00%`.
> The diff coverage is `100.00%`.

```diff
@@           Coverage Diff           @@
##           master     #911   +/-   ##
=======================================
  Coverage   99.94%   99.94%           
=======================================
  Files          32       32           
  Lines       11216    11234   +18     
=======================================
+ Hits        11210    11228   +18     
  Misses          6        6           
```

| [Impacted Files](https://codecov.io/gh/zarr-developers/zarr-python/pull/911?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None) | Coverage Δ | |
|---|---|---|
| [zarr/storage.py](https://codecov.io/gh/zarr-developers/zarr-python/pull/911/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None#diff-emFyci9zdG9yYWdlLnB5) | `100.00% <100.00%> (ø)` | |
| [zarr/tests/test\_storage.py](https://codecov.io/gh/zarr-developers/zarr-python/pull/911/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None#diff-emFyci90ZXN0cy90ZXN0X3N0b3JhZ2UucHk=) | `100.00% <100.00%> (ø)` | |

rabernat: I would like to revive and move forward this PR. Martin, you gave a few helpful comments and ideas above. Would you mind turning these into a more formal review in terms of what you think is _needed_ vs _interesting possibility_.

What we have here is a tightly scoped and tested PR to unblock a rather serious upstream problem. If there is no major technical debt incurred by this PR, could we consider merging as is (modulo doc updates from checklist) rather than expanding scope? If not, please let us know what else would be required.

Also pinging @grlee77 for potential interaction with ongoing refactor of storage classes (#600).
grlee77: > Also pinging @grlee77 and @joshmoore for potential interaction with ongoing refactor of storage classes (#789).

Thanks @rabernat, I commented above. If the `__init__` signature does not change then this should be pretty easy to merge into the work there.
joshmoore: Thanks, @grlee77. From my side, that was the only concern in terms of impact on existing work, @rabernat. @martindurant's point about the general API "burden" seems to be outstanding. Is it that a concrete alternative that doesn't break the API needs proposing from your side, @rabernat?

A side note that testing is currently blocked due to GHA weirdness. @jakirkham and I will try to get that sorted ASAP.
pep8speaks: Hello @rabernat! Thanks for updating this PR. We checked the lines you've touched for [PEP 8](https://www.python.org/dev/peps/pep-0008) issues, and found:

* In the file [`zarr/storage.py`](https://github.com/zarr-developers/zarr-python/blob/48b2bdaa3802431fec27c6668782bdb5ffb03ad6/zarr/storage.py):

> [Line 1329:101](https://github.com/zarr-developers/zarr-python/blob/48b2bdaa3802431fec27c6668782bdb5ffb03ad6/zarr/storage.py#L1329): [E501](https://duckduckgo.com/?q=pep8%20E501) line too long (102 > 100 characters)




rabernat: I feel confused about how to move forward with this PR in terms of how to update the changelog (see https://github.com/zarr-developers/zarr-python/pull/911#discussion_r848818092). Please help me 🙏. I would love to see this merged soon as I believe it is finished. It is not starting to accumulate conflicts.
joshmoore: Sorry for having left you hanging. I think either will be fine @rabernat. Either just add your item to unreleased or copy over from the 2 11 branch. Either way feel free to get this in and I will double check before the next release. (Of whatever branch)
joshmoore: It looks like the semantics of the `update()` method have changed with the refactoring for v3:

```
pytest zarr/tests/test_convenience.py -svx -k2

...snip...

>           store_to_open.update(store)  # copy original store to new unlistable store

zarr/tests/test_convenience.py:305:
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _

self = <zarr.storage.FSStore object at 0x1ba010970>, other = '/var/folders/z5/txc_jj6x5l5cm81r56ck1n9c0000gn/T/tmp0gy4ji7i', kwds = {}

    def update(self, other=(), /, **kwds):
        ''' D.update([E, ]**F) -> None.  Update D from mapping/iterable E and F.
            If E present and has a .keys() method, does:     for k in E: D[k] = E[k]
            If E present and lacks .keys() method, does:     for (k, v) in E: D[k] = v
            In either case, this is followed by: for k, v in F.items(): D[k] = v
        '''
        if isinstance(other, Mapping):
            for key in other:
                self[key] = other[key]
        elif hasattr(other, "keys"):
            for key in other.keys():
                self[key] = other[key]
        else:
>           for key, value in other:
E           ValueError: not enough values to unpack (expected 2, got 1)
```


That doesn't appear to be related to the type hierarchy:
```
2.11.2: FSStore.__mro__: (<class 'zarr.storage.FSStore'>, <class 'zarr._storage.store.Store'>, <class 'zarr._storage.store.BaseStore'>, <class 'collections.abc.MutableMapping'>, <class 'collections.abc.Mapping'>, <class 'collections.abc.Collection'>, <class 'collections.abc.Sized'>, <class 'collections.abc.Iterable'>, <class 'collections.abc.Container'>, <class 'object'>)
2.12.x: FSStore.__mro__: (<class 'zarr.storage.FSStore'>, <class 'zarr._storage.store.Store'>, <class 'zarr._storage.store.BaseStore'>, <class 'collections.abc.MutableMapping'>, <class 'collections.abc.Mapping'>, <class 'collections.abc.Collection'>, <class 'collections.abc.Sized'>, <class 'collections.abc.Iterable'>, <class 'collections.abc.Container'>, <class 'object'>)
```
Still digging in case anyone has an idea (cc: @grlee77)

Patch:
diff --git a/docs/release.rst b/docs/release.rst
index 7a5cf51d..cd9ebbd7 100644
--- a/docs/release.rst
+++ b/docs/release.rst
@@ -9,6 +9,27 @@ Unreleased
 Bug fixes
 ~~~~~~~~~
 
+* Fix bug that made it impossible to create an ``FSStore`` on unlistable filesystems
+  (e.g. some HTTP servers).
+  By :user:`Ryan Abernathey <rabernat>`; :issue:`993`.
+
+Enhancements
+~~~~~~~~~~~~
+
+* **Create FSStore from an existing fsspec filesystem**. If you have created
+  an fsspec filesystem outside of Zarr, you can now pass it as a keyword
+  argument to ``FSStore``.
+  By :user:`Ryan Abernathey <rabernat>`.
+
+
+.. _release_2.11.3:
+
+2.11.3
+------
+
+Bug fixes
+~~~~~~~~~
+
 * Changes the default value of ``write_empty_chunks`` to ``True`` to prevent
   unanticipated data losses when the data types do not have a proper default
   value when empty chunks are read back in.
@@ -322,7 +343,7 @@ Bug fixes
 
 * FSStore: default to normalize_keys=False
   By :user:`Josh Moore <joshmoore>`; :issue:`755`.
-* ABSStore: compatibility with ``azure.storage.python>=12`` 
+* ABSStore: compatibility with ``azure.storage.python>=12``
   By :user:`Tom Augspurger <tomaugspurger>`; :issue:`618`
 
 
@@ -487,7 +508,7 @@ This release will be the last to support Python 3.5, next version of Zarr will b
 
 * `DirectoryStore` now uses `os.scandir`, which should make listing large store
   faster, :issue:`563`
-  
+
 * Remove a few remaining Python 2-isms.
   By :user:`Poruri Sai Rahul <rahulporuri>`; :issue:`393`.
 
@@ -507,7 +528,7 @@ This release will be the last to support Python 3.5, next version of Zarr will b
   ``zarr.errors``  have been replaced by ``ValueError`` subclasses. The corresponding
   ``err_*`` function have been removed. :issue:`590`, :issue:`614`)
 
-* Improve consistency of terminology regarding arrays and datasets in the 
+* Improve consistency of terminology regarding arrays and datasets in the
   documentation.
   By :user:`Josh Moore <joshmoore>`; :issue:`571`.
 
diff --git a/docs/tutorial.rst b/docs/tutorial.rst
index 53ddddb0..b40896c7 100644
--- a/docs/tutorial.rst
+++ b/docs/tutorial.rst
@@ -758,7 +758,7 @@ databases. The :class:`zarr.storage.RedisStore` class interfaces `Redis <https:/
 (an in memory data structure store), and the :class:`zarr.storage.MongoDB` class interfaces
 with `MongoDB <https://www.mongodb.com/>`_ (an object oriented NoSQL database). These stores
 respectively require the `redis-py <https://redis-py.readthedocs.io>`_ and
-`pymongo <https://api.mongodb.com/python/current/>`_ packages to be installed. 
+`pymongo <https://api.mongodb.com/python/current/>`_ packages to be installed.
 
 For compatibility with the `N5 <https://github.com/saalfeldlab/n5>`_ data format, Zarr also provides
 an N5 backend (this is currently an experimental feature). Similar to the zip storage class, an
@@ -897,6 +897,18 @@ The second invocation here will be much faster. Note that the ``storage_options`
 have become more complex here, to account for the two parts of the supplied
 URL.
 
+It is also possible to initialize the filesytem outside of Zarr and then pass
+it through. This requires creating an :class:`zarr.storage.FSStore` object
+explicitly. For example::
+
+    >>> import s3fs  * doctest: +SKIP
+    >>> fs = s3fs.S3FileSystem(anon=True)  # doctest: +SKIP
+    >>> store = zarr.storage.FSStore('/zarr-demo/store', fs=fs)  # doctest: +SKIP
+    >>> g = zarr.open_group(store)  # doctest: +SKIP
+
+This is useful in cases where you want to also use the same fsspec filesystem object
+separately from Zarr.
+
 .. _fsspec: https://filesystem-spec.readthedocs.io/en/latest/
 
 .. _supported by fsspec: https://filesystem-spec.readthedocs.io/en/latest/api.html#built-in-implementations
@@ -1306,18 +1318,18 @@ filters (e.g., byte-shuffle) have been applied.
 
 Empty chunks
 ~~~~~~~~~~~~
- 
+
 As of version 2.11, it is possible to configure how Zarr handles the storage of
 chunks that are "empty" (i.e., every element in the chunk is equal to the array's fill value).
-When creating an array with ``write_empty_chunks=False``, 
+When creating an array with ``write_empty_chunks=False``,
 Zarr will check whether a chunk is empty before compression and storage. If a chunk is empty,
-then Zarr does not store it, and instead deletes the chunk from storage 
-if the chunk had been previously stored. 
+then Zarr does not store it, and instead deletes the chunk from storage
+if the chunk had been previously stored.
 
-This optimization prevents storing redundant objects and can speed up reads, but the cost is 
-added computation during array writes, since the contents of 
-each chunk must be compared to the fill value, and these advantages are contingent on the content of the array. 
-If you know that your data will form chunks that are almost always non-empty, then there is no advantage to the optimization described above. 
+This optimization prevents storing redundant objects and can speed up reads, but the cost is
+added computation during array writes, since the contents of
+each chunk must be compared to the fill value, and these advantages are contingent on the content of the array.
+If you know that your data will form chunks that are almost always non-empty, then there is no advantage to the optimization described above.
 In this case, creating an array with ``write_empty_chunks=True`` (the default) will instruct Zarr to write every chunk without checking for emptiness.
 
 The following example illustrates the effect of the ``write_empty_chunks`` flag on
@@ -1329,7 +1341,7 @@ the time required to write an array with different values.::
     >>> from tempfile import TemporaryDirectory
     >>> def timed_write(write_empty_chunks):
     ...     """
-    ...     Measure the time required and number of objects created when writing 
+    ...     Measure the time required and number of objects created when writing
     ...     to a Zarr array with random ints or fill value.
     ...     """
     ...     chunks = (8192,)
@@ -1368,8 +1380,8 @@ the time required to write an array with different values.::
             Random Data: 0.1359s, 1024 objects stored
             Empty Data: 0.0301s, 0 objects stored
 
-In this example, writing random data is slightly slower with ``write_empty_chunks=True``, 
-but writing empty data is substantially faster and generates far fewer objects in storage.   
+In this example, writing random data is slightly slower with ``write_empty_chunks=True``,
+but writing empty data is substantially faster and generates far fewer objects in storage.
 
 .. _tutorial_rechunking:
 
diff --git a/zarr/storage.py b/zarr/storage.py
index e8a65147..48b6f049 100644
--- a/zarr/storage.py
+++ b/zarr/storage.py
@@ -1262,8 +1262,9 @@ class FSStore(Store):
     Parameters
     ----------
     url : str
-        The destination to map. Should include protocol and path,
-        like "s3://bucket/root"
+        The destination to map. If no fs is provided, should include protocol
+        and path, like "s3://bucket/root". If an fs is provided, can be a path
+        within that filesystem, like "bucket/root"
     normalize_keys : bool
     key_separator : str
         public API for accessing dimension_separator. Never `None`
@@ -1275,7 +1276,19 @@ class FSStore(Store):
         as a missing key
     dimension_separator : {'.', '/'}, optional
         Separator placed between the dimensions of a chunk.
-    storage_options : passed to the fsspec implementation
+    fs : fsspec.spec.AbstractFileSystem, optional
+        An existing filesystem to use for the store.
+    check : bool, optional
+        If True, performs a touch at the root location, to check for write access.
+        Passed to `fsspec.mapping.FSMap` constructor.
+    create : bool, optional
+        If True, performs a mkdir at the rool location.
+        Passed to `fsspec.mapping.FSMap` constructor.
+    missing_exceptions : sequence of Exceptions, optional
+        Exceptions classes to associate with missing files.
+        Passed to `fsspec.mapping.FSMap` constructor.
+    storage_options : passed to the fsspec implementation. Cannot be used
+        together with fs.
     """
     _array_meta_key = array_meta_key
     _group_meta_key = group_meta_key
@@ -1285,18 +1298,37 @@ class FSStore(Store):
                  mode='w',
                  exceptions=(KeyError, PermissionError, IOError),
                  dimension_separator=None,
+                 fs=None,
+                 check=False,
+                 create=False,
+                 missing_exceptions=None,
                  **storage_options):
         import fsspec
-        self.normalize_keys = normalize_keys
 
-        protocol, _ = fsspec.core.split_protocol(url)
-        # set auto_mkdir to True for local file system
-        if protocol in (None, "file") and not storage_options.get("auto_mkdir"):
-            storage_options["auto_mkdir"] = True
+        mapper_options = {"check": check, "create": create}
+        # https://github.com/zarr-developers/zarr-python/pull/911#discussion_r841926292
+        # Some fsspec implementations don't accept missing_exceptions.
+        # This is a workaround to avoid passing it in the most common scenarios.
+        # Remove this and add missing_exceptions to mapper_options when fsspec is released.
+        if missing_exceptions is not None:
+            mapper_options["missing_exceptions"] = missing_exceptions  # pragma: no cover
+
+        if fs is None:
+            protocol, _ = fsspec.core.split_protocol(url)
+            # set auto_mkdir to True for local file system
+            if protocol in (None, "file") and not storage_options.get("auto_mkdir"):
+                storage_options["auto_mkdir"] = True
+            self.map = fsspec.get_mapper(url, **{**mapper_options, **storage_options})
+            self.fs = self.map.fs  # for direct operations
+            self.path = self.fs._strip_protocol(url)
+        else:
+            if storage_options:
+                raise ValueError("Cannot specify both fs and storage_options")
+            self.fs = fs
+            self.path = self.fs._strip_protocol(url)
+            self.map = self.fs.get_mapper(self.path, **mapper_options)
 
-        self.map = fsspec.get_mapper(url, **storage_options)
-        self.fs = self.map.fs  # for direct operations
-        self.path = self.fs._strip_protocol(url)
+        self.normalize_keys = normalize_keys
         self.mode = mode
         self.exceptions = exceptions
         # For backwards compatibility. Guaranteed to be non-None
@@ -1308,8 +1340,6 @@ class FSStore(Store):
 
         # Pass attributes to array creation
         self._dimension_separator = dimension_separator
-        if self.fs.exists(self.path) and not self.fs.isdir(self.path):
-            raise FSPathExistNotDir(url)
 
     def _default_key_separator(self):
         if self.key_separator is None:


Test patch:
diff --git a/zarr/tests/test_convenience.py b/zarr/tests/test_convenience.py
index 097512a2..ce8f03d0 100644
--- a/zarr/tests/test_convenience.py
+++ b/zarr/tests/test_convenience.py
@@ -26,6 +26,7 @@ from zarr.errors import CopyError
 from zarr.hierarchy import Group, group
 from zarr.storage import (
     ConsolidatedMetadataStore,
+    FSStore,
     KVStore,
     MemoryStore,
     atexit_rmtree,
@@ -205,9 +206,18 @@ def test_tree(zarr_version):
 
 
 @pytest.mark.parametrize('zarr_version', [2, 3])
-@pytest.mark.parametrize('with_chunk_store', [False, True], ids=['default', 'with_chunk_store'])
 @pytest.mark.parametrize('stores_from_path', [False, True])
-def test_consolidate_metadata(with_chunk_store, zarr_version, stores_from_path):
+@pytest.mark.parametrize(
+    'with_chunk_store,listable',
+    [(False, True), (True, True), (False, False)],
+    ids=['default-listable', 'with_chunk_store-listable', 'default-unlistable']
+)
+def test_consolidate_metadata(with_chunk_store,
+                              zarr_version,
+                              listable,
+                              monkeypatch,
+                              stores_from_path):
+
     # setup initial data
     if stores_from_path:
         store = tempfile.mkdtemp()
@@ -228,6 +238,10 @@ def test_consolidate_metadata(with_chunk_store, zarr_version, stores_from_path):
         version_kwarg = {}
     path = 'dataset' if zarr_version == 3 else None
     z = group(store, chunk_store=chunk_store, path=path, **version_kwarg)
+
+    # Reload the actual store implementation in case str
+    store_to_copy = z.store
+
     z.create_group('g1')
     g2 = z.create_group('g2')
     g2.attrs['hello'] = 'world'
@@ -278,14 +292,36 @@ def test_consolidate_metadata(with_chunk_store, zarr_version, stores_from_path):
         for key in meta_keys:
             del store[key]
 
+    # https://github.com/zarr-developers/zarr-python/issues/993
+    # Make sure we can still open consolidated on an unlistable store:
+    if not listable:
+        fs_memory = pytest.importorskip("fsspec.implementations.memory")
+        monkeypatch.setattr(fs_memory.MemoryFileSystem, "isdir", lambda x, y: False)
+        monkeypatch.delattr(fs_memory.MemoryFileSystem, "ls")
+        fs = fs_memory.MemoryFileSystem()
+        if zarr_version == 2:
+            store_to_open = FSStore("", fs=fs)
+        else:
+            store_to_open = FSStoreV3("", fs=fs)
+
+        # copy original store to new unlistable store
+        store_to_open.update(store_to_copy)
+
+    else:
+        store_to_open = store
+
     # open consolidated
-    z2 = open_consolidated(store, chunk_store=chunk_store, path=path, **version_kwarg)
+    z2 = open_consolidated(store_to_open, chunk_store=chunk_store, path=path, **version_kwarg)
     assert ['g1', 'g2'] == list(z2)
     assert 'world' == z2.g2.attrs['hello']
     assert 1 == z2.g2.arr.attrs['data']
     assert (z2.g2.arr[:] == 1.0).all()
     assert 16 == z2.g2.arr.nchunks
-    assert 16 == z2.g2.arr.nchunks_initialized
+    if listable:
+        assert 16 == z2.g2.arr.nchunks_initialized
+    else:
+        with pytest.raises(NotImplementedError):
+            _ = z2.g2.arr.nchunks_initialized
 
     if stores_from_path:
         # path string is note a BaseStore subclass so cannot be used to
diff --git a/zarr/tests/test_core.py b/zarr/tests/test_core.py
index ebb24a07..2212b035 100644
--- a/zarr/tests/test_core.py
+++ b/zarr/tests/test_core.py
@@ -2495,6 +2495,13 @@ class TestArrayWithStoreCache(TestArray):
         pass
 
 
+fsspec_mapper_kwargs = {
+    "check": True,
+    "create": True,
+    "missing_exceptions": None
+}
+
+
 @pytest.mark.skipif(have_fsspec is False, reason="needs fsspec")
 class TestArrayWithFSStore(TestArray):
     @staticmethod
@@ -2502,7 +2509,35 @@ class TestArrayWithFSStore(TestArray):
         path = mkdtemp()
         atexit.register(shutil.rmtree, path)
         key_separator = kwargs.pop('key_separator', ".")
-        store = FSStore(path, key_separator=key_separator, auto_mkdir=True)
+        store = FSStore(path, key_separator=key_separator, auto_mkdir=True, **fsspec_mapper_kwargs)
+        cache_metadata = kwargs.pop('cache_metadata', True)
+        cache_attrs = kwargs.pop('cache_attrs', True)
+        write_empty_chunks = kwargs.pop('write_empty_chunks', True)
+        kwargs.setdefault('compressor', Blosc())
+        init_array(store, **kwargs)
+        return Array(store, read_only=read_only, cache_metadata=cache_metadata,
+                     cache_attrs=cache_attrs, write_empty_chunks=write_empty_chunks)
+
+    def expected(self):
+        return [
+           "ab753fc81df0878589535ca9bad2816ba88d91bc",
+           "c16261446f9436b1e9f962e57ce3e8f6074abe8a",
+           "c2ef3b2fb2bc9dcace99cd6dad1a7b66cc1ea058",
+           "6e52f95ac15b164a8e96843a230fcee0e610729b",
+           "091fa99bc60706095c9ce30b56ce2503e0223f56",
+        ]
+
+
+@pytest.mark.skipif(have_fsspec is False, reason="needs fsspec")
+class TestArrayWithFSStoreFromFilesystem(TestArray):
+    @staticmethod
+    def create_array(read_only=False, **kwargs):
+        from fsspec.implementations.local import LocalFileSystem
+        fs = LocalFileSystem(auto_mkdir=True)
+        path = mkdtemp()
+        atexit.register(shutil.rmtree, path)
+        key_separator = kwargs.pop('key_separator', ".")
+        store = FSStore(path, fs=fs, key_separator=key_separator, **fsspec_mapper_kwargs)
         cache_metadata = kwargs.pop('cache_metadata', True)
         cache_attrs = kwargs.pop('cache_attrs', True)
         write_empty_chunks = kwargs.pop('write_empty_chunks', True)
@@ -3148,7 +3183,40 @@ class TestArrayWithFSStoreV3(TestArrayWithPathV3, TestArrayWithFSStore):
         path = mkdtemp()
         atexit.register(shutil.rmtree, path)
         key_separator = kwargs.pop('key_separator', ".")
-        store = FSStoreV3(path, key_separator=key_separator, auto_mkdir=True)
+        store = FSStoreV3(
+            path,
+            key_separator=key_separator,
+            auto_mkdir=True,
+            **fsspec_mapper_kwargs
+        )
+        cache_metadata = kwargs.pop('cache_metadata', True)
+        cache_attrs = kwargs.pop('cache_attrs', True)
+        write_empty_chunks = kwargs.pop('write_empty_chunks', True)
+        kwargs.setdefault('compressor', Blosc())
+        init_array(store, path=array_path, **kwargs)
+        return Array(store, path=array_path, read_only=read_only, cache_metadata=cache_metadata,
+                     cache_attrs=cache_attrs, write_empty_chunks=write_empty_chunks)
+
+    def expected(self):
+        return [
+            "1509abec4285494b61cd3e8d21f44adc3cf8ddf6",
+            "7cfb82ec88f7ecb7ab20ae3cb169736bc76332b8",
+            "b663857bb89a8ab648390454954a9cdd453aa24b",
+            "21e90fa927d09cbaf0e3b773130e2dc05d18ff9b",
+            "e8c1fdd18b5c2ee050b59d0c8c95d07db642459c",
+        ]
+
+
+@pytest.mark.skipif(have_fsspec is False, reason="needs fsspec")
+class TestArrayWithFSStoreV3FromFilesystem(TestArrayWithPathV3, TestArrayWithFSStore):
+    @staticmethod
+    def create_array(array_path='arr1', read_only=False, **kwargs):
+        from fsspec.implementations.local import LocalFileSystem
+        fs = LocalFileSystem(auto_mkdir=True)
+        path = mkdtemp()
+        atexit.register(shutil.rmtree, path)
+        key_separator = kwargs.pop('key_separator', ".")
+        store = FSStoreV3(path, fs=fs, key_separator=key_separator, **fsspec_mapper_kwargs)
         cache_metadata = kwargs.pop('cache_metadata', True)
         cache_attrs = kwargs.pop('cache_attrs', True)
         write_empty_chunks = kwargs.pop('write_empty_chunks', True)
diff --git a/zarr/tests/test_storage.py b/zarr/tests/test_storage.py
index 9fb869d5..abddb696 100644
--- a/zarr/tests/test_storage.py
+++ b/zarr/tests/test_storage.py
@@ -1249,10 +1249,6 @@ class TestFSStore(StoreTests):
 
         assert store[self.root + 'foo'] == b"bar"
 
-        filepath = os.path.join(path, self.root + "foo")
-        with pytest.raises(ValueError):
-            self.create_store(path=filepath, mode='r')
-
     def test_eq(self):
         store1 = self.create_store(path="anypath")
         store2 = self.create_store(path="anypath")
@@ -1339,6 +1335,35 @@ class TestFSStoreWithKeySeparator(StoreTests):
             key_separator=key_separator)
 
 
+@pytest.mark.skipif(have_fsspec is False, reason="needs fsspec")
+class TestFSStoreFromFilesystem(StoreTests):
+
+    def create_store(self, normalize_keys=False,
+                     dimension_separator=".",
+                     path=None,
+                     **kwargs):
+        import fsspec
+        fs = fsspec.filesystem("file")
+
+        if path is None:
+            path = tempfile.mkdtemp()
+            atexit.register(atexit_rmtree, path)
+
+        with pytest.raises(ValueError):
+            # can't specify storage_options when passing an
+            # existing fs object
+            _ = FSStore(path, fs=fs, auto_mkdir=True)
+
+        store = FSStore(
+            path,
+            normalize_keys=normalize_keys,
+            dimension_separator=dimension_separator,
+            fs=fs,
+            **kwargs)
+
+        return store
+
+
 @pytest.fixture()
 def s3(request):
     # writable local S3 system