Skip to content

Commit

Permalink
Move browser management to kaifuku
Browse files Browse the repository at this point in the history
Use webdriver-kaifuku, replacing most of the code in cfme.utils.browser

remove convenience methods surrounding the manager singleton, and update framework interaction with the browser from that singleton

phase one of changing how integration_tests handles browsers, phase two will be to move the manager instance to the contextual classes for UI/SSUI

Address the traceback encoding in pytest hook for artifactor exception hook

FIXES ManageIQ#8131
  • Loading branch information
mshriver committed May 26, 2020
1 parent e08fa48 commit 0579fbd
Show file tree
Hide file tree
Showing 18 changed files with 123 additions and 444 deletions.
7 changes: 2 additions & 5 deletions cfme/base/ssui.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,7 @@
from cfme.utils.appliance.implementations.ssui import navigate_to
from cfme.utils.appliance.implementations.ssui import navigator
from cfme.utils.appliance.implementations.ssui import SSUINavigateStep
from cfme.utils.browser import ensure_browser_open
from cfme.utils.browser import quit
from cfme.utils.browser import manager
from cfme.utils.log import logger
from widgetastic_manageiq import SSUIVerticalNavigation

Expand Down Expand Up @@ -187,12 +186,10 @@ class LoginScreen(SSUINavigateStep):
VIEW = LoginPage

def prerequisite(self):
ensure_browser_open(self.obj.appliance.server.address())
manager.start_at_url(self.obj.appliance.server.address())

def step(self, *args, **kwargs):
# Can be either blank or logged in
del self.view # In order to unbind the browser
quit()
ensure_browser_open(self.obj.appliance.server.address())
if not self.view.is_displayed:
raise Exception('Could not open the login screen')
9 changes: 4 additions & 5 deletions cfme/base/ui.py
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@
from cfme.configure.tasks import TasksView
from cfme.dashboard import DashboardView
from cfme.exceptions import ItemNotFound
from cfme.exceptions import NavigationError
from cfme.intelligence.chargeback import ChargebackView
from cfme.intelligence.rss import RSSView
from cfme.intelligence.timelines import CloudIntelTimelinesView
Expand All @@ -45,6 +46,7 @@
from cfme.utils.appliance.implementations.ui import navigate_to
from cfme.utils.appliance.implementations.ui import navigator
from cfme.utils.appliance.implementations.ui import ViaUI
from cfme.utils.browser import manager
from cfme.utils.log import logger
from cfme.utils.version import Version
from cfme.utils.version import VersionPicker
Expand Down Expand Up @@ -324,8 +326,7 @@ class LoginScreen(CFMENavigateStep):
VIEW = LoginPage

def prerequisite(self):
from cfme.utils.browser import ensure_browser_open
ensure_browser_open(self.obj.appliance.server.address())
manager.start_at_url(self.obj.appliance.server.address())

def step(self, *args, **kwargs):
# Can be either blank or logged in
Expand All @@ -336,10 +337,8 @@ def step(self, *args, **kwargs):
# TODO this is not the place to handle this behavior
if not self.view.wait_displayed(timeout=60):
# Something is wrong
from cfme.utils import browser
del self.view # In order to unbind the browser
browser.quit()
raise
raise NavigationError


@navigator.register(Server)
Expand Down
4 changes: 2 additions & 2 deletions cfme/containers/provider/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@
from cfme.utils.appliance.implementations.ui import CFMENavigateStep
from cfme.utils.appliance.implementations.ui import navigate_to
from cfme.utils.appliance.implementations.ui import navigator
from cfme.utils.browser import browser
from cfme.utils.browser import manager
from cfme.utils.log import logger
from cfme.utils.pretty import Pretty
from cfme.utils.version import LOWEST
Expand Down Expand Up @@ -127,7 +127,7 @@ def get_logging_url(self):
def report_kibana_failure():
raise RuntimeError("Kibana not found in the window title or content")

browser_instance = browser()
browser_instance = manager.browser

all_windows_before = browser_instance.window_handles
appliance_window = browser_instance.current_window_handle
Expand Down
16 changes: 6 additions & 10 deletions cfme/fixtures/browser.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,10 @@

from py.error import ENOENT

import cfme.utils.browser
from cfme.fixtures.artifactor_plugin import fire_art_test_hook
from cfme.utils import browser as browser_util
from cfme.utils import safe_string
from cfme.utils.appliance import find_appliance
from cfme.utils.browser import take_screenshot
from cfme.utils.datafile import template_env
from cfme.utils.log import logger
from cfme.utils.path import log_path
Expand All @@ -31,7 +30,7 @@ def pytest_runtest_setup(item):
return

if set(getattr(item, 'fixturenames', [])) & browser_fixtures:
cfme.utils.browser.ensure_browser_open()
browser_util.manager.start()


def pytest_exception_interact(node, call, report):
Expand All @@ -41,15 +40,12 @@ def pytest_exception_interact(node, call, report):
val = safe_string(call.excinfo.value)
if isinstance(call.excinfo.value, (URLError, BadStatusLine, error)):
logger.error("internal Exception:\n %s", str(call.excinfo))
from cfme.utils.browser import manager
manager.start() # start will quit first and cycle wharf as well
browser_util.manager.start() # start will quit first and cycle wharf as well

last_lines = "\n".join(report.longreprtext.split("\n")[-4:])

short_tb = '{}\n{}\n{}'.format(
last_lines, call.excinfo.type.__name__,
val.encode('ascii', 'xmlcharrefreplace')
)
ascii_val = val.encode('ascii', 'xmlcharrefreplace').decode('ascii')
short_tb = f'{last_lines}\n{call.excinfo.type.__name__}\n{ascii_val}'
fire_art_test_hook(
node, 'filedump',
description="Traceback", contents=report.longreprtext, file_type="traceback",
Expand Down Expand Up @@ -96,7 +92,7 @@ def pytest_exception_interact(node, call, report):
# an isinstance(val, WebDriverException) check in addition to the browser fixture check that
# exists here in commit 825ef50fd84a060b58d7e4dc316303a8b61b35d2

screenshot = take_screenshot()
screenshot = browser_util.take_screenshot()
template_data['screenshot'] = screenshot.png
template_data['screenshot_error'] = screenshot.error
if screenshot.png:
Expand Down
22 changes: 3 additions & 19 deletions cfme/fixtures/rbac.py
Original file line number Diff line number Diff line change
Expand Up @@ -100,8 +100,7 @@ def test_rbac(rbac_role):
from cfme.utils import conf
from cfme.utils import testgen
from cfme.utils.appliance import current_appliance
from cfme.utils.browser import browser
from cfme.utils.browser import ensure_browser_open
from cfme.utils.browser import manager
from cfme.utils.browser import take_screenshot
from cfme.utils.log import logger

Expand Down Expand Up @@ -139,21 +138,6 @@ def save_screenshot(node, ss, sse):
slaveid=store.slaveid)


def really_logout():
"""A convenience function logging out
This function simply ensures that we are logged out and that a new browser is loaded
ready for use.
"""
try:
current_appliance.server.logout()
except AttributeError:
try:
browser().quit()
except AttributeError:
ensure_browser_open()


@pytest.hookimpl(hookwrapper=True)
def pytest_pyfunc_call(pyfuncitem):
"""Inspects and consumes certain exceptions
Expand All @@ -171,7 +155,7 @@ def pytest_pyfunc_call(pyfuncitem):
# Login as the "new" user to run the test under
if 'rbac_role' in pyfuncitem.fixturenames:
user = pyfuncitem._request.getfixturevalue('rbac_role')
really_logout()
manager.start() # will quit browser and create a new one
logger.info(f"setting user to {user}")
user_obj = current_appliance.collections.users.instantiate(
username=conf.credentials[user]['username'],
Expand All @@ -186,7 +170,7 @@ def pytest_pyfunc_call(pyfuncitem):

# Handle the Exception
logger.error(pyfuncitem.location[0])
loc = "{}/{}".format(pyfuncitem.location[0], pyfuncitem.location[2])
loc = f"{pyfuncitem.location[0]}/{pyfuncitem.location[2]}"
# loc = loc[:min([loc.rfind('['), len(loc)])]
logger.error(loc)
# errors = [v for k, v in tests.items() if loc.startswith(k)]
Expand Down
9 changes: 5 additions & 4 deletions cfme/fixtures/sauce.py
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
import pytest

from cfme.utils.browser import manager


@pytest.hookimpl(trylast=True)
def pytest_runtest_teardown(item, nextitem):
if item.config.getoption('sauce'):
from cfme.utils.browser import ensure_browser_open, quit, browser
ensure_browser_open()
browser().execute_script(f"sauce:job-name={item.name}")
quit()
manager.ensure_open()
manager.browser().execute_script(f"sauce:job-name={item.name}")
manager.quit()
4 changes: 2 additions & 2 deletions cfme/tests/configure/test_ntp_server.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
import pytest

from cfme import test_requirements
from cfme.utils.browser import quit
from cfme.utils.browser import manager
from cfme.utils.conf import cfme_data
from cfme.utils.log import logger
from cfme.utils.wait import wait_for
Expand Down Expand Up @@ -155,7 +155,7 @@ def test_ntp_server_check(request, appliance, ntp_servers_keys, empty_ntp_dict):
else:
raise Exception("Failed modifying the system date")
# Calling the browser quit() method to compensate the session after the evm service restart
quit()
manager.quit()


@pytest.mark.tier(3)
Expand Down
8 changes: 3 additions & 5 deletions cfme/tests/configure/test_session_timeout.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,7 @@
import pytest

from cfme import test_requirements
from cfme.utils.browser import ensure_browser_open
from cfme.utils.browser import quit
from cfme.utils.browser import manager
from cfme.utils.wait import wait_for


Expand All @@ -25,13 +24,12 @@ def test_session_timeout(request, appliance):

@request.addfinalizer # Wow, why we did not figure this out before?!
def _finalize():
quit()
ensure_browser_open()
manager.start()
auth_settings.set_session_timeout(hours="24", minutes="0")

auth_settings.set_session_timeout(hours="0", minutes="5")
# Wait 10 minutes
time.sleep(10 * 60)
time.sleep(6 * 60)
# Try getting timeout
# I had to use wait_for because on 5.4 and upstream builds it made weird errors
wait_for(
Expand Down
9 changes: 3 additions & 6 deletions cfme/tests/services/test_myservice.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,11 +12,9 @@
from cfme.services.myservice import MyService
from cfme.services.myservice.ui import MyServiceDetailView
from cfme.services.service_catalogs import ServiceCatalogs
from cfme.utils import browser
from cfme.utils.appliance import ViaUI
from cfme.utils.appliance.implementations.ui import navigate_to
from cfme.utils.blockers import BZ
from cfme.utils.browser import ensure_browser_open
from cfme.utils.log_validator import LogValidator
from cfme.utils.update import update
from cfme.utils.wait import wait_for
Expand All @@ -31,14 +29,13 @@
]


@pytest.fixture
def needs_firefox():
@pytest.fixture(scope='session')
def needs_firefox(appliance):
""" Fixture which skips the test if not run under firefox.
I recommend putting it in the first place.
"""
ensure_browser_open()
if browser.browser().name != "firefox":
if appliance.browser.name != "firefox":
pytest.skip(msg="This test needs firefox to run")


Expand Down
3 changes: 1 addition & 2 deletions cfme/tests/services/test_operations.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@
from cfme.infrastructure.provider.virtualcenter import VMwareProvider
from cfme.markers.env_markers.provider import ONE
from cfme.utils.appliance.implementations.ui import navigate_to
from cfme.utils.browser import browser
from cfme.utils.wait import wait_for

pytestmark = [
Expand Down Expand Up @@ -88,7 +87,7 @@ def generated_request(appliance, provider, provisioning, template_name, vm_name)
provision_request = appliance.collections.requests.instantiate(cells=request_cells)
yield provision_request

browser().get(appliance.url)
appliance.server.logout()
appliance.server.login_admin()

provision_request.remove_request()
Expand Down
12 changes: 5 additions & 7 deletions cfme/utils/appliance/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -2989,8 +2989,8 @@ def push(self, obj):
logger.info(f"Pushed appliance hostname [{obj.hostname}] on stack \n"
f"Previous stack head was {getattr(stack_parent, 'hostname', 'empty')}")
if obj.browser_steal:
from cfme.utils import browser
browser.start()
from cfme.utils.browser import manager
manager.browser.start()

def pop(self):
stack_parent = super().pop()
Expand All @@ -2999,8 +2999,8 @@ def pop(self):
f"Stack head is {getattr(current, 'hostname', 'empty')}")

if stack_parent and stack_parent.browser_steal:
from cfme.utils import browser
browser.start()
from cfme.utils.browser import manager
manager.browser.start()
return stack_parent


Expand Down Expand Up @@ -3029,9 +3029,7 @@ def load_appliances(appliance_list, global_kwargs):
mapping = IPAppliance.CONFIG_MAPPING

if not any(k in mapping for k in kwargs):
raise ValueError(
f"No valid IPAppliance kwargs found in config for appliance #{idx}"
)
raise ValueError(f"No valid IPAppliance kwargs found in config, appliance #{idx}")
appliance = IPAppliance(**{mapping[k]: v for k, v in kwargs.items() if k in mapping})

result.append(appliance)
Expand Down
4 changes: 3 additions & 1 deletion cfme/utils/appliance/implementations/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,9 @@ def __str__(self):

def open_browser(self, url_key=None):
# TODO: self.appliance.server.address() instead of None
return manager.ensure_open(url_key)
manager.ensure_open()
manager.browser.get(self.appliance.server.address())
return manager.browser

def quit_browser(self):
manager.quit()
Expand Down
4 changes: 2 additions & 2 deletions cfme/utils/appliance/implementations/ssui.py
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,7 @@ def am_i_here(self):
return False

def pre_navigate(self, *args, **kwargs):
self.appliance.browser.open_browser(url_key=self.obj.appliance.server.address())
self.appliance.ssui.open_browser()

def do_nav(self, _tries=0, *args, **kwargs):
"""Describes how the navigation should take place."""
Expand Down Expand Up @@ -232,7 +232,7 @@ def __str__(self):
def widgetastic(self):
"""This gives us a widgetastic browser."""
# TODO: Make this a property that could watch for browser change?
browser = self.open_browser(url_key=self.appliance.server.address())
browser = self.open_browser()
wt = MiqSSUIBrowser(browser, self)
manager.add_cleanup(self._reset_cache)
return wt
Loading

0 comments on commit 0579fbd

Please sign in to comment.