added test for ambiguous citkeys

* also reworked the test code to allow to capture output even
  when pubs throws an error.
* empty tags are not added to the metadata anymore (does not affect
  existing instances)

pubs/commands/tag_cmd.py

@@ -55,6 +55,8 @@ def _parse_tag_seq(s):
             if last != 0:
                 raise ValueError('could not match tag expression')
         else:
+            tag = s[last:(m.start())]
+            if len(tag) > 0:
                 tags.append(s[last:(m.start())])
         last = m.start()
     if last == len(s):

pubs/utils.py

@@ -29,11 +29,12 @@ def resolve_citekey(repo, conf, citekey, ui=None, exit_on_fail=True):
     elif citekey not in citekeys:
         if ui is not None:
             citekeys = sorted(citekeys)
-            ui.error("Be more specific; '{}' matches multiples "
+            msg = ["Be more specific; '{}' matches multiples citekeys:".format(citekey)]
-                     "citekeys:".format(citekey))
             for c in citekeys:
                 p = repo.pull_paper(c)
-                ui.message('    {}'.format(pretty.paper_oneliner(p, max_authors=conf['main']['max_authors'])))
+                paper_str = pretty.paper_oneliner(p, max_authors=conf['main']['max_authors'])
+                msg.append('    {}'.format(paper_str))
+            ui.error('\n'.join(msg))
             if exit_on_fail:
                 ui.exit()
     return citekey

tests/fake_env.py

@@ -9,7 +9,7 @@ import dotdot
 
 from pyfakefs import fake_filesystem, fake_filesystem_unittest
 
-from pubs.p3 import input, _fake_stdio, _get_fake_stdio_ucontent
+from pubs.p3 import input
 from pubs import content, filebroker, uis
 
 # code for fake fs
@@ -29,29 +29,6 @@ original_exception_handler = uis.InputUI.handle_exception
 locale.setlocale(locale.LC_ALL, '')
 
 
-# capture output
-
-def capture(f, verbose=False):
-    """Capture the stdout and stderr output.
-
-    Useful for comparing the output with the expected one during tests.
-
-    :param f:        The function to capture output from.
-    :param verbose:  If True, print call will still display their outputs.
-                     If False, they will be silenced.
-
-    """
-    def newf(*args, **kwargs):
-        old_stderr, old_stdout = sys.stderr, sys.stdout
-        sys.stdout = _fake_stdio(additional_out=old_stdout if verbose else None)
-        sys.stderr = _fake_stdio(additional_out=old_stderr if verbose else None)
-        try:
-            return f(*args, **kwargs), _get_fake_stdio_ucontent(sys.stdout), _get_fake_stdio_ucontent(sys.stderr)
-        finally:
-            sys.stderr, sys.stdout = old_stderr, old_stdout
-    return newf
-
-
 # Test helpers
 
 # automating input

tests/test_note_append.py

@@ -46,7 +46,7 @@ class TestNoteAppend(DataCommandTestCase):
         #   * Pass the command split into a command and its args to
         #     execute_cmdsplit, which is called by execute_cmds:
         cmd_split = ['pubs', 'note', 'Page99', '-a', 'xxx yyy']
-        self.execute_cmdsplit(cmd_split, expected_out=None, expected_err=None)
+        self.execute_cmd_capture(cmd_split, expected_out=None, expected_err=None)
         note_lines.append('xxx yyy')
         self.assertFileContentEqual(fin_notes, self._get_note_content(note_lines))
 

tests/test_usecase.py

@@ -17,6 +17,7 @@ import dotdot
 import fake_env
 import mock_requests
 
+
 from pubs import pubs_cmd, color, content, uis, p3, endecoder
 from pubs.config import conf
 
@@ -118,14 +119,12 @@ class CommandTestCase(fake_env.TestFakeFs):
                 input.as_global()
                 try:
                     if capture_output:
