From e8700d7db1f26b276fb2961fea4638ddb897a0c0 Mon Sep 17 00:00:00 2001 From: "Fabien C. Y. Benureau" Date: Tue, 2 Jun 2020 18:55:24 +0900 Subject: [PATCH] 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 | 4 ++- pubs/utils.py | 7 +++--- tests/fake_env.py | 25 +----------------- tests/test_note_append.py | 2 +- tests/test_usecase.py | 53 ++++++++++++++++++++++++++------------- 5 files changed, 45 insertions(+), 46 deletions(-) diff --git a/pubs/commands/tag_cmd.py b/pubs/commands/tag_cmd.py index 485d785..c12bf8f 100644 --- a/pubs/commands/tag_cmd.py +++ b/pubs/commands/tag_cmd.py @@ -55,7 +55,9 @@ def _parse_tag_seq(s): if last != 0: raise ValueError('could not match tag expression') else: - tags.append(s[last:(m.start())]) + tag = s[last:(m.start())] + if len(tag) > 0: + tags.append(s[last:(m.start())]) last = m.start() if last == len(s): raise ValueError('could not match tag expression') diff --git a/pubs/utils.py b/pubs/utils.py index e7878f5..120edfd 100644 --- a/pubs/utils.py +++ b/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 " - "citekeys:".format(citekey)) + msg = ["Be more specific; '{}' matches multiples 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 diff --git a/tests/fake_env.py b/tests/fake_env.py index 41d22d7..1d1865f 100644 --- a/tests/fake_env.py +++ b/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 diff --git a/tests/test_note_append.py b/tests/test_note_append.py index c3009f2..32eebee 100644 --- a/tests/test_note_append.py +++ b/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)) diff --git a/tests/test_usecase.py b/tests/test_usecase.py index 891d9cc..ba94559 100644 --- a/tests/test_usecase.py +++ b/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_cmd.split(), expected_out, expected_err) + actual_out = self.execute_cmd_capture(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( - actual_cmd)) + self.fail('Unexpected input asked by command: {}.'.format(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): - """Run a single command, which has been split into a list containing cmd and args""" - capture_wrap = fake_env.capture(pubs_cmd.execute, - verbose=PRINT_OUTPUT) - _, stdout, stderr = capture_wrap(actual_cmdlist) - actual_out = self.normalize(stdout) - actual_err = self.normalize(stderr) + def execute_cmd_capture(self, cmd, expected_out, expected_err): + """Run a single command, captures the output and and stderr and compare it to the expected ones""" + sys_stdout, sys_stderr = sys.stdout, sys.stderr + sys.stdout = p3._fake_stdio(additional_out=sys_stdout if PRINT_OUTPUT else None) + sys.stderr = p3._fake_stdio(additional_out=sys_stderr if PRINT_OUTPUT else None) + + try: + 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(actual_out), p3.u_maybe(expected_out)) + self.assertEqual(p3.u_maybe(self.captured_stdout), 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): - pass + 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