From 876d81ef2195b12c177f6359f06a06f2bf08b553 Mon Sep 17 00:00:00 2001 From: Huanxuan Ao Date: Wed, 8 Jun 2016 11:41:02 +0800 Subject: [PATCH] Error handling for KeyValueAction class. The set --property command requires that the input match the "key=value" type, but if the type don't match, the return value will be None, and the command still can be implemented successfully, this may confuse the users. I think we should raise exception if the argument type don't match "key=value". So I make some changes in KeyValueAction class in this patch. Change-Id: If4a00ae6da08bc12362ef2fc82012b42839141f0 --- osc_lib/cli/parseractions.py | 4 +++- osc_lib/tests/cli/test_parseractions.py | 17 +++++++---------- 2 files changed, 10 insertions(+), 11 deletions(-) diff --git a/osc_lib/cli/parseractions.py b/osc_lib/cli/parseractions.py index 8125c18..0565372 100644 --- a/osc_lib/cli/parseractions.py +++ b/osc_lib/cli/parseractions.py @@ -35,7 +35,9 @@ class KeyValueAction(argparse.Action): if '=' in values: getattr(namespace, self.dest, {}).update([values.split('=', 1)]) else: - getattr(namespace, self.dest, {}).pop(values, None) + msg = _("Expected 'key=value' type, " + "but got: %s") % (str(values)) + raise argparse.ArgumentTypeError(msg) class MultiKeyValueAction(argparse.Action): diff --git a/osc_lib/tests/cli/test_parseractions.py b/osc_lib/tests/cli/test_parseractions.py index 3225f93..beff04a 100644 --- a/osc_lib/tests/cli/test_parseractions.py +++ b/osc_lib/tests/cli/test_parseractions.py @@ -49,16 +49,13 @@ class TestKeyValueAction(utils.TestCase): self.assertDictEqual(expect, actual) def test_error_values(self): - results = self.parser.parse_args([ - '--property', 'red', - '--property', 'green=100%', - '--property', 'blue', - ]) - - actual = getattr(results, 'property', {}) - # There should be no red or blue - expect = {'green': '100%', 'format': '#rgb'} - self.assertDictEqual(expect, actual) + self.assertRaises( + argparse.ArgumentTypeError, + self.parser.parse_args, + [ + '--property', 'red', + ] + ) class TestMultiKeyValueAction(utils.TestCase):