Merge pull request #134 from Zeit-Labs/shadinaif/new-options-for-extract

feat: add new options merge-po-files and no-segment

i18n/__init__.py

@@ -6,7 +6,7 @@
 
 from . import config
 
-__version__ = '1.1.1'
+__version__ = '1.2.0'
 
 
 class Runner:

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):
     """

tests/test_extract.py

@@ -1,37 +1,53 @@
 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
 
 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,27 +68,40 @@ 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):
         """
         This is a generator.
         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 = '[email protected]'
             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()