Skip to content

Conversation

@zhoxing-ms
Copy link
Contributor

@zhoxing-ms zhoxing-ms commented Oct 11, 2021

Description

Because in the previous PR #5328, the reference of urlopen in the method load_images_from_aliases_doc() was modified to use requests, but the mock path in test_read_images_from_alias_doc() was not synchronously modified.
Screenshot 2021-09-14 172536
code link

So we need to change the mock path azure.cli.command_modules.vm.custom.urlopen to azure.cli.command_modules.vm.custom.requests

Testing Guide

History Notes

[Component Name 1] BREAKING CHANGE: az command a: Make some customer-facing breaking change.
[Component Name 2] az command b: Add some customer-facing feature.


This checklist is used to make sure that common guidelines for a pull request are followed.

@zhoxing-ms zhoxing-ms requested a review from jsntcy as a code owner October 11, 2021 09:37
@zhoxing-ms zhoxing-ms requested a review from jiasli October 11, 2021 09:37
Copy link
Member

Choose a reason for hiding this comment

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

Please remember to rename the parameter mock_urlopen accordingly.

@jiasli
Copy link
Member

jiasli commented Oct 11, 2021

I haven't looked into it in detail, but response will be a MagicMock. Will the mock work? Or why did it work before with the wrong mock?

@zhoxing-ms
Copy link
Contributor Author

zhoxing-ms commented Dec 8, 2021

I haven't looked into it in detail, but response will be a MagicMock. Will the mock work? Or why did it work before with the wrong mock?

Good question!

Previously, the purpose of this mock was to mock the querying results of https://raw.githubusercontent.com/Azure/azure-rest-api-specs/master/arm-compute/quickstart-templates/aliases.json when the method all_images = load_images_from_aliases_doc(cmd.cli_ctx, publisher_name, offer, sku) was called.
I speculate that this is to prevent the test from blocking when the remote file cannot be accessed, so the local cache file aliases.json is used to mock the access results of the remote file.

However, since there was a new PR #10852 to add the logic of reading local json dict when remote files cannot be accessed to the method load_images_from_aliases_doc()

    except requests.exceptions.ConnectionError:
        logger.warning("Failed to retrieve image alias doc '%s'. Error: 'ConnectionError'. Use local copy instead.",
                       target_url)
        dic = json.loads(alias_json)

code link

Since the logic of querying alias_json for fault tolerant in method load_images_from_aliases_doc() has replaced the function of mock logic, so even if the mock logic is wrong, they will not affect the process of load_images_from_aliases_doc. Therefore, these mock logics can be deleted directly.

@zhoxing-ms zhoxing-ms force-pushed the fix_mock_path_for_read_image_test branch from 0da28f9 to 3b80270 Compare December 8, 2021 03:35
@zhoxing-ms zhoxing-ms changed the title {Compute} Fix mock path for test_read_images_from_alias_doc {Compute} Fix mock logic for test_read_images_from_alias_doc Dec 8, 2021
@zhoxing-ms zhoxing-ms merged commit f1c6f85 into Azure:dev Dec 8, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants