Skip to content

Commit 44ee179

Browse files
adewaleotjprescott
authored andcommitted
Fix preview mechanism bug (#161)
* Fix bug where PreviewArgumentAction, does not inherit from registered argument action. * Update test to catch issue. Update fix logic to pass tests. * python2: in test, use print function from __future__
1 parent d933666 commit 44ee179

File tree

4 files changed

+25
-9
lines changed

4 files changed

+25
-9
lines changed

HISTORY.rst

+5
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,11 @@
33
Release History
44
===============
55

6+
0.6.3
7+
+++++
8+
* Fix bug where arguments in preview did not call registered actions. This meant that parameter parsing did not behave
9+
completely as expected.
10+
611
0.6.2
712
+++++
813
* Adds ability to declare that command groups, commands, and arguments are in a preview status and therefore might change or be removed. This is done by passing the kwarg `is_preview=True`.

knack/arguments.py

+3-1
Original file line numberDiff line numberDiff line change
@@ -162,7 +162,9 @@ def _get_parent_class(self, **kwargs):
162162
# wrap any existing action
163163
action = kwargs.get('action', None)
164164
parent_class = argparse.Action
165-
if isinstance(action, argparse.Action):
165+
166+
# action is either a user-defined Action class or a string referring a library-defined Action
167+
if isinstance(action, type) and issubclass(action, argparse.Action):
166168
parent_class = action
167169
elif isinstance(action, str):
168170
parent_class = self.command_loader.cli_ctx.invocation.parser._registries['action'][action] # pylint: disable=protected-access

setup.py

+1-1
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@
99
from codecs import open
1010
from setuptools import setup, find_packages
1111

12-
VERSION = '0.6.2'
12+
VERSION = '0.6.3'
1313

1414
DEPENDENCIES = [
1515
'argcomplete',

tests/test_preview.py

+16-7
Original file line numberDiff line numberDiff line change
@@ -3,17 +3,19 @@
33
# Licensed under the MIT License. See License.txt in the project root for license information.
44
# --------------------------------------------------------------------------------------------
55

6-
from __future__ import unicode_literals
6+
from __future__ import unicode_literals, print_function
77

88
import unittest
99
try:
1010
import mock
1111
except ImportError:
1212
from unittest import mock
13-
from threading import Lock
13+
14+
import sys
15+
import argparse
1416

1517
from knack.arguments import ArgumentsContext
16-
from knack.commands import CLICommand, CLICommandsLoader, CommandGroup
18+
from knack.commands import CLICommandsLoader, CommandGroup
1719

1820
from tests.util import DummyCLI, redirect_io
1921

@@ -148,9 +150,13 @@ def test_preview_command_implicitly(self):
148150
class TestArgumentPreview(unittest.TestCase):
149151

150152
def setUp(self):
151-
152153
from knack.help_files import helps
153154

155+
class LoggerAction(argparse.Action):
156+
157+
def __call__(self, parser, namespace, values, option_string=None):
158+
print("Side-effect from some original action!", file=sys.stderr)
159+
154160
class PreviewTestCommandLoader(CLICommandsLoader):
155161
def load_command_table(self, args):
156162
super(PreviewTestCommandLoader, self).load_command_table(args)
@@ -160,7 +166,7 @@ def load_command_table(self, args):
160166

161167
def load_arguments(self, command):
162168
with ArgumentsContext(self, 'arg-test') as c:
163-
c.argument('arg1', help='Arg1', is_preview=True)
169+
c.argument('arg1', help='Arg1', is_preview=True, action=LoggerAction)
164170

165171
super(PreviewTestCommandLoader, self).load_arguments(command)
166172

@@ -188,8 +194,11 @@ def test_preview_arguments_execute(self):
188194
""" Ensure deprecated arguments can be used. """
189195
self.cli_ctx.invoke('arg-test --arg1 foo --opt1 bar'.split())
190196
actual = self.io.getvalue()
191-
expected = "Argument '--arg1' is in preview. It may be changed/removed in a future release."
192-
self.assertIn(expected, actual)
197+
preview_expected = "Argument '--arg1' is in preview. It may be changed/removed in a future release."
198+
self.assertIn(preview_expected, actual)
199+
200+
action_expected = "Side-effect from some original action!"
201+
self.assertIn(action_expected, actual)
193202

194203

195204
if __name__ == '__main__':

0 commit comments

Comments
 (0)