search

Azure / azure-event-hubs-node

Node client library for Azure Event Hubs https://azure.microsoft.com/services/event-hubs
MIT License
50 stars 46 forks source link

Update expected error in invalid partition id test #187

Closed jeremymeng closed 5 years ago

jeremymeng commented 5 years ago

Description

After upgrading to the latest amqp-common, The type of thrown errors are now ArgumentOutOfRangeError.

Reference to any github issues

amarzavery commented 5 years ago

OPS Build status updates of commit fb4623e:

:clock10: Preparing: average preparing time is 1 min(s) 1 sec(s)

jeremymeng commented 5 years ago

@ramya-rao-a

amarzavery commented 5 years ago

OPS Build status updates of commit fb4623e:

:clock10: Incremental building: average incremental building time is 20 sec(s)

amarzavery commented 5 years ago

OPS Build status updates of commit fb4623e:

:white_check_mark: Validation status: passed

File Status Preview URL Details
client/tests/client.spec.ts :white_check_mark:Succeeded

For more details, please refer to the build report.

Note: If you changed an existing file name or deleted a file, broken links in other files to the deleted or renamed file are listed only in the full build report.

amarzavery commented 5 years ago

OPS Build status updates of commit 4c7e143:

:clock10: Preparing: average preparing time is 1 min(s) 4 sec(s)

amarzavery commented 5 years ago

OPS Build status updates of commit 4c7e143:

:clock10: Incremental building: average incremental building time is 20 sec(s)

amarzavery commented 5 years ago

OPS Build status updates of commit 4c7e143:

:white_check_mark: Validation status: passed

File Status Preview URL Details
client/tests/client.spec.ts :white_check_mark:Succeeded

For more details, please refer to the build report.

Note: If you changed an existing file name or deleted a file, broken links in other files to the deleted or renamed file are listed only in the full build report.

jeremymeng commented 5 years ago

Looks like this is actually a service side change. Without upgrading the amqp-common package the invalid partition id tests failed the same.

ramya-rao-a commented 5 years ago

Looks like this is actually a service side change. Without upgrading the amqp-common package the invalid partition id tests failed the same.

Yes, I see the same.

The error message seems to be more to the point: The specified partition is invalid for an EventHub partition sender or receiver. It should be between 0 and 1

Instead of testing the error name, lets test the error message like we do in https://github.com/Azure/azure-event-hubs-node/blob/v1.0.8-EH-December2018/client/tests/hubruntime.spec.ts#L85

I'll look into if there is any spec for the request we are making.

amarzavery commented 5 years ago

OPS Build status updates of commit 60314fa:

:clock10: Preparing: average preparing time is 1 min(s) 6 sec(s)

amarzavery commented 5 years ago

OPS Build status updates of commit 60314fa:

:clock10: Incremental building: average incremental building time is 20 sec(s)

amarzavery commented 5 years ago

OPS Build status updates of commit 60314fa:

:white_check_mark: Validation status: passed

File Status Preview URL Details
client/tests/client.spec.ts :white_check_mark:Succeeded
client/tests/receiver.spec.ts :white_check_mark:Succeeded
client/tests/sender.spec.ts :white_check_mark:Succeeded

For more details, please refer to the build report.

Note: If you changed an existing file name or deleted a file, broken links in other files to the deleted or renamed file are listed only in the full build report.

jeremymeng commented 5 years ago

@ramya-rao-a Good point on verifying the error messages. I update this test and a few others to do that.