From 389e3e25ffc0e36bd2341b59c75a17fdbf7b548a Mon Sep 17 00:00:00 2001 From: Jason Simeone Date: Fri, 12 Sep 2014 15:25:30 -0400 Subject: [PATCH 1/2] Fixed issue with profile data not merging fully. --- prospector/profiles/profile.py | 21 +++++++++++++-------- tests/profiles/test_profile.py | 14 +------------- 2 files changed, 14 insertions(+), 21 deletions(-) diff --git a/prospector/profiles/profile.py b/prospector/profiles/profile.py index 547f3992..05a3b173 100644 --- a/prospector/profiles/profile.py +++ b/prospector/profiles/profile.py @@ -87,36 +87,41 @@ def parse_profile(name, contents): # parse_profile, for example data = dict(_EMPTY_DATA) else: - data = _merge_dict(_EMPTY_DATA, data, dict1_priority=False) + data = _merge_dict(_EMPTY_DATA, data) return StrictnessProfile(name, data) -def _merge_dict(dict1, dict2, dedup_lists=False, dict1_priority=True): +def _merge_dict(dict1, dict2): newdict = {} newdict.update(dict1) for key, value in dict2.items(): if key not in dict1: newdict[key] = value - elif value is None and dict1[key] is not None: - newdict[key] = dict1[key] - elif dict1[key] is None and value is not None: + + elif dict1[key] is None: newdict[key] = value + + elif value is None: + # Don't blast away existing values with None + pass + elif type(value) != type(dict1[key]): raise ValueError("Could not merge conflicting types %s and %s" % ( type(value), type(dict1[key]), )) + elif isinstance(value, dict): newdict[key] = _merge_dict( dict1[key], value, - dedup_lists, - dict1_priority, ) + elif isinstance(value, (list, tuple)): newdict[key] = list(set(dict1[key]) | set(value)) - elif not dict1_priority: + + else: newdict[key] = value return newdict diff --git a/tests/profiles/test_profile.py b/tests/profiles/test_profile.py index 867b0a90..3ce0677a 100644 --- a/tests/profiles/test_profile.py +++ b/tests/profiles/test_profile.py @@ -94,17 +94,5 @@ def test_dict_merge(self): 'c': 4 } } - self.assertEqual(expected, _merge_dict(a, b, dedup_lists=True, dict1_priority=False)) + self.assertEqual(expected, _merge_dict(a, b)) - expected = { - 'int': 1, - 'str': 'fish', - 'bool': True, - 'list': [1, 2, 3], - 'dict': { - 'a': 1, - 'b': 2, - 'c': 4 - } - } - self.assertEqual(expected, _merge_dict(a, b, dedup_lists=True, dict1_priority=True)) From 8e1dff79287708767c7e963b83bc7d6d38dc6ae2 Mon Sep 17 00:00:00 2001 From: Jason Simeone Date: Sun, 28 Sep 2014 15:42:50 -0400 Subject: [PATCH 2/2] Added a test case to demonstrate profile merging fix. --- tests/profiles/profiles/mergeoptions.yaml | 5 +++++ tests/profiles/test_profile.py | 19 +++++++++++++++++++ 2 files changed, 24 insertions(+) create mode 100644 tests/profiles/profiles/mergeoptions.yaml diff --git a/tests/profiles/profiles/mergeoptions.yaml b/tests/profiles/profiles/mergeoptions.yaml new file mode 100644 index 00000000..cc1c7e3c --- /dev/null +++ b/tests/profiles/profiles/mergeoptions.yaml @@ -0,0 +1,5 @@ +pylint: + options: + max-public-methods: 30 + max-attributes: 10 + diff --git a/tests/profiles/test_profile.py b/tests/profiles/test_profile.py index 3ce0677a..aa8367c4 100644 --- a/tests/profiles/test_profile.py +++ b/tests/profiles/test_profile.py @@ -48,6 +48,25 @@ def test_profile_merge(self): expected = ['C1000', 'C1001', 'E0504', 'W1010', 'W1012'] self.assertEqual(expected, merged_disabled_warnings) + def test_options_merge(self): + base_profile = from_file('strictness_veryhigh') + + # These are the default settings. + self.assertEqual(base_profile.pylint['options']['max-public-methods'], 20) + self.assertEqual(base_profile.pylint['options']['max-attributes'], 7) + + custom_profile = from_file('mergeoptions', self._basedir) + + # These are what we're trying to set with a custom profile. + self.assertEqual(custom_profile.pylint['options']['max-public-methods'], 30) + self.assertEqual(custom_profile.pylint['options']['max-attributes'], 10) + + merged_profile = merge_profiles((base_profile, custom_profile)) + + # The final, merged profile should have our custom options. + self.assertEqual(merged_profile.pylint['options']['max-public-methods'], 30) + self.assertEqual(merged_profile.pylint['options']['max-attributes'], 10) + def test_ignores(self): profile = load_profiles('ignores', basedir=self._basedir) self.assertEqual(['^tests/', '/migrations/'].sort(), profile.ignore.sort())