Bugfix for config merge and unit tests

2 files changed, 157 insertions, 11 deletions

diff --git a/paper2remarkable/ui.py b/paper2remarkable/ui.py
index 6f95e30..1d1e011 100644
--- a/paper2remarkable/ui.py
+++ b/paper2remarkable/ui.py
@@ -21,7 +21,7 @@ from .providers import providers, LocalFile
 from .utils import follow_redirects, is_url
 
 
-def parse_args():
+def build_argument_parser():
     parser = argparse.ArgumentParser(
         description="Paper2reMarkable version %s" % __version__
     )
@@ -129,6 +129,11 @@ def parse_args():
         help="One or more URLs to a paper or paths to local PDF files",
         nargs="+",
     )
+    return parser
+
+
+def parse_args():
+    parser = build_argument_parser()
     return parser.parse_args()
 
 
@@ -199,24 +204,26 @@ def load_config(path=None):
     if path is None:
         path = os.path.join(os.path.expanduser("~"), ".paper2remarkable.yml")
     if not os.path.exists(path):
-        return {"core": {}, "system": {}, "html": {}}
+        return None
     with open(path, "r") as fp:
         config = yaml.safe_load(fp)
     return config
 
 
-def merge_options(config, args):
+def merge_options(args, config=None):
     # command line arguments always overwrite config
+    config = {} if config is None else config
+
     opts = copy.deepcopy(config)
     opts.setdefault("core", {})
     opts.setdefault("system", {})
     opts.setdefault("html", {})
 
-    def set_bool(d, key, value):
+    def set_bool(d, key, value, invert=False):
         if value:
-            d[key] = True
+            d[key] = True ^ invert
         elif not key in d:
-            d[key] = False
+            d[key] = False ^ invert
 
     def set_path(d, key, value):
         if not value is None:
@@ -226,7 +233,7 @@ def merge_options(config, args):
 
     set_bool(opts["core"], "blank", args.blank)
     set_bool(opts["core"], "verbose", args.verbose)
-    set_bool(opts["core"], "upload", not args.no_upload)
+    set_bool(opts["core"], "upload", args.no_upload, invert=True)
     set_bool(opts["core"], "experimental", args.experimental)
 
     if args.center:
@@ -248,14 +255,14 @@ def merge_options(config, args):
         with open(args.css, "r") as fp:
             contents = fp.read()
         opts["html"]["css"] = contents
-    else:
+    elif not "css" in opts["html"]:
         opts["html"]["css"] = None
 
     if args.font_urls and os.path.exists(args.font_urls):
         with open(args.font_urls, "r") as fp:
             urls = [l.strip() for l in fp.readlines()]
         opts["html"]["font_urls"] = urls
-    else:
+    elif not "font_urls" in opts["html"]:
         opts["html"]["font_urls"] = None
 
     return opts
@@ -292,7 +299,7 @@ def main():
         )
 
     config = load_config(path=args.config)
