Created tests for ReleaseCandidateScraper (#160) #165
Conversation
| # -t candidate -p win32 -v 21.0 | ||
| {'args': {'platform': 'win32', | ||
| 'version': '21.0'}, | ||
| 'target': 'firefox-21.0-build3.en-US.win32.exe'}, |
There was a problem hiding this comment.
Similar to the release scraper tests, we should get ´-t candidate´ moved out to a separate test given that it's not used here.
|
Otherwise it looks great! Lets get it updated and we can land it. Yay! |
|
Ok, @whimboo done :) Another review of these changes please. Thanks a lot! |
|
This pull cannot be merged anymore since the tests for the release scraper have been added. Please fix the merge issues. |
| # -p win32 -v 21.0 | ||
| {'args': {'platform': 'win32', | ||
| 'version': '21.0'}, | ||
| 'target': 'firefox-21.0-build3.en-US.win32.exe'}, |
There was a problem hiding this comment.
We need tests for the target_url too, which I missed before.
|
Thanks for the feedback and taking the time to go through this rather large PR. I updated the pull now and added the target_url key to the dicts and test for it like in the release_scraper. |
| tests = firefox_tests + thunderbird_tests | ||
|
|
||
|
|
||
| class ReleaseScraperTest(mhttpd.MozHttpdBaseTest): |
There was a problem hiding this comment.
Why ReleaseScraper? This is ReleaseCandidateScraper.
There was a problem hiding this comment.
This is a copy/paste error that I overlooked. Sorry
|
With the patch I get a lot of INFO output written to the console:
Can we hide that? |
|
I spoke with @davehunt about this last weekend. This has something to do with the logger settings. We thought it best to open a new issue for this once all the tests have been implemented. It is best if during the tests there is only ERROR output, so also no progressbar. I am sure we can tie the progressbar to the information level of the logger. What do you think? |
|
Ok, lets get a new issue filed to hide this logger output. Otherwise lets see how important it would be, and if this additional information wouldn't give us some helpful details while running the tests. |
Please see: #175 |
It should be easy enough to increase the logger output level. In that token, if there is an error in some of the tests, you can still increase the output and re-run when debugging. |
|
The minor nit pick was changed now and a new issue opened for the output. Hopefully that is it now :) Thanks for reviewing |
|
@Nebelhom sorry but we need another update of the pull. It has been bitrotted most likely due to the landing of the daily scraper tests. :/ |
|
Don't worry about it. I will now wait until this one is merged before I look at the other tests. That way, we will hopefully avoid the constant bitrotting. |
|
I cannot run this tests due to a version conflict with mozinfo. Can we get issue #177 fixed first please? |
| @@ -1,4 +1,5 @@ | |||
| [include:base_scraper/manifest.ini] | |||
| [include:candidate_scraper/manifest.ini] | |||
There was a problem hiding this comment.
Just something I found. Can we please rename this to release_candidate_scraper and reorder?
|
ok, fixed. thanks for the reviews :) |
|
Merged as 565d5fa |
This PR addresses issue #160
Tests for the ReleaseCandidateScraper :)