Skip to content

Commit

Permalink
Fix preview mechanism bug (#161)
Browse files Browse the repository at this point in the history
* 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__
  • Loading branch information
adewaleo authored and tjprescott committed Jul 9, 2019
1 parent d933666 commit 44ee179
Show file tree
Hide file tree
Showing 4 changed files with 25 additions and 9 deletions.
5 changes: 5 additions & 0 deletions HISTORY.rst
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,11 @@
Release History
===============

0.6.3
+++++
* Fix bug where arguments in preview did not call registered actions. This meant that parameter parsing did not behave
completely as expected.

0.6.2
+++++
* 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`.
Expand Down
4 changes: 3 additions & 1 deletion knack/arguments.py
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,9 @@ def _get_parent_class(self, **kwargs):
# wrap any existing action
action = kwargs.get('action', None)
parent_class = argparse.Action
if isinstance(action, argparse.Action):

# action is either a user-defined Action class or a string referring a library-defined Action
if isinstance(action, type) and issubclass(action, argparse.Action):
parent_class = action
elif isinstance(action, str):
parent_class = self.command_loader.cli_ctx.invocation.parser._registries['action'][action] # pylint: disable=protected-access
Expand Down
2 changes: 1 addition & 1 deletion setup.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
from codecs import open
from setuptools import setup, find_packages

VERSION = '0.6.2'
VERSION = '0.6.3'

DEPENDENCIES = [
'argcomplete',
Expand Down
23 changes: 16 additions & 7 deletions tests/test_preview.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,17 +3,19 @@
# Licensed under the MIT License. See License.txt in the project root for license information.
# --------------------------------------------------------------------------------------------

from __future__ import unicode_literals
from __future__ import unicode_literals, print_function

import unittest
try:
import mock
except ImportError:
from unittest import mock
from threading import Lock

import sys
import argparse

from knack.arguments import ArgumentsContext
from knack.commands import CLICommand, CLICommandsLoader, CommandGroup
from knack.commands import CLICommandsLoader, CommandGroup

from tests.util import DummyCLI, redirect_io

Expand Down Expand Up @@ -148,9 +150,13 @@ def test_preview_command_implicitly(self):
class TestArgumentPreview(unittest.TestCase):

def setUp(self):

from knack.help_files import helps

class LoggerAction(argparse.Action):

def __call__(self, parser, namespace, values, option_string=None):
print("Side-effect from some original action!", file=sys.stderr)

class PreviewTestCommandLoader(CLICommandsLoader):
def load_command_table(self, args):
super(PreviewTestCommandLoader, self).load_command_table(args)
Expand All @@ -160,7 +166,7 @@ def load_command_table(self, args):

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

super(PreviewTestCommandLoader, self).load_arguments(command)

Expand Down Expand Up @@ -188,8 +194,11 @@ def test_preview_arguments_execute(self):
""" Ensure deprecated arguments can be used. """
self.cli_ctx.invoke('arg-test --arg1 foo --opt1 bar'.split())
actual = self.io.getvalue()
expected = "Argument '--arg1' is in preview. It may be changed/removed in a future release."
self.assertIn(expected, actual)
preview_expected = "Argument '--arg1' is in preview. It may be changed/removed in a future release."
self.assertIn(preview_expected, actual)

action_expected = "Side-effect from some original action!"
self.assertIn(action_expected, actual)


if __name__ == '__main__':
Expand Down

0 comments on commit 44ee179

Please sign in to comment.