From dbd8c1f5d0fb5393111fef11d9cfc6fa79b23d07 Mon Sep 17 00:00:00 2001 From: Louis Date: Thu, 28 Jan 2016 18:24:53 +0100 Subject: [PATCH] Better handling of song parsing errors of chordpro files Closes #191 --- patacrep/songs/chordpro/syntax.py | 8 +- patacrep/tools/convert/__main__.py | 6 +- test/test_patatools/test_convert.py | 57 ++++++-- .../test_convert_errors/song.tsg | 12 -- .../test_convert_failure/song.csg | 122 ++++++++++++++++++ .../test_convert_failure/song.csg.tsg.control | 0 6 files changed, 177 insertions(+), 28 deletions(-) delete mode 100644 test/test_patatools/test_convert_errors/song.tsg create mode 100644 test/test_patatools/test_convert_failure/song.csg create mode 100644 test/test_patatools/test_convert_failure/song.csg.tsg.control diff --git a/patacrep/songs/chordpro/syntax.py b/patacrep/songs/chordpro/syntax.py index 04104ac6..76d2a0d8 100644 --- a/patacrep/songs/chordpro/syntax.py +++ b/patacrep/songs/chordpro/syntax.py @@ -38,6 +38,8 @@ class ChordproParser(Parser): lexer = ChordProLexer(filename=self.filename) ast.AST.lexer = lexer.lexer parsed = self.parser.parse(content, lexer=lexer.lexer) + if parsed is None: + raise ContentError(message='Fatal error during song parsing.') parsed.error_builders.extend(lexer.error_builders) return parsed @@ -325,8 +327,4 @@ class ChordproParser(Parser): def parse_song(content, filename=None): """Parse song and return its metadata.""" - parser = ChordproParser(filename) - parsed_content = parser.parse(content) - if parsed_content is None: - raise ContentError(message='Fatal error during song parsing.') - return parsed_content + return ChordproParser(filename).parse(content) diff --git a/patacrep/tools/convert/__main__.py b/patacrep/tools/convert/__main__.py index f64badc9..683e8535 100644 --- a/patacrep/tools/convert/__main__.py +++ b/patacrep/tools/convert/__main__.py @@ -5,6 +5,7 @@ import logging import sys from patacrep import files +from patacrep.content import ContentError from patacrep.songs import DEFAULT_CONFIG from patacrep.utils import yesno @@ -57,8 +58,8 @@ def main(args=None): sys.exit(1) for file in song_files: - song = renderers[dest][source](file, DEFAULT_CONFIG) try: + song = renderers[dest][source](file, DEFAULT_CONFIG) destname = "{}.{}".format(".".join(file.split(".")[:-1]), dest) if os.path.exists(destname): if not confirm(destname): @@ -66,6 +67,9 @@ def main(args=None): with open(destname, "w") as destfile: destfile.write(song.render()) + except ContentError as error: + LOGGER.error("Cannot parse file '%s'.", file) + sys.exit(1) except NotImplementedError: LOGGER.error("Cannot convert to format '%s'.", dest) sys.exit(1) diff --git a/test/test_patatools/test_convert.py b/test/test_patatools/test_convert.py index 38ee9995..30c1dc24 100644 --- a/test/test_patatools/test_convert.py +++ b/test/test_patatools/test_convert.py @@ -43,7 +43,10 @@ class TestConvert(unittest.TestCase, metaclass=dynamic.DynamicTest): with logging_reduced(): if os.path.exists(destname): os.remove(destname) - self._system(main, args + [in_format, out_format, sourcename]) + self.assertEqual( + self._system(main, args + [in_format, out_format, sourcename]), + 0, + ) expected = controlfile.read().strip().replace( "@TEST_FOLDER@", files.path2posix(resource_filename(__name__, "")), @@ -54,6 +57,26 @@ class TestConvert(unittest.TestCase, metaclass=dynamic.DynamicTest): expected, ) + def assertFailConvert(self, basename, in_format, out_format): + """Test of the "patatools convert" subcommand""" + sourcename = "{}.{}".format(basename, in_format) + destname = "{}.{}".format(basename, out_format) + controlname = "{}.{}.control".format(sourcename, out_format) + for main, args in [ + (tools_main, ["patatools", "convert"]), + (convert_main, ["patatools-convert"]), + ]: + with self.subTest(main=main, args=args): + with self.chdir("test_convert_failure"): + with open_read(controlname) as controlfile: + with logging_reduced(): + if os.path.exists(destname): + os.remove(destname) + self.assertEqual( + self._system(main, args + [in_format, out_format, sourcename]), + 1, + ) + @staticmethod @contextlib.contextmanager def chdir(*path): @@ -65,17 +88,21 @@ class TestConvert(unittest.TestCase, metaclass=dynamic.DynamicTest): @classmethod def _iter_testmethods(cls): """Iterate over song files to test.""" - with cls.chdir("test_convert_success"): - for control in sorted(glob.glob('*.*.*.control')): - [*base, in_format, out_format, _] = control.split('.') - base = '.'.join(base) - yield ( - "test_{}_{}_{}".format(base, in_format, out_format), - cls._create_test(base, in_format, out_format), - ) + for directory, create_test in [ + ("test_convert_success", cls._create_test_success), + ("test_convert_failure", cls._create_test_failure), + ]: + with cls.chdir(directory): + for control in sorted(glob.glob('*.*.*.control')): + [*base, in_format, out_format, _] = control.split('.') + base = '.'.join(base) + yield ( + "test_{}_{}_{}".format(base, in_format, out_format), + create_test(base, in_format, out_format), + ) @classmethod - def _create_test(cls, base, in_format, out_format): + def _create_test_success(cls, base, in_format, out_format): """Return a function testing that `base` compilation from `in_format` to `out_format` format. """ test_parse_render = lambda self: self.assertConvert(base, in_format, out_format) @@ -83,3 +110,13 @@ class TestConvert(unittest.TestCase, metaclass=dynamic.DynamicTest): "Test that '{base}.{in_format}' is correctly converted to '{out_format}'." ).format(base=os.path.basename(base), in_format=in_format, out_format=out_format) return test_parse_render + + @classmethod + def _create_test_failure(cls, base, in_format, out_format): + """Return a function testing that `base` compilation from `in_format` to `out_format` format. + """ + test_parse_render = lambda self: self.assertFailConvert(base, in_format, out_format) + test_parse_render.__doc__ = ( + "Test that '{base}.{in_format}' raises an error when trying to convert it to '{out_format}'." + ).format(base=os.path.basename(base), in_format=in_format, out_format=out_format) + return test_parse_render diff --git a/test/test_patatools/test_convert_errors/song.tsg b/test/test_patatools/test_convert_errors/song.tsg deleted file mode 100644 index c57d0f79..00000000 --- a/test/test_patatools/test_convert_errors/song.tsg +++ /dev/null @@ -1,12 +0,0 @@ -\beginsong{Wonderful song} - [by={Anonymous}] - - \begin{verse} - \[A]La la la\\ - \end{verse} - - \begin{chorus} - \[C]La la la\\ - \end{chorus} - -\endsong diff --git a/test/test_patatools/test_convert_failure/song.csg b/test/test_patatools/test_convert_failure/song.csg new file mode 100644 index 00000000..cb763ba7 --- /dev/null +++ b/test/test_patatools/test_convert_failure/song.csg @@ -0,0 +1,122 @@ +\selectlanguage{english} + +\beginsong{}[ + by={ + }, +] + + +\begin{verse} + \selectlanguage + \songcolumns + \beginsong + \[&y={Traditionnel},cover={traditionnel},al&um={France}] +\end{verse} + + +\begin{verse} + \cover + \gtab + \gtab + \gtab +\end{verse} + + +\begin{verse} + \begin + Cheva\\[C]liers de la Table Ronde + Goûtons \\[G7]voir si le vin est \\[C]bon + \rep + \end +\end{verse} + + +\begin{verse} + \begin + Goûtons \\[F]voir, \echo + Goûtons \\[C]voir, \echo + Goûtons \\[G7]voir si le vin est bon + \rep + \end +\end{verse} + + +\begin{verse} + \begin + S'il est bon, s'il est agréable + J'en boirai jusqu'à mon plaisir + \end +\end{verse} + + +\begin{verse} + \begin + J'en boirai cinq à six bouteilles + Et encore, ce n'est pas beaucoup + \end +\end{verse} + + +\begin{verse} + \begin + Si je meurs, je veux qu'on m'enterre + Dans une cave où il y a du bon vin + \end +\end{verse} + + +\begin{verse} + \begin + Les deux pieds contre la muraille + Et la tête sous le robinet + \end +\end{verse} + + +\begin{verse} + \begin + Et les quatre plus grands ivrognes + Porteront les quatre coins du drap + \end +\end{verse} + + +\begin{verse} + \begin + Pour donner le discours d'usage + On prendra le bistrot du coin + \end +\end{verse} + + +\begin{verse} + \begin + Et si le tonneau se débouche + J'en boirai jusqu'à mon plaisir + \end +\end{verse} + + +\begin{verse} + \begin + Et s'il en reste quelques gouttes + Ce sera pour nous rafraîchir + \end +\end{verse} + + +\begin{verse} + \begin + Sur ma tombe, je veux qu'on inscrive + \emph + \end +\end{verse} + + +\begin{verse} + \endsong +\end{verse} + + + +\endsong \ No newline at end of file diff --git a/test/test_patatools/test_convert_failure/song.csg.tsg.control b/test/test_patatools/test_convert_failure/song.csg.tsg.control new file mode 100644 index 00000000..e69de29b