Skip to content
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

[Network] Bump version from '2020-08-01' to '2020-11-01' #17290

Merged
merged 8 commits into from
Mar 17, 2021

Conversation

msyyc
Copy link
Member

@msyyc msyyc commented Mar 12, 2021

Description

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.

@msyyc
Copy link
Member Author

msyyc commented Mar 12, 2021

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 2 pipeline(s).

Copy link
Member

@qwordy qwordy left a comment

Choose a reason for hiding this comment

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

You upgrated network to 18.0.0. However, in yaml file

- AZURECLI/2.19.1 azsdk-python-azure-mgmt-network/17.1.0

Fake file. Request change.

@yonzhan
Copy link
Collaborator

yonzhan commented Mar 12, 2021

Network

@yonzhan yonzhan added this to the S184 milestone Mar 12, 2021
@msyyc
Copy link
Member Author

msyyc commented Mar 12, 2021

You upgrated network to 18.0.0. However, in yaml file

- AZURECLI/2.19.1 azsdk-python-azure-mgmt-network/17.1.0

Fake file. Request change.

done

@qwordy
Copy link
Member

qwordy commented Mar 12, 2021

@msyyc Replacing keywords in yaml file is dangerous. I am not aware of whether network upgrade breaks vm's tests.

@msyyc
Copy link
Member Author

msyyc commented Mar 12, 2021

@msyyc Yuchao Yan FTE Replacing keywords in yaml file is dangerous. I am not aware of whether network upgrade breaks vm's tests.

I have run the live test for compute. The result is same with weekly live report.

@jsntcy
Copy link
Member

jsntcy commented Mar 12, 2021

@msyyc, only tests that cannot pass live run should use replace method.

@msyyc
Copy link
Member Author

msyyc commented Mar 15, 2021

@msyyc Yuchao Yan FTE, only tests that cannot pass live run should use replace method.

got it

@@ -461,6 +461,7 @@ def test_create_topic(self, resource_group):

self.cmd('az eventgrid topic delete --name {topic_name} --resource-group {rg}')

@unittest.skip('live test always fails, need fix by owners')
Copy link
Member

Choose a reason for hiding this comment

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

Does this test fail in weekly live run?
If yes, we'd better replace "api version" in recording file and create an issue to owner to fix this test.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, it failed in weekly live run. It will fail even with recording mode. So we have to skip it.

@@ -851,7 +851,7 @@ def setUp(self):
self.cmd('extension add -n eventgrid')

def tearDown(self):
self.cmd('extension remove -n eventgrid')
# self.cmd('extension remove -n eventgrid')
Copy link
Member

Choose a reason for hiding this comment

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

Any specific reason for this change which caused #17434?

Copy link
Contributor

Choose a reason for hiding this comment

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

We should not suppose to remove extension after test done to avoid other test when parallel run. For example both NetworkVirtualApplianceScenarioTest and NetworkSecurityPartnerProviderScenarioTest rely on virtual-wan extension. If the first one remove extension when tearDown, it will cause error for unfinished one. We encounter this issue before.

Copy link
Member

Choose a reason for hiding this comment

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

You need @serial_test()

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.

8 participants