Skip to content

Conversation

leducvin
Copy link

This PR adds cleaning the media files from tests/testapp/test-media between each tox env run. It actually does so before each run (if a user wants to inspect the created files after a test run).

Alternatively, the files could be removed after test runs, leaving it all clean.

Additionnally, it adds tox to the optional testing dependency bundle so that when users initially runs pip install -e .[testing], they are ready to go.

Lastly, it fixes the relative path in the make clean recipe of the makefile.

Fix relative path from project root in clean
Add tox to testing dependency bundle
Remove test-media before each tox env run
tox.ini Outdated
commands_pre =
python -I {toxinidir}/tests/manage.py migrate
commands =
-rm -rf tests/testapp/test-media
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, but this is not an acceptable way. adding rm to allowed externals opens up the door for potential abuse.

The solution should be entirely TestCase-bound. That is, clear the test uploaded media directory on tearDown. e.g.

https://github.com/django/django/blob/f5c944b3141c58bb4a5c7bbca61180b2ad7c13aa/tests/file_storage/tests.py#L733-L735

or

https://github.com/django/django/blob/f5c944b3141c58bb4a5c7bbca61180b2ad7c13aa/tests/staticfiles_tests/test_storage.py#L438-L443

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh OK, thanks for the feedback -- I'll change it to do it in tearDown

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, I noticed in

https://github.com/torchbox/wagtailmedia/blob/794bd7e170bb709e5e43084b106b29f34faa9209/tests/manage.py#L57C1-L63C54

that there was already an attempt to clean test media. The problem is that it didn't do anything since the test app used a different MEDIA_ROOT.

So I changed the test app settings to use the same MEDIA_ROOT. Would this be acceptable?

make clean only removes .tox and __pycache__
Do not allow rm external command
Use MEDIA_ROOT and STATIC_ROOT from wagtail.tests.settings
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