[Service Bus] Update Delays in Streaming Receiver

[HarshaNalluru]

Issue
#1147

Description

• Can reuse the DelayStreaming function in all the streaming receiver tests
• Fixed number of iterations is being changed to maxDelay
• delayBetweenRetries, maxDelay are constants and will remain same for all the tests (easier to update them if needed)
• This new function can be used anywhere (not just for streaming receiver)

All the tests in Streaming Receiver passed after the changes(local).
Thanks to @mikeharder - #1147 (comment) .

[ramya-rao-a]

Lets do the same in all tests that use the streaming receiver

[ramya-rao-a]

Maybe we shouldnt remove this check.
What if we got more than 1? then the test should fail isnt it?

[ramya-rao-a]

Same for other cases where we are removing the check

[HarshaNalluru]

The only way we can get more than 1 message is we receive a message after receivedMsgs.length === 1 is verified.
Then, we should also give more scope to receive messages - instead of returning true right after the receivedMsgs.length === 1 check, we can add await delay(retry)[in the DelayStreaming module] and then return true.
Does this make sense ?

[ramya-rao-a]

Ah, thats ok, we can keep it the way it is

[HarshaNalluru]

@ramya-rao-a You want me keep this should.equal(receivedMsgs.length, 1); ?

[ramya-rao-a]

Github is acting weird... I am adding a reply to one comment thread.. it is appearing in another :)

When waiting for the 1 sec inside the loop, it can happen that the receiver gets the second message. So, we should keep the should.equal(receivedMsgs.length, 1) check

[HarshaNalluru]

We are not waiting for 1 sec inside the loop.

export async function checkWithTimeout(
  predicate: () => boolean,
  delayBetweenRetriesInMilliseconds: number = 1000,
  maxWaitTimeInMilliseconds: number = 10000
): Promise<boolean> {
  const maxTime = Date.now() + maxWaitTimeInMilliseconds;
  while (Date.now() < maxTime) {
    if (predicate()) return true;
    await delay(delayBetweenRetriesInMilliseconds);
  }
  return false;
}

• We return true as soon as receivedMsgs.length becomes 1.
• The only way we can get more than 1 message is if we receive a message after receivedMsgs.length === 1 is verified.

---

I suggest we add a await delay(delayBetweenRetriesInMilliseconds);
just before we return true and then keep the check should.equal(receivedMsgs.length, 1); in the tests.

Updated checkWithTimeout looks like

export async function checkWithTimeout(
  predicate: () => boolean,
  delayBetweenRetriesInMilliseconds: number = 1000,
  maxWaitTimeInMilliseconds: number = 10000
): Promise<boolean> {
  const maxTime = Date.now() + maxWaitTimeInMilliseconds;
  while (Date.now() < maxTime) {
    if (predicate()){
        await delay(delayBetweenRetriesInMilliseconds);
        return true;
    } 
    await delay(delayBetweenRetriesInMilliseconds);
  }
  return false;
}

My only concern with this is, it basically increases the the time taken by each test by 1 second.

[mikeharder]

checkWithTimeout() should definitely return immediately when the predicate is true.

[mikeharder]

My feedback is limited to testUtils.ts. The code looks correct to me. I suggested a few optional naming changes.

[mikeharder]

Changing my review from "approve" to "comment"

Meant to review as "comment" instead of "approve"