Various selenium improvements

- Move test case abstraction into Browser helper
- Remove local snapshot code
- Add pytest-compatible reports (inspired by pytest-selenium)
- Fix clearing of cookies

@getsentry/infrastructure

src/sentry/testutils/cases.py

@@ -29,12 +29,8 @@ from django.core.urlresolvers import reverse
 from django.http import HttpRequest
 from django.test import TestCase, TransactionTestCase
 from django.utils.importlib import import_module
-from django.utils.text import slugify
 from exam import before, fixture, Exam
 from rest_framework.test import APITestCase as BaseAPITestCase
-from selenium.webdriver.support.ui import WebDriverWait
-from selenium.webdriver.support import expected_conditions
-from urlparse import urlparse
 
 from sentry import auth
 from sentry.auth.providers.dummy import DummyProvider
@@ -443,98 +439,11 @@ class CliTestCase(TestCase):
         return self.runner.invoke(self.command, args, obj={})
 
 
-@pytest.mark.usefixtures(
-    'browser_class', 'live_server_class', 'percy_class',
-    'screenshots_path_class'
-)
+@pytest.mark.usefixtures('browser_class')
 class AcceptanceTestCase(TransactionTestCase):
-    # Use class setup/teardown to hold Selenium and Percy state across all acceptance tests.
-    # For Selenium, this is done for performance to re-use the same browser across tests.
-    # For Percy, this is done to call initialize and then finalize at the very end after all tests.
     def save_session(self):
-        # XXX(dcramer): "hit a url before trying to set cookies"
-        if not getattr(self.browser, '_has_initialized_cookie_store', False):
-            self.browser.get(self.route('/'))
-            self.wait_until('html')
-            self.browser._has_initialized_cookie_store = True
-
         self.session.save()
