[Communication] - SMS - Add Tests and Clean Up #19149

minnieliu · 2021-03-03T00:11:31Z

All SDK Contribution checklist:

This checklist is used to make sure that common guidelines for a pull request are followed.

Please open PR in Draft mode if it is:
- Work in progress or not intended to be merged.
- Encountering multiple pipeline failures and working on fixes.
If an SDK is being regenerated based on a new swagger spec, a link to the pull request containing these swagger spec changes has been included above.
I have read the contribution guidelines.
The pull request does not introduce breaking changes.

General Guidelines and Best Practices

Title of the pull request is clear and informative.
There are a small number of commits, each of which have an informative message. This means that previously merged commits do not appear in the history of the PR. For more information on cleaning up the commits in your PR, see this page.

Testing Guidelines

Pull request includes test coverage for the included changes.

SDK Generation Guidelines

The generate.cmd file for the SDK has been updated with the version of AutoRest, as well as the commitid of your swagger spec or link to the swagger spec, used to generate the code. (Track 2 only)
The *.csproj and AssemblyInfo.cs files have been updated with the new version of the SDK. Please double check nuget.org current release version.

Additional management plane SDK specific contribution checklist:

Note: Only applies to Microsoft.Azure.Management.[RP] or Azure.ResourceManager.[RP]

Include updated management metadata.
Update AzureRP.props to add/remove version info to maintain up to date API versions.

Management plane SDK Troubleshooting

If this is very first SDK for a services and you are adding new service folders directly under /SDK, please add new service label and/or contact assigned reviewer.
If the check fails at the Verify Code Generation step, please ensure:
- Do not modify any code in generated folders.
- Do not selectively include/remove generated files in the PR.
- Do use generate.ps1/cmd to generate this PR instead of calling autorest directly.
  Please pay attention to the @microsoft.csharp version output after running generate.ps1. If it is lower than current released version (2.3.82), please run it again as it should pull down the latest version,

Old outstanding PR cleanup

Please note:
If PRs (including draft) has been out for more than 60 days and there are no responses from our query or followups, they will be closed to maintain a concise list for our reviewers.

minnieliu · 2021-03-03T00:15:54Z

/azp run net - communication - ci

azure-pipelines · 2021-03-03T00:16:06Z

Azure Pipelines successfully started running 1 pipeline(s).

minnieliu · 2021-03-03T00:45:31Z

/azp run net - communication - tests

azure-pipelines · 2021-03-03T00:45:40Z

Azure Pipelines successfully started running 1 pipeline(s).

beltr0n

minor comments, plus i assume you're going to update the samples

beltr0n · 2021-03-03T00:30:22Z

sdk/communication/Azure.Communication.Sms/tests/SmsClientLiveTests.cs

+                Assert.True(ex.Message.Contains("400"));
+                Console.WriteLine(ex.Message);
+            }
+


beltr0n · 2021-03-03T00:41:53Z

sdk/communication/Azure.Communication.Sms/tests/SmsClientLiveTests.cs

@@ -30,15 +30,13 @@ public async Task SendingSmsMessage()
            #region Snippet:Azure_Communication_Sms_Tests_Troubleshooting
            try
            {
-                #region Snippet:Azure_Communication_Sms_Tests_SendAsync
                SmsSendResult result = await client.SendAsync(
                   //@@ from: "+18001230000" // Phone number acquired on your Azure Communication resource
                   //@@ to: "+18005670000",


can we remove the //@@ if it's no longer in a region?

This is still within the troubleshooting snippet (line 30)

beltr0n · 2021-03-03T00:49:19Z

sdk/communication/Azure.Communication.Sms/README.md

-   message: "Hi");
-Console.WriteLine($"Sms id: {result.MessageId}");
+SmsSendResult sendResult = await smsClient.SendAsync(
+    from: "<from-phone-number>",


I think a comment like this would be helpful, let's them know the format
// Your E.164 formatted phone number used to send SMS

Great Idea, I'll add that

beltr0n

left a minor comment

lsundaralingam · 2021-03-03T01:17:55Z

sdk/communication/Azure.Communication.Sms/tests/SmsClientLiveTests.cs

+            {
+                Console.WriteLine(ex.Message);
+            }
+


Nit: newline

lsundaralingam · 2021-03-03T01:17:58Z

sdk/communication/Azure.Communication.Sms/tests/SmsClientLiveTests.cs


+                Assert.AreNotEqual(firstMessageResult.MessageId, secondMessageResult.MessageId);
+                Assert.True(firstMessageResult.Successful);


Did you consider having a helper method to verify successful messages? You can also use it for the other happy case tests.

Great suggestion! Thanks

Merge Cognitive Services - Language pillar version 2022-05-15-preview to main (Azure#19186) * Updated May-1 preview to include changes from March-1 preview (Azure#18580) * Updated May-1 preview to include changes from March-1 preview * Fixed a missed api version update in an example file Co-authored-by: Abigail Hartman <Abigail.Hartman@microsoft.com> * [Language API 2022-05-01-Preview]Conversation API Async - Conversational Summarization and Conversational PII (Azure#17847) * Adds base for updating Language from version preview/2022-02-01-preview to version 2022-04-01-preview * Updates readme * Updates API version in new specs and examples * [Language API 2022-04-01-Preview] Feature conversation PII redaction and issue resolution summarization * Add Issue Resolution Summary * Address comments * address pretty chek and spell check issues * address file name mismatch issue * typo * Add IRD updates * Add IRD Schema changes * Response Files content update * Address feedback * Address PR comments * Address comments * Add the May API version * Address comments * Remove 04-01 and address comments * Update specification/cognitiveservices/data-plane/Language/preview/2022-05-01-preview/common.json Co-authored-by: Heath Stewart <heaths@microsoft.com> * Update specification/cognitiveservices/data-plane/Language/preview/2022-05-01-preview/common.json Co-authored-by: Heath Stewart <heaths@microsoft.com> * Update specification/cognitiveservices/data-plane/Language/preview/2022-05-01-preview/analyzeconversations.json Co-authored-by: Heath Stewart <heaths@microsoft.com> * Update specification/cognitiveservices/data-plane/Language/preview/2022-05-01-preview/analyzeconversations.json Co-authored-by: Heath Stewart <heaths@microsoft.com> * Update specification/cognitiveservices/data-plane/Language/preview/2022-05-01-preview/textanalytics.json Co-authored-by: Heath Stewart <heaths@microsoft.com> * Address feedback V-1. * Address all comments expect model + enum descriptions * Fx validations * Missed files * Conversation Item modality issue * Update PII Task * Address Spec validation errors * Updates * Fix PII issues * Add readonly * Update schema * Updates * Bring back Text Conversation Item * Update example * Space in Json * Address comments * Address one more comments * Fix model validation * Fix * Update spec * Address comments * Move requestStatistics, prebuiltResult, error and warnings object to common * Address circular dependency Co-authored-by: Abigail Hartman <Abigail.Hahn@microsoft.com> Co-authored-by: Heath Stewart <heaths@microsoft.com> * Rename version from 2022-05-01-preview to 2022-05-15-preview (Azure#18737) * Update CLU swagger (Azure#18739) Co-authored-by: Chong Tang <chot@microsoft.com> * Use azure-sdk-for-net-track2 for validation (Azure#18755) * Bidishac/update field description (#18787) * Update the messaging on the parameter type * Update the documentation of the redactionSource field * Update specification/cognitiveservices/data-plane/Language/preview/2022-05-15-preview/analyzeconversations.json Co-authored-by: Heath Stewart <heaths@microsoft.com> Co-authored-by: Chong Tang <ct4ew@virginia.edu> Co-authored-by: Heath Stewart <heaths@microsoft.com> * Unify the runtime and authoring APIs (Azure#18837) * unifying single classification and multi classification to return array of class (Azure#18902) * Renaming in 2022-05-15-preview (Azure#18929) Co-authored-by: Chong Tang <chot@microsoft.com> * Rename 'summary' in summaryAspects to 'issue' (Azure#18941) * Rename 'summary' in summaryAspects to 'issue' * Merge issues * Update model version value * Merge 2022-05-01-GA authoring updates and "LUIS" string case update into 2022-05-15-preview branch (Azure#18943) * Merge 2022-05-01-GA files changes into 2022-05-15-preview branch * Address Avocado issues and revert case update for Luis * Add the missing error codes * Fix avocado * Avocado fix * Add api-version parameter to cancel job endpoint (Azure#18986) * Add api-version parameter to cancel job endpoint * Fix taskState and jobState * Revert "Fix taskState and jobState" This reverts commit 148dd2183249add89a090721db36a14c2b15d9bd. * Update LUIS to Luis in model names (Azure#19012) * Copy QuestionAnswering definitions to common.json and refer them in analyzeconversations.json (Azure#19031) Co-authored-by: Chong Tang <chot@microsoft.com> * Added fhir feature updates to 2022-05-15-preview (Azure#19092) Co-authored-by: Abigail Hartman <Abigail.Hartman@microsoft.com> * Add cancel endpoint for conversations (Azure#19025) * Add cancel endpoint for conversations * Update examples path * Make the input for transcriptConversationItem optional * Bidishac/update enum name (Azure#19149) * Update name to CreditCard * Update entity categories for PII for build * Keep SSN * Revert "Keep SSN" This reverts commit 77a92119ebd71cf5d5c7f201d402514756289ec2. * Update readme.md Co-authored-by: Abigail Hartman <Abigail.Hahn@microsoft.com> Co-authored-by: Abigail Hartman <Abigail.Hartman@microsoft.com> Co-authored-by: Bidisha Chakraborty <29803402+bidisha-c@users.noreply.github.com> Co-authored-by: Chong Tang <ct4ew@virginia.edu> Co-authored-by: Chong Tang <chot@microsoft.com> Co-authored-by: Mohamed Abbas <moaba@microsoft.com> Co-authored-by: Nourhan <92883092+nourhaanfarid@users.noreply.github.com>

Clean up SMS SDK

2540716

ghost added the Communication label Mar 3, 2021

minnieliu requested review from beltr0n, RezaJooyandeh, lsundaralingam and ankitarorabit March 3, 2021 00:13

Minnie Liu added 2 commits March 2, 2021 16:32

Fix build

864cb0f

Remove fake numbers from the samples

eb2cda7

beltr0n reviewed Mar 3, 2021

View reviewed changes

Address review comments

8edef13

lsundaralingam reviewed Mar 3, 2021

View reviewed changes

lsundaralingam approved these changes Mar 3, 2021

View reviewed changes

Address comments

bd63856

minnieliu merged commit 03e11bc into Azure:master Mar 3, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Communication] - SMS - Add Tests and Clean Up #19149

[Communication] - SMS - Add Tests and Clean Up #19149

minnieliu commented Mar 3, 2021 •

edited

Loading

minnieliu commented Mar 3, 2021

azure-pipelines bot commented Mar 3, 2021

minnieliu commented Mar 3, 2021

azure-pipelines bot commented Mar 3, 2021

beltr0n left a comment

beltr0n Mar 3, 2021

beltr0n Mar 3, 2021

minnieliu Mar 3, 2021

beltr0n Mar 3, 2021

minnieliu Mar 3, 2021

beltr0n left a comment

lsundaralingam Mar 3, 2021

lsundaralingam Mar 3, 2021

minnieliu Mar 3, 2021


		Assert.AreNotEqual(firstMessageResult.MessageId, secondMessageResult.MessageId);
		Assert.True(firstMessageResult.Successful);

[Communication] - SMS - Add Tests and Clean Up #19149

[Communication] - SMS - Add Tests and Clean Up #19149

Conversation

minnieliu commented Mar 3, 2021 • edited Loading

All SDK Contribution checklist:

Additional management plane SDK specific contribution checklist:

Management plane SDK Troubleshooting

Old outstanding PR cleanup

minnieliu commented Mar 3, 2021

azure-pipelines bot commented Mar 3, 2021

minnieliu commented Mar 3, 2021

azure-pipelines bot commented Mar 3, 2021

beltr0n left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

beltr0n left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

minnieliu commented Mar 3, 2021 •

edited

Loading