Merge lp:~gary/html5-browser/incremental-timeouts into lp:html5-browser

[Gary Poster]

This adds incremental timeouts, per the discussion we had. My intent is that, for us, the Javascript will report the incremental results when the test suite starts, and after every test pass or fail (the YUI test runner does not provide events when a test suite starts). I have not tried this actually with LP yet, but it looks like this is what we need on the html5-browser side.

pocketlint appears to be happy (I tried ``pocketlint .`` and then ``pocketlint html5browser/__init__.py html5browser/tests/test_browser.py`` for good measure)

Thanks, Curtis

Gary

[lp:~gary/html5-browser/incremental-timeouts]

28. By Gary Poster on 2011-10-17

make it possible to use html5browser.main as a setuptools console script entry point.

[Curtis Hovey (sinzui)]

Wow! This is great. Thank you very much for solving this problem. Can you move the statement to clear the incremental timeout (line 107) to the top of the block (line 105) because I am paranoid that a timeout can happen when the code has determined all is okay.

I'll merge this and make a release when I get your reply.

review: Approve (code)

[lp:~gary/html5-browser/incremental-timeouts]

29. By Gary Poster on 2011-10-17

clear the incremental timeout first, before clearing status, for safety.

[Gary Poster (gary)]

Cool, thanks.

Yeah, that's reasonable paranoia. I have made the change and pushed.

Thanks again!

=== modified file 'html5browser/__init__.py'
--- html5browser/__init__.py	2011-06-24 16:34:41 +0000
+++ html5browser/__init__.py	2011-10-17 14:19:58 +0000
@@ -74,6 +74,9 @@
 
     STATUS_PREFIX = '::::'
     TIMEOUT = 5000
+    INCREMENTAL_PREFIX = '>>>>'
+    INITIAL_TIMEOUT = None
+    INCREMENTAL_TIMEOUT = None
 
     def __init__(self, show_window=False, hide_console_messages=True,
                  force_internal=False):
@@ -87,21 +90,30 @@
         self.listeners = {}
         self._connect('console-message', self._on_console_message, False)
 
-    def load_page(self, uri, timeout=TIMEOUT):
+    def load_page(self, uri,
+                  timeout=TIMEOUT,
+                  initial_timeout=INITIAL_TIMEOUT,
+                  incremental_timeout=INCREMENTAL_TIMEOUT):
         """Load a page and return the content."""
         if REQUIRES_EXTERNAL and not self.force_internal:
-            self.run_external_browser(uri, timeout)
+            self.run_external_browser(
+                uri, timeout, initial_timeout, incremental_timeout)
         else:
-            self._setup_listening_operation(timeout)
+            self._setup_listening_operation(
+                timeout, initial_timeout, incremental_timeout)
             if uri.startswith('/'):
                 uri = 'file://' + uri
             self.load_uri(uri)
             Gtk.main()
         return self.command
 
-    def run_script(self, script, timeout=TIMEOUT):
+    def run_script(self, script,
+                   timeout=TIMEOUT,
+                   initial_timeout=INITIAL_TIMEOUT,
+                   incremental_timeout=INCREMENTAL_TIMEOUT):
         """Run a script and return the result."""
-        self._setup_listening_operation(timeout)
+        self._setup_listening_operation(
+            timeout, initial_timeout, incremental_timeout)
         self.script = script
         self._connect('notify::load-status', self._on_script_load_finished)
         self.load_string(
@@ -110,10 +122,16 @@
         Gtk.main()
         return self.command
 
-    def run_external_browser(self, uri, timeout):
+    def run_external_browser(self, uri, timeout,
+                             initial_timeout=None, incremental_timeout=None):
         """Load the page and run the script in an external process."""
         self.command = Command()
-        command_line = ['python', HERE, '-t', str(timeout), uri]
+        command_line = ['python', HERE, '-t', str(timeout)]
+        if initial_timeout is not None:
+            command_line.extend(['-i', str(initial_timeout)])
+        if incremental_timeout is not None:
+            command_line.extend(['-s', str(incremental_timeout)])
+        command_line.append(uri)
         browser = subprocess.Popen(
             command_line, stdout=subprocess.PIPE, stderr=subprocess.PIPE)
         content, ignore = browser.communicate()
@@ -131,13 +149,23 @@
             '\\', '\\\\').replace('"', '\\"').replace("'", "\\'").replace(
             '\n', '\\n')
 
-    def _setup_listening_operation(self, timeout):
+    def _setup_listening_operation(self, timeout, initial_timeout,
+                                   incremental_timeout):
         """Setup a one-time listening operation for command's completion."""
         self._create_window()
         self.command = Command()
+        self._last_status = None
+        self._incremental_timeout = incremental_timeout
         self._connect(
             'status-bar-text-changed', self._on_status_bar_text_changed)