-
-        # TODO(dcramer): this should be escaped, but idgaf
-        cookie_data = {
-            'name': settings.SESSION_COOKIE_NAME,
-            'path': '/',
-            'domain': urlparse(self.live_server_url).hostname,
-            'expires': 'Tue, 20 Jun 2025 19:07:44 GMT',
-            'value': self.session.session_key,
-        }
-
-        # XXX(dcramer): PhantomJS does not let us add cookies with the native
-        # selenium API because....
-        # http://stackoverflow.com/questions/37103621/adding-cookies-working-with-firefox-webdriver-but-not-in-phantomjs
-        self.browser.execute_script("document.cookie = '{name}={value}; path={path}; domain={domain}; expires={expires}';\n".format(
-            **cookie_data
-        ))
-
-    def route(self, path, *args, **kwargs):
-        """
-        Return the absolute URI for a given route in Sentry.
-        """
-        return '{}/{}'.format(self.live_server_url, path.strip('/').format(
-            *args, **kwargs
-        ))
-
-    def wait_until(self, selector, timeout=10):
-        """
-        Waits until ``selector`` is found in the browser, or until ``timeout``
-        is hit, whichever happens first.
-        """
-        from selenium.webdriver.common.by import By
-
-        try:
-            WebDriverWait(self.browser, timeout).until(
-                expected_conditions.presence_of_element_located(
-                    (By.CSS_SELECTOR, selector)
-                )
-            )
-        except Exception:
-            print(self.browser.current_url)
-            self.snapshot('wait failed'.format(selector))
-            raise
-
-        return self
-
-    def wait_until_not(self, selector, timeout=10):
-        """
-        Waits until ``selector`` is NOT found in the browser, or until
-        ``timeout`` is hit, whichever happens first.
-        """
-        from selenium.webdriver.common.by import By
-
-        try:
-            WebDriverWait(self.browser, timeout).until_not(
-                expected_conditions.presence_of_element_located(
-                    (By.CSS_SELECTOR, selector)
-                )
-            )
-        except Exception:
-            print(self.browser.current_url)
-            self.snapshot('wait failed'.format(selector))
-            raise
-
-        return self
-
-    def snapshot(self, name):
-        """
-        Capture a screenshot of the current state of the page. Screenshots
-        are captured both locally (in ``cls.screenshots_path``) as well as
-        with Percy (when enabled).
-        """
-        path = os.path.join(
-            self.screenshots_path,
-            slugify(unicode(name)) + '.png',
+        self.browser.save_cookie(
+            name=settings.SESSION_COOKIE_NAME,
+            value=self.session.session_key,
         )
-        print('Saving snapshot {}'.format(path))
-        self.browser.save_screenshot(path)
-        self.percy.snapshot(name=name)
-        return self

src/sentry/utils/pytest.py

@@ -9,6 +9,105 @@ import urllib
 from datetime import datetime
 from django.conf import settings
 from selenium import webdriver
+from selenium.webdriver.support.ui import WebDriverWait
+from selenium.webdriver.support import expected_conditions
+from urlparse import urlparse
+
+
+class Browser(object):
+    def __init__(self, driver, live_server, percy):
+        self.driver = driver
+        self.live_server_url = live_server.url
+        self.percy = percy
+        self.domain = urlparse(self.live_server_url).hostname
+        self._has_initialized_cookie_store = False
+
+    def __getattr__(self, attr):
+        return getattr(self.driver, attr)
+
+    def route(self, path, *args, **kwargs):
+        """
+        Return the absolute URI for a given route in Sentry.
+        """
+        return '{}/{}'.format(self.live_server_url, path.strip('/').format(
+            *args, **kwargs
+        ))
+
+    def get(self, path, *args, **kwargs):
+        self.driver.get(self.route(path), *args, **kwargs)
+        return self
+
+    def post(self, path, *args, **kwargs):
+        self.driver.post(self.route(path), *args, **kwargs)
+        return self
+
+    def put(self, path, *args, **kwargs):
+        self.driver.put(self.route(path), *args, **kwargs)
+        return self
+
+    def delete(self, path, *args, **kwargs):
+        self.driver.delete(self.route(path), *args, **kwargs)
+        return self
+
+    def wait_until(self, selector, timeout=3):
+        """
+        Waits until ``selector`` is found in the browser, or until ``timeout``
+        is hit, whichever happens first.
+        """
+        from selenium.webdriver.common.by import By
+
+        WebDriverWait(self.driver, timeout).until(
+            expected_conditions.presence_of_element_located(
+                (By.CSS_SELECTOR, selector)
+            )
+        )
+
+        return self
+
+    def wait_until_not(self, selector, timeout=3):
+        """
+        Waits until ``selector`` is NOT found in the browser, or until
+        ``timeout`` is hit, whichever happens first.
+        """
+        from selenium.webdriver.common.by import By
+
+        WebDriverWait(self.driver, timeout).until_not(
+            expected_conditions.presence_of_element_located(
+                (By.CSS_SELECTOR, selector)
+            )
+        )
+
+        return self
+
+    def snapshot(self, name):
+        """
+        Capture a screenshot of the current state of the page. Screenshots
+        are captured both locally (in ``cls.screenshots_path``) as well as
+        with Percy (when enabled).
+        """
+        # TODO(dcramer): ideally this would take the executing test package
+        # into account for duplicate names
+        self.percy.snapshot(name=name)
+        return self
+
+    def save_cookie(self, name, value, path='/',
+                    expires='Tue, 20 Jun 2025 19:07:44 GMT'):
+        # XXX(dcramer): "hit a url before trying to set cookies"
+        if not self._has_initialized_cookie_store:
+            self.get('/')
+            self._has_initialized_cookie_store = True
+
+        # XXX(dcramer): PhantomJS does not let us add cookies with the native
+        # selenium API because....
+        # http://stackoverflow.com/questions/37103621/adding-cookies-working-with-firefox-webdriver-but-not-in-phantomjs
+        # TODO(dcramer): this should be escaped, but idgaf
+        self.driver.execute_script("document.cookie = '{name}={value}; path={path}; domain={domain}; expires={expires}';\n".format(
+            name=name,
+            value=value,
+            expires=expires,
+            path=path,
+            domain=self.domain,
+        ))
 
 
 def pytest_configure(config):
@@ -91,6 +190,8 @@ def pytest_configure(config):
     settings.CELERY_ALWAYS_EAGER = False
     settings.CELERY_EAGER_PROPAGATES_EXCEPTIONS = True
 
+    settings.DEBUG_VIEWS = True
+
     settings.DISABLE_RAVEN = True
 
     settings.CACHES = {
@@ -160,7 +261,7 @@ def pytest_runtest_teardown(item):
 
 
 @pytest.fixture(scope='session')
-def browser(request, live_server):
+def driver(request, live_server):
     # Initialize Selenium.
     # NOTE: this relies on the phantomjs binary packaged from npm to be in the right
     # location in node_modules.
@@ -170,33 +271,34 @@ def browser(request, live_server):
         'bin',
         'phantomjs',
     )
-    browser = webdriver.PhantomJS(executable_path=phantomjs_path)
+    driver = webdriver.PhantomJS(executable_path=phantomjs_path)
 
     def fin():
         # Teardown Selenium.
         try:
-            browser.close()
+            driver.close()
         except Exception:
             pass
         # TODO: remove this when fixed in: https://github.com/seleniumhq/selenium/issues/767
-        browser.service.process.send_signal(signal.SIGTERM)
-        browser.quit()
+        driver.service.process.send_signal(signal.SIGTERM)
+        driver.quit()
 
+    request.session._driver = driver
     request.addfinalizer(fin)
-    return browser
+    return driver
 
 
 # TODO(dcramer): ideally we could bundle up more of the browser logic here
 # rather than splitting it between the fixtures and AcceptanceTestCase
 @pytest.fixture(scope='session')
-def percy(request, browser):
+def percy(request, driver):
     import percy
 
     # Initialize Percy.
     loader = percy.ResourceLoader(
         root_dir=settings.STATIC_ROOT,
         base_url=urllib.quote(settings.STATIC_URL),
-        webdriver=browser,
+        webdriver=driver,
     )
     percy_config = percy.Config(default_widths=settings.PERCY_DEFAULT_TESTING_WIDTHS)
     percy = percy.Runner(loader=loader, config=percy_config)
@@ -213,20 +315,8 @@ def live_server_class(request, live_server):
 
 
 @pytest.fixture(scope='class')
-def screenshots_path_class(request, browser):
-    date = datetime.utcnow()
-    # AcceptanceTestCase.snapshot saves local screenshots here
-    path = os.path.normpath(os.path.join(
-        os.path.dirname(__file__), os.pardir, os.pardir, os.pardir, 'tmp', 'selenium-screenshots', date.strftime('%s'))
-    )
-    print('Screenshots will be stored in {}'.format(path))
-    os.makedirs(path)
-    request.cls.screenshots_path = path
-
-
-@pytest.fixture(scope='class')
-def browser_class(request, browser):
-    request.cls.browser = browser
+def browser_class(request, driver, live_server, percy):
+    request.cls.browser = Browser(driver, live_server, percy)
 
 
 @pytest.fixture(scope='class')
@@ -235,11 +325,92 @@ def percy_class(request, percy):
 
 
 @pytest.fixture(scope='function')
-def reset_browser_session(request):
-    if not hasattr(request, 'browser'):
+def setup_selenium_session(request):
+    driver = getattr(request.session, '_driver', None)
+    if not driver:
         return
+    driver.delete_all_cookies()
+
+
+@pytest.mark.tryfirst
+def pytest_runtest_makereport(item, call, __multicall__):
+    report = __multicall__.execute()
+    summary = []
+    extra = getattr(report, 'extra', [])
+    driver = getattr(item.session, '_driver', None)
+    if driver is not None:
+        _gather_url(item, report, driver, summary, extra)
+        _gather_screenshot(item, report, driver, summary, extra)
+        _gather_html(item, report, driver, summary, extra)
+        _gather_logs(item, report, driver, summary, extra)
+    if summary:
+        report.sections.append(('selenium', '\n'.join(summary)))
+    report.extra = extra
+    return report
+
+
+def _gather_url(item, report, driver, summary, extra):
+    try:
+        url = driver.current_url
+    except Exception as e:
+        summary.append('WARNING: Failed to gather URL: {0}'.format(e))
+        return
+    pytest_html = item.config.pluginmanager.getplugin('html')
+    if pytest_html is not None:
+        # add url to the html report
+        extra.append(pytest_html.extras.url(url))
+    summary.append('URL: {0}'.format(url))
+
+
+def _gather_screenshot(item, report, driver, summary, extra):
+    try:
+        screenshot = driver.get_screenshot_as_base64()
+    except Exception as e:
+        summary.append('WARNING: Failed to gather screenshot: {0}'.format(e))
+        return
+    pytest_html = item.config.pluginmanager.getplugin('html')
+    if pytest_html is not None:
+        # add screenshot to the html report
+        extra.append(pytest_html.extras.image(screenshot, 'Screenshot'))
 
-    browser = request.browser
 
-    browser.delete_all_cookies()
-    browser.get('about:blank')
+def _gather_html(item, report, driver, summary, extra):
+    try:
+        html = driver.page_source.encode('utf-8')
+    except Exception as e:
+        summary.append('WARNING: Failed to gather HTML: {0}'.format(e))
+        return
+    pytest_html = item.config.pluginmanager.getplugin('html')
+    if pytest_html is not None:
+        # add page source to the html report
+        extra.append(pytest_html.extras.text(html, 'HTML'))
+
+
+def _gather_logs(item, report, driver, summary, extra):
+    try:
+        types = driver.log_types
+    except Exception as e:
+        # note that some drivers may not implement log types
+        summary.append('WARNING: Failed to gather log types: {0}'.format(e))
+        return
+    for name in types:
+        try:
+            log = driver.get_log(name)
+        except Exception as e:
+            summary.append('WARNING: Failed to gather {0} log: {1}'.format(
+                name, e))
+            return
+        pytest_html = item.config.pluginmanager.getplugin('html')
+        if pytest_html is not None:
+            extra.append(pytest_html.extras.text(
+                format_log(log), '%s Log' % name.title()))
+
+
+def format_log(log):
+    timestamp_format = '%Y-%m-%d %H:%M:%S.%f'
+    entries = [u'{0} {1[level]} - {1[message]}'.format(
+        datetime.utcfromtimestamp(entry['timestamp'] / 1000.0).strftime(
+            timestamp_format), entry).rstrip() for entry in log]
+    log = '\n'.join(entries)
+    log = log.encode('utf-8')
+    return log

src/sentry/web/urls.py

@@ -100,7 +100,7 @@ react_page_view = ReactPageView.as_view()
 
 urlpatterns = patterns('')
 
-if settings.DEBUG:
+if getattr(settings, 'DEBUG_VIEWS', settings.DEBUG):
     from django.views.generic import TemplateView
     import sentry.web.frontend.debug.mail
     from sentry.web.frontend.debug.debug_trigger_error import DebugTriggerErrorView

tests/acceptance/test_auth.py

@@ -8,26 +8,24 @@ class AuthTest(AcceptanceTestCase):
         # disable captcha as it makes these tests flakey (and requires waiting
         # on external resources)
         with self.settings(RECAPTCHA_PUBLIC_KEY=None):
-            self.browser.get(self.route('/auth/login/'))
+            self.browser.get('/auth/login/')
             self.browser.find_element_by_id('id_username').send_keys(username)
             self.browser.find_element_by_id('id_password').send_keys(password)
             self.browser.find_element_by_xpath("//button[contains(text(), 'Login')]").click()
-            # give it one second to make sure the request goes through and we
-            # dont immediately re-set the captcha value
-            self.browser.implicitly_wait(1)
 
-    def test_auth_page(self):
-        self.browser.get(self.live_server_url)
-        self.snapshot(name='login')
+    def test_renders(self):
+        self.browser.get('/auth/login/')
+        self.browser.snapshot(name='login')
 
-    def test_auth_failures(self):
+    def test_no_credentials(self):
         self.enter_auth('', '')
-        self.snapshot(name='login fields required')
+        self.browser.snapshot(name='login fields required')
 
+    def test_invalid_credentials(self):
         self.enter_auth('bad-username', 'bad-username')
-        self.snapshot(name='login fields invalid')
+        self.browser.snapshot(name='login fields invalid')
 
-    def test_auth_success(self):
+    def test_success(self):
         email = 'dummy@example.com'
         password = 'dummy'
         user = self.create_user(email=email)
@@ -35,4 +33,4 @@ class AuthTest(AcceptanceTestCase):
         user.save()
 
         self.enter_auth(email, password)
-        self.snapshot(name='login success')
+        self.browser.snapshot(name='login success')

tests/acceptance/test_issue_details.py

@@ -44,19 +44,19 @@ class IssueDetailsTest(AcceptanceTestCase):
             platform='python',
         )
 
-        self.browser.get(self.route(
-            '/{}/{}/issues/{}/', self.org.slug, self.project.slug, event.group.id
+        self.browser.get('/{}/{}/issues/{}/'.format(
+            self.org.slug, self.project.slug, event.group.id
         ))
-        self.wait_until('.entries')
-        self.snapshot('issue details python')
+        self.browser.wait_until('.entries')
+        self.browser.snapshot('issue details python')
 
     def test_cocoa_event(self):
         event = self.create_sample_event(
             platform='cocoa',
         )
 
-        self.browser.get(self.route(
-            '/{}/{}/issues/{}/', self.org.slug, self.project.slug, event.group.id
+        self.browser.get('/{}/{}/issues/{}/'.format(
+            self.org.slug, self.project.slug, event.group.id
         ))
-        self.wait_until('.entries')
-        self.snapshot('issue details cocoa')
+        self.browser.wait_until('.entries')
+        self.browser.snapshot('issue details cocoa')

tests/acceptance/test_project_issues.py

@@ -23,9 +23,7 @@ class ProjectIssuesTest(AcceptanceTestCase):
             name='Bengal',
         )
         self.login_as(self.user)
-        self.path = self.route(
-            '/{}/{}/', self.org.slug, self.project.slug
-        )
+        self.path = '/{}/{}/'.format(self.org.slug, self.project.slug)
 
     # TODO(dcramer): abstract fixtures into a basic set that is present for
     # all acceptance tests
@@ -33,8 +31,8 @@ class ProjectIssuesTest(AcceptanceTestCase):
         # TODO(dcramer): we should add basic assertions around "i wanted this
         # URL but was sent somewhere else"
         self.browser.get(self.path)
-        self.wait_until('.awaiting-events')
-        self.snapshot('project issues not configured')
+        self.browser.wait_until('.awaiting-events')
+        self.browser.snapshot('project issues not configured')
 
     def test_with_issues(self):
         self.project.update(first_event=timezone.now())
@@ -43,11 +41,11 @@ class ProjectIssuesTest(AcceptanceTestCase):
             message='Foo bar',
         )
         self.browser.get(self.path)
-        self.wait_until('.group-list')
-        self.snapshot('project issues with issues')
+        self.browser.wait_until('.group-list')
+        self.browser.snapshot('project issues with issues')
 
     def test_with_no_issues(self):
         self.project.update(first_event=timezone.now())
         self.browser.get(self.path)
-        self.wait_until('.empty-stream')
-        self.snapshot('project issues without issues')
+        self.browser.wait_until('.empty-stream')
+        self.browser.snapshot('project issues without issues')