Skip to content

{Test} Fix resource group deletion for cross tenant tests#19166

Merged
zhoxing-ms merged 2 commits intoAzure:devfrom
zhoxing-ms:fix_resource_deletion_for_cross_tenant_tests
Aug 9, 2021
Merged

{Test} Fix resource group deletion for cross tenant tests#19166
zhoxing-ms merged 2 commits intoAzure:devfrom
zhoxing-ms:fix_resource_deletion_for_cross_tenant_tests

Conversation

@zhoxing-ms
Copy link
Copy Markdown
Contributor

Description

Related PR: #19159
Use @ResourceGroupPreparer(... ,subscription=cross_tenant_subscrption) to ensure that the cross account resource group will be deleted after the test is completed.

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.

'rg': another_resource_group,
'aux_tenant': '72f988bf-86f1-41af-91ab-2d7cd011db47'
})
self.cmd('group create -g {rg} --location {location} --subscription {aux_sub}',
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

delete previous resource group create command?

@yonzhan
Copy link
Copy Markdown
Collaborator

yonzhan commented Aug 9, 2021

fix cross tenant tests

'ext_rg': another_resource_group,
})

self.cmd('group create -l eastus -n {ext_rg} --subscription {ext_sub}')
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Unnecessary to create rg again as you create it in ResourceGroupPreparer

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

OK~

'rg': another_resource_group,
'aux_tenant': '72f988bf-86f1-41af-91ab-2d7cd011db47'
})
self.cmd('group create -g {rg} --location {location} --subscription {aux_sub}',
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Same issue

@jsntcy
Copy link
Copy Markdown
Member

jsntcy commented Aug 9, 2021

    self.cmd('group create -g {another_rg} --location {location} --subscription {aux_sub}',

same issue


Refers to: src/azure-cli/azure/cli/command_modules/vm/tests/latest/test_vm_commands.py:4180 in 7a0237b. [](commit_id = 7a0237b, deletion_comment = False)

Copy link
Copy Markdown
Member

@jsntcy jsntcy left a comment

Choose a reason for hiding this comment

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

:shipit:

Copy link
Copy Markdown
Contributor

@Juliehzl Juliehzl left a comment

Choose a reason for hiding this comment

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

LGTM. a small suggestion is that we can make aux sub as global variable and used by all tests.

@zhoxing-ms zhoxing-ms merged commit add8066 into Azure:dev Aug 9, 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.

5 participants