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

Testing #121

Closed
wants to merge 26 commits into from
Closed

Testing #121

wants to merge 26 commits into from

Conversation

grimley517
Copy link
Contributor

see issue #120

In tool tests test 4 - a bytes object is recognised by the tools moduel as a string. Should this be the case?

added a line to ignore project artefacts from the cloud 9 IDE
Merge branch 'testing' of github.com:grimley517/moviepy into testing
added travis icon
Corrected .travis.yml in correct repo
Corrected an error - Line 40
@grimley517
Copy link
Contributor Author

I'm not sure if you can access this page -
https://travis-ci.org/grimley517/moviepy/builds/49401075

This is a version specific error. In python versions before python 3 this fails with a byte string. In versions after this behaves as expected.

image

@grimley517
Copy link
Contributor Author

On further reading python2.7 and before do not recognise byte strings.

@grimley517
Copy link
Contributor Author

image

test 4a puts this back in the green by separating out the python 2 and python 3 behaviours.

@Zulko
Copy link
Owner

Zulko commented Feb 4, 2015

Thanks ! This looks awesome, exactly what we needed to get the test suite started. I'll review and merge that when I'm fresh.

@Zulko
Copy link
Owner

Zulko commented Feb 5, 2015

A few questions :

Is the .c9 folder really needed or can I delete it ?

Is there any reason why you chose unittest instead of pytest or anything else ? Just asking, I don't have any preference at the moment.

@grimley517
Copy link
Contributor Author

The .c9 folder is not needed - i added it to the .gitignore after I realised it wasn't there - its an artefact from the IDE I use.

I used unittest as its the simplest if not in terms of actual amount of code written, but more in terms of everything can read it. Unittest tests are picked up by pytest and nosetests. I was just going for simple to start with. I automated these in travis.ci which again can use pytest or nose tests. I'm interested in knowing what happens to the test automation if this is pulled into master - do the automated tests still run on travis if you have not set travis up?

@mgaitan
Copy link
Collaborator

mgaitan commented May 19, 2016

@Zulko IMHO, at this moment is a must to have a test suite that coverages the main features at least, running on travis and appveyor. I believe it will help to reduce the tremendous effort you do to give support (thanks for that btw, it's admirable), and also it may help to increase the number of occasional contributors, making them more confident.

I could help updating this contribution and also migrate it to pytest, if you think that it would be better (probably it would be much less boilerplate).

Moreover, I think we could develop a tool to make a higher-level/functional tests, comparing the result of some edition+export against an expected target, at frames level, conceptually similar to what pytest-mpl does. Altought it could be a bit fragile under some circumstances, it also turn easier to write tests.

Let me know what you think

@Zulko
Copy link
Owner

Zulko commented May 21, 2016

Yes it is a real shame that moviepy doesn't have a test suite. I'd say yes to a test suite, even though I have little time to put into this.
Your comparison to mpl test is very relevant, one thing that drove me out of doing a test suite for moviepy is that it would imply adding a lot of test images and test videos, which makes the size of the source code to explode (this is what happened to matplotlib).
Another difficulty that may arise is that moviepy may have several backends depending on the machine it is installed on (different library used for resizing, different versions of ffmpeg etc.)

@keikoro keikoro added the tests Related to individual tests in the test suite or running the test suite. label Feb 18, 2017
@ghost
Copy link

ghost commented Feb 28, 2017

grimsley517, we used your test_tools.py .. thanks for the submission!

@ghost ghost closed this Feb 28, 2017
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
tests Related to individual tests in the test suite or running the test suite.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants