Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add #injectWS #276

Merged
merged 11 commits into from
Feb 19, 2024
Merged

Add #injectWS #276

merged 11 commits into from
Feb 19, 2024

Conversation

DanieleFedeli
Copy link
Contributor

@DanieleFedeli DanieleFedeli commented Nov 6, 2023 •
edited
Loading

Now is possible to invoke an websocket handler without listening.

closes #35

Checklist

@DanieleFedeli
Add #injectWS
83c40f5 
Now is possible to invoke an websocket handler without listening
airhorns
airhorns reviewed Nov 6, 2023
View reviewed changes
index.js Outdated Show resolved Hide resolved
index.js Show resolved Hide resolved
@DanieleFedeli
Copy link
Contributor Author

Can I have some CI love? on my MacOS latest with node 20 it is passing

Daniele Fedeli added 2 commits November 6, 2023 22:41
mcollina
mcollina approved these changes Nov 7, 2023
View reviewed changes
Copy link
Member

@mcollina mcollina left a comment

Choose a reason for hiding this comment

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

lgtm

remove ws.terminate() in the plugin
28730ae 
The user must manually close the ws in the test
airhorns
airhorns reviewed Nov 7, 2023
View reviewed changes
index.js Show resolved Hide resolved
add upgradeContext parameter in injectWS
efa60b6 
It allows to enhance the request made for upgrading the socket
DanieleFedeli
DanieleFedeli commented Nov 8, 2023
View reviewed changes
index.js Outdated Show resolved Hide resolved
rejects with 'Unexpected server response: <statusCode>'
70c2e4a 
Implementation as close as possibile to ws connectiong error
@DanieleFedeli
Copy link
Contributor Author

Can someone approve the workflow to run?

@mcollina
Copy link
Member

Seems CI is failing

@DanieleFedeli
Copy link
Contributor Author

I am waiting for mafintosh/stream-shift#6 to be solved.

@DanieleFedeli
Copy link
Contributor Author

DanieleFedeli commented Dec 5, 2023 •
edited
Loading

I can see that the issue linked has no activity, I will change my implementation to use mock-socket as @airhorns suggested.

@DanieleFedeli
Copy link
Contributor Author

DanieleFedeli commented Jan 25, 2024 •
edited
Loading

mafintosh/stream-shift#6 looks solved. I ran all the tests locally for node 21. The pipeline now should pass.

Screenshot 2024-01-25 alle 12 13 58

mcollina
mcollina approved these changes Jan 26, 2024
View reviewed changes
Copy link
Member

@mcollina mcollina left a comment

Choose a reason for hiding this comment

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

lgtm

@DanieleFedeli
Copy link
Contributor Author

Is there something I can do to make this mergeable?

@mcollina mcollina merged commit cb3ce0d into fastify:master Feb 19, 2024
19 checks passed
@mcollina
Copy link
Member

Nothing, me shipping it!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@airhorns airhorns airhorns left review comments

@mcollina mcollina mcollina 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.

fastify.inject like testing for WS?
3 participants
@DanieleFedeli @mcollina @airhorns