The default bug view has changed. See this FAQ.

Implement 'timeouts' command

RESOLVED FIXED in mozilla21

Status

Testing
Marionette
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: mdas, Assigned: Ann(Yiming))

Tracking

Trunk
mozilla21
x86
All
Points:
---
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(3 attachments, 1 obsolete attachment)

As described here: http://dvcs.w3.org/hg/webdriver/raw-file/tip/webdriver-spec.html#timeouts-1 and http://code.google.com/p/selenium/wiki/JsonWireProtocol#/session/:sessionId/timeouts, we need a command called 'timeouts' that takes in 'type' and 'ms' arguments so we can set either the script, pageload or implicit wait timeouts in one command.

We already have support for setting the timeout of script and implicit wait timeouts in the server, so now we just need to add support for page load and use the 'timeouts' function to call either of these methods.

I'm setting this bug aside as a ramping up project.
Blocks: 721859
Assignee: nobody → yiyang
No longer blocks: 721859
(Assignee)

Comment 1

4 years ago
Created attachment 700111 [details]
Adding timeout functionality
(Assignee)

Updated

4 years ago
Attachment #700111 - Flags: review?(mdas)
(Assignee)

Comment 2

4 years ago
Created attachment 700115 [details] [diff] [review]
Adding timeout functionality
Attachment #700111 - Attachment is obsolete: true
Attachment #700111 - Flags: review?(mdas)
Attachment #700115 - Flags: review?(mdas)
(Assignee)

Comment 3

4 years ago
Created attachment 700552 [details] [diff] [review]
timeout functionality update
Attachment #700552 - Flags: review?(mdas)
Comment on attachment 700552 [details] [diff] [review]
timeout functionality update

Review of attachment 700552 [details] [diff] [review]:
-----------------------------------------------------------------

Great patch.

I think you just need to undo the change you did to test_findelement.py, and fix up some naming/whitespace issues, and then I'll be ready to give you an r+ :) You just need to upload the new patch and r? me again.

::: testing/marionette/client/marionette/marionette.py
@@ +388,5 @@
>  
>      def navigate(self, url):
>          response = self._send_message('goUrl', 'ok', value=url)
>          return response
> +    

There's some extra whitespace here (4 spaces). Can you remove it?

::: testing/marionette/client/marionette/tests/unit/test_findelement.py
@@ +116,5 @@
>      def test_not_found(self):
>          test_html = self.marionette.absolute_url("test.html")
>          self.marionette.navigate(test_html)
>          self.marionette.set_search_timeout(1000)
> +        self.assertRaises(NoSuchElementException, self.marionette.findeseelement, "id", "I'm not on the page")

there's a typo here, "self.marionette.findeseelement". Are you able to run this test without errors? 

I don't think you want to change this file, either. I'd revert it and resubmit the patch.

::: testing/marionette/client/marionette/tests/unit/test_timeouts.py
@@ +6,5 @@
> +from marionette_test import MarionetteTestCase
> +from marionette import HTMLElement
> +from errors import NoSuchElementException
> +from errors import MarionetteException
> +from errors import JavascriptException, MarionetteException, ScriptTimeoutException

You can merge lines 8-10, so you can import everything in the same line.

@@ +30,5 @@
> +        self.marionette.timeouts("implicit", 1000)
> +        self.assertRaises(NoSuchElementException, self.marionette.find_element, "id", "I'm not on the page")
> +        self.marionette.timeouts("implicit", 0)
> +        self.assertRaises(NoSuchElementException, self.marionette.find_element, "id", "I'm not on the page") 
> +    

Trailing whitespace at the end of line 33 and the beginning of line 34

@@ +37,5 @@
> +        self.marionette.navigate(test_html)
> +        self.marionette.timeouts("implicit", 4000)
> +        self.assertEqual(HTMLElement, type(self.marionette.find_element("id", "newDiv")))
> +
> + 

Trailing whitespace

@@ +42,5 @@
> +    def test_searchtimeout_found(self):
> +        test_html = self.marionette.absolute_url("test.html")
> +        self.marionette.navigate(test_html)
> +        self.assertRaises(NoSuchElementException, self.marionette.find_element, "id", "newDiv")
> +    

Trailing whitespace

::: testing/marionette/marionette-actors.js
@@ +953,5 @@
>      function checkLoad() { 
> +       end = new Date().getTime();
> +       let elapse = end - start;
> +       if (this.pageTimeout == null || elapse <= this.pageTimeout){
> +          if (curWindow.document.readyState == "complete") { 

Trailing whitespace

@@ +957,5 @@
> +          if (curWindow.document.readyState == "complete") { 
> +             sendOk(command_id);
> +             return;
> +          }
> +          else{ 

Trailing whitespace

@@ +1235,5 @@
> +    if (isNaN(timeout)){
> +      this.sendError("Not a Number", 500, null, this.command_id);
> +    }
> +    else{
> +       if (timeout_type == "implicit"){   

There's extra whitespace trailing the {

@@ +1238,5 @@
> +    else{
> +       if (timeout_type == "implicit"){   
> +          aRequest.value = aRequest.ms;
> +          this.setSearchTimeout(aRequest);
> +          

Some more extra whitespace

@@ +1247,5 @@
> +       }
> +       else{
> +          this.pageTimeout = timeout;
> +          this.sendOk(this.command_id);
> +       } 

More whitespace trailing the }

::: testing/marionette/marionette-listener.js
@@ +553,5 @@
> +     }
> +     if (!event.originalTarget.defaultView.frameElement || 
> +          event.originalTarget.defaultView.frameElement == curWindow.frameElement) {
> +        removeEventListener("DOMContentLoaded", onDOMContentLoaded, false);
> +       

More extra whitespace

@@ +565,3 @@
>        }
> +  };
> +  function Timerfunc(){

Timerfunc should be timerFunc. We reserve capitalizing the first letter for "class" type objects.
Attachment #700552 - Flags: review?(mdas) → review-
(Assignee)

Comment 5

4 years ago
Created attachment 701219 [details] [diff] [review]
timeout functionality error/format fix
Attachment #701219 - Flags: review?(mdas)
Comment on attachment 701219 [details] [diff] [review]
timeout functionality error/format fix

Review of attachment 701219 [details] [diff] [review]:
-----------------------------------------------------------------

Great! I'll land this on m-i for you.
Attachment #701219 - Flags: review?(mdas) → review+
landed on m-i:

https://hg.mozilla.org/integration/mozilla-inbound/rev/ef384134776b
https://hg.mozilla.org/mozilla-central/rev/ef384134776b
Status: NEW → RESOLVED
Last Resolved: 4 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla21
Comment on attachment 700115 [details] [diff] [review]
Adding timeout functionality

Review of attachment 700115 [details] [diff] [review]:
-----------------------------------------------------------------

Reviewed a following patch.
Attachment #700115 - Flags: review?(mdas)
You need to log in before you can comment on or make changes to this bug.