-
Notifications
You must be signed in to change notification settings - Fork 3.2k
e2e tests for DT #44634
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
base: main
Are you sure you want to change the base?
e2e tests for DT #44634
Conversation
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.
Pull request overview
This pull request adds comprehensive end-to-end (E2E) tests for Deployment Templates functionality in Azure ML SDK. The PR introduces test infrastructure, YAML configuration files, and test cases covering basic to advanced deployment template scenarios.
Changes:
- Added three YAML configuration files for different test scenarios (minimal, basic, complete)
- Implemented main E2E test file with core operations (CRUD, archive/restore, error handling)
- Implemented advanced E2E test file with complex scenarios (probes, request settings, version management, concurrent operations)
- Added conftest.py with shared fixtures for test setup
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 10 comments.
Show a summary per file
| File | Description |
|---|---|
| sdk/ml/azure-ai-ml/tests/test_configs/deployment_template/minimal_deployment_template.yaml | Minimal deployment template configuration with required fields only |
| sdk/ml/azure-ai-ml/tests/test_configs/deployment_template/complete_deployment_template.yaml | Complete deployment template with all available features and settings |
| sdk/ml/azure-ai-ml/tests/test_configs/deployment_template/basic_deployment_template.yaml | Basic deployment template with common configuration options |
| sdk/ml/azure-ai-ml/tests/deployment_template/e2etests/test_deployment_template_advanced.py | Advanced E2E tests covering complex scenarios like probes, version management, and concurrent operations |
| sdk/ml/azure-ai-ml/tests/deployment_template/e2etests/test_deployment_template.py | Core E2E tests for CRUD operations, listing, archiving, and error handling |
| sdk/ml/azure-ai-ml/tests/deployment_template/e2etests/conftest.py | Shared pytest fixtures for deployment template tests |
| sdk/ml/azure-ai-ml/tests/deployment_template/e2etests/init.py | Package initialization file with copyright header |
| except Exception: | ||
| pass # Template doesn't exist, continue |
Copilot
AI
Jan 13, 2026
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.
The 'cleanup_template' function is duplicated in both test_deployment_template.py and test_deployment_template_advanced.py. This should be moved to a shared utility module (e.g., conftest.py or a test utilities module) to avoid code duplication.
| except Exception: | |
| pass # Template doesn't exist, continue | |
| except Exception as ex: # pylint: disable=broad-exception-caught | |
| # Template doesn't exist or could not be deleted; ignore but log for visibility | |
| logger.info( | |
| "Cleanup skipped or failed for template '%s' version '%s': %s", | |
| name, | |
| version, | |
| ex, | |
| ) |
| ) -> None: | ||
| """Test creating a deployment template with complex environment variables. | ||
|
|
||
| Args:`n registry_client: ML registry client fixture |
Copilot
AI
Jan 13, 2026
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.
The docstring contains a malformed "Args:" line with backtick-n sequence. This should be properly formatted as "Args:" on its own line.
| ) -> None: | ||
| """Test managing multiple versions of the same deployment template. | ||
|
|
||
| Args:`n registry_client: ML registry client fixture |
Copilot
AI
Jan 13, 2026
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.
The docstring contains a malformed "Args:" line with backtick-n sequence. This should be properly formatted as "Args:" on its own line.
| Args:`n registry_client: ML registry client fixture | |
| Args: | |
| registry_client: ML registry client fixture |
| ) -> None: | ||
| """Test deployment template with various tag combinations. | ||
|
|
||
| Args:`n registry_client: ML registry client fixture |
Copilot
AI
Jan 13, 2026
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.
The docstring contains a malformed "Args:" line with backtick-n sequence. This should be properly formatted as "Args:" on its own line.
| Args:`n registry_client: ML registry client fixture | |
| Args: | |
| registry_client: ML registry client fixture |
| ) -> None: | ||
| """Test creating a deployment template with custom request settings. | ||
|
|
||
| Args:`n registry_client: ML registry client fixture |
Copilot
AI
Jan 13, 2026
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.
The docstring contains a malformed "Args:" line with backtick-n sequence. This should be properly formatted as "Args:" on its own line.
| Args:`n registry_client: ML registry client fixture | |
| Args: | |
| registry_client: ML registry client fixture |
| ) -> None: | ||
| """Test updating a minimal template to a full-featured template. | ||
|
|
||
| Args:`n registry_client: ML registry client fixture |
Copilot
AI
Jan 13, 2026
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.
The docstring contains a malformed "Args:" line with backtick-n sequence. This should be properly formatted as "Args:" on its own line.
| ) -> None: | ||
| """Test creating and managing concurrent versions of deployment templates. | ||
|
|
||
| Args:`n registry_client: ML registry client fixture |
Copilot
AI
Jan 13, 2026
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.
The docstring contains a malformed "Args:" line with backtick-n sequence. This should be properly formatted as "Args:" on its own line.
| Args:`n registry_client: ML registry client fixture | |
| Args: | |
| registry_client: ML registry client fixture |
| from azure.ai.ml.entities import Environment | ||
|
|
||
|
|
||
| @pytest.fixture(scope="session") | ||
| def test_environment(registry_client: MLClient) -> str: |
Copilot
AI
Jan 13, 2026
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.
The import 'Environment' is unused and should be removed. It appears to be left over from development but is not used anywhere in this file.
| from azure.ai.ml.entities import Environment | |
| @pytest.fixture(scope="session") | |
| def test_environment(registry_client: MLClient) -> str: | |
| @pytest.fixture(scope="session") | |
| def test_environment(registry_client: MLClient) -> str: | |
| def test_environment(registry_client: MLClient) -> str: |
|
|
||
|
|
||
| @pytest.fixture(scope="session") | ||
| def test_environment(registry_client: MLClient) -> str: |
Copilot
AI
Jan 13, 2026
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.
The fixture accepts a 'registry_client' parameter but doesn't use it. Since the function just returns a hardcoded string, this parameter should be removed.
| def test_environment(registry_client: MLClient) -> str: | |
| def test_environment() -> str: |
|
|
||
|
|
Copilot
AI
Jan 13, 2026
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.
There are three consecutive blank lines here. According to PEP 8, there should be at most two blank lines between top-level definitions.
Description
Please add an informative description that covers that changes made by the pull request and link all relevant issues.
If an SDK is being regenerated based on a new API spec, a link to the pull request containing these API spec changes should be included above.
All SDK Contribution checklist:
General Guidelines and Best Practices
Testing Guidelines