print better message when choices has an empty string1.9.2

Follow on to:
I3bb3621420915a6a2990440294a389b45a23c519

choices can have None (as a default) or an empty string "" (as a
default value or when read from a config file). We need to print
better message in the sample config for these two cases.

Change-Id: Iba483c0e4180fc95181b161f996cce5e627035f5

4 files changed, 35 insertions, 6 deletions

diff --git a/oslo_config/generator.py b/oslo_config/generator.py
index 749a556..ef992fd 100644
--- a/oslo_config/generator.py
+++ b/oslo_config/generator.py
@@ -177,6 +177,13 @@ class _OptFormatter(object):
             lines = ['# ' + help_text + '\n']
         return lines
 
+    def _get_choice_text(self, choice):
+        if choice is None:
+            return '<None>'
+        elif choice == '':
+            return "''"
+        return six.text_type(choice)
+
     def format(self, opt):
         """Format a description of an option to the output file.
 
@@ -195,8 +202,7 @@ class _OptFormatter(object):
         lines = self._format_help(help_text)
 
         if getattr(opt.type, 'choices', None):
-            choices_text = ', '.join([six.text_type(choice)
-                                      if choice else '<None>'
+            choices_text = ', '.join([self._get_choice_text(choice)
                                       for choice in opt.type.choices])
             lines.append('# Allowed values: %s\n' % choices_text)
 
diff --git a/oslo_config/tests/test_cfg.py b/oslo_config/tests/test_cfg.py
index 6accd82..5518f53 100644
--- a/oslo_config/tests/test_cfg.py
+++ b/oslo_config/tests/test_cfg.py
@@ -3479,6 +3479,29 @@ class ChoicesTestCase(BaseTestCase):
         self.assertTrue(hasattr(self.conf, 'foo'))
         self.assertEqual(self.conf.foo, 'bar1')
 
+    def test_conf_file_choice_empty_value(self):
+        self.conf.register_opt(cfg.StrOpt('foo',
+                               choices=['', 'bar1', 'bar2']))
+
+        paths = self.create_tempfiles([('test', '[DEFAULT]\n''foo = \n')])
+
+        self.conf(['--config-file', paths[0]])
+
+        self.assertTrue(hasattr(self.conf, 'foo'))
+        self.assertEqual(self.conf.foo, '')
+
+    def test_conf_file_choice_none_value(self):
+        self.conf.register_opt(cfg.StrOpt('foo',
+                               default=None,
+                               choices=[None, 'bar1', 'bar2']))
+
+        paths = self.create_tempfiles([('test', '[DEFAULT]\n''\n')])
+
+        self.conf(['--config-file', paths[0]])
+
+        self.assertTrue(hasattr(self.conf, 'foo'))
+        self.assertEqual(self.conf.foo, None)
+
     def test_conf_file_bad_choice_value(self):
         self.conf.register_opt(cfg.StrOpt('foo',
                                choices=['bar1', 'bar2']))
diff --git a/oslo_config/tests/test_generator.py b/oslo_config/tests/test_generator.py
index 1c30ca3..144a4f8 100644
--- a/oslo_config/tests/test_generator.py
+++ b/oslo_config/tests/test_generator.py
@@ -50,7 +50,7 @@ class GeneratorTestCase(base.BaseTestCase):
                                      'laborum.'),
         'choices_opt': cfg.StrOpt('choices_opt',
                                   default='a',
-                                  choices=(None, 'a', 'b', 'c'),
+                                  choices=(None, '', 'a', 'b', 'c'),
                                   help='a string with choices'),
         'deprecated_opt': cfg.StrOpt('bar',
                                      deprecated_name='foobar',
@@ -294,7 +294,7 @@ class GeneratorTestCase(base.BaseTestCase):
 #
 
 # a string with choices (string value)
-# Allowed values: <None>, a, b, c
+# Allowed values: <None>, '', a, b, c
 #choices_opt = a
 ''')),
         ('deprecated',
diff --git a/tests/test_generator.py b/tests/test_generator.py
index 3db32c9..d020233 100644
--- a/tests/test_generator.py
+++ b/tests/test_generator.py
@@ -54,7 +54,7 @@ class GeneratorTestCase(base.BaseTestCase):
                                      'laborum.'),
         'choices_opt': cfg.StrOpt('choices_opt',
                                   default='a',
-                                  choices=(None, 'a', 'b', 'c'),
+                                  choices=(None, '', 'a', 'b', 'c'),
                                   help='a string with choices'),
         'deprecated_opt': cfg.StrOpt('bar',
                                      deprecated_name='foobar',
@@ -309,7 +309,7 @@ class GeneratorTestCase(base.BaseTestCase):
 #
 
 # a string with choices (string value)
-# Allowed values: <None>, a, b, c
+# Allowed values: <None>, '', a, b, c
 #choices_opt = a
 ''')),
         ('deprecated',