From 96dd100b70c4b138965471658947a9319597d82e Mon Sep 17 00:00:00 2001 From: Sergei Trofimov Date: Wed, 11 Jul 2018 09:55:10 +0100 Subject: [PATCH] utils/toggle_set: fix merge behavior - Change how "source" and "dest" are handled inside merge() to be more sane and less confusing, ensuring that disabling toggles are merged correctly. - Do not drop disabling values during merge, to ensure that merging is a transitive operation. - Add unit tests for the above fixes. --- tests/test_utils.py | 43 ++++++++++++++++++++++++++++++++++++++++++- wa/utils/types.py | 16 +++++++--------- 2 files changed, 49 insertions(+), 10 deletions(-) diff --git a/tests/test_utils.py b/tests/test_utils.py index 644dd749..232c0324 100644 --- a/tests/test_utils.py +++ b/tests/test_utils.py @@ -21,7 +21,7 @@ from nose.tools import raises, assert_equal, assert_not_equal, assert_in, assert from nose.tools import assert_true, assert_false, assert_raises, assert_is, assert_list_equal from wa.utils.types import (list_or_integer, list_or_bool, caseless_string, - arguments, prioritylist, enum, level) + arguments, prioritylist, enum, level, toggle_set) @@ -149,3 +149,44 @@ class TestEnumLevel(TestCase): s = e.one.to_pod() l = e.from_pod(s) assert_equal(l, e.one) + + +class TestToggleSet(TestCase): + + def test_equality(self): + ts1 = toggle_set(['one', 'two',]) + ts2 = toggle_set(['one', 'two', '~three']) + + assert_not_equal(ts1, ts2) + assert_equal(ts1.values(), ts2.values()) + assert_equal(ts2, toggle_set(['two', '~three', 'one'])) + + def test_merge(self): + ts1 = toggle_set(['one', 'two', 'three', '~four', '~five']) + ts2 = toggle_set(['two', '~three', 'four', '~five']) + + ts3 = ts1.merge_with(ts2) + assert_equal(ts1, toggle_set(['one', 'two', 'three', '~four', '~five'])) + assert_equal(ts2, toggle_set(['two', '~three', 'four', '~five'])) + assert_equal(ts3, toggle_set(['one', 'two', '~three', 'four', '~five'])) + assert_equal(ts3.values(), set(['one', 'two','four'])) + + ts4 = ts1.merge_into(ts2) + assert_equal(ts1, toggle_set(['one', 'two', 'three', '~four', '~five'])) + assert_equal(ts2, toggle_set(['two', '~three', 'four', '~five'])) + assert_equal(ts4, toggle_set(['one', 'two', 'three', '~four', '~five'])) + assert_equal(ts4.values(), set(['one', 'two', 'three'])) + + def test_drop_all_previous(self): + ts1 = toggle_set(['one', 'two', 'three']) + ts2 = toggle_set(['four', '~~', 'five']) + ts3 = toggle_set(['six', 'seven', '~three']) + + ts4 = ts1.merge_with(ts2).merge_with(ts3) + assert_equal(ts4, toggle_set(['four', 'five', 'six', 'seven', '~three', '~~'])) + + ts5 = ts2.merge_into(ts3).merge_into(ts1) + assert_equal(ts5, toggle_set(['four', 'five', '~~'])) + + ts6 = ts2.merge_into(ts3).merge_with(ts1) + assert_equal(ts6, toggle_set(['one', 'two', 'three', 'four', 'five', '~~'])) diff --git a/wa/utils/types.py b/wa/utils/types.py index ed90b4be..a498778a 100644 --- a/wa/utils/types.py +++ b/wa/utils/types.py @@ -39,7 +39,6 @@ else: from urllib import quote, unquote # pylint: disable=no-name-in-module # pylint: disable=wrong-import-position from collections import defaultdict, MutableMapping -from copy import copy from functools import total_ordering from future.utils import with_metaclass @@ -387,10 +386,11 @@ class toggle_set(set): return toggle_set(pod) @staticmethod - def merge(source, dest): - if '~~' in dest: - dest.remove('~~') - return dest + def merge(dest, source): + if '~~' in source: + return toggle_set(source) + + dest = toggle_set(dest) for item in source: if item not in dest: #Disable previously enabled item @@ -411,12 +411,10 @@ class toggle_set(set): set.__init__(self, *args) def merge_with(self, other): - other = copy(other) - return toggle_set.merge(self, toggle_set(other)) + return toggle_set.merge(self, other) def merge_into(self, other): - new_self = copy(self) - return toggle_set.merge(other, new_self) + return toggle_set.merge(other, self) def add(self, item): if item not in self: