[ServiceBus] Fix linter errors and enable linting #18670

jeremymeng · 2021-11-12T19:50:03Z

Some reports are ignored

intentional usage of Function
object/any used in deserialization
empty type {} in public api withResponse<{}>

- for namespace - parameter of `any`

sdk/servicebus/service-bus/src/receivers/receiverCommon.ts

sdk/servicebus/service-bus/test/internal/batchReceiver.spec.ts

sdk/servicebus/service-bus/test/internal/unit/batchingReceiver.spec.ts

sdk/servicebus/service-bus/test/public/amqpAnnotatedMessage.spec.ts

ramya-rao-a · 2021-11-19T19:50:38Z

/azp run js - service-bus - tests

azure-pipelines · 2021-11-19T19:50:51Z

Azure Pipelines successfully started running 1 pipeline(s).

jeremymeng · 2021-11-20T00:48:39Z

sdk/servicebus/service-bus/test/public/atomManagement.spec.ts

@@ -2638,12 +2638,17 @@ versionsToTest(serviceApiVersions, {}, (serviceVersion, {}) => {
              "TestError: Topic path AND subscription path must be passed when invoking tests on rules"
            );
          }
+          if (!ruleOptions?.filter || !ruleOptions.action) {
+            throw new Error(
+              "TestError: ruleOptions.filter and ruleOptions.action should have been set"


Now the live tests actually failed because ruleOptions is undefined. Before this PR we just force-passing undefined for filter and action to atomClient.createRule(). I also see a discrepancy that while createRule doesn't allow undefined values, they are allowed in putResource (as CreateRuleOptions):

azure-sdk-for-js/sdk/servicebus/service-bus/src/serviceBusAtomManagementClient.ts

Line 2198 in 16b6147

| CreateRuleOptions,

Any insights? @HarshaNalluru @ramya-rao-a
Another option is to ignore the linting error if this is indeed expected.

There is no point in creating a rule without a filter, right?

But you can create a rule without action.
We have overloads :)

There is no point in creating a rule without a filter, right?

Then I don't know why the test would pass undefined filter to createRule(). We do create a always true sql filter internally

Hmmm, we can remove the test

azure-sdk-for-js/sdk/servicebus/service-bus/test/public/atomManagement.spec.ts

Line 2569 in 9f454f8

ruleOptions?: Omit<Required<CreateSubscriptionOptions>["defaultRuleOptions"], "name">,

hmm, filter is actually required now because of Required<>. undefined is passed because ruleOptions is undefined.

So how do you want to move forward?

filter will be required

action is optional

check for the existence of filter and throw if it is not provided to avoid ! at the end?

check for the presence of action and call the overloads accordingly?

update/remove the tests as needed(such as the "undefined" rule options one)?

add tests if you feel a code path is missing?

The API we use look fine. I had added throw will update it to only check filter. Don't need to do anything for action since it's optional, right? I will remove the test scenario of undefined ruleOptions as it doesn't apply for createRule()

yes, you are right: calling different overloads based on action value

jeremymeng · 2021-11-22T23:58:49Z

/azp run js - service-bus - tests

azure-pipelines · 2021-11-22T23:59:01Z

Azure Pipelines successfully started running 1 pipeline(s).

jeremymeng · 2021-11-23T01:21:42Z

/azp run js - service-bus - tests

azure-pipelines · 2021-11-23T01:21:54Z

Azure Pipelines successfully started running 1 pipeline(s).

AssertionError: expecting valid error.message but error is [object CloseEvent]: expected undefined to be truthy

jeremymeng · 2021-11-24T23:14:35Z

/azp run js - service-bus - tests

azure-pipelines · 2021-11-24T23:14:47Z

Azure Pipelines successfully started running 1 pipeline(s).

Web ant97 2022 03 01 (Azure#19430) * Adds base for updating Microsoft.Web from version stable/2021-03-01 to version 2022-03-01 * Updates readme * Updates API version in new specs and examples * Carry fwd Microsoft.CertificateRegistration and Microsoft.DomainRegis… (Azure#18460) * Carry fwd Microsoft.CertificateRegistration and Microsoft.DomainRegistration RPs to Api-version 2022-03-01 * Add x-ms-enum for array of inline enums. Fix reference to older api version for common defs * Add examples for App Service Certificate orders * Add suppressions back * Add examples for Microsoft.DomainRegistration RP * Fixes from prettier * Remove unused example * Add Unhealthy CustomDomainStatus for StaticSites (Azure#18557) * Adding VnetRouteAllEnabled, VnetImagePullEnabled, VnetContentShareEnabled site properties to CommonDefinitions (Azure#18627) * Added vnet realted site properties to common definitions * removed tab Co-authored-by: Tanay Bhartia <[email protected]> * Add Ftp/Remote debug properties to Ase network config (Azure#18549) * Add Ftp/Remote debug properties to Ase network config * Switched changes to the right file * Add Networking config to hosting environment creates * Added some of the missing examples * Remove disallowed properties from example Ase GET response * Adjusted responses from other gets * Added a bunch more examples * Added other missing examples for Ase. * Added more missing examples for Ase. * fixed formatting * Edit some examples, add back privatelink apis and try to supress the errors for missing examples. * Fix suppress spelling and fix prettier * Added missing privatelink examples since suppressing did not work * Add Bring your own Backend operations (Azure#18517) * add bring your own backend operations * add missing files * fix json formattin * validation fixes * fix * fix more validation * fix ids * fix model validation * fix descriptions * fix environmentName description * change to 202 * revert change to 202 * customhostnamesites optional param for 2022 API version (Azure#18670) * fix DUPLICATE_PARAMETER * prettier * fix OBJECT_ADDITIONAL_PROPERTIES * fix random sub in examples * add optional hostname param to customHostnameSites RT and add examples * move changes from 2021 api version to 2022 * move examples to 2022 * Revert "add optional hostname param to customHostnameSites RT and add examples" This reverts commit 7fc318e190de2108222dbb869d9a6219a02cae2f. * remove examples from 2021 * fix lintDiff * prettier fix * model validation fix * suppress model validation in readme Co-authored-by: Elle Tojaroon <[email protected]> * [Microsoft.Web] Add CustomDnsSuffixConfiguration to ASE (Azure#18553) * [Microsoft.Web] Add CustomDnsSuffixConfiguration to AppServiceEnvironments * Add examples * Fix style issues * Add customdnssuffix to custom-words * Add CustomDnsSuffixConfiguration to AppServiceEnvironment definition * Address remaining model validation errors * Adjusted networking configuration definitions * Addressed Arm review feedback Co-authored-by: Jarod Aerts <[email protected]> * Fork/web ant97 2022 03 01 (Azure#19259) * Added DeploymentStatus API specs and examples * change status to enum, rm extra dot * suppress missing examples for now * rm id from List Deployment Slot examples * rm location, and rename deploymentId in GetSiteDeploymentStatus examples * change model as string to true, add 202 operations for DeploymentStatus ops to spec * rename operationId to deploymentStatusId, undo deploymentId rename * add long running op, as per linter Co-authored-by: Shubham Dhond <[email protected]> Co-authored-by: Weidong Xu <[email protected]> * PublicNetworkAccess Swagger Change (Azure#19352) * add publicNetworkAccess property in site for swagger * add suppression to fix Model Validation CI * Add allowed values in the description * Add specs for Hosting Environment Maintenance Control (Azure#18691) * Add documentation for Hosting Environment Maintenance Control * Update example file name * Make UpgradePreference non-nullable * Correct indentation to 2 spaces * Fix upgradePreference indentation * Address swagger feedback. Add enums with descriptions. * Move testNotification to request body. Update description text * Fix testNotification * Fix example * Replace TestNotification parameter with TestUpgradeNotification API * Rename to TestUpgradeAvailableNotification * Added logic apps operations (Azure#18604) * Added Initial Operations and some definitions * Adding additional objects to support Logic Apps operations * Updated path and parameters to match testing * Prettier Fixes * oav validat-example fixes * Github validation fixes * Prettier * Updated examples and paths * Prettier * Revert "Prettier" This reverts commit da090adf8005e0bd6fdc2ddebbd4390e5c4b6b20. * Prettier * Fixed model validation errors * Filled in nextLinkName values * Fixing swagger lintdiff errors * added x-ms-long-running to async calls * Adds numberOfWorkers to app service plan (Azure#19475) * Adds numberOfWorkers to app service plan * Add suppression for missing ASP examples * Updating to fix workflow errors (Azure#19490) * Removed conflicting resource definition and added reference to CommonDefinitions.json (Azure#19507) * Removed conflicting resource definition and added reference to CommonDefinitions.json * Updated resource to have workflow version * Updated nextLink object to be correctly tagged (Azure#19560) * Updated nextLink object to be correctly tagged * Adding value placeholder * Updated SKU to remove conflict * Updated x-ms-enum * Added missing model * Adding more missing models * Add suppressions for missing examples (Azure#19563) Co-authored-by: Joseph Lin <[email protected]> Co-authored-by: Tanay Bhartia <[email protected]> Co-authored-by: Tanay Bhartia <[email protected]> Co-authored-by: JarodAertsMs <[email protected]> Co-authored-by: annikel <[email protected]> Co-authored-by: Paviya (Elle) Tojaroon <[email protected]> Co-authored-by: Elle Tojaroon <[email protected]> Co-authored-by: Chris Chen <[email protected]> Co-authored-by: Jarod Aerts <[email protected]> Co-authored-by: edwin-msft <[email protected]> Co-authored-by: Shubham Dhond <[email protected]> Co-authored-by: Weidong Xu <[email protected]> Co-authored-by: jiansong-msft <[email protected]> Co-authored-by: Derek Johnson <[email protected]> Co-authored-by: Alex Karcher <[email protected]>

jeremymeng added 6 commits November 12, 2021 19:42

[ServiceBus] Enable linting

5c90a41

Fixing most linting errors in tests

3d066b2

Fix more linting errors in src

b33df0c

Disable errors

3578452

- for namespace - parameter of `any`

make key const and value mutable

d6e9a52

Add .catch() block for promise

6b12053

jeremymeng requested review from deyaaeldeen, HarshaNalluru, ramya-rao-a and richardpark-msft as code owners November 12, 2021 19:50

ghost added the Service Bus label Nov 12, 2021

ramya-rao-a reviewed Nov 12, 2021

View reviewed changes

sdk/servicebus/service-bus/src/receivers/receiverCommon.ts Outdated Show resolved Hide resolved

ramya-rao-a reviewed Nov 12, 2021

View reviewed changes

sdk/servicebus/service-bus/test/internal/batchReceiver.spec.ts Outdated Show resolved Hide resolved

ramya-rao-a reviewed Nov 12, 2021

View reviewed changes

sdk/servicebus/service-bus/test/internal/unit/batchingReceiver.spec.ts Show resolved Hide resolved

ramya-rao-a reviewed Nov 12, 2021

View reviewed changes

sdk/servicebus/service-bus/test/public/amqpAnnotatedMessage.spec.ts Outdated Show resolved Hide resolved

jeremymeng added 3 commits November 13, 2021 00:43

address CR feedback

d423e55

format

03ba648

Merge remote-tracking branch 'upstream/main' into sb/eslint-fix

dbcda7f

jeremymeng mentioned this pull request Nov 19, 2021

Fix ESlint errors for servicebus #10781

Closed

ramya-rao-a approved these changes Nov 19, 2021

View reviewed changes

jeremymeng commented Nov 20, 2021

View reviewed changes

jeremymeng added 2 commits November 22, 2021 23:36

Delete test input of undefined ruleOptions for createRule() test

626fb09

Use different overloads of crateRule()

5c05d4e

Merge remote-tracking branch 'upstream/main' into sb/eslint-fix

dd49f87

HarshaNalluru approved these changes Nov 23, 2021

View reviewed changes

Update to the new rule name

88db461

in browser, the error caught doesn't have message property!

4a21a6b

AssertionError: expecting valid error.message but error is [object CloseEvent]: expected undefined to be truthy

jeremymeng enabled auto-merge (squash) November 24, 2021 23:14

jeremymeng merged commit 035fa8a into Azure:main Nov 25, 2021

jeremymeng deleted the sb/eslint-fix branch November 29, 2021 18:23

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[ServiceBus] Fix linter errors and enable linting #18670

[ServiceBus] Fix linter errors and enable linting #18670

jeremymeng commented Nov 12, 2021

ramya-rao-a commented Nov 19, 2021

azure-pipelines bot commented Nov 19, 2021

jeremymeng Nov 20, 2021

HarshaNalluru Nov 20, 2021 •

edited

Loading

HarshaNalluru Nov 20, 2021

jeremymeng Nov 22, 2021 •

edited

Loading

HarshaNalluru Nov 22, 2021

jeremymeng Nov 22, 2021

jeremymeng Nov 22, 2021

HarshaNalluru Nov 22, 2021 •

edited

Loading

jeremymeng Nov 22, 2021

jeremymeng Nov 22, 2021

jeremymeng commented Nov 22, 2021

azure-pipelines bot commented Nov 22, 2021

jeremymeng commented Nov 23, 2021

azure-pipelines bot commented Nov 23, 2021

jeremymeng commented Nov 24, 2021

azure-pipelines bot commented Nov 24, 2021

[ServiceBus] Fix linter errors and enable linting #18670

[ServiceBus] Fix linter errors and enable linting #18670

Conversation

jeremymeng commented Nov 12, 2021

ramya-rao-a commented Nov 19, 2021

azure-pipelines bot commented Nov 19, 2021

jeremymeng Nov 20, 2021

Choose a reason for hiding this comment

HarshaNalluru Nov 20, 2021 • edited Loading

Choose a reason for hiding this comment

HarshaNalluru Nov 20, 2021

Choose a reason for hiding this comment

jeremymeng Nov 22, 2021 • edited Loading

Choose a reason for hiding this comment

HarshaNalluru Nov 22, 2021

Choose a reason for hiding this comment

jeremymeng Nov 22, 2021

Choose a reason for hiding this comment

jeremymeng Nov 22, 2021

Choose a reason for hiding this comment

HarshaNalluru Nov 22, 2021 • edited Loading

Choose a reason for hiding this comment

jeremymeng Nov 22, 2021

Choose a reason for hiding this comment

jeremymeng Nov 22, 2021

Choose a reason for hiding this comment

jeremymeng commented Nov 22, 2021

azure-pipelines bot commented Nov 22, 2021

jeremymeng commented Nov 23, 2021

azure-pipelines bot commented Nov 23, 2021

jeremymeng commented Nov 24, 2021

azure-pipelines bot commented Nov 24, 2021

HarshaNalluru Nov 20, 2021 •

edited

Loading

jeremymeng Nov 22, 2021 •

edited

Loading

HarshaNalluru Nov 22, 2021 •

edited

Loading