Dependency Injection for http_client #32
Conversation
|
IMHO, you're modifying too deeply the Apis. If it's for tests purposes, mock is the module to use |
|
I do agree with @dzen .Mocking the urllib2 would be the perfect solution for Image module unit tests. The extractor module does't use Mock at all, it kind of fake it. Id' rather write some unittest first for the existing image module and the go ahead with the download less solution proposed on #31 . I'll have some time next week to improve this. |
|
Yeah in fact I also dislike changing APIs too much. Although I think existing application code shouldn't be affected. I was wondering whether it would make sense to put such stuff directly into the config... Hmm ok... The thing is this: I would like to improve the image extractor. I see that sometimes it fetches the wrong image. (E.g. here http://www.timeforkids.com/news/gaga-goo-goo-clusters/54591 it fetches the author's picture instead of the Article's main image) To implement this, some unit testing is needed in order to prevent destroying cases that work fine today. I.e. specifying HTML as usual and additionally sizes of all referenced images. |
|
I've pushed a work in progress branch. Basic idea here is to Mock urllib2 for testing https://github.com/grangier/python-goose/compare/mocked I'll like to update tests based on this classes before going forward on changing the existing code |
|
@psilva261 I know sometime the image extractor picks up the wrong image. It needs improvement |
|
@grangier Thanks for the preview. Looking forward to work with that |
|
@psilva261 I really advice to wait before hacking the mocked branch. It's not working as expected right now |
|
@grangier Ok ok |
Hey, as proposed in the discussion, dependency injection for http_client. So one can easily write unit tests for the image fetcher. Let me know if it's fine or if you want changes.
Cheers, Philip