From 2b9ad7bdb94d54a46068bd8864fb8bb96161bcf5 Mon Sep 17 00:00:00 2001
From: Mark McLoughlin <markmc@redhat.com>
Date: Mon, 26 Nov 2012 06:47:52 +0000
Subject: Fix set_default() with boolean CLI options

Porting to argparse broke set_default() with boolean CLI options. The
new test case shows this borkage.

The issue is that, by default, argparse differs subtly from optparse in
its handling of defaults for boolean options. Compare:

  >>> p = optparse.OptionParser()
  >>> p.add_option('--foo', action='store_true')
  >>> p.add_option('--bar', action='store_true', default=False)
  >>> p.parse_args([])
  (<Values at 0x7f28aba066c8: {'foo': None, 'bar': False}>, [])

to:

  >>> p = argparse.ArgumentParser()
  >>> p.add_argument('--foo', action='store_true')
  >>> p.add_argument('--bar', action='store_true', default=False)
  >>> p.add_argument('--blaa', action='store_true', default=None)
  >>> p.parse_args([])
  Namespace(bar=False, blaa=None, foo=False)

i.e. unless you specify a default for a boolean option, optparse
defaults to None whereas argparse defaults to False. To get the
same optparse behaviour with argparse, you need default=None.

Change-Id: Ifc92a834c4ba59e939d80ac5de24d7051232f5b5
---
 openstack/common/cfg.py |  3 ++-
 tests/unit/test_cfg.py  | 22 ++++++++++++++++++++++
 2 files changed, 24 insertions(+), 1 deletion(-)

diff --git a/openstack/common/cfg.py b/openstack/common/cfg.py
index 28baa04..9be96cc 100644
--- a/openstack/common/cfg.py
+++ b/openstack/common/cfg.py
@@ -632,7 +632,8 @@ class Opt(object):
             kwargs['dest'] = dest
         else:
             kwargs['nargs'] = '?'
-        kwargs.update({'metavar': self.metavar,
+        kwargs.update({'default': None,
+                       'metavar': self.metavar,
                        'help': self.help, })
         return kwargs
 
diff --git a/tests/unit/test_cfg.py b/tests/unit/test_cfg.py
index 9887411..bf82f04 100644
--- a/tests/unit/test_cfg.py
+++ b/tests/unit/test_cfg.py
@@ -1102,6 +1102,28 @@ class OverridesTestCase(BaseTestCase):
         self.conf.clear_override('foo', group='blaa')
         self.assertEquals(self.conf.blaa.foo, None)
 
+    def test_cli_bool_default(self):
+        self.conf.register_cli_opt(BoolOpt('foo'))
+        self.conf.set_default('foo', True)
+        self.assertTrue(self.conf.foo)
+        self.conf([])
+        self.assertTrue(self.conf.foo)
+        self.conf.set_default('foo', False)
+        self.assertFalse(self.conf.foo)
+        self.conf.clear_default('foo')
+        self.assertTrue(self.conf.foo is None)
+
+    def test_cli_bool_override(self):
+        self.conf.register_cli_opt(BoolOpt('foo'))
+        self.conf.set_override('foo', True)
+        self.assertTrue(self.conf.foo)
+        self.conf([])
+        self.assertTrue(self.conf.foo)
+        self.conf.set_override('foo', False)
+        self.assertFalse(self.conf.foo)
+        self.conf.clear_override('foo')
+        self.assertTrue(self.conf.foo is None)
+
 
 class ResetAndClearTestCase(BaseTestCase):
 
-- 
cgit