From 95747f148acf36856e82c5fa5a4cc32ef021f904 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sta=C5=9B=20Ma=C5=82olepszy?= Date: Fri, 3 Nov 2017 15:34:45 +0100 Subject: [PATCH] Address Pike's feedback --- fluent/migrate/context.py | 13 +++++-- fluent/migrate/merge.py | 2 +- tests/migrate/fixtures/pl/existing.ftl | 0 tests/migrate/test_context.py | 18 +++++++++ tests/migrate/test_copy.py | 11 ------ tests/migrate/test_plural.py | 15 -------- tests/migrate/test_replace.py | 17 --------- tests/migrate/test_source.py | 51 ++++++++++++++++++++++++++ 8 files changed, 79 insertions(+), 48 deletions(-) delete mode 100644 tests/migrate/fixtures/pl/existing.ftl create mode 100644 tests/migrate/test_source.py diff --git a/fluent/migrate/context.py b/fluent/migrate/context.py index 6904ce03..974f2cbe 100644 --- a/fluent/migrate/context.py +++ b/fluent/migrate/context.py @@ -256,10 +256,15 @@ def in_changeset(ident): self, reference, current, transforms, in_changeset ) - # If none of the transforms is in the given changeset, the merged - # snapshot is identical to the current translation. We compare - # JSON trees rather then use filtering by `in_changeset` to account - # for translations removed from `reference`. + # Skip this path if the merged snapshot is identical to the current + # state of the localization file. This may happen when: + # + # - none of the transforms is in the changset, or + # - all messages which would be migrated by the context's + # transforms already exist in the current state. + # + # We compare JSON trees rather then use filtering by `in_changeset` + # to account for translations removed from `reference`. if snapshot.to_json() == current.to_json(): continue diff --git a/fluent/migrate/merge.py b/fluent/migrate/merge.py index 3a2c5987..586393e5 100644 --- a/fluent/migrate/merge.py +++ b/fluent/migrate/merge.py @@ -12,7 +12,7 @@ def merge_resource(ctx, reference, current, transforms, in_changeset): Use the `reference` FTL AST as a template. For each en-US string in the reference, first check for an existing translation in the current FTL - `localization` and return early if it's present; then if the string has + `localization` and use it if it's present; then if the string has a transform defined in the migration specification and if it's in the currently processed changeset, evaluate the transform. """ diff --git a/tests/migrate/fixtures/pl/existing.ftl b/tests/migrate/fixtures/pl/existing.ftl deleted file mode 100644 index e69de29b..00000000 diff --git a/tests/migrate/test_context.py b/tests/migrate/test_context.py index 27fdd3f2..4b7eef32 100644 --- a/tests/migrate/test_context.py +++ b/tests/migrate/test_context.py @@ -397,6 +397,24 @@ def test_existing_target_ftl_existing_string(self): expected ) + def test_existing_target_ftl_with_all_messages(self): + self.ctx.add_transforms('privacy.ftl', 'privacy.ftl', [ + FTL.Message( + id=FTL.Identifier('dnt-description'), + value=COPY( + 'privacy.dtd', + 'doNotTrack.description' + ) + ), + ]) + + # All migrated messages are already in the target FTL and the result of + # merge_changeset is an empty iterator. + self.assertDictEqual( + to_json(self.ctx.merge_changeset()), + {} + ) + class TestNotSupportedError(unittest.TestCase): def test_add_ftl(self): diff --git a/tests/migrate/test_copy.py b/tests/migrate/test_copy.py index c3ff1deb..66eb8c85 100644 --- a/tests/migrate/test_copy.py +++ b/tests/migrate/test_copy.py @@ -9,21 +9,10 @@ except ImportError: PropertiesParser = DTDParser = None -from fluent.migrate.errors import NotSupportedError from fluent.migrate.util import parse, ftl_message_to_json from fluent.migrate.transforms import evaluate, COPY -class TestNotSupportedError(unittest.TestCase): - def test_fluent(self): - pattern = ('Migrating translations from Fluent files is not supported') - with self.assertRaisesRegexp(NotSupportedError, pattern): - FTL.Message( - FTL.Identifier('foo'), - value=COPY('test.ftl', 'foo') - ) - - class MockContext(unittest.TestCase): def get_source(self, path, key): # Ignore path (test.properties) and get translations from self.strings diff --git a/tests/migrate/test_plural.py b/tests/migrate/test_plural.py index 73cdc4e4..0fb21726 100644 --- a/tests/migrate/test_plural.py +++ b/tests/migrate/test_plural.py @@ -9,26 +9,11 @@ except ImportError: PropertiesParser = None -from fluent.migrate.errors import NotSupportedError from fluent.migrate.util import parse, ftl_message_to_json from fluent.migrate.helpers import EXTERNAL_ARGUMENT from fluent.migrate.transforms import evaluate, PLURALS, REPLACE_IN_TEXT -class TestNotSupportedError(unittest.TestCase): - def test_fluent(self): - pattern = ('Migrating translations from Fluent files is not supported') - with self.assertRaisesRegexp(NotSupportedError, pattern): - FTL.Message( - FTL.Identifier('delete-all'), - value=PLURALS( - 'test.ftl', - 'deleteAll', - EXTERNAL_ARGUMENT('num') - ) - ) - - class MockContext(unittest.TestCase): # Static categories corresponding to en-US. plural_categories = ('one', 'other') diff --git a/tests/migrate/test_replace.py b/tests/migrate/test_replace.py index 7f908d66..65576d00 100644 --- a/tests/migrate/test_replace.py +++ b/tests/migrate/test_replace.py @@ -9,28 +9,11 @@ except ImportError: PropertiesParser = None -from fluent.migrate.errors import NotSupportedError from fluent.migrate.util import parse, ftl_message_to_json from fluent.migrate.helpers import EXTERNAL_ARGUMENT from fluent.migrate.transforms import evaluate, REPLACE -class TestNotSupportedError(unittest.TestCase): - def test_fluent(self): - pattern = ('Migrating translations from Fluent files is not supported') - with self.assertRaisesRegexp(NotSupportedError, pattern): - FTL.Message( - FTL.Identifier(u'hello'), - value=REPLACE( - 'test.ftl', - 'hello', - { - '#1': EXTERNAL_ARGUMENT('username') - } - ) - ) - - class MockContext(unittest.TestCase): def get_source(self, path, key): # Ignore path (test.properties) and get translations from self.strings diff --git a/tests/migrate/test_source.py b/tests/migrate/test_source.py new file mode 100644 index 00000000..1da96ac4 --- /dev/null +++ b/tests/migrate/test_source.py @@ -0,0 +1,51 @@ +# coding=utf8 +from __future__ import unicode_literals + +import unittest + +import fluent.syntax.ast as FTL + +from fluent.migrate.errors import NotSupportedError +from fluent.migrate.transforms import Source, COPY, PLURALS, REPLACE +from fluent.migrate.helpers import EXTERNAL_ARGUMENT + + +class TestNotSupportedError(unittest.TestCase): + def test_source(self): + pattern = ('Migrating translations from Fluent files is not supported') + with self.assertRaisesRegexp(NotSupportedError, pattern): + Source('test.ftl', 'foo') + + def test_copy(self): + pattern = ('Migrating translations from Fluent files is not supported') + with self.assertRaisesRegexp(NotSupportedError, pattern): + FTL.Message( + FTL.Identifier('foo'), + value=COPY('test.ftl', 'foo') + ) + + def test_plurals(self): + pattern = ('Migrating translations from Fluent files is not supported') + with self.assertRaisesRegexp(NotSupportedError, pattern): + FTL.Message( + FTL.Identifier('delete-all'), + value=PLURALS( + 'test.ftl', + 'deleteAll', + EXTERNAL_ARGUMENT('num') + ) + ) + + def test_replace(self): + pattern = ('Migrating translations from Fluent files is not supported') + with self.assertRaisesRegexp(NotSupportedError, pattern): + FTL.Message( + FTL.Identifier(u'hello'), + value=REPLACE( + 'test.ftl', + 'hello', + { + '#1': EXTERNAL_ARGUMENT('username') + } + ) + )