Combine exception and file handling cleanups

- combine: Add a proper try/finally block around the conf merge handler.
  Previously still seeing an "unclosed file"  resource warning periodically
  during unit tests.
- Added missing exit code checks to one of the 'combine' unit tests failing for
  Travis.  (Unable to reproduce locally; on a Mac)
- combine:  Use the word 'directory' vs 'folder' uniformly in user output.
- Bump pre-commit-hooks to latest 1.x release (skipping 2.0 for now)

.pre-commit-config.yaml

@@ -2,7 +2,7 @@
 # See http://pre-commit.com/hooks.html for more hooks
 repos:
 - repo: https://github.com/pre-commit/pre-commit-hooks
-  sha: v1.3.0
+  sha: v1.4.0
   hooks:
     - id: trailing-whitespace
       exclude: \.md$

ksconf/commands/combine.py

@@ -161,10 +161,10 @@ def run(self, args):
             self.stderr.write("Must provide the '--target' directory.\n")
             return EXIT_CODE_MISSING_ARG
 
-            self.stderr.write("Combining conf files into {}\n".format(args.target))
+            self.stderr.write("Combining conf files into directory {}\n".format(args.target))
         args.source = list(_expand_glob_list(args.source))
         for src in args.source:
-            self.stderr.write("Reading conf files from {}\n".format(src))
+            self.stderr.write("Reading conf files from directory {}\n".format(src))
 
         marker_file = os.path.join(args.target, CONTROLLED_DIR_MARKER)
         if os.path.isdir(args.target):
@@ -174,9 +174,9 @@ def run(self, args):
                 return EXIT_CODE_COMBINE_MARKER_MISSING
         elif args.dry_run:
             self.stderr.write(
-                "Skipping creating destination folder {0} (dry-run)\n".format(args.target))
+                "Skipping creating destination directory {0} (dry-run)\n".format(args.target))
         else:
-            self.stderr.write("Creating destination folder {0}\n".format(args.target))
+            self.stderr.write("Creating destination directory {0}\n".format(args.target))
             os.mkdir(args.target)
             open(marker_file, "w").write("This directory is managed by KSCONF.  Don't touch\n")
 
@@ -235,17 +235,23 @@ def run(self, args):
                     self.stderr.write(
                         "Copy <{0}>   {1:50}  from {2}\n".format(smart_rc, dest_path, src_file))
             else:
-                # Handle merging conf files
-                dest = ConfFileProxy(os.path.join(args.target, dest_fn), "r+",
-                                     parse_profile=PARSECONF_MID)
-                srcs = [ConfFileProxy(sf, "r", parse_profile=PARSECONF_STRICT) for sf in src_files]
-                # self.stderr.write("Considering {0:50}  CONF MERGE from source:  {1!r}\n".format(dest_fn, src_files[0]))
-                smart_rc = merge_conf_files(dest, srcs, dry_run=args.dry_run,
-                                            banner_comment=args.banner)
-                if smart_rc != SMART_NOCHANGE:
-                    self.stderr.write(
-                        "Merge <{0}>   {1:50}  from {2!r}\n".format(smart_rc, dest_path,
-                                                                    src_files))
+                try:
+                    # Handle merging conf files
+                    dest = ConfFileProxy(os.path.join(args.target, dest_fn), "r+",
+                                         parse_profile=PARSECONF_MID)
+                    srcs = [ConfFileProxy(sf, "r", parse_profile=PARSECONF_STRICT) for sf in src_files]
+                    # self.stderr.write("Considering {0:50}  CONF MERGE from source:  {1!r}\n".format(dest_fn, src_files[0]))
+                    smart_rc = merge_conf_files(dest, srcs, dry_run=args.dry_run,
+                                                banner_comment=args.banner)
+                    if smart_rc != SMART_NOCHANGE:
+                        self.stderr.write(
+                            "Merge <{0}>   {1:50}  from {2!r}\n".format(smart_rc, dest_path,
+                                                                        src_files))
+                finally:
+                    # Protect against any dangling open files:  (ResourceWarning: unclosed file)
+                    dest.close()
+                    for src in srcs:
+                        src.close()
 
         if True and target_extra_files:  # Todo: Allow for cleanup to be disabled via CLI
             self.stderr.write("Cleaning up extra files not part of source tree(s):  {0} files.\n".

tests/test_cli.py

@@ -277,6 +277,7 @@ def test_combine_3dir(self):
         default = twd.get_path("etc/apps/Splunk_TA_aws/default")
         with ksconf_cli:
             ko = ksconf_cli("combine", "--target", default, default + ".d/*")
+            self.assertEquals(ko.returncode, EXIT_CODE_SUCCESS)
             cfg = parse_conf(twd.get_path("etc/apps/Splunk_TA_aws/default/props.conf"))
             self.assertIn("aws:config", cfg)
             self.assertEqual(cfg["aws:config"]["ANNOTATE_PUNCT"], "true")
@@ -297,6 +298,7 @@ def test_combine_3dir(self):
         twd.write_file("etc/apps/Splunk_TA_aws/default/data/dead.conf", "# File to remove")
         with ksconf_cli:
             ko = ksconf_cli("combine", "--dry-run", "--target", default, default + ".d/*")
+            self.assertEquals(ko.returncode, EXIT_CODE_SUCCESS)
             self.assertRegex(ko.stdout, r'[\r\n][-]\s*<view name="search"')
             self.assertRegex(ko.stdout, r"[\r\n][+]TIME_FORMAT = [^\r\n]+%6N")
         with ksconf_cli: