Changed logger info level in tests to ERROR (#175) #186
Conversation
| @@ -17,42 +17,42 @@ | |||
| {'args': {'platform': 'win32'}, | |||
| 'target': '2013-10-01-03-02-04-mozilla-central-firefox-27.0a1.en-US.win32.installer.exe', | |||
| 'target_url': 'firefox/nightly/2013/10/2013-10-01-03-02-04-mozilla-central/firefox-27.0a1.en-US.win32.installer.exe' | |||
| }, | |||
There was a problem hiding this comment.
This are all PEP8 Issues that flared up during commit
|
I'd prefer to suppress the progress bar when running tests too, but I'm easy either way. Also, please keep PEP8 fixes in a separate commit, as it's cleaner if we ever need to revert something without also reverting these style fixes. |
|
regarding the PEP8. Due to the pre-commit hook that we agreed to use, I cannot commit anything with pep8 issues included... |
|
You can still commit, but you need to pass |
|
One truly learns something new everyday. I will remove the pep8 issues and create a separate PR for it. |
|
ok, next try :) thx |
|
I've merged #188, so if you rebase this against master we should see the passing Travis-CI results again. |
|
There is a line in scraper.py one character too long now. I'll fix it now. It actually has something to do with this patch |
|
ok. Go! :) |
|
I think this needs to be rebased - it still contains a lot of the PEP8 related changes. |
|
Ok, I think we can re-activate this PR now. It's been a bit, so just to recap. I added to all tests the log-level as Otherwise, when the tests are run, there is minimal output. Please feel free to suggest improvements, I don't like it yet. feels hackish, but couldn't think of anything better. |
| @@ -35,7 +35,8 @@ def test_download(self): | |||
| test_url = urljoin(self.wdir, 'download_test.txt') | |||
| scraper = mozdownload.DirectScraper(url=test_url, | |||
| directory=self.temp_dir, | |||
| version=None) | |||
| version=None, | |||
| log_level='ERROR') | |||
There was a problem hiding this comment.
Why do we have to pass in the log level here? Aren't you getting it via getEffectiveLevel()? I don't see it used.
There was a problem hiding this comment.
as far as I understand it, This line here
https://github.com/mozilla/mozdownload/blob/master/mozdownload/scraper.py#l85
decides what log level we have. Therefore, when calling a scraper in a test, I need to announce that we want the ERROR log_level. I found it best to do it this way, so that we can selectively switch on the INFO output for each section (depending on which test failed).
RE: relationship between log_level and getEffectiveLevel()
log_level is directly used and not kept in a variable, as a consequence, I need to call getEffectiveLevel() to regain information about the current log level. I could have created self.log_level instead, but I chose that way. Would you prefer it the other way round?
There was a problem hiding this comment.
Doesn't matter to me untill we know the level. It can stay as it is. Thanks for the explanation.
| pbar = progressbar.ProgressBar(widgets=widgets, | ||
| maxval=max_value).start() | ||
|
|
||
| # Get the log level: 20 == INFO; 10 == DEBUG |
There was a problem hiding this comment.
I don't think we need the values in the comment here
|
Just one nit really, otherwise this looks good to me. |
|
Thanks for letting it slide. With my PRs I am not quite sure, when I fixed this. It could have been before you slapped me on the wrist for it. I removed the comment now. Should be good to go. |
|
Thanks, landed in 8e29535 |
This PR addresses issue #175.
It should take care of the flood of information that happens during testing.
If people feel it necessary to remove the progressbar, too, I could look into that. I didn't feel it was necessary.