Skip to content

Use vitest and test WebSocket browser client#71

Merged
dentarg merged 7 commits intocloudamqp:mainfrom
ngbrown-forks:feature/use-vitest-and-test-browser
Jul 21, 2023
Merged

Use vitest and test WebSocket browser client#71
dentarg merged 7 commits intocloudamqp:mainfrom
ngbrown-forks:feature/use-vitest-and-test-browser

Conversation

@ngbrown
Copy link
Copy Markdown
Contributor

@ngbrown ngbrown commented Jul 20, 2023 •
edited by dentarg
Loading

Blocks #68

Includes #70, so merge that first.

This pull request:

  • Switches to testing using RabbitMQ and websocket-tcp-relay containers hosted by docker compose.
  • Switches test framework to vitest.
  • Adds tests for the WebSocket browser client, and tests with a headless browser in CI. Currently only Chromium but Firefox and Webkit are also options.
  • Add new overload for AMQPWebSocketClient constructor to allow setting optional parameters through an init object.

The following websocket browser tests are skipped because they cause an unhandled exception and abort the remaining tests:

  • can handle frames split over socket reads
  • can republish in consume block without race condition

These skipped tests consistently raise errors like the following during test and will be fixed in #68:

RangeError: offset is out of bounds
 ❯ AMQPWebSocketClient.handleMessage http:/localhost:63315/src/amqp-websocket-client.ts:78:28

RangeError: Invalid typed array length: -1251
 ❯ AMQPWebSocketClient.handleMessage http:/localhost:63315/src/amqp-websocket-client.ts:101:28

Vitest links

This was referenced Jul 20, 2023
Merged
Merged
@dentarg
Copy link
Copy Markdown
Member

dentarg commented Jul 21, 2023

Includes #70, so merge that first.

It has been merged now

dentarg
dentarg reviewed Jul 21, 2023
View reviewed changes
Comment thread test-browser/websocket.ts Outdated
dentarg
dentarg approved these changes Jul 21, 2023
View reviewed changes
Copy link
Copy Markdown
Member

@dentarg dentarg left a comment

Choose a reason for hiding this comment

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

Nice work, a minor comment

@dentarg
Copy link
Copy Markdown
Member

dentarg commented Jul 21, 2023

I will note that the test/test.ts and test-browser/websocket.ts looks very similar, do you think it is possible and worthwhile to share code between them and inject the things that differ (settings, the client object used) or would it get too complex and hard to follow? Thinking about the maintenance going forward. No need for you to address that, just wanted to hear your thoughts about it.

@dentarg
Copy link
Copy Markdown
Member

dentarg commented Jul 21, 2023

Also, I'll let you rebase this one :)

@ngbrown ngbrown force-pushed the feature/use-vitest-and-test-browser branch from 276fea2 to f08f3be Compare July 21, 2023 14:02
@ngbrown
Copy link
Copy Markdown
Contributor Author

ngbrown commented Jul 21, 2023

I will note that the test/test.ts and test-browser/websocket.ts looks very similar, do you think it is possible and worthwhile to share code between them and inject the things that differ (settings, the client object used) or would it get too complex and hard to follow? Thinking about the maintenance going forward. No need for you to address that, just wanted to hear your thoughts about it.

I added a commit to make the files dramatically more the same, so they are easier to text compare. I added a getNewClient generator function that all the tests now use. To compare I use VS Code, select both files, and right-click "Compare Selected". I don't think the two files can be made fully the same because the Node.js specific imports would fail in the browser. There are also a few differences in the tests.

@ngbrown
Extract AMQP client creation function
85c7849 
Makes most of the tests common between Node.js and WebSockets and easier to compare.
@ngbrown ngbrown force-pushed the feature/use-vitest-and-test-browser branch from f08f3be to 85c7849 Compare July 21, 2023 14:20
@dentarg dentarg merged commit 08c1aa6 into cloudamqp:main Jul 21, 2023
@dentarg
Copy link
Copy Markdown
Member

dentarg commented Jul 21, 2023

Thanks for this excellent contribution!

@ngbrown ngbrown deleted the feature/use-vitest-and-test-browser branch July 21, 2023 22:52
@dentarg dentarg mentioned this pull request Apr 5, 2025
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@dentarg dentarg dentarg approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@ngbrown @dentarg