-
Notifications
You must be signed in to change notification settings - Fork 11
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
Ticket/18629 #3
Ticket/18629 #3
Conversation
There are still a couple failures, but they seem less trivial than the ones fixed in this commit.
Adding requests to install_requires now has the tests running on Travis, and failing in the same way as the develop branch. http://travis-ci.org/#!/pculture/vidscraper/jobs/955794 I'll find the fix there, then merge it in. |
Conflicts: setup.py
# def test_CantIdentifyUrl_for_video(self): | ||
# self.assertRaises(CantIdentifyUrl, self.suite.get_video, | ||
# 'http://www.google.com/') | ||
|
||
def test_basic_feed_data(self): | ||
feed = self.suite.get_feed( | ||
'file://%s' % os.path.join(self.data_file_dir, 'feed.rss')) | ||
'file://%s' % self._data_file_path('feed/feed.rss')) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe we need a data_file_url
method?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe, but I don't think that should be part of this pull. Maybe when we refactor the feeds?
Conflicts: setup.py vidscraper/suites/fora.py vidscraper/suites/google.py vidscraper/suites/youtube.py
AFAIK this branch is ready. Is that correct? |
Yes, that's correct. |
This refactors the single-video parts of vidscraper to use class-based scraping methods as per bug 18629. The tests all pass locally, but travis seems to be having trouble installing vidscraper for some reason: http://travis-ci.org/#!/pculture/vidscraper/jobs/955735