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

Check YouTube Availability for Video Tests #3882

Merged
merged 1 commit into from
May 29, 2014

Conversation

muhammad-ammar
Copy link
Contributor

Check if YouTube is available before running a Video Test.

@muhammad-ammar
Copy link
Contributor Author

By default we will check the availability of below

But to check an extra URL, use the below statement right above a Scenario in lettuce feature file

@check_youtube(your URL here)

There is any issue with transcript API URL(video.google.com/timedtext), if we HTTP GET this URL we will always get status_code = 404, so we have to use the URL like below explicitly mentioning video id for which we want transcript

http://video.google.com/timedtext?lang=en&v=OEoXaMPEzfM

So question is what is the Best way to handle the above?

@jzoldak @auraz @polesye

@muhammad-ammar
Copy link
Contributor Author

@jzoldak @auraz @polesye Please Review. Thanks

urls = ['https://youtube.com', 'http://www.youtube.com/iframe_api', 'http://gdata.youtube.com/feeds/api/videos/']

# Check if tag is present
tag = [tag for tag in scenario.tags if tag.startswith('check_youtube')]
Copy link
Contributor

Choose a reason for hiding this comment

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

Where are scenario tags set?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

in

  • cms/djangoapps/contentstore/features/video.py
  • lms/djangoapps/courseware/features/video.py

Copy link
Contributor

Choose a reason for hiding this comment

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

These tags are just test decorators, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yeah

@auraz
Copy link
Contributor

auraz commented May 27, 2014

@muhammad-ammar that's ok to ask for "http://video.google.com/timedtext?lang=en&v=OEoXaMPEzfM" with specific video in the link. Default video is fine.

@auraz
Copy link
Contributor

auraz commented May 27, 2014

I think that we can add "http://video.google.com/timedtext?lang=en&v=OEoXaMPEzfM" to list of all urls that need to be checked.

urls.extend(match.group(1).split(','))

if not is_urls_available(urls):
# This is a hakish way to skip a test in lettuce as there is no proper way to skip a test conditionally
Copy link
Contributor

Choose a reason for hiding this comment

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

typo: hakish

@auraz
Copy link
Contributor

auraz commented May 27, 2014

looks good 👍

@muhammad-ammar
Copy link
Contributor Author

I think that we can add "http://video.google.com/timedtext?lang=en&v=OEoXaMPEzfM" to list of all urls that need to be checked.

I am not sure about this. @polesye What is your opinion?

@polesye
Copy link
Contributor

polesye commented May 27, 2014

@muhammad-ammar I think it's good idea. It will be enough to check server availability.

# Here we are adding query parameters for a specific transcript, we are doing this because without query parameters,
# the GET request will always return 404 whether the YouTube is available or not.
# TODO: What should be the URL to check_youtube?
@check_youtube(http://video.google.com/timedtext?lang=en&v=OEoXaMPEzfM)
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd prefer rename it so check_youtube -> is_youtube_available or skip_if_youtube_unavailable

@muhammad-ammar
Copy link
Contributor Author

I am little bit confused.

Now i have added http://video.google.com/timedtext?lang=en&v=OEoXaMPEzfM to list of all urls that need to be checked.

My Question is SHOULD we check the

  • YouTube availability only when skip_if_youtube_unavailable tag is placed above a Scenario
    OR
  • SHOULD we always check YouTube availability whether or not skip_if_youtube_unavailable tag is present above a Scenario

If we go with the second way then we only need to place the skip_if_youtube_unavailable tag if we also want to check the availability of some other URL. And there arises another question, Could this really happen that we need to check some other URL beside the already 4 URLs?

@auraz @polesye

@polesye
Copy link
Contributor

polesye commented May 27, 2014

YouTube availability only when skip_if_youtube_unavailable tag is placed above a Scenario

First.

Could this really happen that we need to check some other URL beside the already 4 URLs?

I don't think so.

@muhammad-ammar
Copy link
Contributor Author

@polesye cool. That would make things simple.

@@ -23,6 +23,11 @@
DELAY = 0.5


@before.each_scenario
Copy link
Contributor

Choose a reason for hiding this comment

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

This won't do what you want. Once this file is loaded the method will be run before every scenario that is executed, not just the video ones. http://lettuce.it/reference/terrain.html#lettuce-hooks

@jzoldak jzoldak changed the title Check YouTube Availabitlity for Video Tests Check YouTube Availability for Video Tests May 28, 2014
@polesye
Copy link
Contributor

polesye commented May 29, 2014

Do we have similar solution for the bok-choy tests?

@muhammad-ammar
Copy link
Contributor Author

Do we have similar solution for the bok-choy tests?

For now there is nothing. After this PR is completed we will provide a solution for bok-choy tests.


"""
for tag in scenario.tags:
print tag
Copy link
Contributor

Choose a reason for hiding this comment

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

accidentally left this in from debugging

@jzoldak
Copy link
Contributor

jzoldak commented May 29, 2014

👍

@polesye
Copy link
Contributor

polesye commented May 29, 2014

👍

muhammad-ammar added a commit that referenced this pull request May 29, 2014
Check YouTube Availability for Video Tests
@muhammad-ammar muhammad-ammar merged commit 026c652 into master May 29, 2014
@muhammad-ammar muhammad-ammar deleted the ammar/test-youtube-availibility branch May 29, 2014 18:54
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.

5 participants