-
Notifications
You must be signed in to change notification settings - Fork 3.2k
fix live test issues #44594
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?
fix live test issues #44594
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 aims to fix live test issues in the cognitive language SDK by modifying pipeline environment variables and test execution behavior.
Key Changes:
- Separates shared environment variables into service-specific variables (conversation, authoring, text analysis) in the pipeline configuration
- Marks orchestration prediction tests (sync and async) to run only in playback mode
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 9 comments.
| File | Description |
|---|---|
| sdk/cognitivelanguage/tests.yml | Changes environment variables from shared $(clu-uri) and $(clu-key) to service-specific variables for conversations, authoring, and text analysis endpoints |
| sdk/cognitivelanguage/azure-ai-language-conversations/tests/test_orchestration_prediction_async.py | Adds @pytest.mark.playback_test_only decorator to skip tests in live mode |
| sdk/cognitivelanguage/azure-ai-language-conversations/tests/test_orchestration_prediction.py | Adds @pytest.mark.playback_test_only decorator to skip tests in live mode |
| TEXT_ANALYSIS_KEY: $(clu-key) | ||
| CONVERSATIONS_ENDPOINT: $(conversation-uri) | ||
| CONVERSATIONS_KEY: $(conversation-key) | ||
| AUTHORING_ENDPOINT: $(authoring-uri) |
Copilot
AI
Jan 8, 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.
Verify that the Azure DevOps pipeline variable $(authoring-uri) exists and is properly configured. The variable name has changed from $(clu-uri) to $(authoring-uri), which is a different naming pattern. If this variable is not defined in the pipeline configuration, tests will fail with missing credentials.
| AUTHORING_ENDPOINT: $(authoring-uri) | |
| AUTHORING_ENDPOINT: $(clu-uri) |
| CONVERSATIONS_ENDPOINT: $(conversation-uri) | ||
| CONVERSATIONS_KEY: $(conversation-key) | ||
| AUTHORING_ENDPOINT: $(authoring-uri) | ||
| AUTHORING_KEY: $(authoring-key) |
Copilot
AI
Jan 8, 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.
Verify that the Azure DevOps pipeline variable $(authoring-key) exists and is properly configured. The variable name has changed from $(clu-key) to $(authoring-key), which is a different naming pattern. If this variable is not defined in the pipeline configuration, tests will fail with missing credentials.
| AUTHORING_KEY: $(authoring-key) | |
| AUTHORING_KEY: $(clu-key) |
| AUTHORING_ENDPOINT: $(authoring-uri) | ||
| AUTHORING_KEY: $(authoring-key) | ||
| TEXT_ANALYSIS_ENDPOINT: $(text_analysis-uri) | ||
| TEXT_ANALYSIS_KEY: $(text_analysis-key) |
Copilot
AI
Jan 8, 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.
Verify that the Azure DevOps pipeline variable $(text_analysis-key) exists and is properly configured. The variable name has changed from $(clu-key) to $(text_analysis-key), which is a different naming pattern (note the underscore in the middle). If this variable is not defined in the pipeline configuration, tests will fail with missing credentials.
| ... | ||
|
|
||
|
|
||
| @pytest.mark.playback_test_only |
Copilot
AI
Jan 8, 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 decorator @pytest.mark.playback_test_only causes these tests to skip when running in live mode. Given that the PR title is "fix live test issues", adding this decorator seems contradictory - it prevents the tests from running live rather than fixing them.
If these tests genuinely cannot run in live mode and should only run in playback mode, this change makes sense, but the PR description should explain why. Otherwise, this may be masking the underlying issue rather than fixing it.
| @pytest.mark.playback_test_only |
| AUTHORING_KEY: $(clu-key) | ||
| TEXT_ANALYSIS_ENDPOINT: $(clu-uri) | ||
| TEXT_ANALYSIS_KEY: $(clu-key) | ||
| CONVERSATIONS_ENDPOINT: $(conversation-uri) |
Copilot
AI
Jan 8, 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.
Verify that the Azure DevOps pipeline variable $(conversation-uri) exists and is properly configured. The variable name has changed from $(clu-uri) to $(conversation-uri), which is a different naming pattern. If this variable is not defined in the pipeline configuration, tests will fail with missing credentials.
| CONVERSATIONS_KEY: $(conversation-key) | ||
| AUTHORING_ENDPOINT: $(authoring-uri) | ||
| AUTHORING_KEY: $(authoring-key) | ||
| TEXT_ANALYSIS_ENDPOINT: $(text_analysis-uri) |
Copilot
AI
Jan 8, 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.
Verify that the Azure DevOps pipeline variable $(text_analysis-uri) exists and is properly configured. The variable name has changed from $(clu-uri) to $(text_analysis-uri), which is a different naming pattern (note the underscore in the middle). If this variable is not defined in the pipeline configuration, tests will fail with missing credentials.
| TEXT_ANALYSIS_ENDPOINT: $(text_analysis-uri) | ||
| TEXT_ANALYSIS_KEY: $(text_analysis-key) |
Copilot
AI
Jan 8, 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's an inconsistency in the naming convention for the Azure DevOps pipeline variables. Lines 21-24 use hyphens without underscores (e.g., conversation-uri, authoring-uri), while lines 25-26 use underscores (e.g., text_analysis-uri, text_analysis-key). This inconsistency could lead to confusion. Consider standardizing the naming convention to either use all hyphens or all underscores consistently.
| TEXT_ANALYSIS_ENDPOINT: $(text_analysis-uri) | |
| TEXT_ANALYSIS_KEY: $(text_analysis-key) | |
| TEXT_ANALYSIS_ENDPOINT: $(text-analysis-uri) | |
| TEXT_ANALYSIS_KEY: $(text-analysis-key) |
| return ConversationAnalysisClient(endpoint, credential) | ||
|
|
||
|
|
||
| @pytest.mark.playback_test_only |
Copilot
AI
Jan 8, 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 decorator @pytest.mark.playback_test_only causes these tests to skip when running in live mode. Given that the PR title is "fix live test issues", adding this decorator seems contradictory - it prevents the tests from running live rather than fixing them.
If these tests genuinely cannot run in live mode and should only run in playback mode, this change makes sense, but the PR description should explain why. Otherwise, this may be masking the underlying issue rather than fixing it.
| @pytest.mark.playback_test_only |
| TEXT_ANALYSIS_ENDPOINT: $(clu-uri) | ||
| TEXT_ANALYSIS_KEY: $(clu-key) | ||
| CONVERSATIONS_ENDPOINT: $(conversation-uri) | ||
| CONVERSATIONS_KEY: $(conversation-key) |
Copilot
AI
Jan 8, 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.
Verify that the Azure DevOps pipeline variable $(conversation-key) exists and is properly configured. The variable name has changed from $(clu-key) to $(conversation-key), which is a different naming pattern. If this variable is not defined in the pipeline configuration, tests will fail with missing credentials.
| CONVERSATIONS_KEY: $(conversation-key) | |
| CONVERSATIONS_KEY: $(clu-key) |
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