-
Notifications
You must be signed in to change notification settings - Fork 3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
{Core} Add random_config_dir param in DummyCli #25689
Changes from 8 commits
43813d2
109201c
b94d580
1b87eea
869aacb
99aeaf0
c29ff94
6a6ca82
60afa32
7a8d2bf
d777ac0
7f9e0d4
0ec2406
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -16,15 +16,19 @@ def __init__(self, commands_loader_cls=None, **kwargs): | |
from azure.cli.core.azlogging import AzCliLogging | ||
from azure.cli.core.cloud import get_active_cloud | ||
from azure.cli.core.parser import AzCliCommandParser | ||
from azure.cli.core.util import random_string | ||
from azure.cli.core._config import GLOBAL_CONFIG_DIR, ENV_VAR_PREFIX | ||
from azure.cli.core._help import AzCliHelp | ||
from azure.cli.core._output import AzOutputProducer | ||
|
||
from knack.completion import ARGCOMPLETE_ENV_NAME | ||
|
||
random_config_dir = kwargs.get('random_config_dir', False) | ||
|
||
super(DummyCli, self).__init__( | ||
cli_name='az', | ||
config_dir=GLOBAL_CONFIG_DIR, | ||
config_dir=os.path.join(GLOBAL_CONFIG_DIR, 'dummy_cli_config_dir', | ||
random_string()) if random_config_dir else GLOBAL_CONFIG_DIR, | ||
Comment on lines
+28
to
+29
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. As an alternative, we may also consider using There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Well, sorry I didn't make my point clear. I am simply providing an alternative solution. You solution is actually better since your location is easy to find. 😉 |
||
config_env_var_prefix=ENV_VAR_PREFIX, | ||
commands_loader_cls=commands_loader_cls or MainCommandsLoader, | ||
parser_cls=AzCliCommandParser, | ||
|
Original file line number | Diff line number | Diff line change | ||
---|---|---|---|---|
|
@@ -81,8 +81,10 @@ def is_empty(self): # pylint: disable=no-self-use | |||
|
||||
class ScenarioTest(ReplayableTest, CheckerMixin, unittest.TestCase): | ||||
def __init__(self, method_name, config_file=None, recording_name=None, | ||||
recording_processors=None, replay_processors=None, recording_patches=None, replay_patches=None): | ||||
self.cli_ctx = get_dummy_cli() | ||||
recording_processors=None, replay_processors=None, recording_patches=None, replay_patches=None, | ||||
random_config_dir=False): | ||||
self.cli_ctx = get_dummy_cli(random_config_dir=random_config_dir) | ||||
self.random_config_dir = random_config_dir | ||||
self.name_replacer = GeneralNameReplacer() | ||||
self.kwargs = {} | ||||
self.test_guid_count = 0 | ||||
|
@@ -137,6 +139,9 @@ def _merge_lists(base, patches): | |||
def tearDown(self): | ||||
for processor in self._processors_to_reset: | ||||
processor.reset() | ||||
if self.random_config_dir: | ||||
import shutil | ||||
shutil.rmtree(self.cli_ctx.config.config_dir, ignore_errors=True) | ||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Well... yes.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Oh, yes. I forget that Windows bug. |
||||
super(ScenarioTest, self).tearDown() | ||||
|
||||
def create_random_name(self, prefix, length): | ||||
|
@@ -181,7 +186,8 @@ class LocalContextScenarioTest(ScenarioTest): | |||
def __init__(self, method_name, config_file=None, recording_name=None, recording_processors=None, | ||||
replay_processors=None, recording_patches=None, replay_patches=None, working_dir=None): | ||||
super(LocalContextScenarioTest, self).__init__(method_name, config_file, recording_name, recording_processors, | ||||
replay_processors, recording_patches, replay_patches) | ||||
replay_processors, recording_patches, replay_patches, | ||||
random_config_dir=True) | ||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Local Context/Param Persist has been deprecated. Consider deleting related tests instead: #24726 |
||||
if self.in_recording: | ||||
self.recording_patches.append(patch_get_current_system_username) | ||||
else: | ||||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -255,6 +255,9 @@ def test_role_definition_scenario(self): | |
|
||
class RoleAssignmentScenarioTest(RoleScenarioTestBase): | ||
|
||
def __init__(self, *arg, **kwargs): | ||
super().__init__(*arg, random_config_dir=True, **kwargs) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, we need to merge #26475 first. |
||
|
||
@ResourceGroupPreparer(name_prefix='cli_role_assign') | ||
@AllowLargeResponse() | ||
def test_role_assignment_scenario(self, resource_group): | ||
|
@@ -568,7 +571,7 @@ def test_role_assignment_handle_conflicted_assignments(self, resource_group): | |
|
||
self.assertGreaterEqual(len(local_defaults_config), 1) | ||
actual = set([(x['name'], x['source'], x['value']) for x in local_defaults_config if x['name'] == 'group']) | ||
expected = set([('group', os.path.join(temp_dir, '.azure', 'config'), self.kwargs['rg'])]) | ||
expected = set([('group', os.path.join(temp_dir, os.path.basename(self.cli_ctx.config.config_dir), 'config'), self.kwargs['rg'])]) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The config path is There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Let's provide some concrete example. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If work dir is
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The local config folder was constructed by https://github.com/microsoft/knack/blob/e496c9590792572e680cb3ec959db175d9ba85dd/knack/config.py#L63 config_dir_name = os.path.basename(self.config_dir)
current_config_dir = os.path.join(current_dir, config_dir_name)
|
||
self.assertEqual(actual, expected) | ||
|
||
# test role assignments on a resource group | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider making it an explicit kwarg.