Skip to content
This repository has been archived by the owner on Dec 18, 2018. It is now read-only.

[TS] Reject handshake if connection closes before response #3000

Merged
merged 1 commit into from
Sep 27, 2018

Conversation

BrennanConroy
Copy link
Member

Updated Jest reference because I was finding it impossible to debug a test failure and found similar issues filed against Jest that have been fixed in 23.X.X. Side-effect is rejects.toThrow no longer works in some of our tests because of another bug jestjs/jest#6675. I can revert the Jest change if we want now that I fixed the test.

try {
let startCompleted = false;
const startPromise = hubConnection.start().then(() => startCompleted = true);
expect(startCompleted).toBe(false);
Copy link
Member

@davidfowl davidfowl Sep 22, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't this be asserted after the await?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, it's asserting that startCompleted is false. It will be true after the await. This is to ensure that the promise doesn't complete until the handshake is received.

@davidfowl
Copy link
Member

Microsoft.AspNetCore.SignalR.Tests.HubConnectionHandlerTests.SendToOthersInGroup(hubType: typeof(Microsoft.AspNetCore.SignalR.Tests.HubT))
13:07:39   Error Message:
13:07:39    System.TimeoutException : Operation in SendToOthersInGroup timed out at /_/test/Microsoft.AspNetCore.SignalR.Tests/HubConnectionHandlerTests.cs:1327

@davidfowl
Copy link
Member

@dotnet-bot test Windows Release x64 Build

"@types/node": "^8.5.2",
"@types/webpack": "^4.4.0",
"jest": "^22.4.3",
"jest": "^23.6.0",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FYI @Eilon. Updated devDependencies (internal use only, not distributed)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Up-roved.

@BrennanConroy BrennanConroy merged commit 3c8c8ba into release/2.2 Sep 27, 2018
@BrennanConroy BrennanConroy deleted the brecon/handshakeClose branch September 27, 2018 15:29
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants