Skip to content

{ARM} Fix race condition in bicep unit tests #26797

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

Merged
zhoxing-ms merged 5 commits into from
Jul 26, 2023
Merged

{ARM} Fix race condition in bicep unit tests #26797

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
471ba65
Remove semver with packaging.version
shenglol Jun 30, 2023
c25e963
Restore requirements files
shenglol Jul 10, 2023
e3ea597
Restore semver
shenglol Jul 17, 2023
841fc57
semver.Version => semver.VersionInfo
shenglol Jul 18, 2023
c106b6d
Fix error
shenglol Jul 20, 2023
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
30 changes: 12 additions & 18 deletions src/azure-cli/azure/cli/command_modules/resource/_bicep.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -93,9 +93,9 @@ def run_bicep_command(cli_ctx, args, auto_install=True):
# Users may continue using the current installed version.
installed_version = _get_bicep_installed_version(installation_path)
latest_release_tag = get_bicep_latest_release_tag() if cache_expired else latest_release_tag
latest_version = _extract_semver(latest_release_tag)
latest_version = _extract_version(latest_release_tag)

if installed_version and latest_version and semver.compare(installed_version, latest_version) < 0:
if installed_version and latest_version and installed_version < latest_version:
_logger.warning(
'A new Bicep release is available: %s. Upgrade now by running "az bicep upgrade".',
latest_release_tag,
Expand All @@ -116,8 +116,8 @@ def ensure_bicep_installation(cli_ctx, release_tag=None, target_platform=None, s
return

installed_version = _get_bicep_installed_version(installation_path)
target_version = _extract_semver(release_tag)
if installed_version and target_version and semver.compare(installed_version, target_version) == 0:
target_version = _extract_version(release_tag)
if installed_version and target_version and installed_version == target_version:
return

installation_dir = os.path.dirname(installation_path)
Expand Down Expand Up @@ -192,22 +192,16 @@ def get_bicep_latest_release_tag():
response = requests.get("https://aka.ms/BicepLatestRelease", verify=_requests_verify)
response.raise_for_status()
return response.json()["tag_name"]
except IOError as err:
except requests.RequestException as err:
raise ClientRequestError(f"Error while attempting to retrieve the latest Bicep version: {err}.")


def bicep_version_greater_than_or_equal_to(version):
system = platform.system()
installation_path = _get_bicep_installation_path(system)
installed_version = _get_bicep_installed_version(installation_path)
return semver.compare(installed_version, version) >= 0


def supports_bicep_publish():
system = platform.system()
installation_path = _get_bicep_installation_path(system)
installed_version = _get_bicep_installed_version(installation_path)
return semver.compare(installed_version, "0.4.1008") >= 0
parsed_version = semver.VersionInfo.parse(version)
return installed_version >= parsed_version


def _bicep_installed_in_ci():
Expand Down Expand Up @@ -260,18 +254,18 @@ def _load_bicep_version_check_result_from_cache():
return None, True


def _refresh_bicep_version_check_cache(lastest_release_tag):
def _refresh_bicep_version_check_cache(latest_release_tag):
with open(_bicep_version_check_file_path, "w+") as version_check_file:
version_check_data = {
"lastCheckTime": datetime.now().strftime(_bicep_version_check_time_format),
"latestReleaseTag": lastest_release_tag,
"latestReleaseTag": latest_release_tag,
}
json.dump(version_check_data, version_check_file)


def _get_bicep_installed_version(bicep_executable_path):
installed_version_output = _run_command(bicep_executable_path, ["--version"])
return _extract_semver(installed_version_output)
return _extract_version(installed_version_output)


def _get_bicep_download_url(system, release_tag, target_platform=None):
Expand Down Expand Up @@ -302,9 +296,9 @@ def _get_bicep_installation_path(system):
raise ValidationError(f'The platform "{system}" is not supported.')


def _extract_semver(text):
def _extract_version(text):
semver_match = re.search(_semver_pattern, text)
return semver_match.group(0) if semver_match else None
return semver.VersionInfo.parse(semver_match.group(0)) if semver_match else None
Copy link
Member

@jiasli jiasli Jul 26, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

If semver is indeed required, we should consider reviving #26523.



def _run_command(bicep_installation_path, args):
Expand Down
2 changes: 0 additions & 2 deletions src/azure-cli/azure/cli/command_modules/resource/custom.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1448,8 +1448,6 @@ def _validate_resource_inputs(resource_group_name, resource_provider_namespace,
raise CLIError('--namespace is required')


# region Custom Commands

def list_resource_groups(cmd, tag=None): # pylint: disable=no-self-use
""" List resource groups, optionally filtered by a tag.
:param str tag:tag to filter by in 'key[=value]' format
Expand Down
42 changes: 22 additions & 20 deletions src/azure-cli/azure/cli/command_modules/resource/tests/latest/test_resource_bicep.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
import os
import contextlib
import unittest
import semver
from unittest import mock

from knack.util import CLIError
Expand All @@ -19,20 +20,21 @@
from azure.cli.core.azclierror import InvalidTemplateError
from azure.cli.core.mock import DummyCli

cli_ctx = DummyCli()


class TestBicep(unittest.TestCase):
def setUp(self):
self.cli_ctx = DummyCli(random_config_dir=True)
Comment on lines +25 to +26
Copy link
Member

@jiasli jiasli Mar 8, 2024

Choose a reason for hiding this comment

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

This contradicts the convention used in #25689 that random_config_dir=True is set in super().__init__ invocation:

azure-cli/src/azure-cli/azure/cli/command_modules/config/tests/latest/test_config.py

Lines 16 to 17 in b223dd9

def __init__(self, *arg, **kwargs):
super().__init__(*arg, random_config_dir=True, **kwargs)

Copy link
Contributor

@bebound bebound Mar 8, 2024

Choose a reason for hiding this comment

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

Its superclass is unittest.TestCase, which does not has random_config_dir param.


@mock.patch("os.path.isfile")
def test_run_bicep_command_raise_error_if_not_installed_and_not_auto_install(self, isfile_stub):
isfile_stub.return_value = False

with contextlib.suppress(FileNotFoundError):
remove_bicep_installation(cli_ctx)
remove_bicep_installation(self.cli_ctx)

cli_ctx.config.set_value("bicep", "use_binary_from_path", "false")
self.cli_ctx.config.set_value("bicep", "use_binary_from_path", "false")
with self.assertRaisesRegex(CLIError, 'Bicep CLI not found. Install it now by running "az bicep install".'):
run_bicep_command(cli_ctx, ["--version"], auto_install=False)
run_bicep_command(self.cli_ctx, ["--version"], auto_install=False)

@mock.patch("os.chmod")
@mock.patch("os.stat")
Expand All @@ -59,22 +61,22 @@ def test_use_bicep_cli_from_path_false_after_install(self, isfile_stub, dirname_
response.read.return_value = b"test"
urlopen_stub.return_value = response

ensure_bicep_installation(cli_ctx, release_tag="v0.14.85", stdout=False)
ensure_bicep_installation(self.cli_ctx, release_tag="v0.14.85", stdout=False)

self.assertTrue(cli_ctx.config.get("bicep", "use_binary_from_path") == "false")
self.assertTrue(self.cli_ctx.config.get("bicep", "use_binary_from_path") == "false")

@mock.patch("shutil.which")
def test_run_bicep_command_raise_error_if_bicep_cli_not_found_when_use_binary_from_path_is_true(self, which_stub):
which_stub.return_value = None
cli_ctx.config.set_value("bicep", "use_binary_from_path", "true")
self.cli_ctx.config.set_value("bicep", "use_binary_from_path", "true")

with self.assertRaisesRegex(
CLIError,
'Could not find the "bicep" executable on PATH. To install Bicep via Azure CLI, set the "bicep.use_binary_from_path" configuration to False and run "az bicep install".',
):
run_bicep_command(cli_ctx, ["--version"], auto_install=False)
run_bicep_command(self.cli_ctx, ["--version"], auto_install=False)

cli_ctx.config.set_value("bicep", "use_binary_from_path", "false")
self.cli_ctx.config.set_value("bicep", "use_binary_from_path", "false")

@mock.patch.dict(os.environ, {"GITHUB_ACTIONS": "true"}, clear=True)
@mock.patch("azure.cli.command_modules.resource._bicep._logger.debug")
Expand All @@ -83,9 +85,9 @@ def test_run_bicep_command_raise_error_if_bicep_cli_not_found_when_use_binary_fr
def test_run_bicep_command_use_bicep_cli_from_path_in_ci(self, which_stub, run_command_stub, debug_mock):
which_stub.return_value = True
run_command_stub.return_value = "Bicep CLI version 0.13.1 (e3ac80d678)"
cli_ctx.config.set_value("bicep", "use_binary_from_path", "if_found_in_ci")
self.cli_ctx.config.set_value("bicep", "use_binary_from_path", "if_found_in_ci")

run_bicep_command(cli_ctx, ["--version"], auto_install=False)
run_bicep_command(self.cli_ctx, ["--version"], auto_install=False)

debug_mock.assert_called_with(
"Using Bicep CLI from PATH. %s",
Expand All @@ -108,12 +110,12 @@ def test_run_bicep_command_check_version(
warning_mock,
):
isfile_stub.return_value = True
_get_bicep_installed_version_stub.return_value = "1.0.0"
_get_bicep_installed_version_stub.return_value = semver.VersionInfo.parse("1.0.0")
get_bicep_latest_release_tag_stub.return_value = "v2.0.0"

cli_ctx.config.set_value("bicep", "check_version", "True")
cli_ctx.config.set_value("bicep", "use_binary_from_path", "false")
run_bicep_command(cli_ctx, ["--version"])
self.cli_ctx.config.set_value("bicep", "check_version", "True")
self.cli_ctx.config.set_value("bicep", "use_binary_from_path", "false")
run_bicep_command(self.cli_ctx, ["--version"])

warning_mock.assert_called_once_with(
'A new Bicep release is available: %s. Upgrade now by running "az bicep upgrade".',
Expand All @@ -139,10 +141,10 @@ def test_run_bicep_command_check_version_cache_read_write(
self._remove_bicep_version_check_file()

isfile_stub.return_value = True
_get_bicep_installed_version_stub.return_value = "1.0.0"
_get_bicep_installed_version_stub.return_value = semver.VersionInfo.parse("1.0.0")
get_bicep_latest_release_tag_stub.return_value = "v2.0.0"

run_bicep_command(cli_ctx, ["--version"])
run_bicep_command(self.cli_ctx, ["--version"])

self.assertTrue(os.path.isfile(_bicep_version_check_file_path))
finally:
Expand All @@ -154,10 +156,10 @@ def test_run_bicep_command_check_version_cache_read_write(
def test_ensure_bicep_installation_skip_download_if_installed_version_matches_release_tag(
self, dirname_mock, _get_bicep_installed_version_stub, isfile_stub
):
_get_bicep_installed_version_stub.return_value = "0.1.0"
_get_bicep_installed_version_stub.return_value = semver.VersionInfo.parse("0.1.0")
isfile_stub.return_value = True

ensure_bicep_installation(cli_ctx, release_tag="v0.1.0")
ensure_bicep_installation(self.cli_ctx, release_tag="v0.1.0")

dirname_mock.assert_not_called()

Expand Down