Fix broken checkout behavior introduced by PR #172.

Finding locally installed optional components was broken both because it did not correctly inspect the status of those components, and because it added components-to-checkout by path instead of by name.

Printing status updates also a bit wonky because 'print_progress' was not correctly passed down.

Slight update to testing docs:  no need to run python 2 and 3 separately, and system tests now exist.

manic/checkout.py

@@ -402,8 +402,10 @@ def main(args):
 
     for comp in args.components:
         if comp not in ext_description.keys():
+            # Note we can't print out the list of found externals because
+            # they were filtered in create_externals_description above.
             fatal_error(
-                "No component {} found in {}".format(
+                "No component {} found in {}.".format(
                     comp, args.externals))
 
     source_tree = SourceTree(root_dir, ext_description, svn_ignore_ancestry=args.svn_ignore_ancestry)

manic/sourcetree.py

@@ -1,5 +1,5 @@
 """
-
+0;10;1c
 FIXME(bja, 2017-11) External and SourceTree have a circular dependancy!
 """
 
@@ -98,12 +98,14 @@ def get_local_path(self):
         """
         return self._local_path
 
-    def status(self, force=False):
+    def status(self, force=False, print_progress=False):
         """
-        Returns status of this component and all subcomponents (if available).
+        Returns status of this component and all subcomponents.
 
-        Returns a dict mapping our local path to an ExternalStatus dict. Any
-        subcomponents will have their own top-level key.
+        Returns a dict mapping our local path (not component name!) to an
+        ExternalStatus dict. Any subcomponents will have their own top-level
+        path keys.  Note the return value includes entries for this and all 
+        subcomponents regardless of whether they are locally installed or not.
 
         Side-effect: If self._stat is empty or force is True, calculates _stat.
         """
@@ -151,7 +153,7 @@ def status(self, force=False):
                 # SourceTree.status() expects to be called from the correct
                 # root directory.
                 os.chdir(self._repo_dir_path)
-                subcomponent_stats = self._externals_sourcetree.status(self._local_path, force=force)
+                subcomponent_stats = self._externals_sourcetree.status(self._local_path, force=force, print_progress=print_progress)
                 os.chdir(cwd)
 
         # Merge our status + subcomponent statuses into one return dict keyed
@@ -311,19 +313,22 @@ def status(self, relative_path_base=LOCAL_PATH_INDICATOR,
                force=False, print_progress=False):
         """Return a dictionary of local path->ExternalStatus.
 
-        Note that all traversed components, whether recursive or top-level, have
-        a top-level key in the returned dictionary.
-
-        Note that all components that are checked out locally, whether required or 
-        optional, ar included in the returned status.
-        """
+        Notes about the returned dictionary:
+          * It is keyed by local path (e.g. 'components/mom'), not by
+            component name (e.g. 'mom').
+          * It contains top-level keys for all traversed components, whether
+            discovered by recursion or top-level.
+          * It contains entries for all components regardless of whether they
+            are locally installed or not, or required or optional.
+x        """
         load_comps = self._all_components.keys()
 
         summary = {}  # Holds merged statuses from all components.
         for comp in load_comps:
             if print_progress:
                 printlog('{0}, '.format(comp), end='')
-            stat = self._all_components[comp].status(force=force)
+            stat = self._all_components[comp].status(force=force,
+                                                     print_progress=print_progress)
 
             # Returned status dictionary is keyed by local path; prepend
             # relative_path_base if not already there.
@@ -342,14 +347,16 @@ def status(self, relative_path_base=LOCAL_PATH_INDICATOR,
 
     def _find_installed_optional_components(self):
         """Returns a list of installed optional component names, if any."""
-        installed_comps = set()
+        installed_comps = []
         for comp_name, ext in self._all_components.items():
             if comp_name in self._required_compnames:
                 continue
             # Note that in practice we expect this status to be cached.
-            stat = ext.status()
-            installed_comps.update(stat.keys())
-        return list(installed_comps)
+            path_to_stat = ext.status()
+            if any(stat.sync_state != ExternalStatus.EMPTY
+                   for stat in path_to_stat.values()):
+                installed_comps.append(comp_name)
+        return installed_comps
 
     def checkout(self, verbosity, load_all, load_comp=None):
         """

test/README.md

@@ -18,28 +18,18 @@ Development environments should be setup for python2 and python3:
 
 ## Unit tests
 
-Tests should be run for both python2 and python3. It is recommended
-that you have seperate terminal windows open python2 and python3
-testing to avoid errors activating and deactivating environments.
-
 ```SH
     cd checkout_externals/test
-    . env_python2/bin/activate
     make utest
-    deactivate
 ```
 
+## System tests
+
 ```SH
     cd checkout_externals/test
-    . env_python2/bin/activate
-    make utest
-    deactivate
+    make stest
 ```
 
-## System tests
-
-Not yet implemented.
-
 ## Static analysis
 
 checkout_externals is difficult to test thoroughly because it relies
@@ -51,9 +41,7 @@ regularly for automatic code formatting and linting.
 
 ```SH
     cd checkout_externals/test
-    . env_python2/bin/activate
     make lint
-    deactivate
 ```
 
 The canonical formatting for the code is whatever autopep8
@@ -68,10 +56,8 @@ coverage, run the code coverage tool:
 
 ```SH
     cd checkout_externals/test
-    . env_python2/bin/activate
     make coverage
     open -a Firefox.app htmlcov/index.html
-    deactivate
 ```