Merge pull request #77 from zopefoundation/reload-follows-redirects

Fix missing HTTP status handling in .reload()
zopefoundation · Jul 12, 2019 · 61792b8 · 61792b8
2 parents 8edbf9b + 8f07706
commit 61792b8
Show file tree

Hide file tree

Showing 7 changed files with 123 additions and 10 deletions.
diff --git a/.coveragerc b/.coveragerc
@@ -4,3 +4,9 @@ source = zope.testbrowser
 
 [report]
 precision = 2
+
+[paths]
+source =
+    src/
+    .tox/*/lib/python*/site-packages/
+    .tox/pypy*/site-packages/
diff --git a/.gitignore b/.gitignore
@@ -18,3 +18,4 @@ parts
 dist
 docs/_build
 tags
+.coverage.*
diff --git a/CHANGES.rst b/CHANGES.rst
@@ -5,7 +5,9 @@ CHANGES
 5.3.4 (unreleased)
 ------------------
 
-- Nothing changed yet.
+- Fix a bug where browser.reload() would not follow redirects or raise
+  exceptions for bad HTTP statuses.  See `issue 75
+  <https://zopefoundation/zope.testbrowser/issue/75>`_.
 
 
 5.3.3 (2019-07-02)

diff --git a/src/zope/testbrowser/browser.py b/src/zope/testbrowser/browser.py
@@ -192,10 +192,10 @@ def reload(self):
         if self._response is None:
             raise BrowserStateError("no URL has yet been .open()ed")
 
-        req = self._response.request
-        with self._preparedRequest(self.url):
-            resp = self.testapp.request(req)
-            self._setResponse(resp)
+        def make_request(args):
+            return self.testapp.request(self._response.request)
+
+        self._processRequest(self.url, make_request)
 
     def goBack(self, count=1):
         """See zope.testbrowser.interfaces.IBrowser"""

diff --git a/src/zope/testbrowser/fixed-bugs.txt b/src/zope/testbrowser/fixed-bugs.txt
@@ -111,7 +111,7 @@ The problem is that e.g. a simple 403 status raises an exception.
 
 This is how it works with a simple open():
 
-    >>> browser.handleErrors=False
+    >>> browser.handleErrors = False
 
     >>> browser.open('http://localhost/set_status.html')
     >>> print(browser.contents)

diff --git a/src/zope/testbrowser/tests/test_browser.py b/src/zope/testbrowser/tests/test_browser.py
@@ -240,10 +240,89 @@ def test_accept_language_header_non_us():
     """
 
 
+def test_redirect_after_reload():
+    r"""
+    When browser is redirected after a page reload, reload() will follow it
+
+    >>> app = YetAnotherTestApp()
+    >>> browser = Browser(wsgi_app=app)
+    >>> html = (b'''\
+    ... <html><body>
+    ...   Please wait, generating the thing
+    ... </body></html>
+    ... ''')
+    >>> content_type = ('Content-Type', 'text/html; charset=UTF-8')
+    >>> app.add_response(html, headers=[content_type])
+
+    >>> redirect = ('Location', 'http://localhost/the_thing')
+    >>> app.add_response(b"Moved", headers=[redirect],
+    ...                  status=302, reason='Found')
+    >>> app.add_response(b"The Thing", headers=[content_type])
+
+    Start conversation
+
+    >>> browser.open("http://localhost/")
+
+    After reload, expect the browser to be redirected
+
+    >>> browser.reload()
+    >>> browser.url
+    'http://localhost/the_thing'
+    >>> browser.contents
+    'The Thing'
+
+    """
+
+
+def test_error_after_reload():
+    r"""
+    When browser is redirected after a page reload, reload() will check
+    for bad HTTP status codes
+
+    >>> app = YetAnotherTestApp()
+    >>> browser = Browser(wsgi_app=app)
+    >>> browser.handleErrors = False
+    >>> html = (b'''\
+    ... <html><body>
+    ...   Please wait, generating the thing
+    ... </body></html>
+    ... ''')
+    >>> content_type = ('Content-Type', 'text/html; charset=UTF-8')
+    >>> app.add_response(html, headers=[content_type])
+
+    >>> app.add_response(b"These are not the droids you're looking for",
+    ...                  status=403, reason='Forbidden')
+
+    Start conversation
+
+    >>> browser.open("http://localhost/")
+
+    After reload, expect the error to be raised
+
+    XXX: I expected
+
+    ## >>> browser.reload()
+    ## Traceback (most recent call last):
+    ##   ...
+    ## HTTPError: HTTP Error 403: Forbidden
+
+    which is what the tests in fixed-bugs.txt get, but what I actually get
+    instead is
+
+    >>> browser.reload()
+    Traceback (most recent call last):
+      ...
+    webtest.app.AppError: Bad response: 403 Forbidden
+    (not 200 OK or 3xx redirect for http://localhost/)
+    These are not the droids you're looking for
+
+    """
+
+
 def test_reload_after_redirect():
     """
     When browser is redirected after form submit, reload() will not resubmit
-    oroginal form data.
+    original form data.
 
     >>> app = YetAnotherTestApp()
     >>> browser = Browser(wsgi_app=app)
@@ -1259,12 +1338,16 @@ def test_additional_hidden_element_with_by_label_search():
 
 
 def test_suite():
-    suite = unittest.TestSuite()
-    suite.addTests([
+    optionflags = (
+        doctest.NORMALIZE_WHITESPACE
+        | doctest.ELLIPSIS
+        | doctest.IGNORE_EXCEPTION_DETAIL
+    )
+    suite = unittest.TestSuite([
         unittest.defaultTestLoader.loadTestsFromName(__name__),
         doctest.DocTestSuite(
             checker=zope.testbrowser.tests.helper.checker,
-            optionflags=doctest.NORMALIZE_WHITESPACE | doctest.ELLIPSIS),
+            optionflags=optionflags),
     ])
     return suite
 

diff --git a/tox.ini b/tox.ini
@@ -16,6 +16,27 @@ deps =
     coverage
 commands =
     coverage run -m zope.testrunner --test-path=src {posargs:-vc}
+setenv =
+    COVERAGE_FILE=.coverage.{envname}
+
+[testenv:coverage-report]
+basepython = python3.6
+deps = coverage
+skip_install = true
+commands =
+    coverage erase
+    coverage combine
+    coverage report -m
+setenv =
+    COVERAGE_FILE=.coverage
+parallel_show_output = true
+depends =
+    py27,
+    py35,
+    py36,
+    py37,
+    pypy,
+    pypy3,
 
 [testenv:docs]
 basepython =