Video tests fix

[vasylnakvasiuk]

Tests for video player were fixed.
@rocha, @chrisndodge please review.

Old pull request on edx-platform-pre-clean: https://github.com/edx/edx-platform-pre-clean/pull/2121

[vasylnakvasiuk]

pylint report for this pull_request:

$ PYTHONPATH=./lms/djangoapps:./lms/lib:./cms/djangoapps:./cms/lib:./common/lib:./common/djangoapps pylint --rcfile=pylintrc --output-format=colorized $(git diff master --name-only | grep -i '\.py$')
************* Module xmodule.video_module
W0511: 98: TODO (vshnayder): This is not being called right now, so the position
W0511:110: TODO (vshnayder): Get and save duration of youtube video, then return
W0223:154:VideoDescriptor: Method 'get_html' is abstract in class 'HTMLSnippet' but is not overridden

@shnayder, can we manage your TODOs?

[shnayder]

I put those in sometime last summer, I think. Feel free to do whatever
makes sense at this point. Thanks!

On Fri, May 31, 2013 at 3:45 AM, Vasyl Nakvasiuk
notifications@github.comwrote:

pylint report for this pull_request:

$
PYTHONPATH=./lms/djangoapps:./lms/lib:./cms/djangoapps:./cms/lib:./common/lib:./common/djangoapps
pylint --rcfile=pylintrc --output-format=colorized $(git diff master
--name-only | grep -i '.py$')
************* Module xmodule.video_module
W0511: 98: TODO (vshnayder): This is not being called right now, so the
position
W0511:110: TODO (vshnayder): Get and save duration of youtube video, then
return
W0223:154:VideoDescriptor: Method 'get_html' is abstract in class
'HTMLSnippet' but is not overridden

@shnayder https://github.com/shnayder, can we manage your TODOs?

—
Reply to this email directly or view it on GitHubhttps://github.com/edx/pull/7#issuecomment-18729389
.

[chrisndodge]

I see build failures, but I don't think it's related to your changes. Can you coordinate with @jzoldak:

VersionConflict: (distribute 0.6.30 (/mnt/virtualenvs/edx-feature-branch-tests/lib/python2.7/site-packages), Requirement.parse('distribute==0.6.28'))

[vasylnakvasiuk]

Reports:

tests + coverage

$ coverage run `which django-admin.py` test --traceback --settings=lms.envs.test --pythonpath=. --logging-clear-handlers $(git diff master --name-only | grep -i '\.py$')
......
-----------------------------------------------------------------------------
6 tests run in 2.7 seconds (6 tests passed)

$ coverage report -m $(git diff master --name-only | grep -i '\.py$')
Name                                               Stmts   Miss  Cover   Missing
--------------------------------------------------------------------------------
common/lib/xmodule/xmodule/video_module               66      0   100%
lms/djangoapps/courseware/tests/__init__              34      0   100%
lms/djangoapps/courseware/tests/test_video_mongo       8      0   100%
lms/djangoapps/courseware/tests/test_video_xml        45      0   100%
--------------------------------------------------------------------------------
TOTAL                                                153      0   100%

pylint

Your code has been rated at 10.00/10 (previous run: 10.00/10)

[chrisndodge]

@jzoldak @cahrens just looking through open PR's. Are we good on this?

@vaxXxa can you merge this or do you need us to press the green button? We're doing a release-branch today, so I'd suggest waiting until the RC has been made before merging this....

[cahrens]

👍 assuming the code is what I reviewed in edx/edx-platform-pre-clean#2121

[rocha]

LGTM 👍 Sorry for the delay, I had already looked that this pull request but got confused with the repo switch over.

[rocha]

One small comment: consider squashing commits before merging (using rebase interactive). Many commits are just small changes.

[vasylnakvasiuk]

@rocha, ok. I will make "rebase -i".

[vasylnakvasiuk]

@rocha, done.

[rocha]

👍 Go ahead and merge 😄

[vasylnakvasiuk]

@rocha, so I can press the green button? :)

[rocha]

Yes, you got two good to go. Go ahead and merge.