-        GObject.timeout_add(timeout, self._on_timeout)
+        self._timeout_source = GObject.timeout_add(timeout, self._on_timeout)
+        if initial_timeout is None:
+            initial_timeout = incremental_timeout
+        if initial_timeout is not None:
+            self._incremental_timeout_source = GObject.timeout_add(
+                initial_timeout, self._on_timeout)
+        else:
+            self._incremental_timeout_source = None
 
     def _create_window(self):
         """Create a window needed to render pages."""
@@ -155,10 +183,22 @@
     def _on_quit(self, widget=None):
         Gtk.main_quit()
 
+    def _clear_status(self):
+            self.execute_script('window.status = "";')
+
     def _on_status_bar_text_changed(self, view, text):
-        if text.startswith(self.STATUS_PREFIX):
+        if text.startswith(self.INCREMENTAL_PREFIX):
+            self._clear_incremental_timeout()
+            self._clear_status()
+            self._last_status = text[4:]
+            if self._incremental_timeout:
+                self._incremental_timeout_source = GObject.timeout_add(
+                    self._incremental_timeout, self._on_timeout)
+        elif text.startswith(self.STATUS_PREFIX):
+            self._clear_timeout()
+            self._clear_incremental_timeout()
             self._disconnect('status-bar-text-changed')
-            self.execute_script('window.status = "";')
+            self._clear_status()
             self.command.status = Command.STATUS_COMPLETE
             self.command.return_code = Command.CODE_SUCCESS
             self.command.content = text[4:]
@@ -173,11 +213,24 @@
         self.execute_script(self.script)
         self.script = None
 
+    def _clear_incremental_timeout(self):
+        if self._incremental_timeout_source is not None:
+            GObject.source_remove(self._incremental_timeout_source)
+            self._incremental_timeout_source = None
+
+    def _clear_timeout(self):
+        if self._timeout_source is not None:
+            GObject.source_remove(self._timeout_source)
+            self._timeout_source = None
+
     def _on_timeout(self):
+        self._clear_timeout()
+        self._clear_incremental_timeout()
         if self.command.status is not Command.STATUS_COMPLETE:
             self._disconnect()
             self.command.status = Command.STATUS_COMPLETE
             self.command.return_code = Command.CODE_FAIL
+            self.command.content = self._last_status
             self._on_quit()
         return False
 
@@ -200,18 +253,24 @@
 from optparse import OptionParser
 
 
-def main(argv):
+def main(argv=None):
     """Load a page an return the result set by a page script."""
+    if argv is None:
+        argv = sys.argv
     (options, uri) = parser_options(args=argv[1:])
     client = Browser(force_internal=True)
-    page = client.load_page(uri, timeout=options.timeout)
+    page = client.load_page(uri, timeout=options.timeout,
+                            initial_timeout=options.initial_timeout,
+                            incremental_timeout=options.incremental_timeout)
+    has_page_content = page.content is not None and page.content.strip() != ''
+    if has_page_content:
+        print page.content
     if page.return_code == page.CODE_FAIL:
         sys.exit(1)
-    elif page.content is None or page.content.strip() == '':
-        # Did not get a report back.
+    elif not has_page_content:
         sys.exit(2)
-    print page.content
-    sys.exit(0)
+    else:
+        sys.exit(0)
 
 
 def parser_options(args):
@@ -224,6 +283,10 @@
     parser = OptionParser(usage=usage, epilog=epilog)
     parser.add_option(
         "-t", "--timeout", type="int", dest="timeout")
+    parser.add_option(
+        "-i", "--initial-timeout", type="int", dest="initial_timeout")
+    parser.add_option(
+        "-s", "--test-timeout", type="int", dest="incremental_timeout")
     parser.set_defaults(timeout=Browser.TIMEOUT)
     (options, uris) = parser.parse_args(args)
     if len(uris) != 1:
@@ -232,4 +295,4 @@
 
 
 if __name__ == '__main__':
-    main(sys.argv)
+    main()
 
=== modified file 'html5browser/tests/test_browser.py'
--- html5browser/tests/test_browser.py	2011-06-23 23:29:42 +0000
+++ html5browser/tests/test_browser.py	2011-10-17 14:19:58 +0000
@@ -19,6 +19,15 @@
     </head><body></body></html>
     """
 
+incremental_timeout_page = """\
+    <html><head>
+    <script type="text/javascript">
+    window.status = '>>>>shazam';
+    </script>
+    </head><body></body></html>
+    """
+
+
 load_page_set_window_status_ignores_non_commands = """\
     <html><head>
     <script type="text/javascript">