-    options = merge_options(config, args)
+    options = merge_options(args, config=config)
 
     filenames = (
         [None] * len(args.input) if not args.filename else args.filename
diff --git a/tests/test_ui.py b/tests/test_ui.py
index d36c5b3..1f4dab7 100644
--- a/tests/test_ui.py
+++ b/tests/test_ui.py
@@ -33,7 +33,11 @@ from paper2remarkable.providers import (
     PubMed,
     Springer,
 )
-from paper2remarkable.ui import choose_provider
+from paper2remarkable.ui import (
+    build_argument_parser,
+    choose_provider,
+    merge_options,
+)
 
 
 class TestUI(unittest.TestCase):
@@ -220,6 +224,141 @@ class TestUI(unittest.TestCase):
         with self.assertRaises(InvalidURLError):
             choose_provider(url)
 
+    def test_merge_options_1(self):
+        config = None
+        source = "/tmp/local.pdf"  # doesn't need to exist
+
+        parser = build_argument_parser()
+        args = parser.parse_args([source])
+
+        # empty config and default args
+        opts = merge_options(args, config)
+        self.assertEqual(opts["core"]["blank"], False)
+        self.assertEqual(opts["core"]["crop"], "left")
+        self.assertEqual(opts["core"]["experimental"], False)
+        self.assertEqual(opts["core"]["upload"], True)
+        self.assertEqual(opts["core"]["verbose"], False)
+
+        test_sys = lambda s: self.assertEqual(opts["system"][s], s)
+        for s in ["gs", "pdftoppm", "pdftk", "qpdf", "rmapi"]:
+            with self.subTest(s):
+                test_sys(s)
+
+        self.assertIsNone(opts["html"]["css"])
+        self.assertIsNone(opts["html"]["font_urls"])
+
+    def test_merge_options_2(self):
+        config = None
+        source = "/tmp/local.pdf"  # doesn't need to exist
+
+        parser = build_argument_parser()
+        args = parser.parse_args(["-v", "-n", source])
+
+        # empty config and default args
+        opts = merge_options(args, config)
+
+        self.assertEqual(opts["core"]["blank"], False)
+        self.assertEqual(opts["core"]["crop"], "left")
+        self.assertEqual(opts["core"]["experimental"], False)
+        self.assertEqual(opts["core"]["upload"], False)
+        self.assertEqual(opts["core"]["verbose"], True)
+
+        test_sys = lambda s: self.assertEqual(opts["system"][s], s)
+        for s in ["gs", "pdftoppm", "pdftk", "qpdf", "rmapi"]:
+            with self.subTest(s):
+                test_sys(s)
+
+        self.assertIsNone(opts["html"]["css"])
+        self.assertIsNone(opts["html"]["font_urls"])
+
+    def test_merge_options_3(self):
+        source = "/tmp/local.pdf"  # doesn't need to exist
+
+        parser = build_argument_parser()
+        args = parser.parse_args(["-v", "-n", "-k", source])
+        config = {"core": {"blank": True, "upload": True, "verbose": False}}
+
+        # empty config and default args
+        opts = merge_options(args, config)
+
+        self.assertEqual(opts["core"]["blank"], True)
+        self.assertEqual(opts["core"]["crop"], "none")
+        self.assertEqual(opts["core"]["experimental"], False)
+        self.assertEqual(opts["core"]["upload"], False)
+        self.assertEqual(opts["core"]["verbose"], True)
+
+        test_sys = lambda s: self.assertEqual(opts["system"][s], s)
+        for s in ["gs", "pdftoppm", "pdftk", "qpdf", "rmapi"]:
+            with self.subTest(s):
+                test_sys(s)
+
+        self.assertIsNone(opts["html"]["css"])
+        self.assertIsNone(opts["html"]["font_urls"])
+
+    def test_merge_options_4(self):
+        source = "/tmp/local.pdf"  # doesn't need to exist
+
+        parser = build_argument_parser()
+        args = parser.parse_args(["-n", "-c", source])
+        gs_path = "/path/to/gs"
+        config = {
+            "core": {"upload": False, "verbose": True},
+            "system": {"gs": gs_path},
+        }
+
+        # empty config and default args
+        opts = merge_options(args, config)
+
+        self.assertEqual(opts["core"]["blank"], False)
+        self.assertEqual(opts["core"]["crop"], "center")
+        self.assertEqual(opts["core"]["experimental"], False)
+        self.assertEqual(opts["core"]["upload"], False)
+        self.assertEqual(opts["core"]["verbose"], True)
+
+        self.assertEqual(opts["system"]["gs"], gs_path)
+        test_sys = lambda s: self.assertEqual(opts["system"][s], s)
+        for s in ["pdftoppm", "pdftk", "qpdf", "rmapi"]:
+            with self.subTest(s):
+                test_sys(s)
+
+        self.assertIsNone(opts["html"]["css"])
+        self.assertIsNone(opts["html"]["font_urls"])
+
+    def test_merge_options_5(self):
+        source = "/tmp/local.pdf"  # doesn't need to exist
+
+        parser = build_argument_parser()
+        args = parser.parse_args(["-n", "-c", source])
+        gs_path = "/path/to/gs"
+        qpdf_path = "/path/to/qpdf"
+        config = {
+            "core": {"upload": False, "verbose": True},
+            "system": {"gs": gs_path, "qpdf": qpdf_path},
+            "html": {
+                "css": "Hello, World!\n",
+                "font_urls": ["url_1", "url_2"],
+            },
+        }
+
+        # empty config and default args
+        opts = merge_options(args, config)
+
+        self.assertEqual(opts["core"]["blank"], False)
+        self.assertEqual(opts["core"]["crop"], "center")
+        self.assertEqual(opts["core"]["experimental"], False)
+        self.assertEqual(opts["core"]["upload"], False)
+        self.assertEqual(opts["core"]["verbose"], True)
+
+        self.assertEqual(opts["system"]["gs"], gs_path)
+        self.assertEqual(opts["system"]["qpdf"], qpdf_path)
+        test_sys = lambda s: self.assertEqual(opts["system"][s], s)
+        for s in ["pdftoppm", "pdftk", "rmapi"]:
+            with self.subTest(s):
+                test_sys(s)
+
+        self.assertEquals(opts["html"]["css"], "Hello, World!\n")
+        self.assertEquals(opts["html"]["font_urls"], ["url_1", "url_2"])
+
 
 if __name__ == "__main__":
     unittest.main()