Skip to content
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

[Tests] Move all our mocha test suites to use old functions instead of arrow functions #13005

Closed
deyaaeldeen opened this issue Dec 22, 2020 · 11 comments
Assignees
Labels
Client This issue points to a problem in the data-plane of the library. EngSys This issue is impacting the engineering system. eslint plugin help wanted This issue is tracking work for which community contributions would be welcomed and appreciated

Comments

@deyaaeldeen
Copy link
Member

deyaaeldeen commented Dec 22, 2020

This is advised by mocha team (see https://mochajs.org/#arrow-functions) and can cause issues that are hard to debug. It was a primary reason for failures in #12819. My concern is if we were to access/update mocha context later in our unit tests, we will run into this issue if the unit tests are still written using the lambda syntax. This could cause hours in lost productivity.

@deyaaeldeen deyaaeldeen added the test-reliability Issue that causes tests to be unreliable label Dec 22, 2020
@deyaaeldeen deyaaeldeen added this to the Backlog milestone Dec 22, 2020
openapi-sdkautomation bot pushed a commit to AzureSDKAutomation/azure-sdk-for-js that referenced this issue May 11, 2021
PR for new api-version: Microsoft.Cache/redis/stable/2020-12-01 [rebased + refactored] (Azure#14023)

* Adds base for updating Microsoft.Cache from version stable/2020-06-01 to version 2020-12-01

* Updates readme

* Updates API version in new specs and examples

* Adding Redis Version to PUT request (Azure#12675)

* Adding Redis Version to PUT request

* Include RedisVersion in PUT and PATCH, and update description to clarify major version

* Adding new properties isPrimary and replicasPerPrimary (Azure#13005)

* Adding new properties isPrimary and replicasPerPrimary

* Addressing PR feedback

Co-authored-by: Lavanya Singampalli <lasingam@microsoft.com>

* Refactor the latest redis.json API spec proposal to use the definitions ProxyResource, TrackedResource and OperationListResult from common-types/resource-management/v2. (I hope CI likes this!)

* Cleanup  now-unreferenced #definitions which were still flagged as duplicate definitions.

* Fix linter errors for camelCasing of API names and formatting of integer types.

* Fix more linter errors by adding pagable, refactoring LinkedServers and FirewallRules. These will be between-versions breaking changes for the SDK, in the name of more API consistency.

* Fix linter error 'PUT' operation 'PrivateEndpointConnections_Put' should use method name 'Create'. Note: If you have already shipped an SDK on top of this spec, fixing this warning may introduce a breaking change.

* Fix some duplicate example names.

* Revert "Fix linter error 'PUT' operation 'PrivateEndpointConnections_Put' should use method name 'Create'. Note: If you have already shipped an SDK on top of this spec, fixing this warning may introduce a breaking change." -- because it introduces new linter errors.

* Revert "Fix more linter errors by adding pagable, refactoring LinkedServers and FirewallRules. These will be between-versions breaking changes for the SDK, in the name of more API consistency."

This reverts commit c0e305128834947066bb59de2c4247b29feef5ee.

* Fix the example name for CheckNameAvailability.

* Add x-ms-pageable suggested by the linter.

* Revert the linter fix for CheckNameAvailabiltiy camelCasing to avoid introducing new linter errors.

* Use same FirewallRule response schema for PUT and GET, addressing linter errors, and have RedisFirewallRuleCreateParameters inherit it in hopes that helps with SDK backcompat.

* Fix indenting.

* Fix broken $ref reference syntax.

* Add back the description for RedisFirewallRuleCreateParameters.

* Actually run prettier.

* With print width 20

* Suppress the warnings that booleans are not descriptive, since to change them would be a breaking change.

* Delete unintended extra character

* Revert enum type-hinting and refactors to common-types that are getting flagged as breaking changes.

* Add back the refactorings to use Resource/ProxyResource/TrackedResource from common-types. Since otherwise we get "Duplicate Schema named Resource -- properties.id.description: undefined => "Fully qualified resource ID for the resource. Ex - /subscriptions/subscriptionId/resourceGroups/resourceGroupName/providers/resourceProviderNamespace/resourceType/resourceName",

properties.name.description: undefined => "The name of the resource","

Co-authored-by: MECHANDR <30642185+MECHANDR@users.noreply.github.com>
Co-authored-by: lasingam <68033674+lasingam@users.noreply.github.com>
Co-authored-by: Lavanya Singampalli <lasingam@microsoft.com>
@ramya-rao-a
Copy link
Contributor

Even if we fix this, we should invest in a linter rule for this to avoid future failures

@ramya-rao-a ramya-rao-a added eslint plugin EngSys This issue is impacting the engineering system. and removed test-reliability Issue that causes tests to be unreliable labels Sep 21, 2021
@kurtzeborn kurtzeborn added the Client This issue points to a problem in the data-plane of the library. label Oct 18, 2021
@jeremymeng jeremymeng added the help wanted This issue is tracking work for which community contributions would be welcomed and appreciated label Dec 1, 2021
@ramya-rao-a
Copy link
Contributor

Tips for folks interested in contributing to this issue:

  • Set up your dev env if you have not already
  • Fix one package at a time
    • Pick any package under the sdk folder that has tests. Look for sdk/service-name/package-name/test
    • Update the tests so that they use the arrow functions
    • send PR
  • Once all tests in this repo use the arrow functions for test suites, add a linter rule for this in the ESLint Plugin

@jeremymeng
Copy link
Member

@StevanFreeborn
Copy link
Contributor

StevanFreeborn commented Nov 8, 2022

@ramya-rao-a, @jeremymeng, or @deyaaeldeen could I be assigned this issue to work on? Or do I need to be assigned the issue to work on it?

@xirzec
Copy link
Member

xirzec commented Nov 8, 2022

@StevanFreeborn happy to let you work on it. Was there a particular SDK you were thinking of starting with?

@StevanFreeborn
Copy link
Contributor

StevanFreeborn commented Nov 8, 2022

@xirzec Since all the tests for all the sdks need to be re-visited I figured I'd just work alphabetically.

So it looks like the first tests to be modified would be here:
sdk/advisor/arm-advisor/test/sampleTest.ts

Any objections to that approach? Or have a particular one in mind to start with?

Do you want separate PRs for each update to the individual SDKs?

@xirzec
Copy link
Member

xirzec commented Nov 8, 2022

@StevanFreeborn perhaps avoid arm- packages initially since I think dataplane packages have more tests. I think separate PRs per SDK will be easiest to get review / merge.

Perhaps search would be a fine starting point? Most of the unit tests should be pretty straightforward to convert, e.g.
https://github.com/Azure/azure-sdk-for-js/blob/main/sdk/search/search-documents/test/internal/base64.spec.ts

@StevanFreeborn
Copy link
Contributor

@xirzec ah okay good to know. I shall work accordingly. And agreed separate PRs for each SDK seems like a good way to organize it. Thank you for the direction. Very much appreciated.

@xirzec
Copy link
Member

xirzec commented Nov 8, 2022

@xirzec ah okay good to know. I shall work accordingly. And agreed separate PRs for each SDK seems like a good way to organize it. Thank you for the direction. Very much appreciated.

Thank you for your time and effort! Please feel free to comment here or open a draft PR if you have any trouble and we can take a look. 😄

@StevanFreeborn
Copy link
Contributor

StevanFreeborn commented Nov 8, 2022

Of course I very much appreciate the opportunity to contribute. I've gone ahead and opened an initial PR ( #23761 ) for making the necessary changes to the search-documents package to confirm I'm moving in the right direction.

I ran the test suite locally for that package using rushx test before and after making the changes also to verify things remain as they should.

deyaaeldeen pushed a commit that referenced this issue Nov 10, 2022
### Packages impacted by this PR

`sdk\attestation\attestation`

### Issues associated with this PR

#13005 

### Describe the problem that is addressed by this PR

The existing mocha tests for the `sdk\attestation\attestation` made use of the arrow syntax for callback functions. Mocha recommends not to do this because you lose access to the mocha context (https://mochajs.org/#arrow-functions).

### What are the possible designs available to address the problem? If there are more than one possible design, why was the one in this PR chosen?

The reason for utilizing the function keyword instead of an arrow syntax to write the callback functions in these mocha tests is to maintain access to the mocha context.

### Are there test cases added in this PR? _(If not, why?)_

No additional test cases were added in this PR as the change only required modifying existing test cases.

### Provide a list of related PRs _(if any)_

#23761 - Same fix, but for the `sdk/search/search-documents` package

### Command used to generate this PR:**_(Applicable only to SDK release request PRs)_

**_Not applicable_**

### Checklists
- [x] Added impacted package name to the issue description
- [ ] Does this PR needs any fixes in the SDK Generator?** _(If so, create an Issue in the [Autorest/typescript](https://github.com/Azure/autorest.typescript) repository and link it here)_
  - **_I don't believe this is relevant._**
- [ ] Added a changelog (if necessary)
  - **_I don't believe this is necessary_**

### Notes

I did run the package's test suite before and after the changes to confirm all remains as it should be.
deyaaeldeen pushed a commit that referenced this issue Nov 14, 2022
### Packages impacted by this PR

`sdk\batch\batch`

### Issues associated with this PR

#13005 

### Describe the problem that is addressed by this PR

The existing mocha tests for the `sdk\attestation\attestation` made use of the arrow syntax for callback functions. Mocha recommends not to do this because you lose access to the mocha context (https://mochajs.org/#arrow-functions).

### What are the possible designs available to address the problem? If there are more than one possible design, why was the one in this PR chosen?

The reason for utilizing the function keyword instead of an arrow syntax to write the callback functions in these mocha tests is to maintain access to the mocha context.

### Are there test cases added in this PR? _(If not, why?)_

No additional test cases were added in this PR as the change only required modifying existing test cases.

### Provide a list of related PRs _(if any)_

#23761 - Same fix, but for the `sdk\search\search-documents` package
#23789 - Same fix but for the `sdk\attestation\attestation` package

### Command used to generate this PR:**_(Applicable only to SDK release request PRs)_

**_Not applicable_**

### Checklists
- [x] Added impacted package name to the issue description
- [ ] Does this PR needs any fixes in the SDK Generator?** _(If so, create an Issue in the [Autorest/typescript](https://github.com/Azure/autorest.typescript) repository and link it here)_
   - **_I don't believe this is relevant._**
- [ ] Added a changelog (if necessary)
  - **_I don't believe this is necessary_**
deyaaeldeen pushed a commit that referenced this issue Nov 15, 2022
### Packages impacted by this PR

`sdk\cognitivelanguage\ai-language-conversations`

### Issues associated with this PR

#13005 

### Describe the problem that is addressed by this PR

The existing mocha tests for the `sdk\cognitivelanguage\ai-language-conversations` made use of the arrow syntax for callback functions. Mocha recommends not to do this because you lose access to the mocha context (https://mochajs.org/#arrow-functions).

### What are the possible designs available to address the problem? If there are more than one possible design, why was the one in this PR chosen?

The reason for utilizing the function keyword instead of an arrow syntax to write the callback functions in these mocha tests is to maintain access to the mocha context.

### Are there test cases added in this PR? _(If not, why?)_

No additional test cases were added in this PR as the change only required modifying existing test cases.

### Provide a list of related PRs _(if any)_

#23761 - Same fix, but for the `sdk\search\search-documents` package
#23789 - Same fix but for the `sdk\attestation\attestation` package
#23835 - Same fix but for the `sdk\batch\batch` package

### Command used to generate this PR:**_(Applicable only to SDK release request PRs)_

**_Not applicable_**

### Checklists
- [x] Added impacted package name to the issue description
- [ ] Does this PR needs any fixes in the SDK Generator?** _(If so, create an Issue in the [Autorest/typescript](https://github.com/Azure/autorest.typescript) repository and link it here)_
   - **_I don't believe this is relevant._**
- [ ] Added a changelog (if necessary)
  - **_I don't believe this is necessary_**
xirzec pushed a commit that referenced this issue Feb 6, 2023
### Packages impacted by this PR

`sdk\cognitiveservices\cognitiveservices-luis-runtime`

### Issues associated with this PR

#13005 

### Describe the problem that is addressed by this PR

The existing mocha tests for the
`sdk\cognitiveservices\cognitiveservices-luis-runtime` made use of the
arrow syntax for callback functions. Mocha recommends not to do this
because you lose access to the mocha context
(https://mochajs.org/#arrow-functions).

### What are the possible designs available to address the problem? If
there are more than one possible design, why was the one in this PR
chosen?

The reason for utilizing the function keyword instead of an arrow syntax
to write the callback functions in these mocha tests is to maintain
access to the mocha context.

### Are there test cases added in this PR? _(If not, why?)_

No additional test cases were added in this PR as the change only
required modifying existing test cases.

### Provide a list of related PRs _(if any)_

#23761 - Same fix, but for the `sdk\search\search-documents` package
#23789 - Same fix but for the `sdk\attestation\attestation` package
#23835 - Same fix but for the `sdk\batch\batch` package
#23850 - Same fix but for the
`sdk\cognitivelanguage\ai-language-conversations` package
#23881 - Same fix but for the
`sdk\cognitiveservices\cognitiveservices-luis-authoring`

### Command used to generate this PR:**_(Applicable only to SDK release
request PRs)_

**_Not applicable_**

### Checklists
- [x] Added impacted package name to the issue description
- [ ] Does this PR needs any fixes in the SDK Generator?** _(If so,
create an Issue in the
[Autorest/typescript](https://github.com/Azure/autorest.typescript)
repository and link it here)_
   - **_I don't believe this is relevant._**
- [ ] Added a changelog (if necessary)
  - **_I don't believe this is necessary_**
jeremymeng pushed a commit that referenced this issue Feb 7, 2023
### Packages impacted by this PR

`sdk\communication\communication-common`

### Issues associated with this PR

#13005 

### Describe the problem that is addressed by this PR

The existing mocha tests for the
`sdk\communication\communication-common` made use of the arrow syntax
for callback functions. Mocha recommends not to do this because you lose
access to the mocha context (https://mochajs.org/#arrow-functions).

### What are the possible designs available to address the problem? If
there are more than one possible design, why was the one in this PR
chosen?

The reason for utilizing the function keyword instead of an arrow syntax
to write the callback functions in these mocha tests is to maintain
access to the mocha context.

### Are there test cases added in this PR? _(If not, why?)_

No additional test cases were added in this PR as the change only
required modifying existing test cases.

### Provide a list of related PRs _(if any)_

#23761 - Same fix, but for the `sdk\search\search-documents` package
#23789 - Same fix but for the `sdk\attestation\attestation` package
#23835 - Same fix but for the `sdk\batch\batch` package
#23850 - Same fix but for the
`sdk\cognitivelanguage\ai-language-conversations` package
#23881 - Same fix but for the
`sdk\cognitiveservices\cognitiveservices-luis-authoring` package
#24126 - Same fix but for the
`sdk\cognitiveservices\cognitiveservices-luis-runtime` package
#21470 - Same fix but for the `sdk\communication\communication-chat`
package

### Command used to generate this PR:**_(Applicable only to SDK release
request PRs)_

**_Not applicable_**

### Checklists
- [x] Added impacted package name to the issue description
- [ ] Does this PR needs any fixes in the SDK Generator?** _(If so,
create an Issue in the
[Autorest/typescript](https://github.com/Azure/autorest.typescript)
repository and link it here)_
   - **_I don't believe this is relevant._**
- [ ] Added a changelog (if necessary)
  - **_I don't believe this is necessary_**
jeremymeng pushed a commit that referenced this issue Feb 8, 2023
### Packages impacted by this PR

`sdk\communication\communication-email`

### Issues associated with this PR

#13005 

### Describe the problem that is addressed by this PR

The existing mocha tests for the `sdk\communication\communication-email`
made use of the arrow syntax for callback functions. Mocha recommends
not to do this because you lose access to the mocha context
(https://mochajs.org/#arrow-functions).

### What are the possible designs available to address the problem? If
there are more than one possible design, why was the one in this PR
chosen?

The reason for utilizing the function keyword instead of an arrow syntax
to write the callback functions in these mocha tests is to maintain
access to the mocha context.

### Are there test cases added in this PR? _(If not, why?)_

No additional test cases were added in this PR as the change only
required modifying existing test cases.

### Provide a list of related PRs _(if any)_

#23761 - Same fix, but for the `sdk\search\search-documents` package
#23789 - Same fix but for the `sdk\attestation\attestation` package
#23835 - Same fix but for the `sdk\batch\batch` package
#23850 - Same fix but for the
`sdk\cognitivelanguage\ai-language-conversations` package
#23881 - Same fix but for the
`sdk\cognitiveservices\cognitiveservices-luis-authoring` package
#24126 - Same fix but for the
`sdk\cognitiveservices\cognitiveservices-luis-runtime` package
#21470 - Same fix but for the `sdk\communication\communication-chat`
package
#24746 - Same fix but for the `sdk\communication\communication-common`
package

### Command used to generate this PR:**_(Applicable only to SDK release
request PRs)_

**_Not applicable_**

### Checklists
- [x] Added impacted package name to the issue description
- [ ] Does this PR needs any fixes in the SDK Generator?** _(If so,
create an Issue in the
[Autorest/typescript](https://github.com/Azure/autorest.typescript)
repository and link it here)_
   - **_I don't believe this is relevant._**
- [ ] Added a changelog (if necessary)
  - **_I don't believe this is necessary_**
petrsvihlik pushed a commit that referenced this issue Feb 9, 2023
### Packages impacted by this PR

`sdk\communication\communication-identity`

### Issues associated with this PR

#13005 

### Describe the problem that is addressed by this PR

The existing mocha tests for the
`sdk\communication\communication-identity` made use of the arrow syntax
for callback functions. Mocha recommends not to do this because you lose
access to the mocha context (https://mochajs.org/#arrow-functions).

### What are the possible designs available to address the problem? If
there are more than one possible design, why was the one in this PR
chosen?

The reason for utilizing the function keyword instead of an arrow syntax
to write the callback functions in these mocha tests is to maintain
access to the mocha context.

### Are there test cases added in this PR? _(If not, why?)_

No additional test cases were added in this PR as the change only
required modifying existing test cases.

### Provide a list of related PRs _(if any)_

#23761 - Same fix, but for the `sdk\search\search-documents` package
#23789 - Same fix but for the `sdk\attestation\attestation` package
#23835 - Same fix but for the `sdk\batch\batch` package
#23850 - Same fix but for the
`sdk\cognitivelanguage\ai-language-conversations` package
#23881 - Same fix but for the
`sdk\cognitiveservices\cognitiveservices-luis-authoring` package
#24126 - Same fix but for the
`sdk\cognitiveservices\cognitiveservices-luis-runtime` package
#21470 - Same fix but for the `sdk\communication\communication-chat`
package
#24746 - Same fix but for the `sdk\communication\communication-common`
package
#24747 - Same fix but for the `sdk\communication\communication-email`
package

### Command used to generate this PR:**_(Applicable only to SDK release
request PRs)_

**_Not applicable_**

### Checklists
- [x] Added impacted package name to the issue description
- [ ] Does this PR needs any fixes in the SDK Generator?** _(If so,
create an Issue in the
[Autorest/typescript](https://github.com/Azure/autorest.typescript)
repository and link it here)_
   - **_I don't believe this is relevant._**
- [ ] Added a changelog (if necessary)
  - **_I don't believe this is necessary_**
anujissarMS pushed a commit that referenced this issue Feb 14, 2023
### Packages impacted by this PR

`sdk\communication\communication-rooms`

### Issues associated with this PR

#13005 

### Describe the problem that is addressed by this PR

The existing mocha tests for the `sdk\communication\communication-rooms`
made use of the arrow syntax for callback functions. Mocha recommends
not to do this because you lose access to the mocha context
(https://mochajs.org/#arrow-functions).

### What are the possible designs available to address the problem? If
there are more than one possible design, why was the one in this PR
chosen?

The reason for utilizing the function keyword instead of an arrow syntax
to write the callback functions in these mocha tests is to maintain
access to the mocha context.

### Are there test cases added in this PR? _(If not, why?)_

No additional test cases were added in this PR as the change only
required modifying existing test cases.

### Provide a list of related PRs _(if any)_

#23761 - Same fix, but for the `sdk\search\search-documents` package
#23789 - Same fix but for the `sdk\attestation\attestation` package
#23835 - Same fix but for the `sdk\batch\batch` package
#23850 - Same fix but for the
`sdk\cognitivelanguage\ai-language-conversations` package
#23881 - Same fix but for the
`sdk\cognitiveservices\cognitiveservices-luis-authoring` package
#24126 - Same fix but for the
`sdk\cognitiveservices\cognitiveservices-luis-runtime` package
#21470 - Same fix but for the `sdk\communication\communication-chat`
package
#24746 - Same fix but for the `sdk\communication\communication-common`
package
#24747 - Same fix but for the `sdk\communication\communication-email`
package
#24797 - Same fix but for the `sdk\communication\communication-identity`
package
#24799 - Same fix but for the
`sdk\communication\communication-job-router` package

### Command used to generate this PR:**_(Applicable only to SDK release
request PRs)_

**_Not applicable_**

### Checklists
- [x] Added impacted package name to the issue description
- [ ] Does this PR needs any fixes in the SDK Generator?** _(If so,
create an Issue in the
[Autorest/typescript](https://github.com/Azure/autorest.typescript)
repository and link it here)_
   - **_I don't believe this is relevant._**
- [ ] Added a changelog (if necessary)
  - **_I don't believe this is necessary_**
jeremymeng pushed a commit that referenced this issue Feb 14, 2023
### Packages impacted by this PR

`sdk\communication\communication-chat`

### Issues associated with this PR

#13005 

### Describe the problem that is addressed by this PR

The existing mocha tests for the `sdk\communication\communication-chat`
made use of the arrow syntax for callback functions. Mocha recommends
not to do this because you lose access to the mocha context
(https://mochajs.org/#arrow-functions).

### What are the possible designs available to address the problem? If
there are more than one possible design, why was the one in this PR
chosen?

The reason for utilizing the function keyword instead of an arrow syntax
to write the callback functions in these mocha tests is to maintain
access to the mocha context.

### Are there test cases added in this PR? _(If not, why?)_

No additional test cases were added in this PR as the change only
required modifying existing test cases.

### Provide a list of related PRs _(if any)_

#23761 - Same fix, but for the `sdk\search\search-documents` package
#23789 - Same fix but for the `sdk\attestation\attestation` package
#23835 - Same fix but for the `sdk\batch\batch` package
#23850 - Same fix but for the
`sdk\cognitivelanguage\ai-language-conversations` package
#23881 - Same fix but for the
`sdk\cognitiveservices\cognitiveservices-luis-authoring`
#24126 - Same fix but for the
`sdk\cognitiveservices\cognitiveservices-luis-runtime`

### Command used to generate this PR:**_(Applicable only to SDK release
request PRs)_

**_Not applicable_**

### Checklists
- [x] Added impacted package name to the issue description
- [ ] Does this PR needs any fixes in the SDK Generator?** _(If so,
create an Issue in the
[Autorest/typescript](https://github.com/Azure/autorest.typescript)
repository and link it here)_
   - **_I don't believe this is relevant._**
- [ ] Added a changelog (if necessary)
  - **_I don't believe this is necessary_**
jeremymeng pushed a commit that referenced this issue Feb 21, 2023
…s in test directory (#24865)

### Packages impacted by this PR

`sdk\communication\communication-job-router`

### Issues associated with this PR

#13005 

### Describe the problem that is addressed by this PR

The existing mocha tests for the
`sdk\communication\communication-job-router` made use of the arrow
syntax for callback functions. Mocha recommends not to do this because
you lose access to the mocha context
(https://mochajs.org/#arrow-functions).

### What are the possible designs available to address the problem? If
there are more than one possible design, why was the one in this PR
chosen?

The reason for utilizing the function keyword instead of an arrow syntax
to write the callback functions in these mocha tests is to maintain
access to the mocha context.

### Are there test cases added in this PR? _(If not, why?)_

No additional test cases were added in this PR as the change only
required modifying existing test cases.

### Provide a list of related PRs _(if any)_

#23761 - Same fix, but for the `sdk\search\search-documents` package
#23789 - Same fix but for the `sdk\attestation\attestation` package
#23835 - Same fix but for the `sdk\batch\batch` package
#23850 - Same fix but for the
`sdk\cognitivelanguage\ai-language-conversations` package
#23881 - Same fix but for the
`sdk\cognitiveservices\cognitiveservices-luis-authoring` package
#24126 - Same fix but for the
`sdk\cognitiveservices\cognitiveservices-luis-runtime` package
#21470 - Same fix but for the `sdk\communication\communication-chat`
package
#24746 - Same fix but for the `sdk\communication\communication-common`
package
#24747 - Same fix but for the `sdk\communication\communication-email`
package
#24797 - Same fix but for the `sdk\communication\communication-identity`
package

### Command used to generate this PR:**_(Applicable only to SDK release
request PRs)_

**_Not applicable_**

### Checklists
- [x] Added impacted package name to the issue description
- [ ] Does this PR needs any fixes in the SDK Generator?** _(If so,
create an Issue in the
[Autorest/typescript](https://github.com/Azure/autorest.typescript)
repository and link it here)_
   - **_I don't believe this is relevant._**
- [ ] Added a changelog (if necessary)
  - **_I don't believe this is necessary_**
jeremymeng pushed a commit that referenced this issue Mar 13, 2023
### Packages impacted by this PR

`sdk\confidentialledger\confidential-ledger-rest`

### Issues associated with this PR

#13005 

### Describe the problem that is addressed by this PR

The existing mocha tests for the
`sdk\confidentialledger\confidential-ledger-rest` made use of the arrow
syntax for callback functions. Mocha recommends not to do this because
you lose access to the mocha context
(https://mochajs.org/#arrow-functions).

### What are the possible designs available to address the problem? If
there are more than one possible design, why was the one in this PR
chosen?

The reason for utilizing the function keyword instead of an arrow syntax
to write the callback functions in these mocha tests is to maintain
access to the mocha context.

### Are there test cases added in this PR? _(If not, why?)_

No additional test cases were added in this PR as the change only
required modifying existing test cases.

### Provide a list of related PRs _(if any)_

#23761 - Same fix, but for the `sdk\search\search-documents` package
#23789 - Same fix but for the `sdk\attestation\attestation` package
#23835 - Same fix but for the `sdk\batch\batch` package
#23850 - Same fix but for the
`sdk\cognitivelanguage\ai-language-conversations` package
#23881 - Same fix but for the
`sdk\cognitiveservices\cognitiveservices-luis-authoring` package
#24126 - Same fix but for the
`sdk\cognitiveservices\cognitiveservices-luis-runtime` package
#21470 - Same fix but for the `sdk\communication\communication-chat`
package
#24746 - Same fix but for the `sdk\communication\communication-common`
package
#24747 - Same fix but for the `sdk\communication\communication-email`
package
#24797 - Same fix but for the `sdk\communication\communication-identity`
package
#24865 - Same fix but for the
`sdk\communication\communication-job-router` package

### Command used to generate this PR:**_(Applicable only to SDK release
request PRs)_

**_Not applicable_**

### Checklists
- [x] Added impacted package name to the issue description
- [ ] Does this PR needs any fixes in the SDK Generator?** _(If so,
create an Issue in the
[Autorest/typescript](https://github.com/Azure/autorest.typescript)
repository and link it here)_
   - **_I don't believe this is relevant._**
- [ ] Added a changelog (if necessary)
  - **_I don't believe this is necessary_**
jeremymeng pushed a commit that referenced this issue Jul 28, 2023
### Packages impacted by this PR

`sdk\communication\communication-sms`

### Issues associated with this PR

#13005 

### Describe the problem that is addressed by this PR

The existing mocha tests for the `sdk\communication\communication-sms`
made use of the arrow syntax for callback functions. Mocha recommends
not to do this because you lose access to the mocha context
(https://mochajs.org/#arrow-functions).

### What are the possible designs available to address the problem? If
there are more than one possible design, why was the one in this PR
chosen?

The reason for utilizing the function keyword instead of an arrow syntax
to write the callback functions in these mocha tests is to maintain
access to the mocha context.

### Are there test cases added in this PR? _(If not, why?)_

No additional test cases were added in this PR as the change only
required modifying existing test cases.

### Provide a list of related PRs _(if any)_

#23761 - Same fix, but for the `sdk\search\search-documents` package
#23789 - Same fix but for the `sdk\attestation\attestation` package
#23835 - Same fix but for the `sdk\batch\batch` package
#23850 - Same fix but for the
`sdk\cognitivelanguage\ai-language-conversations` package
#23881 - Same fix but for the
`sdk\cognitiveservices\cognitiveservices-luis-authoring` package
#24126 - Same fix but for the
`sdk\cognitiveservices\cognitiveservices-luis-runtime` package
#21470 - Same fix but for the `sdk\communication\communication-chat`
package
#24746 - Same fix but for the `sdk\communication\communication-common`
package
#24747 - Same fix but for the `sdk\communication\communication-email`
package
#24797 - Same fix but for the `sdk\communication\communication-identity`
package
#24800 - Same fix but for the `sdk\communication\communication-rooms`
package
#24865 - Same fix but for the
`sdk\communication\communication-job-router` package
#25148 - Same fix but for the
`sdk\confidentialledger\confidential-ledger-rest` package

### Command used to generate this PR:**_(Applicable only to SDK release
request PRs)_

**_Not applicable_**

### Checklists
- [x] Added impacted package name to the issue description
- [ ] Does this PR needs any fixes in the SDK Generator?** _(If so,
create an Issue in the
[Autorest/typescript](https://github.com/Azure/autorest.typescript)
repository and link it here)_
- [ ] Added a changelog (if necessary)
dgetu pushed a commit that referenced this issue Sep 6, 2023
### Packages impacted by this PR

`sdk\communication\communication-sms`

### Issues associated with this PR

#13005 

### Describe the problem that is addressed by this PR

The existing mocha tests for the `sdk\communication\communication-sms`
made use of the arrow syntax for callback functions. Mocha recommends
not to do this because you lose access to the mocha context
(https://mochajs.org/#arrow-functions).

### What are the possible designs available to address the problem? If
there are more than one possible design, why was the one in this PR
chosen?

The reason for utilizing the function keyword instead of an arrow syntax
to write the callback functions in these mocha tests is to maintain
access to the mocha context.

### Are there test cases added in this PR? _(If not, why?)_

No additional test cases were added in this PR as the change only
required modifying existing test cases.

### Provide a list of related PRs _(if any)_

#23761 - Same fix, but for the `sdk\search\search-documents` package
#23789 - Same fix but for the `sdk\attestation\attestation` package
#23835 - Same fix but for the `sdk\batch\batch` package
#23850 - Same fix but for the
`sdk\cognitivelanguage\ai-language-conversations` package
#23881 - Same fix but for the
`sdk\cognitiveservices\cognitiveservices-luis-authoring` package
#24126 - Same fix but for the
`sdk\cognitiveservices\cognitiveservices-luis-runtime` package
#21470 - Same fix but for the `sdk\communication\communication-chat`
package
#24746 - Same fix but for the `sdk\communication\communication-common`
package
#24747 - Same fix but for the `sdk\communication\communication-email`
package
#24797 - Same fix but for the `sdk\communication\communication-identity`
package
#24800 - Same fix but for the `sdk\communication\communication-rooms`
package
#24865 - Same fix but for the
`sdk\communication\communication-job-router` package
#25148 - Same fix but for the
`sdk\confidentialledger\confidential-ledger-rest` package

### Command used to generate this PR:**_(Applicable only to SDK release
request PRs)_

**_Not applicable_**

### Checklists
- [x] Added impacted package name to the issue description
- [ ] Does this PR needs any fixes in the SDK Generator?** _(If so,
create an Issue in the
[Autorest/typescript](https://github.com/Azure/autorest.typescript)
repository and link it here)_
- [ ] Added a changelog (if necessary)
Copy link

Hi @deyaaeldeen, we deeply appreciate your input into this project. Regrettably, this issue has remained inactive for over 2 years, leading us to the decision to close it. We've implemented this policy to maintain the relevance of our issue queue and facilitate easier navigation for new contributors. If you still believe this topic requires attention, please feel free to create a new issue, referencing this one. Thank you for your understanding and ongoing support.

@github-actions github-actions bot closed this as not planned Won't fix, can't repro, duplicate, stale Mar 20, 2024
@github-actions github-actions bot locked and limited conversation to collaborators Mar 20, 2024
@xirzec xirzec removed this from the Backlog milestone May 7, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Client This issue points to a problem in the data-plane of the library. EngSys This issue is impacting the engineering system. eslint plugin help wanted This issue is tracking work for which community contributions would be welcomed and appreciated
Projects
None yet
Development

No branches or pull requests

6 participants