-
Notifications
You must be signed in to change notification settings - Fork 3.1k
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
{testsdk} Move azure-devtools
's code to azure-cli-testsdk
#20601
Conversation
@@ -94,7 +94,7 @@ interactions: | |||
api_version_constraint)\r\nfrom knack.util import CLIError\r\nfrom azure.cli.core.profiles | |||
import ResourceType\r\n\r\nfrom azure.cli.command_modules.storage._client_factory | |||
import MISSING_CREDENTIALS_ERROR_MESSAGE\r\nfrom ..storage_test_util import | |||
StorageScenarioMixin\r\nfrom azure_devtools.scenario_tests import AllowLargeResponse\r\n\r\n\r\n@api_version_constraint(ResourceType.MGMT_STORAGE, | |||
StorageScenarioMixin\r\nfrom azure.cli.testsdk.scenario_tests import AllowLargeResponse\r\n\r\n\r\n@api_version_constraint(ResourceType.MGMT_STORAGE, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It don't think it is a good idea to upload the source code py
to Storage for testing. @evelyn-ys
# https://docs.python.org/3/library/tempfile.html#tempfile.NamedTemporaryFile | ||
# delete=False must be set for Windows, because Windows doesn't allow opening a temporary file | ||
# that is already opened. | ||
with tempfile.NamedTemporaryFile(delete=False) as f: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changed the code from https://github.com/Azure/azure-python-devtools/blob/1.2.0/src/azure_devtools/scenario_tests/tests/test_utilities.py#L46-L48 to make the test pass on Windows.
See https://docs.python.org/3/library/tempfile.html#tempfile.NamedTemporaryFile
result = MockTest().run() | ||
self.assertTrue(result.skipped) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changed the code from https://github.com/Azure/azure-python-devtools/blob/1.2.0/src/azure_devtools/scenario_tests/tests/test_integration_test_base.py#L67 to make the test pass on latest Python.
See https://docs.python.org/3/library/unittest.html#unittest.TestCase.run
This module is vendored from | ||
https://github.com/Azure/azure-python-devtools/tree/1.2.0/src/azure_devtools/scenario_tests | ||
|
||
More info: https://github.com/Azure/azure-cli/pull/20601 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added comments for where this module comes from.
azure-devtools
's code to azure-cli-testsdk
azure-devtools
's code to azure-cli-testsdk
Background from Azure SDK team
From @lmazuel:
azure-devtools
used to be a repo shared across SDK and CLI to share the test infra. We forked our own version a few years ago. We did try to re-share it (see Azure/azure-python-devtools#60 to backport SDK changes into that repo). This PR never got merged, and SDK continued to use different version ofazure_devtools
, time making them more and more different.We made a more important refactoring, since we don’t use
vcrpy
anymore to record tests. I think we reached a level where we can now more officially say that we won’t ever merge back again those two projects. Which means to me thatazure-devtools
is now officially used only by CLI team. SDK team didn’t use this repo for that last 3 years and don’t plan to use it.Q&A
Q: Is
azure_devtools
is still used by Python SDKs or other tools?A: Not used by SDK for 3 years.
Q: Where is the release pipeline for
azure_devtools
?A: It’s used to be plugged with TravisCI.
Q: Is it a good practice to vendor/incorporate
azure_devtools
directly intoazure-cli-testsdk
?A: I think you should vendor that code indeed. I would suggest to archive that repo and migrate the code into the CLI tests code.
Reference
Email: Destiny of azure_devtools
Required changes for extensions
If you import from
azure_devtools
, change it toazure.cli.testsdk
, such as