Review patch for GH-63

- Moved mask_value_if_secret til after the method using it. Most
  important/interesting method first, lower level/more detailed later.
- Made the logic in mask_value_if_secret simpler to understand, I think,
  by making the two different cases more obvious.
- Split the test in two, as it actually tested two different cases.
- Changed the test names to explain what should happen, and not just
  what is tested.
- Add a new test that tests the case which caused the crash: a setting
  being None.

mopidy/utils/settings.py

@@ -129,14 +129,6 @@ def validate_settings(defaults, settings):
 
     return errors
 
-def mask_value_if_secret(key, value):
-    masked_value = value
-    
-    if key.endswith('PASSWORD') and value:
-         masked_value = u'********'
-         
-    return masked_value
-
 def list_settings_optparse_callback(*args):
     """
     Prints a list of all settings.
@@ -158,3 +150,9 @@ def list_settings_optparse_callback(*args):
             lines.append(u'  Error: %s' % errors[key])
     print u'Settings: %s' % indent('\n'.join(lines), places=2)
     sys.exit(0)
+
+def mask_value_if_secret(key, value):
+    if key.endswith('PASSWORD') and value:
+        return u'********'
+    else:
+        return value

tests/utils/settings_test.py

@@ -45,14 +45,19 @@ class ValidateSettingsTest(unittest.TestCase):
     def test_two_errors_are_both_reported(self):
         result = validate_settings(self.defaults,
             {'FOO': '', 'BAR': ''})
-        self.assertEquals(len(result), 2)
-
-    def test_mask_if_secret(self):
-        not_secret = mask_value_if_secret('SPOTIFY_USERNAME', 'foo')
-        self.assertEquals('foo', not_secret)
+        self.assertEqual(len(result), 2)
 
+    def test_masks_value_if_secret(self):
         secret = mask_value_if_secret('SPOTIFY_PASSWORD', 'bar')
-        self.assertEquals(u'********', secret)
+        self.assertEqual(u'********', secret)
+
+    def test_does_not_mask_value_if_not_secret(self):
+        not_secret = mask_value_if_secret('SPOTIFY_USERNAME', 'foo')
+        self.assertEqual('foo', not_secret)
+
+    def test_does_not_mask_value_if_none(self):
+        not_secret = mask_value_if_secret('SPOTIFY_USERNAME', None)
+        self.assertEqual(None, not_secret)
 
 
 class SettingsProxyTest(unittest.TestCase):