Skip to content

Conversation

gismya
Copy link
Contributor

@gismya gismya commented May 22, 2024

  • I have added automatic tests where applicable
  • The PR title is suitable as a release note
  • The PR contains a description of what has been changed
  • The description contains manual test instructions

Changes

PR to address #238. Adds a disconnect method to the event hub

Test

@gismya gismya requested a review from a team as a code owner May 22, 2024 11:41
@ffMathy
Copy link
Contributor

ffMathy commented May 22, 2024

Thank you so much for this! ❤️


test("Disconnecting should disconnect the socket", () => {
eventHub.disconnect();
expect(disconnectCalled).toBe(true);
Copy link
Contributor

@jimmycallin jimmycallin May 22, 2024

Choose a reason for hiding this comment

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

Suggested change
expect(disconnectCalled).toBe(true);
expect(eventHub._socketIo.disconnect).toHaveBeenCalled();

think you can do something like this, a bit more idiomatic

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, I'm setting _socketIo to undefined as part of the disconnect.

Copy link
Contributor

@jimmycallin jimmycallin May 22, 2024

Choose a reason for hiding this comment

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

ah, I guess you then could do this:

const disconnectFn = vi.fn() ... disconnect: disconnectFn, ... expect(disconnectFn).toHaveBeenCalled() 

but we can go with this approach as well

@gismya gismya merged commit 817259e into main May 23, 2024
@gismya gismya deleted the explicitly-disconnect-event-hub branch May 23, 2024 05:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

3 participants