#5425 closed defect (fixed)
Allow tests to pass without audio/video dependencies again
| Reported by: | Christopher Allan Webber | Owned by: | |
|---|---|---|---|
| Priority: | major | Milestone: | 0.9.0 |
| Component: | programming | Keywords: | |
| Cc: | Boris Bobrov | Parent Tickets: |
Description
A change was made to test_submission.py a while back which did the nice work of adding audio / video upload support. Unfortunately it also switched the codebase so that without the audio/video media type dependencies installed, tests would be failed, when really they should be skipped.
Attachments (1)
Change History (7)
comment:1 by , 8 years ago
| Cc: | added |
|---|
by , 8 years ago
| Attachment: | 0001-Remove-requirement-that-audio-video-dependencies-mus.patch added |
|---|
comment:3 by , 8 years ago
Firstly, I don't understand why the old skipping didn't work.
Secondly, I dislike the fact that we skip video testing. Video is a very important part of mediagoblin and we get a lot of complaints about it. On the other side, the tests there are not really unit tests, they are more like functional tests.
I'm merging the patch, but we should look into getting better tests for video.
comment:4 by , 8 years ago
| Milestone: | → 0.9.0 |
|---|---|
| Resolution: | → fixed |
| Status: | new → closed |
fixed in cf9eae4
comment:5 by , 8 years ago
The old skipping didn't work because the submission tests all used the same mediagoblin app config, which would enable video whether you had the dependencies for it or not. If you didn't have them, *all* tests would fail across test_submit.py, and even in many other places.
I agree that we need more video testing fwiw. We have some tests now between this and test_video.py, but we could use a lot better testing.
Maybe one could argue that if you're running tests, then you're probably a MediaGoblin developer, and if you're a developer, you should be testing all the most important plugins, and that especially includes video. I'm open to that world view, but that's not how we've been doing things. I could be convinced of it for going forward, though.
Anwyay, thanks for the review/merge!
comment:6 by , 8 years ago
It looks like this never got merged, or at least not pushed! So I just merged and pushed.
I've submitted a patch that adds back the ability to skip these tests in the right circumstances. What do you think, breton?
This doesn't affect the main codebase so I think it would be okay to merge for 0.9.0.