Skip to content
This repository has been archived by the owner on Dec 3, 2019. It is now read-only.

Commit

Permalink
[Telemetry] Fix flaky browser test due to duplicated system stubbing
Browse files Browse the repository at this point in the history
All the browser tests were flaky because we fail to restore the |sys| module in
desktop_browser_finder. The reason were because the module
desktop_browser_finder were overriden twice, hence in the second time
its original module |desktop_browser_finder.sys| is already stubbed. The
system_stub framework then tries to restore desktop_browser_finder.sys with this
"original" sys module, hence failing to restore desktop_browser_finder to real
original state.

This CL remove the duplicate stubbing & add assertion in system_stub to avoid overriding a module twice.

BUG=catapult:#3074

Review-Url: https://codereview.chromium.org/2574453002
  • Loading branch information
nedn authored and Commit bot committed Dec 12, 2016
1 parent f9f1644 commit b0ae6e2
Show file tree
Hide file tree
Showing 2 changed files with 8 additions and 7 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -32,15 +32,12 @@ def setUp(self):
self._catapult_path_stubs = system_stub.Override(
desktop_browser_finder.path_module.catapult_util, ['os', 'sys'])
self._util_stubs = system_stub.Override(util, ['os', 'sys'])
self._browser_finder_stubs = system_stub.Override(desktop_browser_finder,
['os', 'sys'])

def tearDown(self):
self._finder_stubs.Restore()
self._path_stubs.Restore()
self._catapult_path_stubs.Restore()
self._util_stubs.Restore()
self._browser_finder_stubs.Restore()

@property
def _files(self):
Expand Down Expand Up @@ -68,7 +65,6 @@ def setUp(self):
self._finder_stubs.sys.platform = 'win32'
self._path_stubs.sys.platform = 'win32'
self._util_stubs.sys.platform = 'win32'
self._browser_finder_stubs.sys.platform = 'win32'

def testFindProgramFiles(self):
if not self.CanFindAvailableBrowsers():
Expand Down Expand Up @@ -104,7 +100,6 @@ def setUp(self):
self._finder_stubs.sys.platform = 'win32'
self._path_stubs.sys.platform = 'win32'
self._util_stubs.sys.platform = 'win32'
self._browser_finder_stubs.sys.platform = 'win32'

def testFindBuild(self):
if not self.CanFindAvailableBrowsers():
Expand Down Expand Up @@ -134,7 +129,6 @@ def setUp(self):
self._finder_stubs.sys.platform = 'darwin'
self._path_stubs.sys.platform = 'darwin'
self._util_stubs.sys.platform = 'darwin'
self._browser_finder_stubs.sys.platform = 'darwin'
self._files.append('/Applications/Google Chrome Canary.app/'
'Contents/MacOS/Google Chrome Canary')
self._files.append('/Applications/Google Chrome.app/' +
Expand Down Expand Up @@ -261,7 +255,6 @@ def setUp(self):
self._finder_stubs.sys.platform = 'win32'
self._path_stubs.sys.platform = 'win32'
self._util_stubs.sys.platform = 'win32'
self._browser_finder_stubs.sys.platform = 'win32'
self._path_stubs.os.local_app_data = 'c:\\Users\\Someone\\AppData\\Local'
self._files.append('c:\\tmp\\chrome.exe')
self._files.append('..\\..\\..\\build\\Release\\chrome.exe')
Expand Down
8 changes: 8 additions & 0 deletions telemetry/telemetry/testing/system_stub.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,9 @@


class Override(object):

_overidden_modules = set()

def __init__(self, base_module, module_list):
self.cloud_storage = None
self.open = None
Expand All @@ -41,6 +44,9 @@ def __init__(self, base_module, module_list):
self.subprocess = None
self.sys = None

assert base_module not in self._overidden_modules, (
'%s is already overridden' % base_module.__name__)
self._overidden_modules.add(base_module)
self._base_module = base_module
self._overrides = {}

Expand All @@ -65,6 +71,8 @@ def Restore(self):
else:
setattr(self._base_module, module_name, original_module)
self._overrides = {}
self._overidden_modules.remove(self._base_module)
self._base_module = None


class AdbDevice(object):
Expand Down

0 comments on commit b0ae6e2

Please sign in to comment.