search

stephenmcd / filebrowser-safe

File manager for Mezzanine
Other
41 stars 104 forks source link

Use own tests instead of running mezzanine test suite #117

Closed vstoykov closed 5 years ago

vstoykov commented 5 years ago

With the current porting initiative to Django 2+ there is chicken and egg problem with filebrowser-safe and mezzanine. Both of them need to be Django 2 compatible in order both travis CI to pass. This is not very good.

I know that the existence of filebrowser-safe is solely for Mezzanine and probably there is no project out there which is using filebrowser-safe without Mezzanine. Still I'm wondering if it is a good idea filebrowser-safe to have separate test suite only for it and integrate it will Travis CI instead of running Mezzanine's test suite.

What you think?

stephenmcd commented 5 years ago

Sounds good.

vstoykov commented 5 years ago

The bad news is that the code depends on mezzanine:

filebrowser_safe/base.py:                from mezzanine.core.exceptions import FileSystemEncodingChanged
filebrowser_safe/functions.py:    from mezzanine.conf import settings as mezz_settings
filebrowser_safe/functions.py:    from mezzanine.utils.sites import current_site_id
filebrowser_safe/static/filebrowser/js/filebrowser-popup.js: *  2. js:  mezzanine/js/%s' % settings.JQUERY_FILENAME
filebrowser_safe/templates/filebrowser/custom_field.html:{% load mezzanine_tags i18n %}
filebrowser_safe/templates/filebrowser/include/filelisting.html:{% load i18n static fb_tags mezzanine_tags %}
filebrowser_safe/templates/filebrowser/upload.html:{% load i18n l10n static fb_tags mezzanine_tags %}
filebrowser_safe/views.py:from mezzanine.utils.importing import import_dotted_path
filebrowser_safe/views.py:    from mezzanine.utils.html import escape
filebrowser_safe/views.py:    from mezzanine.conf import settings

In order this to happens filebrowser_safe should not use anything from mezzanine.

stephenmcd commented 5 years ago

Running tests from FB simply runs the Mezzanine test suite right?

Presumably there was some benefit when this was added, but I don't think it's strictly necessary we have this - if it's easier to remove this altogether then I'm fine with that. WDYT?

vstoykov commented 5 years ago

Running the Mezzanine's test suite was not testing very much from FileBrowser. Basically it tests that urls.py can be imported and that's it. If FileBrowser has it's own tests it will be better IMHO.

I was able to create test suite with one fake test and add mezzanine as dependency in setup.py in test_requires and the test suite ran correctly with python setup.py test without the need of tests.sh.

I just need to prepare some tests which will test something in FileBrowser. This will still have benefit if Mezzanine is not ported to work with latest Django version in the future but the parts needed by FileBrowser are ok with it. Then we will have passing test suite for FileBrowser even if Mezzanine is not fully compatible with latest Django.

After few days I will try to finish with tests (if i have enough free time) and I will create PR.

vstoykov commented 5 years ago

I created PR with a test suite that will test some of the functionality. You can look at #121 and say if you like it or not.

stephenmcd commented 5 years ago

Thank you

github-actions[bot] commented 4 years ago

:tada: This issue has been resolved in version 1.0.0-alpha.1 :tada:

The release is available on:

Your semantic-release bot :package::rocket:

github-actions[bot] commented 3 years ago

:tada: This issue has been resolved in version 1.0.0-rc.1 :tada:

The release is available on:

Your semantic-release bot :package::rocket:

github-actions[bot] commented 2 years ago

:tada: This issue has been resolved in version 1.0.0 :tada:

The release is available on:

Your semantic-release bot :package::rocket: