Skip to content

{testsdk} Initialize random config dir in setUp #28849

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.

Sign up for GitHub

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

Draft
jiasli wants to merge 1 commit into
base: dev
Choose a base branch
from
Draft

{testsdk} Initialize random config dir in setUp #28849

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 2 additions & 25 deletions src/azure-cli-core/azure/cli/core/mock.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,41 +8,18 @@

class DummyCli(AzCli):
"""A dummy CLI instance can be used to facilitate automation"""
def __init__(self, commands_loader_cls=None, random_config_dir=False, **kwargs):
def __init__(self, commands_loader_cls=None, **kwargs):
import os
from unittest.mock import patch

from azure.cli.core import MainCommandsLoader
from azure.cli.core.commands import AzCliCommandInvoker
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, ENV_VAR_TEST_LIVE
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
from knack.util import ensure_dir

if random_config_dir:
config_dir = os.path.join(GLOBAL_CONFIG_DIR, 'dummy_cli_config_dir', random_string())
# Knack prioritizes the AZURE_CONFIG_DIR env over the config_dir param, and other functions may call
# get_config_dir directly. We need to set the env to make sure the config_dir is used.
self.env_patch = patch.dict(os.environ, {'AZURE_CONFIG_DIR': config_dir})
self.env_patch.start()

# In recording mode, copy login credentials from global config dir to the dummy config dir
if os.getenv(ENV_VAR_TEST_LIVE, '').lower() == 'true':
if os.path.exists(GLOBAL_CONFIG_DIR):
Copy link
Member Author

@jiasli jiasli Apr 28, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

GLOBAL_CONFIG_DIR will always exist.

Copy link
Contributor

@bebound bebound Apr 29, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In practice, if the user is using --live, .azure should exist.

But from the code, .azure is created by AzCli:

azure-cli/src/azure-cli-core/azure/cli/core/__init__.py

Lines 77 to 78 in f98ba80

azure_folder = self.config.config_dir
ensure_dir(azure_folder)

The super().__init__() is not called yet when this line is executed, and the folder might not exist.

ensure_dir(config_dir)
import shutil
for file in ['azureProfile.json', 'msal_token_cache.bin', 'clouds.config', 'msal_token_cache.json',
'service_principal_entries.json']:
try:
shutil.copy(os.path.join(GLOBAL_CONFIG_DIR, file), config_dir)
except FileNotFoundError:
pass

super(DummyCli, self).__init__(
cli_name='az',
Expand Down
34 changes: 29 additions & 5 deletions src/azure-cli-testsdk/azure/cli/testsdk/base.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,6 @@ 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,
random_config_dir=False):
self.cli_ctx = get_dummy_cli(random_config_dir=random_config_dir)
Copy link
Member

@wangzelin007 wangzelin007 Apr 29, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some tests reported the following errors:
AttributeError: 'xxxScenarioTest' object has no attribute 'cli_ctx'
https://dev.azure.com/azclitools/public/_build/results?buildId=153167&view=logs&j=9ccb9437-6abb-5c1a-b909-dd7aa8d5d916&t=da342424-93bf-5216-b53c-16ba29f2062e&l=674

Copy link
Contributor

@bebound bebound Apr 29, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cli_ctx is moved to setUp mothod, which is executed after __init__.
If cli_ctx is used in __init__, this error is raised.

self.random_config_dir = random_config_dir
self.name_replacer = GeneralNameReplacer()
self.kwargs = {}
Expand Down Expand Up @@ -136,13 +135,38 @@ def _merge_lists(base, patches):
recording_name=recording_name
)

def setUp(self):
if self.random_config_dir:
from unittest.mock import patch
from azure.cli.core.util import random_string, rmtree_with_retry
from azure.cli.core._config import GLOBAL_CONFIG_DIR
from knack.util import ensure_dir

config_dir = os.path.join(GLOBAL_CONFIG_DIR, 'dummy_cli_config_dir', random_string())
self.addCleanup(rmtree_with_retry, config_dir)
logger.warning('Using random config dir: %s', config_dir)
# Knack prioritizes the AZURE_CONFIG_DIR env over the config_dir param, and other functions may call
# get_config_dir directly. We need to set the env to make sure the config_dir is used.
env_patch = patch.dict(os.environ, {'AZURE_CONFIG_DIR': config_dir})
env_patch.start()
self.addCleanup(env_patch.stop)
Copy link
Member Author

@jiasli jiasli Apr 28, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The official doc of unittest.mock also prefers addCleanup over tearDown: https://docs.python.org/3/library/unittest.mock.html#patch-methods-start-and-stop

image


# In recording mode, copy login credentials from global config dir to the dummy config dir
if self.in_recording:
ensure_dir(config_dir)
import shutil
for file in ['azureProfile.json', 'clouds.config', 'msal_token_cache.bin', 'msal_token_cache.json',
'service_principal_entries.bin', 'service_principal_entries.json']:
try:
shutil.copy(os.path.join(GLOBAL_CONFIG_DIR, file), config_dir)
except FileNotFoundError:
pass
self.cli_ctx = get_dummy_cli()
super(ScenarioTest, self).setUp()

def tearDown(self):
for processor in self._processors_to_reset:
processor.reset()
if self.random_config_dir:
Copy link
Member Author

@jiasli jiasli Apr 28, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This if block is also eliminated by using addCleanup.

from azure.cli.core.util import rmtree_with_retry
rmtree_with_retry(self.cli_ctx.config.config_dir)
self.cli_ctx.env_patch.stop()
super(ScenarioTest, self).tearDown()

def create_random_name(self, prefix, length):
Expand Down