Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Very basic autotesting #28

Open
wants to merge 4 commits into
base: develop
Choose a base branch
from

Conversation

SupraSummus
Copy link

@SupraSummus SupraSummus commented Jul 6, 2020

This is proposal for a basic test infrastructure. This hasn't been consulted with anyone prior this PR. Idea is to test behaviour of mounted filesystem using os module. At the same time implementation interface is tested using mocks. All of this is done in single python process, using multiple threads. Mountpoints are obtained via tempfile module.

Python-level mounting code is extracted and generalised from ipfs-api-mount.

There are few not-so-ideal solutions in this PR and things not sorted out yet:

  1. There is busy waiting for FS to be ready. I don't know how to do it better. FUSE doesn't seem to provide "FS ready" hook.
  2. Mounting code requires /proc/mounts file. AFAIK this is linux-specific. Good part is that it can be easily substituted.
  3. Test runner needs to be configured. I ran tests using python setup.py test, but this seems to be deprecated. Deprecation message mentions tox as a recommended solution.
  4. Documentation (even basic one) on tests are missing

Let me know if this is a desired approach to testing refuse. If it is then I can put some work in it - up to this point it's a copy-paste effort. In my opinion after resolving points 3. and 4. it'll be ready for merging.

Related issues #13, fusepy#80

@SupraSummus SupraSummus changed the base branch from master to develop July 6, 2020 18:19
@s-m-e s-m-e added the enhancement New feature or request label Jul 10, 2020
@s-m-e
Copy link
Member

s-m-e commented Jul 10, 2020

Thanks for the effort and the pull request!

I am not entirely sure if I understand this correctly. You're testing and mocking against refuse / libfuse directly. I.e. there is no actual FUSE filesystem involved. What are those tests telling you about the state of refuse?

python setup.py test and deprecation: Yes, it is indeed considered "legacy". There are better and more modern test runners and test frameworks. tox is one of them. I personally prefer pytest (in conjunction with tox if required).


I have considered to write an in-memory file-system for proper tests, e.g. similar to memfs or kungfuse, by e.g. "upgrading" loggedfs-python. This would allow tests against a "real" file system while being able to inspect the file system's contents and feedback while testing. It's a bit time consuming, to be honest.

@SupraSummus
Copy link
Author

SupraSummus commented Jul 11, 2020

You are right. These tests doesn't depend on any "actual FS" implementation. This is, in my opinion, good because we want to be testing libfuse binding for python (passing stuff from libfuse to python code and from py to libfuse). We don't want to be testing dummy FS implementation. Of course dummy implementation is unavoidable - here I implemented very simple FS using mocks. With more complex test more complex implementation(s) may come. The point is that the simpler the FS implementation the smaller the chance for covering (hiding) potential refuse bugs with FS implementation.

What the tests are telling me?

Any test that loads refuse tests the following properties:

  1. The code has no syntax errors. (This can be done using flake maybe. But it seems it isn't configured.)
  2. The code executes properly enough to be loaded.

Six tests I provided test the following properties:

  1. Init/destroy/ls(high) calls are passed to py code. Parameters passed in look okay. By overriding init/destroy/ls(high) methods on operations class FS author can provide desired handlers.
  2. LS results are returned properly to libfuse (high).

@SupraSummus
Copy link
Author

I got somewhat familiar with travis+tox+pytest setup when updating CI configuration and test suite in ipfs-api-mount. I can port it here if there is some chance it will be merged eventually.

@s-m-e
Copy link
Member

s-m-e commented Jul 19, 2020

It's an improvement for sure so there no reason to not merge it. I need to find some time to look through it, though.

@SupraSummus
Copy link
Author

I've updated tests to be pytest-style. Also I configured tox.

I've got configuration for flake ready, but that will come in another PR to keep this one in managable size

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants