From acc8055c6bd0f6b692a58235515f5eefc94bbe55 Mon Sep 17 00:00:00 2001 From: Yuxuan Lu Date: Mon, 7 Nov 2022 13:39:57 +0800 Subject: [PATCH 1/6] Add tests for method signatures --- fsspec/implementations/tests/conftest.py | 8 ++++++-- fsspec/implementations/tests/test_common.py | 17 +++++++++++++++++ 2 files changed, 23 insertions(+), 2 deletions(-) diff --git a/fsspec/implementations/tests/conftest.py b/fsspec/implementations/tests/conftest.py index 7dce3ee4e..021fc8081 100644 --- a/fsspec/implementations/tests/conftest.py +++ b/fsspec/implementations/tests/conftest.py @@ -2,6 +2,7 @@ import pytest +import fsspec from fsspec.implementations.arrow import ArrowFSWrapper from fsspec.implementations.local import LocalFileSystem from fsspec.implementations.memory import MemoryFileSystem @@ -24,14 +25,17 @@ class MultiProtocolFileSystem(LocalFileSystem): @pytest.fixture(scope="function") def fs(request): - pyarrow_fs = pytest.importorskip("pyarrow.fs") - FileSystem = pyarrow_fs.FileSystem if request.param == "arrow": + pyarrow_fs = pytest.importorskip("pyarrow.fs") + FileSystem = pyarrow_fs.FileSystem fs = ArrowFSWrapper(FileSystem.from_uri("file:///")[0]) return fs cls = FILESYSTEMS[request.param] return cls() +@pytest.fixture(scope="function") +def fscls(request): + return fsspec.get_filesystem_class(request.param) @pytest.fixture(scope="function") def temp_file(): diff --git a/fsspec/implementations/tests/test_common.py b/fsspec/implementations/tests/test_common.py index fe88dc373..9b6934726 100644 --- a/fsspec/implementations/tests/test_common.py +++ b/fsspec/implementations/tests/test_common.py @@ -5,6 +5,7 @@ from fsspec import AbstractFileSystem from fsspec.implementations.tests.conftest import READ_ONLY_FILESYSTEMS +from inspect import signature, isfunction @pytest.mark.parametrize("fs", ["local"], indirect=["fs"]) @@ -31,3 +32,19 @@ def test_modified(fs: AbstractFileSystem, temp_file): assert modified > created finally: fs.rm(temp_file) + +# TODO: add more filesystems +@pytest.mark.parametrize("fscls", ["file", "memory", "hdfs"], indirect=["fscls"]) +def test_signature(fscls): + abstract_fs = AbstractFileSystem + for method in dir(abstract_fs): + if isfunction(getattr(abstract_fs, method)): + print(method) + if method.startswith('_'): continue + abs_signature = signature(getattr(abstract_fs, method)) + fs_signature = signature(getattr(fscls, method)) + # assert abs_signature == fs_signature + for k in abs_signature.parameters: + if k in ['self', 'args', 'kwargs']: continue + assert abs_signature.parameters[k] == fs_signature.parameters[k] + assert False \ No newline at end of file From fe62c94e1763102d9a94c971c5536b0711cf4573 Mon Sep 17 00:00:00 2001 From: Yuxuan Lu Date: Mon, 7 Nov 2022 13:46:32 +0800 Subject: [PATCH 2/6] Add assert message --- fsspec/implementations/tests/test_common.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/fsspec/implementations/tests/test_common.py b/fsspec/implementations/tests/test_common.py index 9b6934726..a320ca91e 100644 --- a/fsspec/implementations/tests/test_common.py +++ b/fsspec/implementations/tests/test_common.py @@ -39,12 +39,12 @@ def test_signature(fscls): abstract_fs = AbstractFileSystem for method in dir(abstract_fs): if isfunction(getattr(abstract_fs, method)): - print(method) if method.startswith('_'): continue abs_signature = signature(getattr(abstract_fs, method)) fs_signature = signature(getattr(fscls, method)) # assert abs_signature == fs_signature - for k in abs_signature.parameters: - if k in ['self', 'args', 'kwargs']: continue - assert abs_signature.parameters[k] == fs_signature.parameters[k] + for k1, k2 in zip(abs_signature.parameters, fs_signature.parameters): + if k1 in ['self', 'args', 'kwargs']: continue + assert abs_signature.parameters[k1].kind == fs_signature.parameters[k2].kind, f"Paramete {k2} of {method} in {fscls.__name__} don't have the same kind with the one in base class" + assert abs_signature.parameters[k1].default == fs_signature.parameters[k2].default, f"Paramete {k2} of {method} in {fscls.__name__} don't have the same default value with the one in base class" assert False \ No newline at end of file From ce8fed8d5d88ea06a437fdc48cee5d2de8b5fe08 Mon Sep 17 00:00:00 2001 From: Yuxuan Lu Date: Mon, 7 Nov 2022 13:51:24 +0800 Subject: [PATCH 3/6] Fix format & add more tests --- fsspec/implementations/tests/conftest.py | 2 + fsspec/implementations/tests/test_common.py | 56 ++++++++++++++++++--- 2 files changed, 50 insertions(+), 8 deletions(-) diff --git a/fsspec/implementations/tests/conftest.py b/fsspec/implementations/tests/conftest.py index 021fc8081..1e0f6c1d6 100644 --- a/fsspec/implementations/tests/conftest.py +++ b/fsspec/implementations/tests/conftest.py @@ -33,10 +33,12 @@ def fs(request): cls = FILESYSTEMS[request.param] return cls() + @pytest.fixture(scope="function") def fscls(request): return fsspec.get_filesystem_class(request.param) + @pytest.fixture(scope="function") def temp_file(): with tempfile.TemporaryDirectory() as temp_dir: diff --git a/fsspec/implementations/tests/test_common.py b/fsspec/implementations/tests/test_common.py index a320ca91e..6664af9d0 100644 --- a/fsspec/implementations/tests/test_common.py +++ b/fsspec/implementations/tests/test_common.py @@ -1,11 +1,11 @@ import datetime import time +from inspect import isfunction, signature import pytest from fsspec import AbstractFileSystem from fsspec.implementations.tests.conftest import READ_ONLY_FILESYSTEMS -from inspect import signature, isfunction @pytest.mark.parametrize("fs", ["local"], indirect=["fs"]) @@ -33,18 +33,58 @@ def test_modified(fs: AbstractFileSystem, temp_file): finally: fs.rm(temp_file) -# TODO: add more filesystems -@pytest.mark.parametrize("fscls", ["file", "memory", "hdfs"], indirect=["fscls"]) + +@pytest.mark.parametrize( + "fscls", + [ + "file", + "memory", + "http", + "zip", + "tar", + "sftp", + "ssh", + "ftp", + "webhdfs", + "cached", + "blockcache", + "filecache", + "simplecache", + "dask", + "dbfs", + "github", + "git", + "smb", + "jupyter", + "libarchive", + "reference", + "hdfs", + ], + indirect=["fscls"], +) def test_signature(fscls): abstract_fs = AbstractFileSystem for method in dir(abstract_fs): if isfunction(getattr(abstract_fs, method)): - if method.startswith('_'): continue + if method.startswith("_"): + continue abs_signature = signature(getattr(abstract_fs, method)) fs_signature = signature(getattr(fscls, method)) # assert abs_signature == fs_signature for k1, k2 in zip(abs_signature.parameters, fs_signature.parameters): - if k1 in ['self', 'args', 'kwargs']: continue - assert abs_signature.parameters[k1].kind == fs_signature.parameters[k2].kind, f"Paramete {k2} of {method} in {fscls.__name__} don't have the same kind with the one in base class" - assert abs_signature.parameters[k1].default == fs_signature.parameters[k2].default, f"Paramete {k2} of {method} in {fscls.__name__} don't have the same default value with the one in base class" - assert False \ No newline at end of file + if k1 in ["self", "args", "kwargs"]: + continue + assert ( + abs_signature.parameters[k1].kind + == fs_signature.parameters[k2].kind + ), ( + f"Paramete {k2} of {method} in {fscls.__name__} " + "don't have the same kind with the one in base class" + ) + assert ( + abs_signature.parameters[k1].default + == fs_signature.parameters[k2].default + ), ( + f"Paramete {k2} of {method} in {fscls.__name__} don't" + " have the same default value with the one in base class" + ) From afda73d1d85ce3e2200ad8626050bec793a39698 Mon Sep 17 00:00:00 2001 From: Yuxuan Lu Date: Mon, 7 Nov 2022 13:54:42 +0800 Subject: [PATCH 4/6] Skip fs that isn't installed --- fsspec/implementations/tests/conftest.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/fsspec/implementations/tests/conftest.py b/fsspec/implementations/tests/conftest.py index 1e0f6c1d6..9762516ee 100644 --- a/fsspec/implementations/tests/conftest.py +++ b/fsspec/implementations/tests/conftest.py @@ -36,7 +36,10 @@ def fs(request): @pytest.fixture(scope="function") def fscls(request): - return fsspec.get_filesystem_class(request.param) + try: + return fsspec.get_filesystem_class(request.param) + except ImportError: + pytest.skip(f"filesystem {request.param} not installed") @pytest.fixture(scope="function") From bd45955497a97d90005560950d592da8f8dd3266 Mon Sep 17 00:00:00 2001 From: Yuxuan Lu Date: Tue, 8 Nov 2022 23:42:57 +0800 Subject: [PATCH 5/6] Update tests --- fsspec/implementations/tests/test_common.py | 111 +++++++++++++++----- 1 file changed, 86 insertions(+), 25 deletions(-) diff --git a/fsspec/implementations/tests/test_common.py b/fsspec/implementations/tests/test_common.py index 6664af9d0..4e43ba2a8 100644 --- a/fsspec/implementations/tests/test_common.py +++ b/fsspec/implementations/tests/test_common.py @@ -34,6 +34,72 @@ def test_modified(fs: AbstractFileSystem, temp_file): fs.rm(temp_file) +@pytest.mark.parametrize( + "method", + [ + 'cat', + 'cat_file', + 'cat_ranges', + 'checksum', + 'copy', + 'cp', + 'cp_file', + 'created', + 'delete', + 'disk_usage', + 'download', + 'du', + 'end_transaction', + 'exists', + 'expand_path', + 'find', + 'from_json', + 'get', + 'get_file', + 'get_mapper', + 'glob', + 'head', + 'info', + 'invalidate_cache', + 'isdir', + 'isfile', + 'lexists', + 'listdir', + 'ls', + 'makedir', + 'makedirs', + 'mkdir', + 'mkdirs', + 'modified', + 'move', + 'mv', + 'open', + 'pipe', + 'pipe_file', + 'put', + 'put_file', + 'read_block', + 'read_bytes', + 'read_text', + 'rename', + 'rm', + 'rm_file', + 'rmdir', + 'sign', + 'size', + 'sizes', + 'start_transaction', + 'stat', + 'tail', + 'to_json', + 'touch', + 'ukey', + 'unstrip_protocol', + 'upload', + 'walk', + 'write_bytes', + 'write_text'] +) @pytest.mark.parametrize( "fscls", [ @@ -62,29 +128,24 @@ def test_modified(fs: AbstractFileSystem, temp_file): ], indirect=["fscls"], ) -def test_signature(fscls): +def test_signature(fscls, method): abstract_fs = AbstractFileSystem - for method in dir(abstract_fs): - if isfunction(getattr(abstract_fs, method)): - if method.startswith("_"): - continue - abs_signature = signature(getattr(abstract_fs, method)) - fs_signature = signature(getattr(fscls, method)) - # assert abs_signature == fs_signature - for k1, k2 in zip(abs_signature.parameters, fs_signature.parameters): - if k1 in ["self", "args", "kwargs"]: - continue - assert ( - abs_signature.parameters[k1].kind - == fs_signature.parameters[k2].kind - ), ( - f"Paramete {k2} of {method} in {fscls.__name__} " - "don't have the same kind with the one in base class" - ) - assert ( - abs_signature.parameters[k1].default - == fs_signature.parameters[k2].default - ), ( - f"Paramete {k2} of {method} in {fscls.__name__} don't" - " have the same default value with the one in base class" - ) + abs_signature = signature(getattr(abstract_fs, method)) + fs_signature = signature(getattr(fscls, method)) + # We only compare parameters shown in the abstract class + # We allow extra parameters in the subclass + for k1, k2 in zip(abs_signature.parameters, fs_signature.parameters): + if k1 in ["self", "args", "kwargs"]: + continue + assert ( + abs_signature.parameters[k1].kind + == fs_signature.parameters[k2].kind + ), ( + f"{abs_signature} != {fs_signature}" + ) + assert ( + abs_signature.parameters[k1].default + == fs_signature.parameters[k2].default + ), ( + f"{abs_signature} != {fs_signature}" + ) From cc406855ff1be3dd2a08066c04d1cecf3bdb83d4 Mon Sep 17 00:00:00 2001 From: Yuxuan Lu Date: Tue, 8 Nov 2022 23:44:19 +0800 Subject: [PATCH 6/6] Add xfail --- fsspec/implementations/tests/test_common.py | 142 ++++++++++---------- 1 file changed, 69 insertions(+), 73 deletions(-) diff --git a/fsspec/implementations/tests/test_common.py b/fsspec/implementations/tests/test_common.py index 4e43ba2a8..18533ed72 100644 --- a/fsspec/implementations/tests/test_common.py +++ b/fsspec/implementations/tests/test_common.py @@ -1,6 +1,6 @@ import datetime import time -from inspect import isfunction, signature +from inspect import signature import pytest @@ -34,71 +34,73 @@ def test_modified(fs: AbstractFileSystem, temp_file): fs.rm(temp_file) +@pytest.mark.xfail @pytest.mark.parametrize( "method", [ - 'cat', - 'cat_file', - 'cat_ranges', - 'checksum', - 'copy', - 'cp', - 'cp_file', - 'created', - 'delete', - 'disk_usage', - 'download', - 'du', - 'end_transaction', - 'exists', - 'expand_path', - 'find', - 'from_json', - 'get', - 'get_file', - 'get_mapper', - 'glob', - 'head', - 'info', - 'invalidate_cache', - 'isdir', - 'isfile', - 'lexists', - 'listdir', - 'ls', - 'makedir', - 'makedirs', - 'mkdir', - 'mkdirs', - 'modified', - 'move', - 'mv', - 'open', - 'pipe', - 'pipe_file', - 'put', - 'put_file', - 'read_block', - 'read_bytes', - 'read_text', - 'rename', - 'rm', - 'rm_file', - 'rmdir', - 'sign', - 'size', - 'sizes', - 'start_transaction', - 'stat', - 'tail', - 'to_json', - 'touch', - 'ukey', - 'unstrip_protocol', - 'upload', - 'walk', - 'write_bytes', - 'write_text'] + "cat", + "cat_file", + "cat_ranges", + "checksum", + "copy", + "cp", + "cp_file", + "created", + "delete", + "disk_usage", + "download", + "du", + "end_transaction", + "exists", + "expand_path", + "find", + "from_json", + "get", + "get_file", + "get_mapper", + "glob", + "head", + "info", + "invalidate_cache", + "isdir", + "isfile", + "lexists", + "listdir", + "ls", + "makedir", + "makedirs", + "mkdir", + "mkdirs", + "modified", + "move", + "mv", + "open", + "pipe", + "pipe_file", + "put", + "put_file", + "read_block", + "read_bytes", + "read_text", + "rename", + "rm", + "rm_file", + "rmdir", + "sign", + "size", + "sizes", + "start_transaction", + "stat", + "tail", + "to_json", + "touch", + "ukey", + "unstrip_protocol", + "upload", + "walk", + "write_bytes", + "write_text", + ], ) @pytest.mark.parametrize( "fscls", @@ -138,14 +140,8 @@ def test_signature(fscls, method): if k1 in ["self", "args", "kwargs"]: continue assert ( - abs_signature.parameters[k1].kind - == fs_signature.parameters[k2].kind - ), ( - f"{abs_signature} != {fs_signature}" - ) + abs_signature.parameters[k1].kind == fs_signature.parameters[k2].kind + ), f"{abs_signature} != {fs_signature}" assert ( - abs_signature.parameters[k1].default - == fs_signature.parameters[k2].default - ), ( - f"{abs_signature} != {fs_signature}" - ) + abs_signature.parameters[k1].default == fs_signature.parameters[k2].default + ), f"{abs_signature} != {fs_signature}"