[event-hubs] Enable unit-test for node.js in CI by chradek · Pull Request #17492 · Azure/azure-sdk-for-js

chradek · 2021-09-07T16:53:18Z

Replaces #14568

Uses the @azure/mock-hub package to start a local mocked version of Event Hubs to run tests against.

There are a lot of whitespace changes (indentation) so I recommend viewing with whitespace changes hidden.

…ock service

…nst the mock test target, to allow packages that read environment variables (e.g. identity) to function properly

richardpark-msft · 2021-09-07T20:32:25Z

CC: @jhendrixMSFT

(this is the CI-friendly eventhubs tester I mentioned the other day)

richardpark-msft · 2021-09-07T21:53:51Z

sdk/eventhub/event-hubs/scripts/generateCerts.js

+  "-out",
+  `${resolvePath(certsDirectory, "my-server.crt.pem")}`,
+  "-days",
+  "5"


We need to work on our live test times.

sdk/eventhub/event-hubs/test/public/utils/wrapper.ts

richardpark-msft · 2021-09-07T21:58:28Z

sdk/eventhub/mock-hub/src/server/mockServer.ts


    for (const m of incomingMessages) {
-      if (m.body?.content) {
+      if (m.body.multiple && m.body?.content) {


Just curious what this is about. Is this the "message was too big for a single transfer frame" or similar condition?

This is for batched messages. When I initially wrote this, I noticed m.body.content was an array if an AMQP message contained multiple messages. But it turns out sequence bodies also cause content to be an array, so without the multiple check I was converting every item in content to a separate message when the body was a sequence.

Add a comment and I think you're good.

sdk/eventhub/mock-hub/src/services/eventHubs.ts

HarshaNalluru · 2021-09-15T17:29:08Z

sdk/eventhub/event-hubs/karma.conf.js

      "EVENTHUB_NAME",
-      "IOTHUB_EH_COMPATIBLE_CONNECTION_STRING"
+      "IOTHUB_EH_COMPATIBLE_CONNECTION_STRING",
+      "TEST_TARGET"


Make it TEST_MODE?

The multiversion test helper makes some assumptions with TEST_MODE that causes issue with what we're trying to do. I'm using the multiversion test helper to facilitate testing mock and live versions of the service, not multiple API versions of the service.

This works fine if I don't set TEST_MODE at all, but things break if I set TEST_MODE to live because it assumes I want different behavior than just running my tests against the live service. So, that's why I made TEST_TARGET instead.

HarshaNalluru · 2021-09-15T20:02:01Z

sdk/eventhub/event-hubs/test/internal/auth.spec.ts

-
-        await producerClient.close();
-      });
+testWithServiceTypes("internal/auth.spec.ts", (serviceVersion) => {


why do you need the file paths?

Strictly speaking, they aren't needed. However I found it made it really easy to find out where a test that fails lives.

[NIT] Test objects/blocks do have the file path in them, which is what I used to locate the recordings folder in the recorder.
Since you have the wrapper and using describe blocks, I'm wondering if you can automate that instead of writing in each of the files.

Do you mean there's an API to get the file path? Does it also work for browsers?

Yes.

Something like this.

Here, the testConext is the this object of the function from it test.

Hmm, this does mean I need to wrap my describe(this.file, ...) in another describe so that I have access to the mocha suite context object when defining the describe that has the file path. I decided to go ahead and do that.

Unfortunately, that does not seem to work in browsers, but I'll accept that.

Interesting

richardpark-msft

Looks good.

Just that one spot where I think a comment is needed to explain the .content hoisting, but otherwise, no issues.

…d on current mocha test suite context to avoid hard-coding filepaths

chradek · 2021-09-15T23:09:31Z

/azp run js - event-hubs - tests

azure-pipelines · 2021-09-15T23:09:44Z

Azure Pipelines successfully started running 1 pipeline(s).

chradek · 2021-09-15T23:31:03Z

/azp run js - event-hubs - tests

azure-pipelines · 2021-09-15T23:31:16Z

Azure Pipelines successfully started running 1 pipeline(s).

… set to live

…min-max environments

chradek · 2021-09-17T16:35:15Z

I had to use an additional environment variable/add a check since TEST_MODE was being set automatically in some of our live test jobs, but live tests also pass:
https://dev.azure.com/azure-sdk/internal/_build/results?buildId=1101647&view=results

ghost · 2021-09-17T16:42:19Z

Hello @chradek!

Because this pull request has the auto-merge label, I will be glad to assist with helping to merge this pull request once all check-in policies pass.

p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (`@msftbot`) and give me an instruction to get started! Learn more here.

Microsoft.App version 2022-01-01-preview (Azure#17820) * New Swagger Spec File * New Swagger Example Spec File * New Readme Config File * New Azure AZ Readme Config File * New Azure CLI Readme Config File * New Go Language Readme Config File * New Python Language Readme Config File * New Typescript Language Readme Config File * New C# Language Readme Config File * Adding new API version 2022-01-01-preview for the new service Microsoft.App (Azure#17135) * Adding swagger and examples * Fix samples * Fix linting errors * fix errors * fix more errors * prettier fixes * Fix the VNET properties * fix the vnet props * Attempt to remove x-ms-identifiers * Add x-ms-identifiers back * Revert "Add x-ms-identifiers back" This reverts commit 44525ab5ace45d9cb1bc85bee2751f015dcaffc6. * Addsourcecontrolapis (Azure#17287) * add sourcecontrol apis * remove space * prettier fix * typo * avocado fix * lint fix * add replicas apis (Azure#17501) * Remove Dapr components from ContainerApp spec. Not breaking because the version hasn't been released yet. (Azure#17479) * Remove dapr components from the ContainerApp object * Fix example * add descriptions * fix * change the auto-rest parameters * Support volume mounts for containerApp (Azure#17530) * add volume mounts * add identifier * refine volume definition * Fix samples (Azure#17534) * Adding managed identity (Azure#17569) * Adding managed identity * prettier fix. * Microsof.app 2022 01 01 preview/add custom domains (Azure#17385) * add support for Custom domains and certificates * add Certificates * Ccertificate as child resource of Managed Env. * Support default custom domain * PUT/DELETE certificate are not long-running * Add Custom Domain Verification Id * domains for all revisions and adding examples * missing examples * one more missing example * Examples+missing paths * Adding missing envelope properties * Addressing PR comments * Removing AKV and Free cert related properties * Prettier and semantic validation fixes * Fixing semantic validations and examples * More fixes * Addressing more PR comments * Updating examples * fixing type * fixing types * Extra properties and responses * misplaced response * whitespace * fix security section * fixing ManageEnvironment securityDefinitions * add 204 delete response * Removing virtual IP and IP Based option * change modelAsString * Addressing ARM PR comments * Removing 404 response from example * renaming custom hostname analysis operation * mark certificate as tracked resource * fix sample * Use Certificate Id instead of Certificate name Co-authored-by: Ruslan Yakushev 🚴 <ruslany@microsoft.com> Co-authored-by: vinisoto <vinisoto@hotmail.com> * Add new properties for ContainerApp (Azure#17483) * add ephemeral storage * add outbound ip * add listsecrets * fix CI * fix example * fix * add identifier * fix * add example * mars as secret * Add storages operation for managedEnvironment (Azure#17545) * add storage * fix * fix typo * Add EasyAuth configuration APIs for ContainerApp (Azure#17492) * Add Easy Auth Config related APIs for ContainerApp * Use common type ProxyResource * Update description * update per validation * typo fix * fix validation error * Update sample and description * Update because ARM prefer string than boolean * Add static web identity provider * Add container probes (Azure#17535) * Add container probes * minor fix * Use execute instead of exec'd, add identifier * remove exec from preview * use integer instead of intorstring * Add `internal` property under VnetConfiguration for internalOnly environments (Azure#17656) * Add internal property under VnetConfiguration for internalOnly environments * Update examples * Add Dapr Components collection APIs (Azure#17552) * Add daprComponents * update readme * Fix linting errors * More lint fixes * prettier fixes * make dapr component a tracked resource * fix the patch * fix lint errors * Revert "fix lint errors" This reverts commit 045f1d94bddf3527eab98b7a376070ab30fdd760. * Revert "fix the patch" This reverts commit 14521103e848e09762185f832c0270c16ed16efd. * Revert "make dapr component a tracked resource" This reverts commit 239268eda070ff37f26e8a772adacdd26bbf0937. * Fix linter issues * fix wrong fix * fix linter * fix the operationids (Azure#17809) * correct resource name (Azure#17846) * Add custom open id providers support (Azure#17855) * Add custom open id providers support * Update description Co-authored-by: Xingjian Wang <79332479+xwang971@users.noreply.github.com> Co-authored-by: Zunli Hu <zuh@microsoft.com> Co-authored-by: Vaclav Turecek <vturecek@microsoft.com> Co-authored-by: Vini Soto <18271663+vinisoto@users.noreply.github.com> Co-authored-by: vinisoto <vinisoto@hotmail.com> Co-authored-by: erich-wang <eriwan@microsoft.com> Co-authored-by: Mike Vu <mdhvu@uwaterloo.ca> Co-authored-by: Sanchit Mehta <sanmeht@microsoft.com>

chradek added 5 commits September 3, 2021 19:28

[mock-hub] make updates and fixes to pass JavaScript CI tests

c768669

[event-hubs] add wrapper and mock service for testing

bb18612

[event-hubs] update npm scripts and dependencies to support running m…

f9ecef6

…ock service

[event-hubs] update tests to use test target wrapper

6eaee29

[event-hubs] enable unit-test in ci

ba970cf

ghost added the Event Hubs label Sep 7, 2021

chradek added 5 commits September 7, 2021 10:51

[event-hubs] fix eslint errors

9b9951f

[event-hubs] tests now inject environment variables when running agai…

0691e8f

…nst the mock test target, to allow packages that read environment variables (e.g. identity) to function properly

[mock-hub] update samples

1e172da

[event-hubs] improve token credential mock

cdcd0e6

[event-hubs] fix lint errors

702e273

chradek marked this pull request as ready for review September 7, 2021 19:58

chradek requested review from HarshaNalluru, praveenkuttappan and richardpark-msft as code owners September 7, 2021 19:58

chradek mentioned this pull request Sep 7, 2021

[event-hubs] add unit-test:node script using mock-hub #14568

Closed

richardpark-msft reviewed Sep 7, 2021

View reviewed changes

sdk/eventhub/event-hubs/test/public/utils/wrapper.ts Outdated Show resolved Hide resolved

richardpark-msft reviewed Sep 7, 2021

View reviewed changes

sdk/eventhub/event-hubs/test/public/utils/wrapper.ts Outdated Show resolved Hide resolved

richardpark-msft reviewed Sep 7, 2021

View reviewed changes

sdk/eventhub/mock-hub/src/services/eventHubs.ts Show resolved Hide resolved

chradek added 2 commits September 7, 2021 19:30

[event-hubs] rename "wrapper" test helper to "testWithServiceTypes"

d5c5739

fix imports

564f72c

HarshaNalluru reviewed Sep 15, 2021

View reviewed changes

richardpark-msft approved these changes Sep 15, 2021

View reviewed changes

chradek added 2 commits September 15, 2021 16:02

[event-hubs] improve testWithServiceTypes helper to get filepath base…

26fa11c

…d on current mocha test suite context to avoid hard-coding filepaths

[mock-hub] add context around message.body.multiple check

565dd30

Merge remote-tracking branch 'upstream/main' into eh-mock-hub-merger

c0a57c8

HarshaNalluru approved these changes Sep 15, 2021

View reviewed changes

chradek added 4 commits September 16, 2021 12:58

debugging - add console logs

e0c63fd

debugging - more logs

4a6a66f

[event-hubs] disable testing multiple service types when TEST_MODE is…

2a1b174

… set to live

[event-hubs] add additional serviceVersion check in test wrapper for …

8763cb3

…min-max environments

[event-hubs] remove console.log from test

d93496a

chradek added the auto-merge label Sep 17, 2021

ghost merged commit bcb1827 into Azure:main Sep 17, 2021

This pull request was closed.

Conversation

chradek commented Sep 7, 2021

Uh oh!

richardpark-msft commented Sep 7, 2021

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

HarshaNalluru Sep 15, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

HarshaNalluru Sep 15, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

richardpark-msft left a comment

Choose a reason for hiding this comment

Uh oh!

chradek commented Sep 15, 2021

Uh oh!

azure-pipelines bot commented Sep 15, 2021

Uh oh!

chradek commented Sep 15, 2021

Uh oh!

azure-pipelines bot commented Sep 15, 2021

Uh oh!

chradek commented Sep 17, 2021

Uh oh!

ghost commented Sep 17, 2021

p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (@msftbot) and give me an instruction to get started! Learn more here.

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

HarshaNalluru Sep 15, 2021 •

edited

Loading

HarshaNalluru Sep 15, 2021 •

edited

Loading

p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (`@msftbot`) and give me an instruction to get started! Learn more here.