Bug 1237396 - Convert testSafeBrowsing_initialDownload Mozmill test to Marionette. r=whimboo, maja_zf #328
Conversation
|
So you run into timeout issues, this is most likely because you do not wait long enough or even better let Firefox download the files earlier. For the latter please read through the bug and check comment 5. Setting this preference should help us a lot here. |
|
|
||
| from firefox_puppeteer.testcases import FirefoxTestCase | ||
|
|
||
| from firefox_puppeteer.ui.browser.window import BrowserWindow |
There was a problem hiding this comment.
Please read https://www.python.org/dev/peps/pep-0008/#imports in how the coding style for imports should look like.
|
@whimboo Would you suggest both a 5 min timout and the nextupdatetime variable set? |
|
For setting the preference you can use the following method: http://firefox-puppeteer.readthedocs.org/en/latest/api/prefs.html#firefox_puppeteer.api.prefs.Preferences.set_pref |
|
5 minutes is definitely too long. With the preferences set the files should appear within seconds. Maybe you record some timings to see how fast they download? |
| self.test_url = 'https://mozqa.com' | ||
|
|
||
| # Set Browser Preferences | ||
| self.prefs.set_pref('browser.safebrowsing.provider.google.lastupdatetime', 1) |
There was a problem hiding this comment.
I think that browser.safebrowsing.provider.google.lastupdatetime is not needed (sorry about that, it was my fault for mentioning it in the first place).
| # Set Browser Preferences | ||
| self.prefs.set_pref('browser.safebrowsing.provider.google.nextupdatetime', 1) | ||
| self.prefs.set_pref('browser.safebrowsing.provider.mozilla.nextupdatetime', 1) | ||
| self.prefs.set_pref('browser.safebrowsing.provider.mozilla.lastupdatetime', 1) |
There was a problem hiding this comment.
lastupdatetime shouldn't be necessary.
| @@ -11,6 +11,7 @@ tags = local | |||
| tags = local | |||
| [test_safe_browsing_notification.py] | |||
| [test_safe_browsing_warning_pages.py] | |||
| [test_safe_browsing_primary_download.py] | |||
There was a problem hiding this comment.
nit: please sort the files alphabetically.
There was a problem hiding this comment.
Benny, this still needs a correction.
|
|
||
| def test_safe_browsing(self): | ||
| with self.marionette.using_context('content'): | ||
| self.marionette.navigate(self.test_url) |
There was a problem hiding this comment.
Is there a reason why we have to navigate to this page? I don't see that this would trigger the download of the wanted files.
|
I hope this is everything! |
|
@whimboo Please let me know if the platform list variable name is okay |
| import os | ||
| import sys | ||
|
|
||
| from marionette_driver import By, Wait |
There was a problem hiding this comment.
nit: By is also not used in this test, so no need to import it.
Hoping everything checks out on this, as it is my first ever contribution on github. Please feel free to tear it up!