From 08b712e34652018bf66b2667eb16f9ec2bd6cfc4 Mon Sep 17 00:00:00 2001 From: Igor Bogoslavskyi Date: Mon, 12 Jun 2017 16:34:00 +0200 Subject: [PATCH] Make execution parallel (#9) * make everything run in parallel * use block printer to have some status messages on screen * new parameter: `-j` or `--num_threads` for controlling the level of parallelism * new parameter: `-no_status` to disable printing status info on screen --- catkin_tools_fetch/cli.py | 65 +++++++++++++-- catkin_tools_fetch/lib/dependency_parser.py | 8 +- catkin_tools_fetch/lib/downloader.py | 91 +++++++++++++++------ catkin_tools_fetch/lib/printer.py | 47 +++++++++++ catkin_tools_fetch/lib/tools.py | 36 ++++---- catkin_tools_fetch/lib/update.py | 74 +++++++++++------ setup.py | 18 ++-- tests/test_downloader.py | 16 ++-- tests/test_git_bridge.py | 27 ++++-- tests/test_tools.py | 2 +- tests/test_updater.py | 9 +- 11 files changed, 290 insertions(+), 103 deletions(-) create mode 100644 catkin_tools_fetch/lib/printer.py diff --git a/catkin_tools_fetch/cli.py b/catkin_tools_fetch/cli.py index bed312b..51db73c 100644 --- a/catkin_tools_fetch/cli.py +++ b/catkin_tools_fetch/cli.py @@ -63,6 +63,12 @@ def prepare_arguments(parser): config_group.add_argument( '--default_url', default="{package}", help='Where to look for packages by default.') + config_group.add_argument('--no_status', action='store_true', + help='Do not use progress status when cloning.') + config_group.add_argument('--num_threads', '-j', + type=int, + default=4, + help='Number of threads run in parallel.') # Behavior behavior_group = parser.add_argument_group( @@ -101,6 +107,14 @@ def prepare_arguments_deps(parser): action='store_true', default=False, help='Print output from commands.') + parent_parser.add_argument('--no_status', + action='store_true', + default=False, + help='Do not use progress status when cloning.') + parent_parser.add_argument('--num_threads', '-j', + type=int, + default=4, + help='Number of threads run in parallel.') packages_help_msg = """ Packages for which the dependencies are analyzed. @@ -144,6 +158,7 @@ def prepare_arguments_deps(parser): metavar='PKGNAME', nargs='*', help=packages_help_msg) + return parser @@ -163,6 +178,15 @@ def main(opts): else: log.setLevel(logging.getLevelName("INFO")) + if opts.no_status: + log.info(" Not printing status messages while cloning.") + use_preprint = False + else: + log.info(" Will print status messages while cloning.") + use_preprint = True + + log.info(" Using %s threads.", opts.num_threads) + context = Context.load(opts.workspace, opts.profile, opts, append=True) default_url = Tools.prepare_default_url(opts.default_url) if not opts.workspace: @@ -189,16 +213,26 @@ def main(opts): return fetch(packages=opts.packages, workspace=opts.workspace, context=context, - default_url=default_url) + default_url=default_url, + use_preprint=use_preprint, + num_threads=opts.num_threads) if opts.subverb == 'update': return update(packages=opts.packages, workspace=opts.workspace, context=context, default_url=default_url, - conflict_strategy=opts.on_conflict) - - -def update(packages, workspace, context, default_url, conflict_strategy): + conflict_strategy=opts.on_conflict, + use_preprint=use_preprint, + num_threads=opts.num_threads) + + +def update(packages, + workspace, + context, + default_url, + conflict_strategy, + use_preprint, + num_threads): """Update packages from the available remotes. Args: @@ -206,6 +240,7 @@ def update(packages, workspace, context, default_url, conflict_strategy): workspace (str): Path to a workspace (without src/ in the end). context (Context): Current context. Needed to find current packages. default_url (str): A default url with a {package} placeholder in it. + use_preprint (bool): Show status messages while cloning Returns: int: Return code. 0 if success. Git error code otherwise. @@ -214,12 +249,21 @@ def update(packages, workspace, context, default_url, conflict_strategy): workspace_packages = find_packages(context.source_space_abs, exclude_subspaces=True, warnings=[]) - updater = Updater(ws_path, workspace_packages, conflict_strategy) + updater = Updater(ws_path=ws_path, + packages=workspace_packages, + conflict_strategy=conflict_strategy, + use_preprint=use_preprint, + num_threads=num_threads) updater.update_packages(packages) return 0 -def fetch(packages, workspace, context, default_url): +def fetch(packages, + workspace, + context, + default_url, + use_preprint, + num_threads): """Fetch dependencies of a package. Args: @@ -227,6 +271,7 @@ def fetch(packages, workspace, context, default_url): workspace (str): Path to a workspace (without src/ in the end). context (Context): Current context. Needed to find current packages. default_url (str): A default url with a {package} placeholder in it. + use_preprint (bool): Show status messages while cloning Returns: int: Return code. 0 if success. Git error code otherwise. @@ -267,7 +312,11 @@ def fetch(packages, workspace, context, default_url): # to download dependencies for one project only. packages.add(new_dep_name) try: - downloader = Downloader(ws_path, available_pkgs, ignore_pkgs) + downloader = Downloader(ws_path=ws_path, + available_pkgs=available_pkgs, + ignore_pkgs=ignore_pkgs, + use_preprint=use_preprint, + num_threads=num_threads) except ValueError as e: log.critical(" Encountered error. Abort.") log.critical(" Error message: %s", e.message) diff --git a/catkin_tools_fetch/lib/dependency_parser.py b/catkin_tools_fetch/lib/dependency_parser.py index a34c266..bfa3fa8 100644 --- a/catkin_tools_fetch/lib/dependency_parser.py +++ b/catkin_tools_fetch/lib/dependency_parser.py @@ -9,6 +9,7 @@ from xml.dom import minidom from catkin_tools_fetch.lib.tools import Tools +from catkin_tools_fetch.lib.printer import Printer log = logging.getLogger('deps') @@ -55,6 +56,7 @@ def __init__(self, download_mask, pkg_name): '`download_mask` must contain a "{package}" placeholder.') self.__download_mask = download_mask self.pkg_name = pkg_name + self.printer = Printer() def get_dependencies(self, package_folder): """Find and parse package.xml file and return a dict of dependencies. @@ -76,9 +78,9 @@ def get_dependencies(self, package_folder): deps = Parser.__node_to_list(xmldoc, tag) deps = Parser.__fix_dependencies(deps, self.pkg_name) all_deps += deps - log.info(" %-21s: Found %s dependencies.", - Tools.decorate(self.pkg_name), - len(all_deps)) + msg = " {}: Found {} dependencies".format( + Tools.decorate(self.pkg_name), len(all_deps)) + self.printer.print_msg(msg) log.debug(" Dependencies: %s", all_deps) deps_with_urls = self.__init_dep_dict(all_deps) return Parser.__update_explicit_values(xmldoc, deps_with_urls) diff --git a/catkin_tools_fetch/lib/downloader.py b/catkin_tools_fetch/lib/downloader.py index 632bcbf..21ec675 100644 --- a/catkin_tools_fetch/lib/downloader.py +++ b/catkin_tools_fetch/lib/downloader.py @@ -6,9 +6,12 @@ import logging from os import path +from termcolor import colored +from concurrent import futures from catkin_tools_fetch.lib.tools import Tools from catkin_tools_fetch.lib.tools import GitBridge +from catkin_tools_fetch.lib.printer import Printer log = logging.getLogger('deps') @@ -22,12 +25,19 @@ class Downloader(object): ws_path (str): Workspace path. This is where packages live. """ - IGNORE_TAG = "[IGNORED]" - NOT_FOUND_TAG = "[NOT FOUND]" + IGNORE_TAG = colored("[IGNORED]", 'yellow') + NOT_FOUND_TAG = colored("[NOT FOUND]", 'red') + CLONING_TAG = "[CLONING]" + CHECKING_TAG = "[CHECKING]" NO_ERROR = 0 - def __init__(self, ws_path, available_pkgs, ignore_pkgs): + def __init__(self, + ws_path, + available_pkgs, + ignore_pkgs, + use_preprint=True, + num_threads=4): """Init a downloader. Args: @@ -44,6 +54,9 @@ def __init__(self, ws_path, available_pkgs, ignore_pkgs): self.ws_path = ws_path self.available_pkgs = available_pkgs self.ignore_pkgs = ignore_pkgs + self.thread_pool = futures.ThreadPoolExecutor(max_workers=num_threads) + self.use_preprint = use_preprint + self.printer = Printer() def download_dependencies(self, dep_dict): """Check and download dependencies from a dependency dictionary. @@ -59,6 +72,14 @@ def download_dependencies(self, dep_dict): checked_deps = self.__check_dependencies(dep_dict) return self.__clone_dependencies(checked_deps) + def __clone_dependency(self, pkg_name, url, dep_path, branch): + """Clone a single dependency. Return a future to the clone process.""" + if self.use_preprint: + msg = " {}: {}".format(Tools.decorate(pkg_name), + Downloader.CLONING_TAG) + self.printer.add_msg(pkg_name, msg) + return GitBridge.clone(pkg_name, url, dep_path, branch) + def __clone_dependencies(self, checked_deps): """Clone dependencies. @@ -73,28 +94,39 @@ def __clone_dependencies(self, checked_deps): return Downloader.NO_ERROR log.info(" Cloning valid dependencies:") error_code = Downloader.NO_ERROR + # store all tasks in a futures list + futures_list = [] for name, dependency in checked_deps.items(): url = dependency.url branch = dependency.branch if not branch: branch = "master" if name in self.available_pkgs: - log.info(" %-21s: %s", - Tools.decorate(name), - GitBridge.EXISTS_TAG) + msg = " {}: {}".format( + Tools.decorate(name), GitBridge.EXISTS_TAG) + self.printer.purge_msg(name, msg) continue dep_path = path.join(self.ws_path, name) - clone_result = GitBridge.clone(url, dep_path, branch) - if clone_result in [GitBridge.CLONED_TAG.format(branch=branch), - GitBridge.EXISTS_TAG]: - log.info(" %-21s: %s", Tools.decorate(name), clone_result) - elif clone_result == GitBridge.ERROR_TAG: - log.error(" %-21s: %s", Tools.decorate(name), clone_result) + future = self.thread_pool.submit( + self.__clone_dependency, name, url, dep_path, branch) + futures_list.append(future) + # we have all the futures ready. Now just wait for them to finish. + for future in futures.as_completed(futures_list): + pkg_name, clone_result = future.result() + msg = " {}: {}".format( + Tools.decorate(pkg_name), clone_result) + self.printer.purge_msg(pkg_name, msg) + if clone_result == GitBridge.ERROR_TAG: error_code = 1 - else: - log.error(" undefined result of clone.") return error_code + def __check_dependency(self, dependency): + if self.use_preprint: + msg = " {}: {}".format( + Tools.decorate(dependency.name), Downloader.CHECKING_TAG) + self.printer.add_msg(dependency.name, msg) + return GitBridge.repository_exists(dependency) + def __check_dependencies(self, dep_dict): """Check dependencies for validity. @@ -111,18 +143,25 @@ def __check_dependencies(self, dep_dict): if not dep_dict: # exit early if there are no new dependencies return checked_deps + futures_list = [] log.info(" Checking merged dependencies:") - for name, dependency in dep_dict.items(): - url = dependency.url - if name in self.ignore_pkgs: - log.info(" %-21s: %s", - Tools.decorate(name), - Downloader.IGNORE_TAG) - elif GitBridge.repository_exists(url): - log.info(" %-21s: %s", Tools.decorate(name), url) - checked_deps[name] = dependency + for dependency in dep_dict.values(): + if dependency.name in self.ignore_pkgs: + msg = " {}: {}".format( + Tools.decorate(dependency.name), Downloader.IGNORE_TAG) + self.printer.add_msg(dependency.name, msg) + continue + futures_list.append(self.thread_pool.submit( + self.__check_dependency, dependency)) + for future in futures.as_completed(futures_list): + dependency, repo_found = future.result() + if repo_found: + msg = " {}: {}".format( + Tools.decorate(dependency.name), dependency.url) + self.printer.purge_msg(dependency.name, msg) + checked_deps[dependency.name] = dependency else: - log.info(" %-21s: %s", - Tools.decorate(name), - Downloader.NOT_FOUND_TAG) + msg = " {}: {}".format( + Tools.decorate(dependency.name), Downloader.NOT_FOUND_TAG) + self.printer.purge_msg(dependency.name, msg) return checked_deps diff --git a/catkin_tools_fetch/lib/printer.py b/catkin_tools_fetch/lib/printer.py new file mode 100644 index 0000000..758de43 --- /dev/null +++ b/catkin_tools_fetch/lib/printer.py @@ -0,0 +1,47 @@ +"""Module for printing a block of text overwriting previous text.""" + +import sys +from threading import RLock + + +class Printer: + """Reprints messages wiping unneeded lines. Supports multiple threads.""" + + __rlock = RLock() + + def __init__(self, line_length=70): + """Initialize object.""" + self.__msgs = {} + self.__line_length = line_length + + def add_msg(self, key, msg): + """Add a new message and print it on last line.""" + with self.__rlock: + self.__msgs[key] = msg + print(self.__msgs[key].ljust(self.__line_length, " ")) + + def print_msg(self, msg): + """Print a single message.""" + print(msg.ljust(self.__line_length, " ")) + + def purge_msg(self, key, last_msg): + """Print the last message on top active line and move lower.""" + with self.__rlock: + self.__move_up() + if key in self.__msgs: + del self.__msgs[key] + print(last_msg.ljust(self.__line_length, " ")) + self.__print_active(move_up=False) + + def __print_active(self, move_up=False): + """Print all active messages overwriting console.""" + # Clear previous text by overwritig non-spaces with spaces + if move_up: + self.__move_up() + for key in self.__msgs.keys(): + print(self.__msgs[key].ljust(self.__line_length, " ")) + + def __move_up(self): + """Move cursor to the top active line.""" + for _ in range(len(self.__msgs)): + sys.stdout.write("\033[A") diff --git a/catkin_tools_fetch/lib/tools.py b/catkin_tools_fetch/lib/tools.py index fde8b3d..c92a1c6 100644 --- a/catkin_tools_fetch/lib/tools.py +++ b/catkin_tools_fetch/lib/tools.py @@ -7,6 +7,8 @@ import logging import re +from termcolor import colored + log = logging.getLogger('deps') @@ -21,9 +23,9 @@ class GitBridge(object): BRANCH_REGEX = re.compile("## (?!HEAD)([\w\-_]+)") - EXISTS_TAG = "[ALREADY EXISTS]" - CLONED_TAG = "[CLONED] [BRANCH: '{branch}']" - ERROR_TAG = "[ERROR]" + EXISTS_TAG = colored("[ALREADY EXISTS]", "green") + CLONED_TAG = colored("[CLONED]", "green") + " [BRANCH: '{branch}']" + ERROR_TAG = colored("[ERROR]", "red") @staticmethod def status(repo_folder): @@ -50,7 +52,7 @@ def pull(repo_folder, branch): return output @staticmethod - def clone(url, clone_path, branch="master"): + def clone(name, url, clone_path, branch="master"): """Clone the repo from url into clone_path.""" cmd_clone = GitBridge.CLONE_CMD_MASK.format(url=url, path=clone_path, @@ -59,17 +61,16 @@ def clone(url, clone_path, branch="master"): subprocess.check_output(cmd_clone, stderr=subprocess.STDOUT, shell=True) - return GitBridge.CLONED_TAG.format(branch=branch) + return name, GitBridge.CLONED_TAG.format(branch=branch) except subprocess.CalledProcessError as e: out_str = e.output.decode("utf8") - print(out_str) - log.debug(" Clone output: %s", out_str) if "already exists" in out_str: - return GitBridge.EXISTS_TAG - return GitBridge.ERROR_TAG + return name, GitBridge.EXISTS_TAG + log.critical("Git error: %s", out_str) + return name, GitBridge.ERROR_TAG @staticmethod - def repository_exists(url): + def repository_exists(dependency): """Check if repository exists. Uses `git ls-remote` to check if the repository exists. @@ -80,17 +81,17 @@ def repository_exists(url): Returns: bool: True if exists, False otherwise """ - if url == "": - return False - git_cmd = GitBridge.CHECK_CMD_MASK.format(url=url) + if dependency.url == "": + return dependency, False + git_cmd = GitBridge.CHECK_CMD_MASK.format(url=dependency.url) try: subprocess.check_call(git_cmd, stdout=subprocess.PIPE, stderr=subprocess.PIPE, shell=True) - return True + return dependency, True except subprocess.CalledProcessError: - return False + return dependency, False @staticmethod def get_branch_name(git_status_output): @@ -166,9 +167,10 @@ def list_all_ros_pkgs(): return set(Tools.default_ros_packages) @staticmethod - def decorate(pkg_name): + def decorate(pkg_name, max_width=25): """Decorate a package name.""" - return "[" + pkg_name + "]" + decorated = "[" + pkg_name + "]" + return decorated.ljust(max_width) default_ros_packages = ['actionlib', 'actionlib_msgs', diff --git a/catkin_tools_fetch/lib/update.py b/catkin_tools_fetch/lib/update.py index cf958aa..91ae754 100644 --- a/catkin_tools_fetch/lib/update.py +++ b/catkin_tools_fetch/lib/update.py @@ -6,9 +6,12 @@ import logging import subprocess from os import path +from termcolor import colored +from concurrent import futures from catkin_tools_fetch.lib.tools import Tools from catkin_tools_fetch.lib.tools import GitBridge +from catkin_tools_fetch.lib.printer import Printer log = logging.getLogger('deps') @@ -21,6 +24,7 @@ class Updater(object): OK_TAGS = [PULLED_TAG, UP_TO_DATE_TAG] CHANGES_TAG = "[UNCOMMITTED CHANGES]" + RUNNING_TAG = "[RUNNING]" NO_TRACK_TAG = "[NO BRANCH]" ERROR_TAG = "[GIT ERROR]" CONFLICT_TAG = "[MERGE CONFLICT]" @@ -28,7 +32,13 @@ class Updater(object): UP_TO_DATE_MSG = "Already up-to-date" CONFLICT_MSG = "Automatic merge failed" - def __init__(self, ws_path, packages, conflict_strategy): + def __init__(self, + ws_path, + packages, + conflict_strategy, + use_preprint=True, + colored=True, + num_threads=4): """Initialize the updater. Args: @@ -40,6 +50,10 @@ def __init__(self, ws_path, packages, conflict_strategy): self.ws_path = ws_path self.packages = packages self.conflict_strategy = conflict_strategy + self.thread_pool = futures.ThreadPoolExecutor(max_workers=num_threads) + self.printer = Printer() + self.colored = colored + self.use_preprint = use_preprint def filter_packages(self, selected_packages): """Filter the packages based on user input. @@ -58,6 +72,22 @@ def filter_packages(self, selected_packages): filtered_packages[ws_folder] = package return filtered_packages + def pick_tag(self, folder, package): + """Pick result tag for a folder.""" + if self.use_preprint: + msg = " {}: {}".format(Tools.decorate( + package.name), Updater.RUNNING_TAG) + self.printer.add_msg(package.name, msg) + output, branch, has_changes = GitBridge.status(folder) + if has_changes: + return package, Updater.CHANGES_TAG + try: + output = GitBridge.pull(folder, branch) + return package, Updater.tag_from_output(output) + except subprocess.CalledProcessError as e: + log.debug(" git pull returned error: %s", e) + return package, Updater.ERROR_TAG + def update_packages(self, selected_packages): """Update all the folders to match the remote. Considers the branch. @@ -70,34 +100,21 @@ def update_packages(self, selected_packages): log.info(" Pulling packages:") packages = self.filter_packages(selected_packages) status_msgs = [] - # some helpful vars - abort_on_conflict = self.conflict_strategy == Strategy.ABORT - # stash_on_conflict = self.conflict_strategy == Strategy.STASH + futures_list = [] for ws_folder, package in packages.items(): - log_func = log.info picked_tag = None folder = path.join(self.ws_path, ws_folder) - output, branch, has_changes = GitBridge.status(folder) - if has_changes: - picked_tag = Updater.CHANGES_TAG - else: - try: - output = GitBridge.pull(folder, branch) - picked_tag = Updater.tag_from_output(output) - except subprocess.CalledProcessError as e: - picked_tag = Updater.ERROR_TAG - log.debug(" git pull returned error: %s", e) + futures_list.append( + self.thread_pool.submit(self.pick_tag, folder, package)) + for future in futures.as_completed(futures_list): + package, picked_tag = future.result() # change logger for warning if something is wrong - if picked_tag not in Updater.OK_TAGS: - log_func = log.warning + if self.colored: + picked_tag = Updater.colorize_tag(picked_tag) # now show the results to the user status_msgs.append((package.name, picked_tag)) - log_func(" %-21s: %s", Tools.decorate(package.name), picked_tag) - - # abort if the user wants it - if abort_on_conflict and log_func == log.warning: - log.info(" Abort due to picked strategy: '%s'", Strategy.ABORT) - break + msg = " {}: {}".format(Tools.decorate(package.name), picked_tag) + self.printer.purge_msg(package.name, msg) return status_msgs @staticmethod @@ -113,6 +130,17 @@ def tag_from_output(output): return Updater.CONFLICT_TAG return Updater.PULLED_TAG + @staticmethod + def colorize_tag(picked_tag): + """Colorize the tag.""" + if picked_tag in Updater.OK_TAGS: + return colored(picked_tag, 'green') + + if picked_tag == Updater.CHANGES_TAG: + # this is a warning + return colored(picked_tag, 'yellow') + return colored(picked_tag, 'red') + class Strategy(object): """An enum of stategies for update.""" diff --git a/setup.py b/setup.py index 82be846..1b87ee8 100644 --- a/setup.py +++ b/setup.py @@ -1,3 +1,4 @@ +"""Setup module for catkin_tools_fetch.""" import os import sys from stat import ST_MODE @@ -6,12 +7,15 @@ from setuptools import find_packages from setuptools.command.install import install +version_str = '0.2.0' + # Setup installation dependencies install_requires = [ 'catkin-pkg > 0.2.9', 'catkin_tools >= 0.4.2', 'mock', 'setuptools', + 'termcolor' ] if sys.version_info[0] == 2 and sys.version_info[1] <= 6: install_requires.append('argparse') @@ -28,7 +32,10 @@ class PermissiveInstall(install): + """A class for permissive install.""" + def run(self): + """Run the install procedure.""" install.run(self) if os.name == 'posix': for file in self.get_outputs(): @@ -38,7 +45,6 @@ def run(self): os.chmod(file, mode) -version_str = '0.1.0' github_url = 'https://github.com/niosus/catkin_tools_fetch' setup( @@ -60,11 +66,11 @@ def run(self): 'Intended Audience :: Developers', 'License :: OSI Approved :: Apache Software License', ], - description="A new verb 'fetch' for catkin_tools", - long_description=""" -Provides a new verb 'fetch' for catkin_tools. Allows fetching dependencies of -the packages found inside the catkin workspace. -""", + description="""A new verb 'dependencies' to manage project dependencies with +catkin_tools""", + long_description="""Provides a new verb 'dependencies' or 'deps' for +catkin_tools. Allows fetching dependencies of the packages found inside the +catkin workspace and updating all the packages to the final state.""", test_suite='tests', entry_points={ 'catkin_tools.commands.catkin.verbs': [ diff --git a/tests/test_downloader.py b/tests/test_downloader.py index a7cc5c4..e25d2e4 100644 --- a/tests/test_downloader.py +++ b/tests/test_downloader.py @@ -21,14 +21,14 @@ def tearDown(self): def test_init_empty(self): """Test that initialization is empty.""" ws_path = path.join(path.dirname(__file__), 'data') - downloader = Downloader(ws_path, [], []) + downloader = Downloader(ws_path, [], [], use_preprint=False) self.assertEqual(downloader.ws_path, ws_path) self.assertEqual(downloader.available_pkgs, []) self.assertEqual(downloader.ignore_pkgs, []) def test_download_dependencies_simple(self): """Test simple dependencies downloader.""" - downloader = Downloader(self.test_dir, [], []) + downloader = Downloader(self.test_dir, [], [], use_preprint=False) dependency = Dependency( name="fetch", url="https://github.com/niosus/catkin_tools_fetch") @@ -41,7 +41,7 @@ def test_download_dependencies_simple(self): def test_download_dependencies_again(self): """Test that downloading them again breaks nothing.""" - downloader = Downloader(self.test_dir, [], []) + downloader = Downloader(self.test_dir, [], [], use_preprint=False) dependency = Dependency( name="fetch", url="https://github.com/niosus/catkin_tools_fetch") @@ -55,7 +55,7 @@ def test_download_dependencies_again(self): def test_no_download_for_ros_deps(self): """Test that we skip ROS packages.""" - downloader = Downloader(self.test_dir, [], []) + downloader = Downloader(self.test_dir, [], [], use_preprint=False) roscpp_dep = Dependency(name="roscpp", url="irrelevant_link") std_msgs_dep = Dependency(name="std_msgs", url="irrelevant_link") dep_dict = {"roscpp": roscpp_dep, "std_msgs": std_msgs_dep} @@ -67,7 +67,7 @@ def test_no_download_for_ros_deps(self): def test_no_download_for_wrong_link(self): """Test that we download nothing for a wrong link.""" - downloader = Downloader(self.test_dir, [], []) + downloader = Downloader(self.test_dir, [], [], use_preprint=False) dependency = Dependency(name="fetch", url="wrong_link") dep_dict = {"fetch": dependency} downloader.download_dependencies(dep_dict) @@ -76,7 +76,7 @@ def test_no_download_for_wrong_link(self): def test_no_download_for_wrong_branch(self): """Test that we download nothing for a wrong branch.""" - downloader = Downloader(self.test_dir, [], []) + downloader = Downloader(self.test_dir, [], [], use_preprint=False) dependency = Dependency( name="fetch", url="https://github.com/niosus/catkin_tools_fetch", @@ -89,14 +89,14 @@ def test_no_download_for_wrong_branch(self): def test_init_death(self): """Test death when init is wrong.""" try: - Downloader("blah", [], []) + Downloader("blah", [], [], use_preprint=False) self.fail() except ValueError as e: self.assertTrue(isinstance(e, ValueError)) def test_download_dependencies_death(self): """Test that we throw an exception for wrong input dict.""" - downloader = Downloader(self.test_dir, [], []) + downloader = Downloader(self.test_dir, [], [], use_preprint=False) not_a_dict = {"not", "a dictionary"} self.assertRaises(ValueError, downloader.download_dependencies, diff --git a/tests/test_git_bridge.py b/tests/test_git_bridge.py index 8374349..d61f4f9 100644 --- a/tests/test_git_bridge.py +++ b/tests/test_git_bridge.py @@ -4,6 +4,7 @@ import shutil import tempfile from catkin_tools_fetch.lib.tools import GitBridge +from catkin_tools_fetch.lib.dependency_parser import Dependency class TestGitBridge(unittest.TestCase): @@ -20,7 +21,8 @@ def tearDown(self): def test_status(self): """Test that git status gives us branch and status.""" http_url = "https://github.com/niosus/catkin_tools_fetch" - result = GitBridge.clone(http_url, self.test_dir) + name, result = GitBridge.clone("test", http_url, self.test_dir) + self.assertEqual(name, "test") self.assertEqual(result, GitBridge.CLONED_TAG.format(branch="master")) output, branch, has_changes = GitBridge.status(self.test_dir) expected_output = b"## master...origin/master\n" @@ -40,18 +42,20 @@ def test_status(self): def test_clone(self): """Test if cloning works as expected.""" wrong_url = "https://github.com/niosus" - result = GitBridge.clone(wrong_url, self.test_dir) + name, result = GitBridge.clone("name", wrong_url, self.test_dir) + self.assertEqual(name, "name") self.assertEqual(result, GitBridge.ERROR_TAG) http_url = "https://github.com/niosus/catkin_tools_fetch" - result = GitBridge.clone(http_url, self.test_dir) + _, result = GitBridge.clone("", http_url, self.test_dir) self.assertEqual(result, GitBridge.CLONED_TAG.format(branch="master")) - result = GitBridge.clone(http_url, ".") + _, result = GitBridge.clone("", http_url, ".") self.assertEqual(result, GitBridge.EXISTS_TAG) def test_pull(self): """Test pulling a repository.""" http_url = "https://github.com/niosus/catkin_tools_fetch" - output = GitBridge.clone(http_url, self.test_dir) + _, output = GitBridge.clone( + "catkin_tools_fetch", http_url, self.test_dir) output = GitBridge.pull(self.test_dir, "master") expected_msg = b"""From https://github.com/niosus/catkin_tools_fetch * branch master -> FETCH_HEAD @@ -62,11 +66,18 @@ def test_pull(self): def test_repository_exists(self): """Test behavior if repository exists.""" http_url = "https://github.com/niosus/catkin_tools_fetch" - self.assertTrue(GitBridge.repository_exists(http_url)) + dependency = Dependency(name='test', url=http_url) + dep_res, exists = GitBridge.repository_exists(dependency) + self.assertTrue(exists) + self.assertEqual(dep_res.name, dependency.name) wrong_url = "https://github.com/niosus" - self.assertFalse(GitBridge.repository_exists(wrong_url)) - self.assertFalse(GitBridge.repository_exists("")) + dependency = Dependency(name='test', url=wrong_url) + dep_res, exists = GitBridge.repository_exists(dependency) + self.assertFalse(exists) + dependency = Dependency(name='empty', url='') + dep_res, exists = GitBridge.repository_exists(dependency) + self.assertFalse(exists) def test_get_branch_name(self): """Test getting the branch name.""" diff --git a/tests/test_tools.py b/tests/test_tools.py index 2b8f673..fb93004 100644 --- a/tests/test_tools.py +++ b/tests/test_tools.py @@ -25,7 +25,7 @@ def test_prepare_default_url(self): def test_decorate(self): """Test how we decorate something.""" - self.assertEqual('[blah]', Tools.decorate('blah')) + self.assertEqual('[blah]'.ljust(25), Tools.decorate('blah')) def test_default_ros_pkgs(self): """Test that we actually remove the default ros packages from list.""" diff --git a/tests/test_updater.py b/tests/test_updater.py index ea9b9ba..094427b 100644 --- a/tests/test_updater.py +++ b/tests/test_updater.py @@ -2,6 +2,7 @@ import unittest import shutil import tempfile +from termcolor import colored from mock import MagicMock, PropertyMock from catkin_tools_fetch.lib.update import Updater from catkin_tools_fetch.lib.update import Strategy @@ -83,7 +84,8 @@ def test_filter_packages_none(self): def test_tag_from_output(self): """Test getting a tag from a pull output.""" http_url = "https://github.com/niosus/catkin_tools_fetch" - output = GitBridge.clone(http_url, self.test_dir) + _, output = GitBridge.clone( + "catkin_tools_fetch", http_url, self.test_dir) output = GitBridge.pull(self.test_dir, "master") tag = Updater.tag_from_output(output) self.assertEqual(tag, Updater.UP_TO_DATE_TAG) @@ -107,7 +109,7 @@ def test_merge_success(self): def test_update_full_simple(self): """Test updater end to end on single repo.""" http_url = "https://github.com/niosus/catkin_tools_fetch" - GitBridge.clone(http_url, self.test_dir) + GitBridge.clone("fetch", http_url, self.test_dir) pkg = MagicMock() type(pkg).name = PropertyMock(return_value="pkg") packages = { @@ -118,7 +120,8 @@ def test_update_full_simple(self): status_msgs = updater.update_packages(selected_packages) self.assertEquals(len(status_msgs), 1) self.assertEquals(status_msgs[0][0], "pkg") - self.assertEquals(status_msgs[0][1], Updater.UP_TO_DATE_TAG) + self.assertEquals(status_msgs[0][1], colored( + Updater.UP_TO_DATE_TAG, "green")) def test_list_strategies(self): """Test the list of all strategies."""