From b90f379398afffa7e8668af27ed2741b28d41cc8 Mon Sep 17 00:00:00 2001 From: Shadi Naif Date: Fri, 18 Aug 2023 11:48:57 +0300 Subject: [PATCH] feat: add two new options, merge_js and overrite_from_partial Refs: FC-0012 OEP-58 --- i18n/extract.py | 115 ++++++++++++++++++++++++++++++------------ tests/test_extract.py | 113 ++++++++++++++++++++++++++++++++++++++--- 2 files changed, 188 insertions(+), 40 deletions(-) diff --git a/i18n/extract.py b/i18n/extract.py index 8b5aaa3..f8b9430 100644 --- a/i18n/extract.py +++ b/i18n/extract.py @@ -26,7 +26,7 @@ from path import Path from i18n import Runner -from i18n.execute import execute +from i18n.execute import remove_file, execute from i18n.segment import segment_pofiles @@ -57,6 +57,22 @@ def add_args(self): Adds arguments """ self.parser.description = __doc__ + self.parser.add_argument( + '--merge-js', + action='store_true', + help='Merge djangojs.po with django.po using msgcat' + ) + self.parser.add_argument( + '--combine-partial', + action='store_true', + help=( + 'Renames (django-partial.po) and (djangojs-partial.po) to (django.po) and (djangojs.po) respectively. ' + 'This will also remove (django-saved.po) and (djangojs-saved.po) that are used to restore original ' + '(django.po) and (djangojs.po) after extract is completed. ' + 'This is helpful when this `extract` command output is needed to be used immediately ' + 'with no segment/merge workflow in small repositories.' + ) + ) def rename_source_file(self, src, dst): """ @@ -95,35 +111,7 @@ def run(self, args): else: stderr = DEVNULL - # --keyword informs Babel that `interpolate()` is an expected - # gettext function, which is necessary because the `tokenize` function - # in the `markey` module marks it as such and passes it to Babel. - # (These functions are called in the django-babel-underscore module.) - babel_cmd_template = ( - 'pybabel {verbosity} extract --mapping={config} ' - '--add-comments="Translators:" --keyword="interpolate" ' - '. --output={output}' - ) - - babel_mako_cfg = self.base(configuration.locale_dir, 'babel_mako.cfg') - if babel_mako_cfg.exists(): - babel_mako_cmd = babel_cmd_template.format( - verbosity=babel_verbosity, - config=babel_mako_cfg, - output=self.base(configuration.source_messages_dir, 'mako.po'), - ) - - execute(babel_mako_cmd, working_directory=configuration.root_dir, stderr=stderr) - - babel_underscore_cfg = self.base(configuration.locale_dir, 'babel_underscore.cfg') - if babel_underscore_cfg.exists(): - babel_underscore_cmd = babel_cmd_template.format( - verbosity=babel_verbosity, - config=babel_underscore_cfg, - output=self.base(configuration.source_messages_dir, 'underscore.po'), - ) - - execute(babel_underscore_cmd, working_directory=configuration.root_dir, stderr=stderr) + self.babel_extract(stderr, babel_verbosity) makemessages = f"django-admin makemessages -l en -v{args.verbose}" ignores = " ".join(f'--ignore="{d}/*"' for d in configuration.ignore_dirs) @@ -177,9 +165,70 @@ def run(self, args): for filename in files_to_clean: clean_pofile(self.source_msgs_dir.joinpath(filename)) - # Restore the saved .po files. - self.rename_source_file('django-saved.po', 'django.po') - self.rename_source_file('djangojs-saved.po', 'djangojs.po') + if args.merge_js: + self.merge_js(stderr) + + if args.combine_partial: + # Overwrite django.po and djangojs.po from django-partial.po and djangojs-partial.po + self.rename_source_file('django-partial.po', 'django.po') + self.rename_source_file('djangojs-partial.po', 'djangojs.po') + remove_file(self.source_msgs_dir.joinpath('django-saved.po')) + remove_file(self.source_msgs_dir.joinpath('djangojs-saved.po')) + else: + # Restore the saved .po files. + self.rename_source_file('django-saved.po', 'django.po') + self.rename_source_file('djangojs-saved.po', 'djangojs.po') + + def merge_js(self, stderr): + """ + Merge djangojs.po with django.po using msgcat + """ + # Some projects don't have any javascript, so there is no djangojs-partial.po + if not file_exists(self.source_msgs_dir.joinpath('djangojs-partial.po')): + return + + execute( + 'msgcat django-partial.po djangojs-partial.po -o django-partial.po', + working_directory=self.source_msgs_dir, + stderr=stderr, + ) + remove_file(self.source_msgs_dir.joinpath('djangojs-partial.po')) + + def babel_extract(self, stderr, verbosity): + """ + Extract strings from mako templates. + """ + configuration = self.configuration + + # --keyword informs Babel that `interpolate()` is an expected + # gettext function, which is necessary because the `tokenize` function + # in the `markey` module marks it as such and passes it to Babel. + # (These functions are called in the django-babel-underscore module.) + babel_cmd_template = ( + 'pybabel {verbosity} extract --mapping={config} ' + '--add-comments="Translators:" --keyword="interpolate" ' + '. --output={output}' + ) + + babel_mako_cfg = self.base(configuration.locale_dir, 'babel_mako.cfg') + if babel_mako_cfg.exists(): + babel_mako_cmd = babel_cmd_template.format( + verbosity=verbosity, + config=babel_mako_cfg, + output=self.base(configuration.source_messages_dir, 'mako.po'), + ) + + execute(babel_mako_cmd, working_directory=configuration.root_dir, stderr=stderr) + + babel_underscore_cfg = self.base(configuration.locale_dir, 'babel_underscore.cfg') + if babel_underscore_cfg.exists(): + babel_underscore_cmd = babel_cmd_template.format( + verbosity=verbosity, + config=babel_underscore_cfg, + output=self.base(configuration.source_messages_dir, 'underscore.po'), + ) + + execute(babel_underscore_cmd, working_directory=configuration.root_dir, stderr=stderr) def clean_pofile(path_name): diff --git a/tests/test_extract.py b/tests/test_extract.py index 32eaedc..727768d 100644 --- a/tests/test_extract.py +++ b/tests/test_extract.py @@ -1,6 +1,9 @@ from datetime import datetime, timedelta +from functools import wraps import os +from unittest import mock +import ddt import polib from path import Path @@ -9,12 +12,51 @@ from . import I18nToolTestCase, MOCK_DJANGO_APP_DIR +def extract_options(): + """ + Decorator for test methods in TestExtract class. + + It wraps the test method in a function that calls (extract.main) with various options using (ddt.data) + + Sets the following attributes: + - ddt_flag_merge_js: True if the test method should be run with merge_js=True + - ddt_flag_combine_partial: True if the test method should be run with combine_partial=True + """ + def decorator(test_method): + """ + The decorator itself + """ + @wraps(test_method) + @ddt.data( + (True, True), + (True, False), + (False, True), + (False, False), + ) + @ddt.unpack + def wrapped(self, flag_merge_js, flag_combine_partial): + """ + The wrapper function + """ + self.ddt_flag_merge_js = flag_merge_js + self.ddt_flag_combine_partial = flag_combine_partial + extract.main( + verbosity=0, + config=self.configuration._filename, + root_dir=MOCK_DJANGO_APP_DIR, + merge_js=flag_merge_js, + combine_partial=flag_combine_partial, + ) + test_method(self) + return wrapped + return decorator + + +@ddt.ddt class TestExtract(I18nToolTestCase): """ Tests functionality of i18n/extract.py """ - generated_files = ('django-partial.po', 'djangojs-partial.po', 'mako.po') - def setUp(self): super().setUp() @@ -31,8 +73,28 @@ def setUp(self): ) self.configuration = config.Configuration(root_dir=MOCK_DJANGO_APP_DIR) - # Run extraction script - extract.main(verbosity=0, config=self.configuration._filename, root_dir=MOCK_DJANGO_APP_DIR) + # These will be set by extract_options decorator. Also get_files will fail if they are None (to remind us + # to use the decorator all tests methods) + self.ddt_flag_merge_js = None + self.ddt_flag_combine_partial = None + + @property + def django_po(self): + """ + Returns django.po or django-partial.po according to the combine_partial flag + """ + return 'django{partial_prefix}.po'.format( + partial_prefix='' if self.ddt_flag_combine_partial else '-partial' + ) + + @property + def djangojs_po(self): + """ + Returns djangojs.po or djangojs-partial.po according to the combine_partial flag + """ + return 'djangojs{partial_prefix}.po'.format( + partial_prefix='' if self.ddt_flag_combine_partial else '-partial' + ) def get_files(self): """ @@ -40,13 +102,24 @@ def get_files(self): Returns the fully expanded filenames for all extracted files Fails assertion if one of the files doesn't exist. """ - for filename in self.generated_files: + assert self.ddt_flag_merge_js is not None, "Use extract_options decorator" + assert self.ddt_flag_combine_partial is not None, "Use extract_options decorator" + + # According to merge_js and combine_partial flags, we may have generated different files + # combine_partial = True: no partial files are generated, because they have overwritten the original ones + # merge_js = True: no djangojs*.po is generated, because it has been merged into django*.po + generated_files = ('mako.po', self.django_po,) + if not self.ddt_flag_merge_js: + generated_files += (self.djangojs_po,) + + for filename in generated_files: path = Path.joinpath(self.configuration.source_messages_dir, filename) exists = Path.exists(path) self.assertTrue(exists, msg='Missing file: %s' % path) - if exists: - yield path + yield path + + @extract_options() def test_files(self): """ Asserts that each auto-generated file has been modified since 'extract' was launched. @@ -56,6 +129,7 @@ def test_files(self): self.assertTrue(datetime.fromtimestamp(os.path.getmtime(path)) > self.start_time, msg='File not recently modified: %s' % path) + @extract_options() def test_is_keystring(self): """ Verifies is_keystring predicate @@ -67,6 +141,7 @@ def test_is_keystring(self): self.assertTrue(extract.is_key_string(entry1.msgid)) self.assertFalse(extract.is_key_string(entry2.msgid)) + @extract_options() def test_headers(self): """ Verify all headers have been modified @@ -79,6 +154,7 @@ def test_headers(self): msg='Missing header in %s:\n"%s"' % (path, header) ) + @extract_options() def test_metadata(self): """ Verify all metadata has been modified @@ -90,6 +166,7 @@ def test_metadata(self): expected = 'openedx-translation@googlegroups.com' self.assertEquals(expected, value) + @extract_options() def test_metadata_no_create_date(self): """ Verify `POT-Creation-Date` metadata has been removed @@ -98,3 +175,25 @@ def test_metadata_no_create_date(self): po = polib.pofile(path) metadata = po.metadata self.assertIsNone(metadata.get('POT-Creation-Date')) + + @extract_options() + def test_merge_js(self): + """ + Verify that djangojs*.po is generated only if merge_js is False + """ + assert self.ddt_flag_merge_js != Path.exists( + Path.joinpath(self.configuration.source_messages_dir, self.djangojs_po,) + ) + + @extract_options() + def test_combine_partial(self): + """ + Verify that partial files are not generated if combine_partial is True + """ + assert self.ddt_flag_combine_partial != Path.exists( + Path.joinpath(self.configuration.source_messages_dir, 'django-partial.po',) + ) + if not self.ddt_flag_merge_js: + assert self.ddt_flag_combine_partial != Path.exists( + Path.joinpath(self.configuration.source_messages_dir, 'djangojs-partial.po',) + )