@@ -35,6 +44,16 @@
     <html><head></head><body></body></html>
     """
 
+initial_long_wait_page = """\
+    <html><head>
+    <script type="text/javascript">
+    setTimeout(function() {
+      window.status = '>>>>initial';
+      setTimeout(function() {window.status = '::::ended'}, 200);
+    }, 1000);
+    </script>
+    </head><body></body></html>"""
+
 
 class BrowserTestCase(unittest.TestCase):
     """Verify Browser methods."""
@@ -93,6 +112,71 @@
         self.assertEqual(Command.CODE_SUCCESS, command.return_code)
         self.assertEqual('pting', command.content)
 
+    def test_load_page_initial_timeout(self):
+        # If a initial_timeout is set, it can cause a timeout.
+        self.file.write(timeout_page)
+        self.file.flush()
+        browser = Browser()
+        command = browser.load_page(
+            self.file.name, initial_timeout=1000, timeout=30000)
+        self.assertEqual(Command.STATUS_COMPLETE, command.status)
+        self.assertEqual(Command.CODE_FAIL, command.return_code)
+
+    def test_load_page_incremental_timeout(self):
+        # If an incremental_timeout is set, it can cause a timeout.
+        self.file.write(timeout_page)
+        self.file.flush()
+        browser = Browser()
+        command = browser.load_page(
+            self.file.name, incremental_timeout=1000, timeout=30000)
+        self.assertEqual(Command.STATUS_COMPLETE, command.status)
+        self.assertEqual(Command.CODE_FAIL, command.return_code)
+
+    def test_load_page_initial_timeout_has_precedence_first(self):
+        # If both an initial_timeout and an incremental_timeout are set,
+        # initial_timeout takes precedence for the first wait.
+        self.file.write(initial_long_wait_page)
+        self.file.flush()
+        browser = Browser()
+        command = browser.load_page(
+            self.file.name, initial_timeout=3000,
+            incremental_timeout=500, timeout=30000)
+        self.assertEqual(Command.STATUS_COMPLETE, command.status)
+        self.assertEqual(Command.CODE_SUCCESS, command.return_code)
+        self.assertEqual('ended', command.content)
+
+    def test_load_page_incremental_timeout_has_precedence_second(self):
+        # If both an initial_timeout and an incremental_timeout are set,
+        # incremental_timeout takes precedence for the second wait.
+        self.file.write(initial_long_wait_page)
+        self.file.flush()
+        browser = Browser()
+        command = browser.load_page(
+            self.file.name, initial_timeout=3000,
+            incremental_timeout=100, timeout=30000)
+        self.assertEqual(Command.STATUS_COMPLETE, command.status)
+        self.assertEqual(Command.CODE_FAIL, command.return_code)
+        self.assertEqual('initial', command.content)
+
+    def test_load_page_timeout_always_wins(self):
+        # If timeout, initial_timeout, and incremental_timeout are set,
+        # the main timeout will still be honored.
+        self.file.write(initial_long_wait_page)
+        self.file.flush()
+        browser = Browser()
+        command = browser.load_page(
+            self.file.name, initial_timeout=3000,
+            incremental_timeout=3000, timeout=100)
+        self.assertEqual(Command.STATUS_COMPLETE, command.status)
+        self.assertEqual(Command.CODE_FAIL, command.return_code)
+        self.assertEqual('', command.content)
+
+    def test_load_page_default_timeout_values(self):
+        # Verify our expected class defaults.
+        self.assertEqual(5000, Browser.TIMEOUT)
+        self.assertEqual(None, Browser.INITIAL_TIMEOUT)
+        self.assertEqual(None, Browser.INCREMENTAL_TIMEOUT)
+
     def test_load_page_timeout(self):
         # A page that does not set window.status in 5 seconds will timeout.
         self.file.write(timeout_page)
@@ -101,7 +185,16 @@
         command = browser.load_page(self.file.name, timeout=1000)
         self.assertEqual(Command.STATUS_COMPLETE, command.status)
         self.assertEqual(Command.CODE_FAIL, command.return_code)
-        self.assertEqual(5000, Browser.TIMEOUT)
+
+    def test_load_page_set_window_status_incremental_timeout(self):
+        # Any incremental information is returned on a timeout.
+        self.file.write(incremental_timeout_page)
+        self.file.flush()
+        browser = Browser()
+        command = browser.load_page(self.file.name, timeout=1000)
+        self.assertEqual(Command.STATUS_COMPLETE, command.status)
+        self.assertEqual(Command.CODE_FAIL, command.return_code)
+        self.assertEqual('shazam', command.content)
 
     def test_run_script_timeout(self):
         # A script that does not set window.status in 5 seconds will timeout.
 
=== modified file 'setup.py'
--- setup.py	2011-06-24 16:34:41 +0000
+++ setup.py	2011-10-17 14:19:58 +0000
@@ -20,7 +20,7 @@
 setup(
     name="html5browser",
     description="A HTML5 browser that can be driven by python code.",
-    version="0.0.8",
+    version="0.0.9",
     maintainer="Curtis C. Hovey",
     maintainer_email="[e-mail]",
     url="https://launchpad.net/html5-browser",