Closed
Bug 931781
Opened 11 years ago
Closed 11 years ago
Use timeout arg on the command line to change timeout for desktop/device
Categories
(Firefox OS Graveyard :: Gaia::UI Tests, defect)
Tracking
(firefox28 fixed, b2g-v1.2 fixed)
RESOLVED
FIXED
People
(Reporter: zcampbell, Assigned: bitgeeky)
Details
(Whiteboard: [mentor=davehunt][lang=py])
Attachments
(1 file, 10 obsolete files)
We'd like to be able to use a timeout function to set the `_default_timeout` from the command line.
Tasks:
* Update gaia_test.py where required
* Retain default/current timeout for device tests
* Update Travis command line to use 10 second timeout and see how the test run goes
Updated•11 years ago
|
Whiteboard: [mentor=davehunt][lang=py]
Comment 1•11 years ago
|
||
To get started you will need to clone the gaia repository at https://github.com/mozilla-b2g/gaia and install the Python package. I recommend using virtual environments for this, which you can install by running 'pip install virtualenv'.
Then change to the gaia-ui-tests directory, create and activate a virtual environment, and install gaiatest:
cd tests/python/gaia-ui-tests/
virtualenv venv
source venv/bin/activate
python setup.py develop
The gaiatest harness currently sets a search and script timeout here: https://github.com/mozilla-b2g/gaia/blob/c8e1eb8b06bc2e3e02fcf57e206f07b1be02ecc4/tests/python/gaia-ui-tests/gaiatest/gaia_test.py#L440 but we want to be able to control these via the command line --timeout argument.
The --timeout argument is already functioning, so we just need to remove anywhere we override the timeouts, and allow for suitable defaults. Here are a couple of examples of the changes we'd need:
In https://github.com/mozilla-b2g/gaia/blob/c8e1eb8b06bc2e3e02fcf57e206f07b1be02ecc4/tests/python/gaia-ui-tests/gaiatest/gaia_test.py#L117 we currently specify a default timeout of 30 seconds. If we set the default value to None, we can then use something like `timeout = timeout or self.marionette.timeout or 30` we can use the passed value, falling back on the --timeout command line value if present, and finally falling back to a default value of 30 seconds.
There are a number of places where we currently set timeouts, and all of these will need reviewing for this patch. In most cases we will want to use the value from self.marionette.timeout in favour of a hard-coded value.
If you need any further help with this please ask here, or in IRC.
Assignee: nobody → mozpankaj1994
Assignee | ||
Comment 2•11 years ago
|
||
Thanks Dave , after working on it , I modified it as https://gist.github.com/bitgeeky/7255081
I have put #changed on every line that I have changed
Hope this was what you wanted , please let me know if its correct.
Assignee | ||
Comment 3•11 years ago
|
||
Attachment #825635 -
Flags: feedback?(dave.hunt)
Assignee | ||
Comment 4•11 years ago
|
||
Attachment #825635 -
Attachment is obsolete: true
Attachment #825635 -
Flags: feedback?(dave.hunt)
Attachment #825660 -
Flags: feedback?(dave.hunt)
Comment 5•11 years ago
|
||
Comment on attachment 825660 [details] [diff] [review]
931781.patch
Review of attachment 825660 [details] [diff] [review]:
-----------------------------------------------------------------
Wow, thanks for coming up with a patch so quickly, this is a great start! I've added some comments, and I think you missed the timeouts from apps/base.py. Looking forward to seeing the next version!
::: tests/python/gaia-ui-tests/gaiatest/gaia_test.py
@@ +115,5 @@
> def runningApps(self):
> return self.marionette.execute_script("return GaiaApps.getRunningApps()")
>
> + def switch_to_frame(self, app_frame, url=None, timeout=None):
> + timeout = timeout or self.marionette.timeout or 30
Thinking about this, marionette.timeout is in milliseconds, so we will want to divide it by 1000 if it's set. Also, please take care not to add white space to the end of lines.
@@ -137,4 @@
> self.testvars = testvars or {}
> js = os.path.abspath(os.path.join(__file__, os.path.pardir, 'atoms', "gaia_data_layer.js"))
> self.marionette.import_script(js)
> - self.marionette.set_search_timeout(10000)
In this case we should be safe to remove this line. We'll be relying on Marionette's default of 10 seconds or use the value set by the --timeout argument.
@@ +162,5 @@
> result = self.marionette.execute_async_script('return GaiaDataLayer.insertContact(%s);' % json.dumps(contact), special_powers=True)
> assert result, 'Unable to insert contact %s' % contact
>
> + def remove_all_contacts(self, default_script_timeout=None):
> + default_script_timeout=default_script_timeout or self.marionette.default_script_timeout or 60000
I would remove this argument, and define timeout directly as:
timeout = max(self.marionette.timeout or 60000, 1000 * len(self.all_contacts))
@@ +167,2 @@
> self.marionette.switch_to_frame()
> self.marionette.set_script_timeout(max(default_script_timeout, 1000 * len(self.all_contacts)))
Let's remove this set_script_timeout as it means we have to then set it back. Instead we can set the timeout for just this call, by adding timeout=timeout as an argument for the execute_async_script call below.
@@ +416,4 @@
> self.marionette.wait_for_port()
> self.marionette.start_session()
> if self.is_android_build:
> + script_timeout= self.marionette.script_timeout or 60000
In this case, let's keep the hard-coded timeout. We're waiting for B2G to start, and that may take a while.
@@ +445,5 @@
> self.restart = kwargs.pop('restart', False)
> kwargs.pop('iterations', None)
> kwargs.pop('checkpoint_interval', None)
> MarionetteTestCase.__init__(self, *args, **kwargs)
> + _default_timeout=marionette._default_timeout
We shouldn't need this now.
@@ +467,5 @@
> # the emulator can be really slow!
> + _script_timeout=self.marionette._script_timeout or 60000
> + self.marionette.set_script_timeout(_script_timeout)
> + _search_timeout=self.marionette._search_timeout or 10000
> + self.marionette.set_search_timeout(_search_timeout)
We can remove these lines now and rely on the --timeout arg setting them.
@@ +616,5 @@
> def screen_orientation(self):
> return self.marionette.execute_script('return window.screen.mozOrientation')
>
> + def wait_for_element_present(self, by, locator, timeout=None):
> + timeout= timeout or self.marionette._default_timeout or 30
You need a space either side of the = character. Also, use self.marionette.timeout (divided by 1000 to get the seconds) if it's set rather than _default_timeout.
@@ +621,1 @@
> timeout = float(timeout) + time.time()
Rather than redefine timeout, we could rename this variable to end_time like it is in wait_for_condition.
Attachment #825660 -
Flags: feedback?(dave.hunt) → feedback+
Assignee | ||
Comment 6•11 years ago
|
||
Changed both gaia_test.py and apps/base.py
Attachment #825660 -
Attachment is obsolete: true
Attachment #825931 -
Flags: feedback?(dave.hunt)
Comment 7•11 years ago
|
||
Comment on attachment 825931 [details] [diff] [review]
931781v2.patch
Review of attachment 825931 [details] [diff] [review]:
-----------------------------------------------------------------
Mostly looks good. Please be careful not to introduce unwanted whitespace. We should be ready for a review soon, at which point you should submit a pull request so we get some results in Travis-CI.
::: tests/python/gaia-ui-tests/gaiatest/apps/base.py
@@ +26,5 @@
>
> + def wait_for_element_present(self, by, locator, timeout=None):
> + if self.marionette.timeout:
> + marionette_timeout = self.marionette.timeout/1000
> + timeout = timeout or marionette_timeout or 30
I don't like that we mix seconds with milliseconds, especially without documenting these methods. That said, we shouldn't change this as our tests rely on it. What I would suggest is to simplify this to one line:
timeout = timeout or (self.marionette.timeout and self.marionette.timeout / 1000) or 60
@@ +102,1 @@
> end_time = time.time() + timeout
Elsewhere we cast timeout to a float and have the components in the opposite order. Let's stay consistent.
::: tests/python/gaia-ui-tests/gaiatest/gaia_test.py
@@ +68,4 @@
> (app_name, permission_name, value))
>
> def launch(self, name, switch_to_frame=True, url=None, launch_timeout=None):
> + launch_timeout = launch_timeout or self.marionette.launch_timeout
Marionette does not have a 'launch_timeout' attribute. In this case I would just use the passed timeout, so you can remove this change.
@@ +162,4 @@
> result = self.marionette.execute_async_script('return GaiaDataLayer.insertContact(%s);' % json.dumps(contact), special_powers=True)
> assert result, 'Unable to insert contact %s' % contact
>
> + def remove_all_contacts(self):
Remove the trailing whitespace.
@@ +166,2 @@
> self.marionette.switch_to_frame()
> + self.marionette.set_script_timeout(max(self.marionette.timeout or 60000 , 1000 * len(self.all_contacts)))
We shouldn't use set_script_timeout anymore, we should define the value for the timeout, and then pass it was an argument for the execute_async_script call on the next line. For example:
timeout = max(self.marionette.timeout or 60000, 1000 * len(self.all_contacts))
result = self.marionette.execute_async_script('return GaiaDataLayer.removeAllContacts();', special_powers=True, script_timeout=timeout)
@@ +170,3 @@
>
> def get_setting(self, name):
> + return self.marionette.execute_async_script('return GaiaDataLayer.getSetting("%s")' % name, special_powers=True ,timeout = timeout)
This change is not needed as the default timeout will be used.
@@ +420,4 @@
> window.removeEventListener('mozbrowserloadend', loaded);
> marionetteScriptFinished();
> }
> +});""", 60000)
There should be no change here.
@@ +461,3 @@
> self.device.start_b2g()
>
> # the emulator can be really slow!
We can remove this comment too, as it was related to the timeout lines.
@@ +608,4 @@
> def screen_orientation(self):
> return self.marionette.execute_script('return window.screen.mozOrientation')
>
> + def wait_for_element_present(self, by, locator, timeout=None):
Remove trailing whitespace.
Attachment #825931 -
Flags: feedback?(dave.hunt) → feedback+
Assignee | ||
Comment 8•11 years ago
|
||
Removed the trailing white spaces , fixed the problem with spaces and tabs and also modified the files as mentioned in comment.
Attachment #825931 -
Attachment is obsolete: true
Attachment #828636 -
Flags: feedback?(dave.hunt)
Assignee | ||
Comment 9•11 years ago
|
||
Pointer to Github pull-request
Assignee | ||
Updated•11 years ago
|
Attachment #828771 -
Flags: review?(dave.hunt)
Assignee | ||
Comment 10•11 years ago
|
||
Pointer to Github pull-request
Assignee | ||
Comment 11•11 years ago
|
||
Comment on attachment 828774 [details]
Pointer to Github pull request: https://github.com/mozilla-b2g/gaia/pull/13478#attch-to-bugzilla
Pointer to Pull Request : https://github.com/mozilla-b2g/gaia/pull/13478
Attachment #828774 -
Flags: review?(dave.hunt)
Updated•11 years ago
|
Attachment #828774 -
Attachment is obsolete: true
Attachment #828774 -
Flags: review?(dave.hunt)
Updated•11 years ago
|
Attachment #828636 -
Attachment is obsolete: true
Attachment #828636 -
Flags: feedback?(dave.hunt)
Comment 12•11 years ago
|
||
Comment on attachment 828771 [details]
Pointer to Github pull request: https://github.com/mozilla-b2g/gaia/pull/13478
This is really close now! Just a few comments added to the pull request. I also think it's worth getting Zac's feedback on your next patch version.
Attachment #828771 -
Flags: review?(dave.hunt) → review-
Updated•11 years ago
|
Status: NEW → ASSIGNED
Assignee | ||
Comment 13•11 years ago
|
||
Pointer to Github pull-request
Assignee | ||
Comment 14•11 years ago
|
||
Comment on attachment 830155 [details]
Pointer to Github pull request: https://github.com/mozilla-b2g/gaia/pull/13478
:zac i made the changes as mentioned on the pull request , squashed and rebased the commits , please have a look at the pull request.
Attachment #830155 -
Flags: feedback?(zcampbell)
Reporter | ||
Comment 15•11 years ago
|
||
Comment on attachment 830155 [details]
Pointer to Github pull request: https://github.com/mozilla-b2g/gaia/pull/13478
Issue with default timeout/implicit wait, see comments in pull.
I'd also like to see some basic documentation in the readme.md. Just declare that the command line exists and recommend to use seconds or microseconds.
Re-mark the f? when it's done, I am a fan of this pull :)
Attachment #830155 -
Flags: feedback?(zcampbell) → feedback-
Comment 16•11 years ago
|
||
I agree with Zac that we should mention the timeout in the README, but it's not good that we should expect failures if no timeout is specified. Zac: Would you object to setting the search timeout to 10 seconds if none has been set on the command line? We would do this just once per session in setUp.
Other that that, we should add a --timeout=10000 to the Travis script here: https://github.com/mozilla-b2g/gaia/blob/a04340f5d52ef95008ae0199b770ebd773bdcc96/tests/travis_ci/gaia_ui_tests/script#L14 and add a higher timeout for device testing in Jenkins, but that would be separate from this patch.
Flags: needinfo?(zcampbell)
Reporter | ||
Comment 17•11 years ago
|
||
+1, +1 and +1!
10 second default I think should be reasonably even for device.
I am comfortable because I think in most cases where we need a larger timeout we have overridden the default in the test/page object itself (eg waiting for a SMS).
Agree let's do the script updates separately.
Flags: needinfo?(zcampbell)
Assignee | ||
Comment 18•11 years ago
|
||
Please have a look at this patch and suggest changes if any
Attachment #831560 -
Flags: feedback?(dave.hunt)
Flags: needinfo?(zcampbell)
Updated•11 years ago
|
Attachment #828771 -
Attachment is obsolete: true
Updated•11 years ago
|
Attachment #830155 -
Attachment is obsolete: true
Comment 19•11 years ago
|
||
Comment on attachment 831560 [details] [diff] [review]
931781v4.patch
Please don't attach a new patch, but add this to the open pull request.
+ --timeout < time in milliseconds > to set the default timeout
We should clarify that this sets the page load timeout, the search timeout, and the script timeout. We should also mention the the defaults if this is not specified is 30s for page load, 10s for search, and 10s for script.
+ def setUp(self, timeout=None):
+ _search_timeout = timeout or self.marionette.timeout or 10000
+ self.marionette.set_search_timeout(_search_timeout)
Have you tested this? I think this is before we have a Marionette session so suspect this would fail. Let's restore the line at https://github.com/mozilla-b2g/gaia/blob/1b5247170863865327ea31480bec5469a747d435/tests/python/gaia-ui-tests/gaiatest/gaia_test.py#L491
Also, we shouldn't add a timeout argument to setUp. In fact, we only need to set this if nothing has been specified on the command line. So it should look something like:
if not self.marionette.timeout:
self.marionette.set_search_timeout(10000)
You also pointed out on IRC that we're forcing the search timeout in is_element_present here: https://github.com/mozilla-b2g/gaia/blob/1b5247170863865327ea31480bec5469a747d435/tests/python/gaia-ui-tests/gaiatest/apps/base.py#L108
We should change this so we set it to self.marionette.timeout if it's set, falling back to 10000. It's not ideal to have this value hard-coded in two places, but I think we'll have to live with it for now.
Attachment #831560 -
Flags: feedback?(dave.hunt) → feedback-
Assignee | ||
Comment 20•11 years ago
|
||
Pointer to Github pull-request
Assignee | ||
Comment 21•11 years ago
|
||
Comment on attachment 831859 [details]
Pointer to Github pull request: https://github.com/mozilla-b2g/gaia/pull/13478
:davehunt please have a look at the last commit that i pushed , if the pull looks fine to you i may squash and rebase my commits.
Attachment #831859 -
Flags: review?(dave.hunt)
Updated•11 years ago
|
Attachment #831560 -
Attachment is obsolete: true
Comment 22•11 years ago
|
||
Comment on attachment 831859 [details]
Pointer to Github pull request: https://github.com/mozilla-b2g/gaia/pull/13478
This looks great! I've triggered an ad-hoc device testrun here: http://qa-selenium.mv.mozilla.com:8080/view/B2G%20Hamachi/job/b2g.hamachi.mozilla-central.ui.adhoc/42/ (unfortunately you won't be able to see this without VPN access) if the results are comparable to without the patch applied then we should merge. In the meantime, please rebase and squash your commits.
Attachment #831859 -
Flags: review?(dave.hunt) → review+
Updated•11 years ago
|
Flags: needinfo?(zcampbell)
Updated•11 years ago
|
Flags: needinfo?(dave.hunt)
Comment 23•11 years ago
|
||
Okay, the results are in and it looks good. Most of the failures are known intermittents, but there are a couple of timeouts that we can probably address in this patch.
It appears that we're failing to connect to WiFi within the default timeout, which isn't too surprising at 10 seconds. Let's increase the timeout to 60 seconds or marionette.timeout (whichever is greater).
This would be added here: https://github.com/mozilla-b2g/gaia/blob/61ac5d3bc764e66dd35eb1d43b74282b1b0232c2/tests/python/gaia-ui-tests/gaiatest/gaia_test.py#L269 as a script_timeout argument. I would use max(self.marionette.timeout, 60000) as the timeout value so it can still be overridden on the command line but would be no less than 60 seconds.
Similarly, we should do this for the WiFi UI test, here: https://github.com/mozilla-b2g/gaia/blob/61ac5d3bc764e66dd35eb1d43b74282b1b0232c2/tests/python/gaia-ui-tests/gaiatest/apps/settings/regions/wifi.py#L45 adding a timeout argument (note that this is in seconds, not milliseconds).
Flags: needinfo?(dave.hunt)
Assignee | ||
Comment 24•11 years ago
|
||
Pointer to Github pull-request
Assignee | ||
Comment 25•11 years ago
|
||
Comment on attachment 832309 [details]
Pointer to Github pull request: https://github.com/mozilla-b2g/gaia/pull/13478
:davehunt , made the changes please have a look at pull , and suggest changes if any
Attachment #832309 -
Flags: review?(dave.hunt)
Reporter | ||
Comment 26•11 years ago
|
||
Hey bitgeeky I just noticed that your pull has merge conflicts.
If you rebase it before Dave reviews it next he'll have an easier time.
Comment 27•11 years ago
|
||
Comment on attachment 832309 [details]
Pointer to Github pull request: https://github.com/mozilla-b2g/gaia/pull/13478
See comments in pull request. When resubmitting for review you can just change the previous r- to a r? if the location of the patch has not changed. If you create a new attachment please be sure to mark any obsolete ones appropriately.
Attachment #832309 -
Flags: review?(dave.hunt) → review-
Updated•11 years ago
|
Attachment #831859 -
Attachment is obsolete: true
Assignee | ||
Comment 28•11 years ago
|
||
Pointer to Github pull-request
Assignee | ||
Updated•11 years ago
|
Attachment #832309 -
Attachment is obsolete: true
Assignee | ||
Comment 29•11 years ago
|
||
Comment on attachment 8335251 [details]
Pointer to Github pull request: https://github.com/mozilla-b2g/gaia/pull/13478/files
made the change , according to the comment on the pull
Attachment #8335251 -
Flags: review?(dave.hunt)
Comment 30•11 years ago
|
||
Comment on attachment 8335251 [details]
Pointer to Github pull request: https://github.com/mozilla-b2g/gaia/pull/13478/files
See comment in pull request. Please avoid squashing your commits in your next push so I can see your fix without checking the full diff.
Attachment #8335251 -
Flags: review?(dave.hunt) → review-
Comment 31•11 years ago
|
||
Comment on attachment 8335251 [details]
Pointer to Github pull request: https://github.com/mozilla-b2g/gaia/pull/13478/files
Apologies, I couldn't see what was right there! :) Let's run an adhoc job and see how it goes.
Attachment #8335251 -
Flags: review- → review+
Comment 32•11 years ago
|
||
I've triggered an adhoc job: http://qa-selenium.mv.mozilla.com:8080/view/B2G%20Hamachi/job/b2g.hamachi.mozilla-central.ui.adhoc/47/console
Updated•11 years ago
|
Flags: needinfo?(dave.hunt)
Comment 33•11 years ago
|
||
I noticed this failure:
Traceback (most recent call last):
File "/var/jenkins/workspace/b2g.hamachi.mozilla-central.ui.adhoc/.env/local/lib/python2.7/site-packages/marionette_client-0.6.2-py2.7.egg/marionette/marionette_test.py", line 143, in run
testMethod()
File "/var/jenkins/workspace/b2g.hamachi.mozilla-central.ui.adhoc/tests/python/gaia-ui-tests/gaiatest/tests/functional/settings/test_settings_wifi.py", line 26, in test_connect_to_wifi_via_settings_app
wifi_settings.connect_to_network(self.testvars['wifi'])
File "/var/jenkins/workspace/b2g.hamachi.mozilla-central.ui.adhoc/tests/python/gaia-ui-tests/gaiatest/apps/settings/regions/wifi.py", line 46, in connect_to_network
timeout = max(self.marionette.timeout / 1000, 60))
TypeError: unsupported operand type(s) for /: 'NoneType' and 'int'
Here we're trying to divide a None object by 1000. You could fix this by first checking if the timeout is set before converting it to seconds:
timeout = max(self.marionette.timeout and self.marionette.timeout / 1000, 60))
Flags: needinfo?(dave.hunt)
Comment 34•11 years ago
|
||
Comment on attachment 8335251 [details]
Pointer to Github pull request: https://github.com/mozilla-b2g/gaia/pull/13478/files
Flipping review state due to failure.
Attachment #8335251 -
Flags: review+ → review-
Assignee | ||
Comment 35•11 years ago
|
||
Comment on attachment 8335251 [details]
Pointer to Github pull request: https://github.com/mozilla-b2g/gaia/pull/13478/files
Updated the Pull
Attachment #8335251 -
Flags: review- → review?(dave.hunt)
Comment 36•11 years ago
|
||
Running another adhoc job: http://qa-selenium.mv.mozilla.com:8080/view/B2G Hamachi/job/b2g.hamachi.mozilla-central.ui.adhoc/51
Zac: when it's done could you look over any failures and comment if any are suspicious? We may need to add a --timeout to Jenkins if 10 seconds is not long enough for our waits.
Flags: needinfo?(zcampbell)
Reporter | ||
Comment 37•11 years ago
|
||
Dave, I've run it again because last week's build was poor. Keeping the ni?
Comment 38•11 years ago
|
||
Okay, just keep in mind that the branch we're running against will be out of date with master.
Reporter | ||
Comment 39•11 years ago
|
||
(In reply to Dave Hunt (:davehunt) from comment #36)
> Zac: when it's done could you look over any failures and comment if any are
> suspicious? We may need to add a --timeout to Jenkins if 10 seconds is not
> long enough for our waits.
For all of the failures I don't recognise there appears to be a pattern around wifi failing to connect. I feel like we should push them up to 30 seconds but for UI interactions 10 would be better.
Thus I'm happy to take on this patch with --timeout=30 for Jenkins as you suggest and then tune the wifi timin in the future and reduce the Jenkins back to --timeout=10
One issue I have though, why can't you use a canonical/global variable for what the marionette timeout is in seconds so that we can refer to it from within page objects?
It is calculated every time in gaia_test.py and we'd have to do the same in the page object. Every time it just introduces a chance for it to be messed up or done wrong.
Flags: needinfo?(zcampbell)
Comment 40•11 years ago
|
||
(In reply to Zac C (:zac) from comment #39)
> One issue I have though, why can't you use a canonical/global variable for
> what the marionette timeout is in seconds so that we can refer to it from
> within page objects?
> It is calculated every time in gaia_test.py and we'd have to do the same in
> the page object. Every time it just introduces a chance for it to be messed
> up or done wrong.
It's only necessary to convert it from milliseconds because the wait_for methods in gaiatest expect seconds. Ideally we'd just use milliseconds, but changing this now would be rather disruptive. When we have a Wait support class we could perhaps switch to milliseconds and deprecate the wait_for methods.
We could provide a method such as timeout_in_seconds that takes a marionette instance and returns the timeout converted to seconds, however there's still a chance for this not to be called and the marionette.timeout is used directly. Also, such a method would need to be duplicated in the test base and page base classes.
Reporter | ||
Comment 41•11 years ago
|
||
(In reply to Dave Hunt (:davehunt) from comment #40)
> We could provide a method such as timeout_in_seconds that takes a marionette
> instance and returns the timeout converted to seconds, however there's still
> a chance for this not to be called and the marionette.timeout is used
> directly. Also, such a method would need to be duplicated in the test base
> and page base classes.
Couldn't you just do it once in the setUp?
I probably didn't describe the use case very well. Here is an example:
https://github.com/mozilla-b2g/gaia/blob/master/tests/python/gaia-ui-tests/gaiatest/apps/browser/app.py#L50
How would we change that to use the default timeout from the command line in the python method declaration yet be overridden from the test itself when required?
Comment 42•11 years ago
|
||
(In reply to Zac C (:zac) from comment #41)
> (In reply to Dave Hunt (:davehunt) from comment #40)
> > We could provide a method such as timeout_in_seconds that takes a marionette
> > instance and returns the timeout converted to seconds, however there's still
> > a chance for this not to be called and the marionette.timeout is used
> > directly. Also, such a method would need to be duplicated in the test base
> > and page base classes.
>
> Couldn't you just do it once in the setUp?
Where would we store it that would be available to test cases and page objects alike? It's always available from the marionette object, and that's where we're currently getting it from.
> I probably didn't describe the use case very well. Here is an example:
> https://github.com/mozilla-b2g/gaia/blob/master/tests/python/gaia-ui-tests/
> gaiatest/apps/browser/app.py#L50
>
> How would we change that to use the default timeout from the command line in
> the python method declaration yet be overridden from the test itself when
> required?
That's ultimately calling wait_for_condition, which with this patch will now wait for the duration specified on the command line, or a default of 30 seconds, or the value passed to it - in this case 30 seconds. Doesn't this achieve what you need?
Reporter | ||
Comment 43•11 years ago
|
||
>
> That's ultimately calling wait_for_condition, which with this patch will now
> wait for the duration specified on the command line, or a default of 30
> seconds, or the value passed to it - in this case 30 seconds. Doesn't this
> achieve what you need?
Ok so you would just pass in None and get the default timeout, gotcha!
Updated•11 years ago
|
Attachment #8335251 -
Flags: review?(dave.hunt) → review+
Comment 44•11 years ago
|
||
Landed in:
https://github.com/mozilla-b2g/gaia/commit/47f77aafd283485885ed59ea636326706f21a4b0 (master)
https://github.com/mozilla-b2g/gaia/commit/9e7309f802225fccfd6b850f5b955be75cb060ec (v1.2)
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
status-b2g-v1.2:
--- → fixed
status-firefox28:
--- → fixed
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•