-
Notifications
You must be signed in to change notification settings - Fork 4.8k
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
Media services 2019 05 01 preview #8929
Media services 2019 05 01 preview #8929
Conversation
Specs commit Azure/azure-rest-api-specs#6147. Skip tests that use new version.
@erich-wang , can we please get a review of these changes? Thanks |
/azp run net - mgmt - ci |
Azure Pipelines could not run because the pipeline triggers exclude this branch/path. |
/azp run net - mgmt - ci |
Azure Pipelines successfully started running 1 pipeline(s). |
@@ -12,7 +12,7 @@ namespace Media.Tests.ScenarioTests | |||
{ | |||
public class LiveEventTests : MediaScenarioTestBase | |||
{ | |||
[Fact] | |||
[Fact(Skip = "Need to rerun for 2019-05-01-preview")] |
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.
Hi Brian, as public preview is still shipped to all the customers, we need to ensure its quality aka proper test coverage. Wouldn't want to skip them
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.
@isra-fel, the tests that are marked skip don't test the new feature. We want to skip them to save the feature team from having to rerecord the test with APIs that use the version 2019-05-01-preview and then rerecord them again when we move the feature into the stable version - 2018-07-01, regardless if they choose to update the test with new API calls.
The new feature that we're previewing in 2019-05-01-preview has been thoroughly tested with both the .NET SDK and the Java SDK.
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.
@isra-fel, I have removed the skip attribute and rerecorded the tests. Please review.
Thanks
@isra-fel , are you going to merge this PR with the skipped tests? |
Hi Brian, I understand that you want to save the effort of devs, it's just we don't want to release anything not tested, especially when its for public. Besides tests, there's supposed to be a metadata file under |
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.
@isra-fel , I have updated the tests per your request.
@giakas, @sanbhatt This is the public preview of the Live Transcription feature.