From 14419a67f97e3ee9fcbc031906510cd37b2e2612 Mon Sep 17 00:00:00 2001 From: Shadi Naif Date: Fri, 18 Aug 2023 11:48:57 +0300 Subject: [PATCH 1/2] feat: add new options merge-po-files and no-segment no-segment: no partial files are generated, because they replaced the original django.po and djangojs.po. Also no segmet workflow is processed merge-po-files: no djangojs*.po is generated, because it has been merged into django*.po Refs: FC-0012 OEP-58 --- i18n/extract.py | 53 ++++++++++++++++-- tests/test_extract.py | 126 +++++++++++++++++++++++++++++++++++++----- 2 files changed, 159 insertions(+), 20 deletions(-) diff --git a/i18n/extract.py b/i18n/extract.py index 89203b55..d3f0a034 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 @@ -65,6 +65,21 @@ def add_args(self): Adds arguments """ self.parser.description = __doc__ + self.parser.add_argument( + '--merge-po-files', + action='store_true', + help='Merge djangojs.po with django.po using msgcat' + ) + self.parser.add_argument( + '--no-segment', + action='store_true', + help=( + 'Ignore the `segment` section in the `config.yaml` configurations file. Also rename ' + '(django-partial.po) and (djangojs-partial.po) to (django.po) and (djangojs.po) respectively, ' + 'which means that the original files will be overwritten after the extraction is completed. This is ' + 'helpful when running the `extract` command in small repositories with no segment/merge workflow.' + ) + ) def rename_source_file(self, src, dst): """ @@ -147,8 +162,9 @@ def run(self, args): execute(babel_cmd, working_directory=app_dir, stderr=stderr) # Segment the generated files. - segmented_files = segment_pofiles(configuration, configuration.source_locale) - files_to_clean.update(segmented_files) + if not args.no_segment: + segmented_files = segment_pofiles(configuration, configuration.source_locale) + files_to_clean.update(segmented_files) # Add partial files to the list of files to clean. files_to_clean.update((DJANGO_PARTIAL_PO, DJANGOJS_PARTIAL_PO)) @@ -157,9 +173,19 @@ 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_po_files: + self.merge_po_files(stderr) + + if args.no_segment: + # 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 babel_extract(self, stderr, verbosity): """ @@ -197,6 +223,21 @@ def babel_extract(self, stderr, verbosity): execute(babel_underscore_cmd, working_directory=configuration.root_dir, stderr=stderr) + def merge_po_files(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()): + 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 clean_pofile(path_name): """ diff --git a/tests/test_extract.py b/tests/test_extract.py index c057f605..5a89ea32 100644 --- a/tests/test_extract.py +++ b/tests/test_extract.py @@ -1,7 +1,10 @@ import os from datetime import datetime, timedelta from functools import wraps +import itertools +import ddt +import mock import polib from i18n import extract, config from path import Path @@ -9,29 +12,42 @@ from . import I18nToolTestCase, MOCK_DJANGO_APP_DIR -def perform_extract(): +def perform_extract_with_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_po_files: True if the test method should be run with merge-po-files=True + - ddt_flag_no_segment: True if the test method should be run with no-segment=True """ def decorator(test_method): """ The decorator itself """ @wraps(test_method) - def wrapped(self): + @ddt.data(*itertools.product([True, False], repeat=2)) # all combinations of flags + @ddt.unpack + def wrapped(self, flag_merge_po_files, flag_no_segment): """ The wrapper function """ + self.ddt_flag_merge_po_files = flag_merge_po_files + self.ddt_flag_no_segment = flag_no_segment extract.main( verbosity=0, config=self.configuration._filename, root_dir=MOCK_DJANGO_APP_DIR, + merge_po_files=flag_merge_po_files, + no_segment=flag_no_segment, ) test_method(self) return wrapped return decorator +@ddt.ddt class TestExtract(I18nToolTestCase): """ Tests functionality of i18n/extract.py @@ -52,19 +68,24 @@ def setUp(self): ) self.configuration = config.Configuration(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 in all tests methods) + self.ddt_flag_merge_po_files = None + self.ddt_flag_no_segment = None + @property def django_po(self): """ - Returns the name of the generated django file + Returns file or partial file name according to the no-segment flag """ - return extract.DJANGO_PARTIAL_PO + return extract.DJANGO_PO if self.ddt_flag_no_segment else extract.DJANGO_PARTIAL_PO @property def djangojs_po(self): """ - Returns the name of the generated djangojs file + Returns jsfile or partial jsfile name according to the no-segment flag """ - return extract.DJANGOJS_PARTIAL_PO + return extract.DJANGOJS_PO if self.ddt_flag_no_segment else extract.DJANGOJS_PARTIAL_PO def get_files(self): """ @@ -72,7 +93,15 @@ def get_files(self): Returns the fully expanded filenames for all extracted files Fails assertion if one of the files doesn't exist. """ - generated_files = (extract.MAKO_PO, self.django_po, self.djangojs_po,) + assert self.ddt_flag_merge_po_files is not None, "Use perform_extract decorator" + assert self.ddt_flag_no_segment is not None, "Use perform_extract decorator" + + # Depending on how the merge-po-files and no-segment options are set, we may have generated different files + # no-segment: no partial files are generated, because they replaced the original django.po and djangojs.po + # merge-po-files: no djangojs*.po is generated, because it has been merged into django*.po + generated_files = (self.django_po,) + if not self.ddt_flag_merge_po_files: + generated_files += (self.djangojs_po,) for filename in generated_files: path = Path.joinpath(self.configuration.source_messages_dir, filename) @@ -81,17 +110,27 @@ def get_files(self): yield path - @perform_extract() + def is_file_generated(self, file_path): + """ + Helper to check if the given file has been generated or not + + Using only (Path.exists) is misleading because all possible files are already exist at the beginning + of the test. Therefore, we need to check the file's modification time too + """ + if not Path.exists(file_path): + return False + return datetime.fromtimestamp(os.path.getmtime(file_path)) > self.start_time + + @perform_extract_with_options() def test_files(self): """ Asserts that each auto-generated file has been modified since 'extract' was launched. Intended to show that the file has been touched by 'extract'. """ for path in self.get_files(): - self.assertTrue(datetime.fromtimestamp(os.path.getmtime(path)) > self.start_time, - msg='File not recently modified: %s' % path) + self.assertTrue(self.is_file_generated(path), msg='File not recently modified: %s' % path) - @perform_extract() + @perform_extract_with_options() def test_is_keystring(self): """ Verifies is_keystring predicate @@ -103,7 +142,7 @@ def test_is_keystring(self): self.assertTrue(extract.is_key_string(entry1.msgid)) self.assertFalse(extract.is_key_string(entry2.msgid)) - @perform_extract() + @perform_extract_with_options() def test_headers(self): """ Verify all headers have been modified @@ -116,7 +155,7 @@ def test_headers(self): msg='Missing header in %s:\n"%s"' % (path, header) ) - @perform_extract() + @perform_extract_with_options() def test_metadata(self): """ Verify all metadata has been modified @@ -128,7 +167,7 @@ def test_metadata(self): expected = 'openedx-translation@googlegroups.com' self.assertEquals(expected, value) - @perform_extract() + @perform_extract_with_options() def test_metadata_no_create_date(self): """ Verify `POT-Creation-Date` metadata has been removed @@ -137,3 +176,62 @@ def test_metadata_no_create_date(self): po = polib.pofile(path) metadata = po.metadata self.assertIsNone(metadata.get('POT-Creation-Date')) + + @perform_extract_with_options() + def test_merge_po_files(self): + """ + Verify that djangojs*.po is generated only if merge-po-files is not set + """ + assert self.ddt_flag_merge_po_files != self.is_file_generated( + Path.joinpath(self.configuration.source_messages_dir, self.djangojs_po,) + ) + + @perform_extract_with_options() + def test_no_segment_guard_to_verify_names_in_tests(self): + """ + Verify that (django_po) and (djangojs_po) properties return the correct file names + according to no-segment flag + """ + assert self.ddt_flag_no_segment != ('-partial' in self.django_po) + assert self.ddt_flag_no_segment != ('-partial' in self.djangojs_po) + + @perform_extract_with_options() + def test_no_segment_partial_files(self): + """ + Verify that partial files are not generated if no-segment is True + """ + # We can't use (django_po) and (djangojs_po) properties here because we need to always check + # for (partial) files + assert self.ddt_flag_no_segment != self.is_file_generated( + Path.joinpath(self.configuration.source_messages_dir, extract.DJANGO_PARTIAL_PO,) + ) + if not self.ddt_flag_merge_po_files: + assert self.ddt_flag_no_segment != self.is_file_generated( + Path.joinpath(self.configuration.source_messages_dir, extract.DJANGOJS_PARTIAL_PO,) + ) + + def test_no_segment_off_do_call_segment_pofiles(self): + """ + Verify that (segment_pofiles) is called if no-segment is False + """ + with mock.patch('i18n.extract.segment_pofiles') as mock_segment_pofiles: + extract.main( + verbosity=0, + config=self.configuration._filename, + root_dir=MOCK_DJANGO_APP_DIR, + no_segment=False, + ) + mock_segment_pofiles.assert_called_once() + + def test_no_segment_on_dont_call_segment_pofiles(self): + """ + Verify that (segment_pofiles) is not called if no-segment is True + """ + with mock.patch('i18n.extract.segment_pofiles') as mock_segment_pofiles: + extract.main( + verbosity=0, + config=self.configuration._filename, + root_dir=MOCK_DJANGO_APP_DIR, + no_segment=True, + ) + mock_segment_pofiles.assert_not_called() From afabc56ff02077327e4db591409635dbc045d9a3 Mon Sep 17 00:00:00 2001 From: Shadi Naif Date: Mon, 11 Sep 2023 10:41:01 +0300 Subject: [PATCH 2/2] feat: bump version to (1.2.0) Bump version to include --merge-js and --no-partial options --- i18n/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/i18n/__init__.py b/i18n/__init__.py index 247ce766..5a1d1917 100644 --- a/i18n/__init__.py +++ b/i18n/__init__.py @@ -6,7 +6,7 @@ from . import config -__version__ = '1.1.1' +__version__ = '1.2.0' class Runner: