Skip to content

Commit

Permalink
[Service Bus] Remove test reference in src and fix test failures (Azu…
Browse files Browse the repository at this point in the history
…re#8664)

* translatedError instanceof MessagingError

* Making nonRetryableError a MessagingError

* // Skipping the "disconnect" tests in the browser since they fail.
  • Loading branch information
HarshaNalluru authored May 2, 2020
1 parent 8579032 commit 893bfc9
Show file tree
Hide file tree
Showing 4 changed files with 31 additions and 6 deletions.
4 changes: 2 additions & 2 deletions sdk/servicebus/service-bus/src/core/messageReceiver.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@ import { ServiceBusMessageImpl, DispositionType, ReceiveMode } from "../serviceB
import { getUniqueName, calculateRenewAfterDuration } from "../util/utils";
import { MessageHandlerOptions } from "../models";
import { DispositionStatusOptions } from "./managementClient";
import { isMessagingError } from "../../test/utils/testUtils";

/**
* @internal
Expand Down Expand Up @@ -909,7 +908,8 @@ export class MessageReceiver extends LinkEntity {
// - Any non MessagingError because such errors do not get
// translated by `@azure/core-amqp` to a MessagingError
// - More details here - https://github.com/Azure/azure-sdk-for-js/pull/8580#discussion_r417087030
let shouldReopen = isMessagingError(translatedError) ? translatedError.retryable : true;
let shouldReopen =
translatedError instanceof MessagingError ? translatedError.retryable : true;

// Non-retryable errors that aren't caused by disconnect
// will have already been forwarded to the user's error handler.
Expand Down
8 changes: 8 additions & 0 deletions sdk/servicebus/service-bus/test/batchReceiver.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import {
} from "./utils/testutils2";
import { ReceivedMessageWithLock } from "../src/serviceBusMessage";
import { AbortController } from "@azure/abort-controller";
import { isNode } from "@azure/core-amqp";

const should = chai.should();
chai.use(chaiAsPromised);
Expand Down Expand Up @@ -1034,6 +1035,13 @@ describe("Batching - disconnects", function(): void {
return serviceBusClient.test.after();
});

beforeEach(function() {
if (!isNode) {
// Skipping the "disconnect" tests in the browser since they fail.
// More info - https://github.com/Azure/azure-sdk-for-js/pull/8664#issuecomment-622651713
this.skip();
}
});
function afterEachTest(): Promise<void> {
return serviceBusClient.test.afterEach();
}
Expand Down
9 changes: 9 additions & 0 deletions sdk/servicebus/service-bus/test/managementClient.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import chaiAsPromised from "chai-as-promised";
import { delay, Sender, Receiver, ReceivedMessageWithLock } from "../src";
import { TestClientType, TestMessage } from "./utils/testUtils";
import { ServiceBusClientForTests, createServiceBusClientForTests } from "./utils/testutils2";
import { isNode } from "@azure/core-amqp";
chai.should();
chai.use(chaiAsPromised);

Expand All @@ -27,6 +28,14 @@ describe("ManagementClient - disconnects", function(): void {
return serviceBusClient.test.after();
});

beforeEach(function() {
if (!isNode) {
// Skipping the "disconnect" tests in the browser since they fail.
// More info - https://github.com/Azure/azure-sdk-for-js/pull/8664#issuecomment-622651713
this.skip();
}
});

function afterEachTest(): Promise<void> {
return serviceBusClient.test.afterEach();
}
Expand Down
16 changes: 12 additions & 4 deletions sdk/servicebus/service-bus/test/streamingReceiver.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ import {
testPeekMsgsLength
} from "./utils/testutils2";
import { getDeliveryProperty } from "./utils/misc";
import { translate, MessagingError } from "@azure/core-amqp";
import { translate, MessagingError, isNode } from "@azure/core-amqp";

const should = chai.should();
chai.use(chaiAsPromised);
Expand Down Expand Up @@ -1214,7 +1214,7 @@ describe("Streaming - onDetached", function(): void {
await receiverIsActive;

// Simulate onDetached being called with a non-retryable error.
const nonRetryableError = translate(new Error(`I break systems.`));
const nonRetryableError = new MessagingError(`I break systems.`);
(nonRetryableError as MessagingError).retryable = false;
await (receiverClient as any)["_context"].streamingReceiver!.onDetached(
nonRetryableError,
Expand Down Expand Up @@ -1252,7 +1252,7 @@ describe("Streaming - onDetached", function(): void {
await receiverIsActive;

// Simulate onDetached being called multiple times with non-retryable errors.
const nonRetryableError = translate(new Error(`I break systems.`));
const nonRetryableError = new MessagingError(`I break systems.`);
(nonRetryableError as MessagingError).retryable = false;
await Promise.all([
(receiverClient as any)["_context"].streamingReceiver!.onDetached(nonRetryableError, true),
Expand Down Expand Up @@ -1290,7 +1290,7 @@ describe("Streaming - onDetached", function(): void {
await receiverIsActive;

// Simulate onDetached being called multiple times with non-retryable and then retryable errors.
const nonRetryableError = translate(new Error(`I break systems.`));
const nonRetryableError = new MessagingError(`I break systems.`);
(nonRetryableError as MessagingError).retryable = false;
const retryableError = new Error("I temporarily break systems.");
(retryableError as any).retryable = true;
Expand Down Expand Up @@ -1324,6 +1324,14 @@ describe("Streaming - disconnects", function(): void {
);
}

beforeEach(function() {
if (!isNode) {
// Skipping the "disconnect" tests in the browser since they fail.
// More info - https://github.com/Azure/azure-sdk-for-js/pull/8664#issuecomment-622651713
this.skip();
}
});

afterEach(async () => {
await serviceBusClient.test.afterEach();
});
Expand Down

0 comments on commit 893bfc9

Please sign in to comment.