Skip to content

Conversation

@ianthomas23
Copy link
Contributor

This adds testing using the abstract test harness introduced into fsspec in fsspec/filesystem_spec#1216. It is not intended to be merged yet, my intention is to leave it as an open PR that I can keep pushing to and rerun the CI as I add more tests to fsspec that will be picked up here.

It demonstrates what is required for a class derived from fsspec.AbstractFileSystem to inherit and run the new test suite. The s3fs_test.py file will run all tests from the fsspec.Abstract<whatever>Tests by inheriting from them. If a test is not appropriate for this fsspec implementation then it would be pytest.skipped in this file. Most of the new code is setting up a filesystem in a repeatable way for the tests, and most of this is copy-and-pasted (with minor modifications) from the directory above. I've duplicated this code rather than importing it from the existing code as I'd like to keep the new tests isolated from the old ones initially. It might be possible to replace all of the old tests with new ones within the new test harness but it is too early to say whether this will be likely yet.

This is all very experimental still. We need to add a fairly comprehensive set of tests to this framework before we can be sure that it will work and be easy to maintain.

@ianthomas23
Copy link
Contributor Author

Tests fail in CI when trying to import fsspec.tests and the directory is not present. In CI we install the latest fsspec commit from the github repo, and this builds it as a wheel and installs the wheel. The wheel does not include any of the fsspec test code so this approach isn't going to work.

I can think of two options here:

  • Install fsspec in a different way in CI so that the tests are present.
  • Change fsspec to include the tests in the wheel.

@ianthomas23
Copy link
Contributor Author

There is a workaround in CI now.

@ianthomas23 ianthomas23 force-pushed the abstract_test_harness branch from 94f994b to 0ec0be4 Compare May 30, 2023 08:49
@ianthomas23
Copy link
Contributor Author

This is ready now. It successfully runs and passes all tests inherited from the fsspec abstract test harness.

@martindurant
Copy link
Member

Hurray!

@martindurant martindurant merged commit ab460d5 into fsspec:main May 30, 2023
@ianthomas23 ianthomas23 deleted the abstract_test_harness branch May 31, 2023 08:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants