From 595afa32c4ca76670fcd8801630048641435b252 Mon Sep 17 00:00:00 2001 From: aiolos Date: Mon, 21 Sep 2015 15:26:55 -0700 Subject: [PATCH] Clean up check for dependency_info. Review URL: https://codereview.chromium.org/1359723002 Cr-Commit-Position: refs/heads/master@{#350048} --- .../dependency_manager/dependency_manager.py | 8 ++++---- .../dependency_manager/dependency_manager_unittest.py | 15 ++++++++++++--- 2 files changed, 16 insertions(+), 7 deletions(-) diff --git a/tools/telemetry/catapult_base/dependency_manager/dependency_manager.py b/tools/telemetry/catapult_base/dependency_manager/dependency_manager.py index 3ab987c24261..d7395acf13e6 100644 --- a/tools/telemetry/catapult_base/dependency_manager/dependency_manager.py +++ b/tools/telemetry/catapult_base/dependency_manager/dependency_manager.py @@ -85,7 +85,8 @@ class DependencyManager(object): FileNotFoundError: If an attempted download was otherwise unsuccessful. """ - if not self._lookup_dict or not dependency in self._lookup_dict: + dependency_info = self._GetDependencyInfo(dependency, platform) + if not dependency_info: # TODO(aiolos): Replace the support_binaries call with an error once all # binary dependencies are moved over to the new system. @@ -101,7 +102,6 @@ class DependencyManager(object): platform_os) logging.info('Looking for dependency %s, on platform %s in the dependency ' 'manager.' % (dependency, platform)) - dependency_info = self._GetDependencyInfo(dependency, platform) path = self._LocalPath(dependency_info) if not path or not os.path.exists(path): path = self._CloudStoragePath(dependency_info) @@ -131,9 +131,9 @@ class DependencyManager(object): """ # TODO(aiolos): Replace the support_binaries call with an error once all # binary dependencies are moved over to the new system. - if not self._lookup_dict or not dependency in self._lookup_dict: - return support_binaries.FindLocallyBuiltPath(dependency) dependency_info = self._GetDependencyInfo(dependency, platform) + if not dependency_info: + return support_binaries.FindLocallyBuiltPath(dependency) local_path = self._LocalPath(dependency_info) if not local_path or not os.path.exists(local_path): raise exceptions.NoPathFoundError(dependency, platform) diff --git a/tools/telemetry/catapult_base/dependency_manager/dependency_manager_unittest.py b/tools/telemetry/catapult_base/dependency_manager/dependency_manager_unittest.py index d034cefafa4a..eadb70ab7edc 100644 --- a/tools/telemetry/catapult_base/dependency_manager/dependency_manager_unittest.py +++ b/tools/telemetry/catapult_base/dependency_manager/dependency_manager_unittest.py @@ -54,17 +54,17 @@ class DependencyManagerTest(unittest.TestCase): sb_path = 'sb_path' local_path = 'local_path' cs_path = 'cs_path' - dep_info = 'dep_info' local_path_mock.return_value = local_path cs_path_mock.return_value = cs_path sb_find_path_mock.return_value = sb_path - dep_info_mock.return_value = dep_info + dep_info_mock.return_value = None # Empty lookup_dict found_path = dep_manager.FetchPath('dep', 'plat_arch_x86') self.assertEqual(sb_path, found_path) self.assertFalse(local_path_mock.call_args) self.assertFalse(cs_path_mock.call_args) + dep_info_mock.assert_called_once_with('dep', 'plat_arch_x86') sb_find_path_mock.assert_called_once_with('dep', 'arch_x86', 'plat') local_path_mock.reset_mock() cs_path_mock.reset_mock() @@ -80,6 +80,7 @@ class DependencyManagerTest(unittest.TestCase): self.assertEqual(sb_path, found_path) self.assertFalse(local_path_mock.call_args) self.assertFalse(cs_path_mock.call_args) + dep_info_mock.assert_called_once_with('dep', 'plat_arch_x86') sb_find_path_mock.assert_called_once_with('dep', 'arch_x86', 'plat') local_path_mock.reset_mock() cs_path_mock.reset_mock() @@ -105,6 +106,7 @@ class DependencyManagerTest(unittest.TestCase): local_path_mock.return_value = local_path cs_path_mock.return_value = cs_path sb_find_path_mock.return_value = sb_path + # The DependencyInfo returned should be passed through to LocalPath. dep_info_mock.return_value = dep_info @@ -238,13 +240,16 @@ class DependencyManagerTest(unittest.TestCase): dep_info = 'dep_info' local_path_mock.return_value = local_path sb_find_path_mock.return_value = sb_path - dep_info_mock.return_value = dep_info + + # GetDependencyInfo should return None when missing from the lookup dict. + dep_info_mock.return_value = None # Empty lookup_dict found_path = dep_manager.LocalPath('dep', 'plat') self.assertEqual(sb_path, found_path) self.assertFalse(local_path_mock.call_args) sb_find_path_mock.assert_called_once_with('dep') + dep_info_mock.assert_called_once_with('dep', 'plat') local_path_mock.reset_mock() sb_find_path_mock.reset_mock() dep_info_mock.reset_mock() @@ -258,10 +263,14 @@ class DependencyManagerTest(unittest.TestCase): self.assertEqual(sb_path, found_path) self.assertFalse(local_path_mock.call_args) sb_find_path_mock.assert_called_once_with('dep') + dep_info_mock.assert_called_once_with('dep', 'plat') local_path_mock.reset_mock() sb_find_path_mock.reset_mock() dep_info_mock.reset_mock() + # The DependencyInfo returned should be passed through to LocalPath. + dep_info_mock.return_value = dep_info + # Non-empty lookup dict that contains the dependency we're looking for. # Local path exists. dep_manager._lookup_dict = {'dep1': mock.MagicMock(), -- 2.11.4.GIT