-
Notifications
You must be signed in to change notification settings - Fork 29.6k
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.
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
test: improve code coverage for streams/duplexify #41862
test: improve code coverage for streams/duplexify #41862
Conversation
If we have to use |
(We can also land this and try to refactor things later to use public APIs where possible.) |
If could is unreachable, you can delete it. If you want to be cautious, you can replace it with an internal assertion. (https://github.com/nodejs/node/blob/e46c680bf2b211bbd52cf959ca17ee98c7f657f5/lib/internal/cluster/shared_handle.js is an example of something that uses the internal assertion.) |
Signed-off-by: Erick Wendel <[email protected]>
Just updated it using Public APIs |
Perfect, I'll create another PR to land this, ok? |
Signed-off-by: Erick Wendel <[email protected]>
Signed-off-by: Erick Wendel <[email protected]>
Landed in 0185464 |
PR-URL: nodejs#41862 Reviewed-By: Benjamin Gruenbaum <[email protected]> Reviewed-By: Juan José Arboleda <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Luigi Pinca <[email protected]>
PR-URL: nodejs#41862 Reviewed-By: Benjamin Gruenbaum <[email protected]> Reviewed-By: Juan José Arboleda <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Luigi Pinca <[email protected]>
PR-URL: #41862 Reviewed-By: Benjamin Gruenbaum <[email protected]> Reviewed-By: Juan José Arboleda <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Luigi Pinca <[email protected]>
@ErickWendel this breaks tests when landing in v16.x-staging. Do you mind creating a backport PR for the v16.x line? Thank you |
Heyy. Sure! Do you have an example of how to do it? I'm not sure if it's just to make it works on the v16.x |
PR-URL: #41862 Backport-PR-URL: #42788 Reviewed-By: Benjamin Gruenbaum <[email protected]> Reviewed-By: Juan José Arboleda <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Luigi Pinca <[email protected]>
PR-URL: #41862 Backport-PR-URL: #42788 Reviewed-By: Benjamin Gruenbaum <[email protected]> Reviewed-By: Juan José Arboleda <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Luigi Pinca <[email protected]>
PR-URL: nodejs/node#41862 Backport-PR-URL: nodejs/node#42788 Reviewed-By: Benjamin Gruenbaum <[email protected]> Reviewed-By: Juan José Arboleda <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Luigi Pinca <[email protected]>
Improve code coverage for streams duplexify.js
Refs:
I would add that those lines are unreachable:
In those cases, what could I do to cover it?