From 7621d1bd34375d8a30e8df15073325c4216a0be0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sta=C5=9B=20Ma=C5=82olepszy?= Date: Mon, 19 Jun 2017 14:59:17 +0200 Subject: [PATCH 1/9] Add BaseNode.equals --- fluent/syntax/ast.py | 48 ++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 48 insertions(+) diff --git a/fluent/syntax/ast.py b/fluent/syntax/ast.py index 02cefafc..d0dabb8c 100644 --- a/fluent/syntax/ast.py +++ b/fluent/syntax/ast.py @@ -1,6 +1,7 @@ from __future__ import unicode_literals import sys import json +from itertools import ifilterfalse, imap, izip def to_json(value): @@ -27,6 +28,38 @@ def from_json(value): return value +def fields_equal(field1, field2, with_spans): + (key1, value1), (key2, value2) = field1, field2 + + if key1 != key2: + return False + + return values_equal(value1, value2, with_spans) + + +def values_equal(value1, value2, with_spans): + if type(value1) != type(value2): + return False + + if isinstance(value1, BaseNode): + return value1.equals(value2, with_spans) + + if isinstance(value1, list): + value_pairs = izip(value1, value2) + diffs = imap( + lambda pair: not values_equal(*pair, with_spans), + value_pairs) + return not any(diffs) + + else: + return value1 == value2 + + +def is_span(item): + _, value = item + return isinstance(value, Span) + + class BaseNode(object): """Base class for all Fluent AST nodes. @@ -61,6 +94,21 @@ def visit(value): return fun(node) + def equals(self, other, with_spans=True): + self_vars = vars(self).iteritems() + other_vars = vars(other).iteritems() + + if not with_spans: + self_vars = ifilterfalse(is_span, self_vars) + other_vars = ifilterfalse(is_span, other_vars) + + field_pairs = izip(self_vars, other_vars) + diffs = imap( + lambda pair: not fields_equal(*pair, with_spans), + field_pairs) + + return not any(diffs) + def to_json(self): obj = { name: to_json(value) From 07d9c03ed7f1a0d307af14608eae8582a4c57ed7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sta=C5=9B=20Ma=C5=82olepszy?= Date: Mon, 19 Jun 2017 17:02:58 +0200 Subject: [PATCH 2/9] Use sets and for loops instead of any/imap --- fluent/syntax/ast.py | 78 ++++++++++++++++++++++++-------------------- 1 file changed, 43 insertions(+), 35 deletions(-) diff --git a/fluent/syntax/ast.py b/fluent/syntax/ast.py index d0dabb8c..0db61691 100644 --- a/fluent/syntax/ast.py +++ b/fluent/syntax/ast.py @@ -1,7 +1,7 @@ from __future__ import unicode_literals import sys import json -from itertools import ifilterfalse, imap, izip +from itertools import izip def to_json(value): @@ -28,36 +28,17 @@ def from_json(value): return value -def fields_equal(field1, field2, with_spans): - (key1, value1), (key2, value2) = field1, field2 - - if key1 != key2: +def fields_equal(field1, field2, with_spans=True): + if type(field1) != type(field2): return False - return values_equal(value1, value2, with_spans) - + if isinstance(field1, BaseNode): + return field1.equals(field2, with_spans) -def values_equal(value1, value2, with_spans): - if type(value1) != type(value2): + if field1 != field2: return False - if isinstance(value1, BaseNode): - return value1.equals(value2, with_spans) - - if isinstance(value1, list): - value_pairs = izip(value1, value2) - diffs = imap( - lambda pair: not values_equal(*pair, with_spans), - value_pairs) - return not any(diffs) - - else: - return value1 == value2 - - -def is_span(item): - _, value = item - return isinstance(value, Span) + return True class BaseNode(object): @@ -95,19 +76,46 @@ def visit(value): return fun(node) def equals(self, other, with_spans=True): - self_vars = vars(self).iteritems() - other_vars = vars(other).iteritems() + self_keys = set(vars(self).keys()) + other_keys = set(vars(other).keys()) if not with_spans: - self_vars = ifilterfalse(is_span, self_vars) - other_vars = ifilterfalse(is_span, other_vars) + self_keys.discard('span') + other_keys.discard('span') + + if self_keys != other_keys: + return False + + keys = sorted(self_keys) + + self_fields = [getattr(self, key) for key in keys] + other_fields = [getattr(other, key) for key in keys] + + for key, field1, field2 in izip(keys, self_fields, other_fields): + if isinstance(field1, list) and isinstance(field2, list): + if len(field1) != len(field2): + return False + + field_sorting = { + # 'annotations': lambda elem: elem.message, + 'attributes': lambda elem: elem.id.name, + 'tags': lambda elem: elem.name.name, + 'variants': lambda elem: elem.key.name, + } + + if key in field_sorting.keys(): + sorting = field_sorting[key] + field1 = sorted(field1, key=sorting) + field2 = sorted(field2, key=sorting) + + for elem1, elem2 in izip(field1, field2): + if not fields_equal(elem1, elem2, with_spans): + return False - field_pairs = izip(self_vars, other_vars) - diffs = imap( - lambda pair: not fields_equal(*pair, with_spans), - field_pairs) + elif not fields_equal(field1, field2, with_spans): + return False - return not any(diffs) + return True def to_json(self): obj = { From 9fc95976f31feff3e4e270e57bdad600c415a110 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sta=C5=9B=20Ma=C5=82olepszy?= Date: Mon, 19 Jun 2017 17:16:39 +0200 Subject: [PATCH 3/9] scalars_equal is for Nodes and primitives, not lists --- fluent/syntax/ast.py | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/fluent/syntax/ast.py b/fluent/syntax/ast.py index 0db61691..f3351820 100644 --- a/fluent/syntax/ast.py +++ b/fluent/syntax/ast.py @@ -28,17 +28,14 @@ def from_json(value): return value -def fields_equal(field1, field2, with_spans=True): +def scalars_equal(field1, field2, with_spans=True): if type(field1) != type(field2): return False if isinstance(field1, BaseNode): return field1.equals(field2, with_spans) - if field1 != field2: - return False - - return True + return field1 == field2 class BaseNode(object): @@ -109,10 +106,10 @@ def equals(self, other, with_spans=True): field2 = sorted(field2, key=sorting) for elem1, elem2 in izip(field1, field2): - if not fields_equal(elem1, elem2, with_spans): + if not scalars_equal(elem1, elem2, with_spans): return False - elif not fields_equal(field1, field2, with_spans): + elif not scalars_equal(field1, field2, with_spans): return False return True From 6e13b16b0ebe5f1afacc8d8b24904080d4540758 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sta=C5=9B=20Ma=C5=82olepszy?= Date: Mon, 19 Jun 2017 17:23:28 +0200 Subject: [PATCH 4/9] Address @Pike's feedback --- fluent/syntax/ast.py | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/fluent/syntax/ast.py b/fluent/syntax/ast.py index f3351820..fbe1284d 100644 --- a/fluent/syntax/ast.py +++ b/fluent/syntax/ast.py @@ -83,12 +83,10 @@ def equals(self, other, with_spans=True): if self_keys != other_keys: return False - keys = sorted(self_keys) + for key in self_keys: + field1 = getattr(self, key) + field2 = getattr(other, key) - self_fields = [getattr(self, key) for key in keys] - other_fields = [getattr(other, key) for key in keys] - - for key, field1, field2 in izip(keys, self_fields, other_fields): if isinstance(field1, list) and isinstance(field2, list): if len(field1) != len(field2): return False @@ -100,7 +98,7 @@ def equals(self, other, with_spans=True): 'variants': lambda elem: elem.key.name, } - if key in field_sorting.keys(): + if key in field_sorting: sorting = field_sorting[key] field1 = sorted(field1, key=sorting) field2 = sorted(field2, key=sorting) From cd7e7d40ca6de9d5e9403f948d3337eb4e02b57c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sta=C5=9B=20Ma=C5=82olepszy?= Date: Mon, 19 Jun 2017 17:24:41 +0200 Subject: [PATCH 5/9] Make with_spans default to False --- fluent/syntax/ast.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/fluent/syntax/ast.py b/fluent/syntax/ast.py index fbe1284d..d53e0163 100644 --- a/fluent/syntax/ast.py +++ b/fluent/syntax/ast.py @@ -28,7 +28,7 @@ def from_json(value): return value -def scalars_equal(field1, field2, with_spans=True): +def scalars_equal(field1, field2, with_spans=False): if type(field1) != type(field2): return False @@ -72,7 +72,7 @@ def visit(value): return fun(node) - def equals(self, other, with_spans=True): + def equals(self, other, with_spans=False): self_keys = set(vars(self).keys()) other_keys = set(vars(other).keys()) From 908129a2247cc49c83ed2842f94d453f687bdd79 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sta=C5=9B=20Ma=C5=82olepszy?= Date: Wed, 21 Jun 2017 15:35:46 +0200 Subject: [PATCH 6/9] Add tests and docstrings --- fluent/syntax/ast.py | 25 +++- tests/syntax/test_equals.py | 228 ++++++++++++++++++++++++++++++++++++ 2 files changed, 247 insertions(+), 6 deletions(-) create mode 100644 tests/syntax/test_equals.py diff --git a/fluent/syntax/ast.py b/fluent/syntax/ast.py index d53e0163..a10e1e2e 100644 --- a/fluent/syntax/ast.py +++ b/fluent/syntax/ast.py @@ -28,14 +28,16 @@ def from_json(value): return value -def scalars_equal(field1, field2, with_spans=False): - if type(field1) != type(field2): +def scalars_equal(node1, node2, with_spans=False): + """Compare two nodes which are not lists.""" + + if type(node1) != type(node2): return False - if isinstance(field1, BaseNode): - return field1.equals(field2, with_spans) + if isinstance(node1, BaseNode): + return node1.equals(node2, with_spans) - return field1 == field2 + return node1 == node2 class BaseNode(object): @@ -73,6 +75,13 @@ def visit(value): return fun(node) def equals(self, other, with_spans=False): + """Compare two entries. + + Entries are deeply compared on a field by field basis. If possible, + False is returned early. When comparing attributes, tags and variants + in SelectExpressions, the order doesn't matter. + """ + self_keys = set(vars(self).keys()) other_keys = set(vars(other).keys()) @@ -87,12 +96,16 @@ def equals(self, other, with_spans=False): field1 = getattr(self, key) field2 = getattr(other, key) + # List-typed nodes are compared item-by-item. When comparing + # attributes, tags and variants, the order of items doesn't matter. if isinstance(field1, list) and isinstance(field2, list): if len(field1) != len(field2): return False + # These functions are used to sort lists of items for when + # order doesn't matter. Annotations are also lists but they + # can't be keyed on any of their fields reliably. field_sorting = { - # 'annotations': lambda elem: elem.message, 'attributes': lambda elem: elem.id.name, 'tags': lambda elem: elem.name.name, 'variants': lambda elem: elem.key.name, diff --git a/tests/syntax/test_equals.py b/tests/syntax/test_equals.py new file mode 100644 index 00000000..870a661b --- /dev/null +++ b/tests/syntax/test_equals.py @@ -0,0 +1,228 @@ +from __future__ import unicode_literals +import unittest +import sys + +sys.path.append('.') + +from tests.syntax import dedent_ftl +from fluent.syntax.parser import FluentParser + + +class TestEntryEqualToSelf(unittest.TestCase): + def setUp(self): + self.parser = FluentParser() + + def parse_ftl_entry(self, string): + return self.parser.parse_entry(dedent_ftl(string)) + + def test_same_simple_message(self): + message1 = self.parse_ftl_entry("""\ + foo = Foo + """) + + self.assertTrue(message1.equals(message1)) + + def test_same_selector_message(self): + message1 = self.parse_ftl_entry("""\ + foo = + { $num -> + [one] One + [two] Two + [few] Few + [many] Many + *[other] Other + } + """) + + self.assertTrue(message1.equals(message1)) + + def test_same_complex_placeable_message(self): + message1 = self.parse_ftl_entry("""\ + foo = Foo { NUMBER($num, style: "decimal") } Bar + """) + + self.assertTrue(message1.equals(message1)) + + def test_same_message_with_attribute(self): + message1 = self.parse_ftl_entry("""\ + foo + .attr = Attr + """) + + self.assertTrue(message1.equals(message1)) + + def test_same_message_with_attributes(self): + message1 = self.parse_ftl_entry("""\ + foo + .attr1 = Attr 1 + .attr2 = Attr 2 + """) + + self.assertTrue(message1.equals(message1)) + + def test_same_message_with_tag(self): + message1 = self.parse_ftl_entry("""\ + foo = Foo + #tag + """) + + self.assertTrue(message1.equals(message1)) + + def test_same_message_with_tags(self): + message1 = self.parse_ftl_entry("""\ + foo = Foo + #tag1 + #tag2 + """) + + self.assertTrue(message1.equals(message1)) + + def test_same_junk(self): + message1 = self.parse_ftl_entry("""\ + foo = Foo { + """) + + self.assertTrue(message1.equals(message1)) + + +class TestOrderEquals(unittest.TestCase): + def setUp(self): + self.parser = FluentParser() + + def parse_ftl_entry(self, string): + return self.parser.parse_entry(dedent_ftl(string)) + + def test_attributes(self): + message1 = self.parse_ftl_entry("""\ + foo + .attr1 = Attr1 + .attr2 = Attr2 + """) + message2 = self.parse_ftl_entry("""\ + foo + .attr2 = Attr2 + .attr1 = Attr1 + """) + + self.assertTrue(message1.equals(message2)) + self.assertTrue(message2.equals(message1)) + + def test_tags(self): + message1 = self.parse_ftl_entry("""\ + foo = Foo + #tag1 + #tag2 + """) + message2 = self.parse_ftl_entry("""\ + foo = Foo + #tag2 + #tag1 + """) + + self.assertTrue(message1.equals(message2)) + self.assertTrue(message2.equals(message1)) + + def test_variants(self): + message1 = self.parse_ftl_entry("""\ + foo = + { $num -> + [a] A + *[b] B + } + """) + message2 = self.parse_ftl_entry("""\ + foo = + { $num -> + *[b] B + [a] A + } + """) + + self.assertTrue(message1.equals(message2)) + self.assertTrue(message2.equals(message1)) + + +class TestSpansEqual(unittest.TestCase): + def test_default_behavior(self): + parser = FluentParser() + + strings = [ + ("foo = Foo", "foo = Foo"), + ("foo = Foo", "foo = Foo"), + ("foo = { $arg }", "foo = { $arg }"), + ] + + messages = [ + (parser.parse_entry(a), parser.parse_entry(b)) + for a, b in strings + ] + + for a, b in messages: + self.assertTrue(a.equals(b)) + + def test_parser_without_spans(self): + parser = FluentParser(with_spans=False) + + strings = [ + ("foo = Foo", "foo = Foo"), + ("foo = Foo", "foo = Foo"), + ("foo = { $arg }", "foo = { $arg }"), + ] + + messages = [ + (parser.parse_entry(a), parser.parse_entry(b)) + for a, b in strings + ] + + for a, b in messages: + self.assertTrue(a.equals(b)) + + def test_equals_with_spans(self): + parser = FluentParser() + + strings = [ + ("foo = Foo", "foo = Foo"), + ("foo = { $arg }", "foo = { $arg }"), + ] + + messages = [ + (parser.parse_entry(a), parser.parse_entry(b)) + for a, b in strings + ] + + for a, b in messages: + self.assertTrue(a.equals(b, with_spans=True)) + + def test_differ_with_spans(self): + parser = FluentParser() + + strings = [ + ("foo = Foo", "foo = Foo"), + ("foo = { $arg }", "foo = { $arg }"), + ] + + messages = [ + (parser.parse_entry(a), parser.parse_entry(b)) + for a, b in strings + ] + + for a, b in messages: + self.assertFalse(a.equals(b, with_spans=True)) + + def test_parser_without_spans_equals_with_spans(self): + parser = FluentParser(with_spans=False) + + strings = [ + ("foo = Foo", "foo = Foo"), + ("foo = Foo", "foo = Foo"), + ("foo = { $arg }", "foo = { $arg }"), + ("foo = { $arg }", "foo = { $arg }"), + ] + + messages = [ + (parser.parse_entry(a), parser.parse_entry(b)) + for a, b in strings + ] + + for a, b in messages: + self.assertTrue(a.equals(b, with_spans=True)) From 147b70cb1146d65a7a2bab26b75f2d6f914f8768 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sta=C5=9B=20Ma=C5=82olepszy?= Date: Wed, 21 Jun 2017 16:35:43 +0200 Subject: [PATCH 7/9] Remove izip --- fluent/syntax/ast.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/fluent/syntax/ast.py b/fluent/syntax/ast.py index a10e1e2e..450c456d 100644 --- a/fluent/syntax/ast.py +++ b/fluent/syntax/ast.py @@ -1,7 +1,6 @@ from __future__ import unicode_literals import sys import json -from itertools import izip def to_json(value): @@ -116,7 +115,7 @@ def equals(self, other, with_spans=False): field1 = sorted(field1, key=sorting) field2 = sorted(field2, key=sorting) - for elem1, elem2 in izip(field1, field2): + for elem1, elem2 in zip(field1, field2): if not scalars_equal(elem1, elem2, with_spans): return False From 4158edcde67ac2c7d80adae41e65886c79aefcfc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sta=C5=9B=20Ma=C5=82olepszy?= Date: Thu, 22 Jun 2017 12:54:37 +0200 Subject: [PATCH 8/9] Amend the CHANGELOG --- CHANGELOG.md | 7 ++++++- fluent/syntax/ast.py | 9 +++++---- 2 files changed, 11 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c9aef32e..e7d80e4d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,7 +2,12 @@ ## Unreleased - - … + - Add BaseNode.equals for deep-equality testing. + + Nodes are deeply compared on a field by field basis. If possible, False is + returned early. When comparing attributes, tags and variants in + SelectExpressions, the order doesn't matter. By default, spans are not + taken into account. ## fluent 0.4.0 (June 13th, 2017) diff --git a/fluent/syntax/ast.py b/fluent/syntax/ast.py index 450c456d..01bcbde2 100644 --- a/fluent/syntax/ast.py +++ b/fluent/syntax/ast.py @@ -74,11 +74,12 @@ def visit(value): return fun(node) def equals(self, other, with_spans=False): - """Compare two entries. + """Compare two nodes. - Entries are deeply compared on a field by field basis. If possible, - False is returned early. When comparing attributes, tags and variants - in SelectExpressions, the order doesn't matter. + Nodes are deeply compared on a field by field basis. If possible, False + is returned early. When comparing attributes, tags and variants in + SelectExpressions, the order doesn't matter. By default, spans are not + taken into account. """ self_keys = set(vars(self).keys()) From db57aa8d8eb514ac841df45c79f9c61484941836 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sta=C5=9B=20Ma=C5=82olepszy?= Date: Thu, 22 Jun 2017 13:09:48 +0200 Subject: [PATCH 9/9] Apply @Pike's feedback to tests --- tests/syntax/test_equals.py | 30 +++++++++++++++++++++--------- 1 file changed, 21 insertions(+), 9 deletions(-) diff --git a/tests/syntax/test_equals.py b/tests/syntax/test_equals.py index 870a661b..4c6b9f12 100644 --- a/tests/syntax/test_equals.py +++ b/tests/syntax/test_equals.py @@ -8,6 +8,10 @@ from fluent.syntax.parser import FluentParser +def identity(node): + return node + + class TestEntryEqualToSelf(unittest.TestCase): def setUp(self): self.parser = FluentParser() @@ -21,6 +25,7 @@ def test_same_simple_message(self): """) self.assertTrue(message1.equals(message1)) + self.assertTrue(message1.equals(message1.traverse(identity))) def test_same_selector_message(self): message1 = self.parse_ftl_entry("""\ @@ -35,6 +40,7 @@ def test_same_selector_message(self): """) self.assertTrue(message1.equals(message1)) + self.assertTrue(message1.equals(message1.traverse(identity))) def test_same_complex_placeable_message(self): message1 = self.parse_ftl_entry("""\ @@ -42,6 +48,7 @@ def test_same_complex_placeable_message(self): """) self.assertTrue(message1.equals(message1)) + self.assertTrue(message1.equals(message1.traverse(identity))) def test_same_message_with_attribute(self): message1 = self.parse_ftl_entry("""\ @@ -50,6 +57,7 @@ def test_same_message_with_attribute(self): """) self.assertTrue(message1.equals(message1)) + self.assertTrue(message1.equals(message1.traverse(identity))) def test_same_message_with_attributes(self): message1 = self.parse_ftl_entry("""\ @@ -59,6 +67,7 @@ def test_same_message_with_attributes(self): """) self.assertTrue(message1.equals(message1)) + self.assertTrue(message1.equals(message1.traverse(identity))) def test_same_message_with_tag(self): message1 = self.parse_ftl_entry("""\ @@ -67,6 +76,7 @@ def test_same_message_with_tag(self): """) self.assertTrue(message1.equals(message1)) + self.assertTrue(message1.equals(message1.traverse(identity))) def test_same_message_with_tags(self): message1 = self.parse_ftl_entry("""\ @@ -76,6 +86,7 @@ def test_same_message_with_tags(self): """) self.assertTrue(message1.equals(message1)) + self.assertTrue(message1.equals(message1.traverse(identity))) def test_same_junk(self): message1 = self.parse_ftl_entry("""\ @@ -83,6 +94,7 @@ def test_same_junk(self): """) self.assertTrue(message1.equals(message1)) + self.assertTrue(message1.equals(message1.traverse(identity))) class TestOrderEquals(unittest.TestCase): @@ -142,7 +154,7 @@ def test_variants(self): self.assertTrue(message2.equals(message1)) -class TestSpansEqual(unittest.TestCase): +class TestEqualWithSpans(unittest.TestCase): def test_default_behavior(self): parser = FluentParser() @@ -193,11 +205,13 @@ def test_equals_with_spans(self): for a, b in messages: self.assertTrue(a.equals(b, with_spans=True)) - def test_differ_with_spans(self): - parser = FluentParser() + def test_parser_without_spans_equals_with_spans(self): + parser = FluentParser(with_spans=False) strings = [ + ("foo = Foo", "foo = Foo"), ("foo = Foo", "foo = Foo"), + ("foo = { $arg }", "foo = { $arg }"), ("foo = { $arg }", "foo = { $arg }"), ] @@ -207,15 +221,13 @@ def test_differ_with_spans(self): ] for a, b in messages: - self.assertFalse(a.equals(b, with_spans=True)) + self.assertTrue(a.equals(b, with_spans=True)) - def test_parser_without_spans_equals_with_spans(self): - parser = FluentParser(with_spans=False) + def test_differ_with_spans(self): + parser = FluentParser() strings = [ - ("foo = Foo", "foo = Foo"), ("foo = Foo", "foo = Foo"), - ("foo = { $arg }", "foo = { $arg }"), ("foo = { $arg }", "foo = { $arg }"), ] @@ -225,4 +237,4 @@ def test_parser_without_spans_equals_with_spans(self): ] for a, b in messages: - self.assertTrue(a.equals(b, with_spans=True)) + self.assertFalse(a.equals(b, with_spans=True))