4teamwork / ftw.testbrowser

A testing browser for Plone.
http://ftwtestbrowser.readthedocs.io/
5 stars 1 forks source link

Drivers not initialized in all cases #46

Closed lukasgraf closed 7 years ago

lukasgraf commented 7 years ago

Apparently, the drivers (Browser.drivers) only get initialized during Browser.get_driver().

This will be done early enough (for example, during Browser.open()) in most cases, but not all of them. Browser.login() for example doesn't issue a self.get_driver(), and drivers therefore aren't initialized after login(), leading to surprising behavior.

For example, this doesn't work as expected (doesn't clear the header):

@browsing
def test_foo(self, browser):
        browser.login()
        browser.clear_request_header('X-zope-handle-errors')

I think the logic to actually initialize drivers and get the current one should probably be separated. The drivers then should be initialized explicitly at an appropriate point. (Maybe Browser.__enter__?) I'm not too sure about the details of the lifecycle of the Browser / drivers, maybe a different place is more appropriate.

/cc @jone

jone commented 7 years ago

Historically it is possible to switch the driver within the context manager:

with browser:
    browser.request_library = LIB_REQUESTS
    browser.open()

As long as we maintain backwards compatibility regarding the requests_library attribute we do not know at the time we __enter__ the context manager which driver will be used. In fact it is even possible to switch / mix drivers within a test.

The request headers are therefore "cached" in the browser instance and applied to new driver instances uppon initialization. Since login is only an Authorization request header, this applies to login too.

The issue with the example above is that the X-zope-handle-errors header is added by default by the driver in order to have a good default behavior. This header does only make sense when using the mechdriver.

Todo: If the X-zope-handle-errors is cleared before the mechbrowser is instantiated, the browser must ensure to clear the header after instantiating the driver at any time.

jone commented 7 years ago

I'm actually quite confused about the X-zope-handle-errors header. It seems that we always have an exception with the mechanize browser:

    @browsing
    def test_handle_errors_header(self, browser):
        browser.open()
        browser.replace_request_header('X-zope-handle-errors', 'False')  # <<-- ftw.testbrowser default
        browser.open(view='lets-produce-a-404')

⬇️

Traceback (most recent call last):
...
  File "/Users/jone/projects/python/cache/eggs/Zope2-2.13.24-py2.7.egg/ZPublisher/BaseRequest.py", line 525, in traverse
    return response.notFoundError(URL)
  File "/Users/jone/projects/python/cache/eggs/Zope2-2.13.24-py2.7.egg/ZPublisher/HTTPResponse.py", line 718, in notFoundError
    "<p><b>Resource:</b> %s</p>" % escape(entry))
NotFound:   <h2>Site Error</h2>
  <p>An error was encountered while publishing this resource.
  </p>
  <p><strong>Resource not found</strong></p>

    @browsing
    def test_handle_errors_header(self, browser):
        browser.open()
        browser.replace_request_header('X-zope-handle-errors', 'True')
        browser.open(view='lets-produce-a-404')

⬇️

Traceback (most recent call last):
...
  File "/Users/jone/projects/python/cache/eggs/mechanize-0.2.5-py2.7.egg/mechanize/_mechanize.py", line 203, in open
    return self._mech_open(url, data, timeout=timeout)
  File "/Users/jone/projects/python/cache/eggs/mechanize-0.2.5-py2.7.egg/mechanize/_mechanize.py", line 255, in _mech_open
    raise response
httperror_seek_wrapper: HTTP Error 404: Not Found

This applies to the current master as well as the last released. I'm not sure what we want to support.

jone commented 7 years ago

@lukasgraf I have cleaned up the exception handling in #48 and addressed this issue in this context.