-                        actual_out = self.execute_cmdsplit(
+                        actual_out = self.execute_cmd_capture(actual_cmd.split(), expected_out, expected_err)
-                            actual_cmd.split(), expected_out, expected_err)
                         outs.append(color.undye(actual_out))
                     else:
                         pubs_cmd.execute(actual_cmd.split())
                 except fake_env.FakeInput.UnexpectedInput:
-                    self.fail('Unexpected input asked by command: {}.'.format(
+                    self.fail('Unexpected input asked by command: {}.'.format(actual_cmd))
-                        actual_cmd))
             return outs
         except SystemExit as exc:
             exc_class, exc, tb = sys.exc_info()
@@ -145,21 +144,25 @@ class CommandTestCase(fake_env.TestFakeFs):
             pass
         return s
 
-    def execute_cmdsplit(self, actual_cmdlist, expected_out, expected_err):
+    def execute_cmd_capture(self, cmd, expected_out, expected_err):
-        """Run a single command, which has been split into a list containing cmd and args"""
+        """Run a single command, captures the output and and stderr and compare it to the expected ones"""
-        capture_wrap = fake_env.capture(pubs_cmd.execute,
+        sys_stdout, sys_stderr = sys.stdout, sys.stderr
-                                        verbose=PRINT_OUTPUT)
+        sys.stdout = p3._fake_stdio(additional_out=sys_stdout if PRINT_OUTPUT else None)
-        _, stdout, stderr = capture_wrap(actual_cmdlist)
+        sys.stderr = p3._fake_stdio(additional_out=sys_stderr if PRINT_OUTPUT else None)
-        actual_out = self.normalize(stdout)
-        actual_err = self.normalize(stderr)
-        if expected_out is not None:
-            self.assertEqual(p3.u_maybe(actual_out), p3.u_maybe(expected_out))
-        if expected_err is not None:
-            self.assertEqual(p3.u_maybe(actual_err), p3.u_maybe(expected_err))
-        return actual_out
 
-    def tearDown(self):
+        try:
-        pass
+            pubs_cmd.execute(cmd)
+        finally:
+            # capturing output even if exception was raised.
+            self.captured_stdout = self.normalize(p3._get_fake_stdio_ucontent(sys.stdout))
+            self.captured_stderr = self.normalize(p3._get_fake_stdio_ucontent(sys.stderr))
+            sys.stderr, sys.stdout = sys_stderr, sys_stdout
+
+        if expected_out is not None:
+            self.assertEqual(p3.u_maybe(self.captured_stdout), p3.u_maybe(expected_out))
+        if expected_err is not None:
+            self.assertEqual(p3.u_maybe(self.captured_stderr), p3.u_maybe(expected_err))
+        return self.captured_stdout
 
     def update_config(self, config_update, path=None):
         """Allow to set the config parameters. Must have done a `pubs init` beforehand."""
@@ -624,6 +627,7 @@ class TestTag(DataCommandTestCase):
         with self.assertRaises(FakeSystemExit):
             self.execute_cmds(cmds)
 
+
 class TestURL(DataCommandTestCase):
 
     def setUp(self):
@@ -1126,6 +1130,21 @@ class TestUsecase(DataCommandTestCase):
         self.execute_cmds(cmds, capture_output=True)
 #        self.assertEqual(correct, self.execute_cmds(cmds, capture_output=True))
 
+    def test_ambiguous_citekey(self):
+        cmds = ['pubs init',
+                'pubs add data/pagerank.bib',
+                'pubs add data/pagerank.bib', # now we have Page99 and Page99a
+                'pubs edit Page',
+                ]
+        output = '\n'.join(["error: Be more specific; 'Page' matches multiples citekeys:",
+                            "    [Page99] Page, Lawrence et al. \"The PageRank Citation Ranking: Bringing Order to the Web.\" (1999) ",
+                            "    [Page99a] Page, Lawrence et al. \"The PageRank Citation Ranking: Bringing Order to the Web.\" (1999) \n"])
+
+        with self.assertRaises(FakeSystemExit):
+            self.execute_cmds(cmds)
+
+        self.assertEqual(self.captured_stderr, output)
+
 
 
 @ddt.ddt