-
Notifications
You must be signed in to change notification settings - Fork 30k
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
inspector: add NodeRuntime.waitingForDebugger event #51560
inspector: add NodeRuntime.waitingForDebugger event #51560
Conversation
Review requested:
|
linter seems to fail |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This appears to be failing in Linters.
7c3a8e3
to
ac5867d
Compare
`NodeRuntime.waitingForDebugger` is a new Inspector Protocol event that will fire when the process being inspected is waiting for the debugger (for example, when `inspector.waitForDebugger()` is called). This allows inspecting processes to know when the inspected process is waiting for a `Runtime.runIfWaitingForDebugger` message to resume execution. It allows tooling to resume execution of the inspected process as soon as it deems necessary, without having to guess if the inspected process is waiting or not, making the workflow more deterministic. With a more deterministic workflow, it is possible to update Node.js core tests to avoid race conditions that can cause flakiness. Therefore, tests were also changed as following: * Remove no-op Runtime.runIfWaitingForDebugger from tests that don't need it * Use NodeRuntime.waitingForDebugger in all tests that need Runtime.runIfWaitingForDebugger, to ensure order of operations is predictable and correct * Simplify test-inspector-multisession-ws There might be value in adding `NodeWorker.waitingForDebugger` in a future patch, but as of right now, no Node.js core inspector tests using worker threads are not failing due to race conditions. Fixes: nodejs#34730
ac5867d
to
9ec34cb
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
RSLGTM
Stress test for flaked tests on RHEL boxes: https://ci.nodejs.org/view/Stress/job/node-stress-single-test/475/ (reference nodejs/reliability#787) |
Landed in 0161ad0 |
`NodeRuntime.waitingForDebugger` is a new Inspector Protocol event that will fire when the process being inspected is waiting for the debugger (for example, when `inspector.waitForDebugger()` is called). This allows inspecting processes to know when the inspected process is waiting for a `Runtime.runIfWaitingForDebugger` message to resume execution. It allows tooling to resume execution of the inspected process as soon as it deems necessary, without having to guess if the inspected process is waiting or not, making the workflow more deterministic. With a more deterministic workflow, it is possible to update Node.js core tests to avoid race conditions that can cause flakiness. Therefore, tests were also changed as following: * Remove no-op Runtime.runIfWaitingForDebugger from tests that don't need it * Use NodeRuntime.waitingForDebugger in all tests that need Runtime.runIfWaitingForDebugger, to ensure order of operations is predictable and correct * Simplify test-inspector-multisession-ws There might be value in adding `NodeWorker.waitingForDebugger` in a future patch, but as of right now, no Node.js core inspector tests using worker threads are not failing due to race conditions. Fixes: #34730 PR-URL: #51560 Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com> Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
`NodeRuntime.waitingForDebugger` is a new Inspector Protocol event that will fire when the process being inspected is waiting for the debugger (for example, when `inspector.waitForDebugger()` is called). This allows inspecting processes to know when the inspected process is waiting for a `Runtime.runIfWaitingForDebugger` message to resume execution. It allows tooling to resume execution of the inspected process as soon as it deems necessary, without having to guess if the inspected process is waiting or not, making the workflow more deterministic. With a more deterministic workflow, it is possible to update Node.js core tests to avoid race conditions that can cause flakiness. Therefore, tests were also changed as following: * Remove no-op Runtime.runIfWaitingForDebugger from tests that don't need it * Use NodeRuntime.waitingForDebugger in all tests that need Runtime.runIfWaitingForDebugger, to ensure order of operations is predictable and correct * Simplify test-inspector-multisession-ws There might be value in adding `NodeWorker.waitingForDebugger` in a future patch, but as of right now, no Node.js core inspector tests using worker threads are not failing due to race conditions. Fixes: #34730 PR-URL: #51560 Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com> Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
`NodeRuntime.waitingForDebugger` is a new Inspector Protocol event that will fire when the process being inspected is waiting for the debugger (for example, when `inspector.waitForDebugger()` is called). This allows inspecting processes to know when the inspected process is waiting for a `Runtime.runIfWaitingForDebugger` message to resume execution. It allows tooling to resume execution of the inspected process as soon as it deems necessary, without having to guess if the inspected process is waiting or not, making the workflow more deterministic. With a more deterministic workflow, it is possible to update Node.js core tests to avoid race conditions that can cause flakiness. Therefore, tests were also changed as following: * Remove no-op Runtime.runIfWaitingForDebugger from tests that don't need it * Use NodeRuntime.waitingForDebugger in all tests that need Runtime.runIfWaitingForDebugger, to ensure order of operations is predictable and correct * Simplify test-inspector-multisession-ws There might be value in adding `NodeWorker.waitingForDebugger` in a future patch, but as of right now, no Node.js core inspector tests using worker threads are not failing due to race conditions. Fixes: #34730 PR-URL: #51560 Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com> Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
`NodeRuntime.waitingForDebugger` is a new Inspector Protocol event that will fire when the process being inspected is waiting for the debugger (for example, when `inspector.waitForDebugger()` is called). This allows inspecting processes to know when the inspected process is waiting for a `Runtime.runIfWaitingForDebugger` message to resume execution. It allows tooling to resume execution of the inspected process as soon as it deems necessary, without having to guess if the inspected process is waiting or not, making the workflow more deterministic. With a more deterministic workflow, it is possible to update Node.js core tests to avoid race conditions that can cause flakiness. Therefore, tests were also changed as following: * Remove no-op Runtime.runIfWaitingForDebugger from tests that don't need it * Use NodeRuntime.waitingForDebugger in all tests that need Runtime.runIfWaitingForDebugger, to ensure order of operations is predictable and correct * Simplify test-inspector-multisession-ws There might be value in adding `NodeWorker.waitingForDebugger` in a future patch, but as of right now, no Node.js core inspector tests using worker threads are not failing due to race conditions. Fixes: #34730 PR-URL: #51560 Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com> Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
`NodeRuntime.waitingForDebugger` is a new Inspector Protocol event that will fire when the process being inspected is waiting for the debugger (for example, when `inspector.waitForDebugger()` is called). This allows inspecting processes to know when the inspected process is waiting for a `Runtime.runIfWaitingForDebugger` message to resume execution. It allows tooling to resume execution of the inspected process as soon as it deems necessary, without having to guess if the inspected process is waiting or not, making the workflow more deterministic. With a more deterministic workflow, it is possible to update Node.js core tests to avoid race conditions that can cause flakiness. Therefore, tests were also changed as following: * Remove no-op Runtime.runIfWaitingForDebugger from tests that don't need it * Use NodeRuntime.waitingForDebugger in all tests that need Runtime.runIfWaitingForDebugger, to ensure order of operations is predictable and correct * Simplify test-inspector-multisession-ws There might be value in adding `NodeWorker.waitingForDebugger` in a future patch, but as of right now, no Node.js core inspector tests using worker threads are not failing due to race conditions. Fixes: #34730 PR-URL: #51560 Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com> Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
`NodeRuntime.waitingForDebugger` is a new Inspector Protocol event that will fire when the process being inspected is waiting for the debugger (for example, when `inspector.waitForDebugger()` is called). This allows inspecting processes to know when the inspected process is waiting for a `Runtime.runIfWaitingForDebugger` message to resume execution. It allows tooling to resume execution of the inspected process as soon as it deems necessary, without having to guess if the inspected process is waiting or not, making the workflow more deterministic. With a more deterministic workflow, it is possible to update Node.js core tests to avoid race conditions that can cause flakiness. Therefore, tests were also changed as following: * Remove no-op Runtime.runIfWaitingForDebugger from tests that don't need it * Use NodeRuntime.waitingForDebugger in all tests that need Runtime.runIfWaitingForDebugger, to ensure order of operations is predictable and correct * Simplify test-inspector-multisession-ws There might be value in adding `NodeWorker.waitingForDebugger` in a future patch, but as of right now, no Node.js core inspector tests using worker threads are not failing due to race conditions. Fixes: nodejs#34730 PR-URL: nodejs#51560 Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com> Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Use the `NodeRuntime.waitingForDebugger` event. Refs: nodejs#51560
Refs: nodejs#51560 PR-URL: nodejs#54827 Reviewed-By: Chengzhong Wu <legendecas@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
Use the `NodeRuntime.waitingForDebugger` event. Refs: nodejs#51560 PR-URL: nodejs#55058 Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com> Reviewed-By: Chemi Atlow <chemi@atlow.co.il> Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Refs: nodejs#51560 PR-URL: nodejs#54827 Reviewed-By: Chengzhong Wu <legendecas@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
Use the `NodeRuntime.waitingForDebugger` event. Refs: nodejs#51560 PR-URL: nodejs#55058 Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com> Reviewed-By: Chemi Atlow <chemi@atlow.co.il> Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
NodeRuntime.waitingForDebugger
is a new Inspector Protocol event that will fire when the process being inspected is waiting for the debugger (for example, wheninspector.waitForDebugger()
is called). This allows inspecting processes to know when the inspected process is waiting for aRuntime.runIfWaitingForDebugger
message to resume execution. It allows tooling to resume execution of the inspected process as soon as it deems necessary, without having to guess if the inspected process is waiting or not, making the workflow more deterministic. With a more deterministic workflow, it is possible to update Node.js core tests to avoid race conditions that can cause flakiness. Therefore, tests were also changed as following:There might be value in adding
NodeWorker.waitingForDebugger
in a future patch, but as of right now, no Node.js core inspector tests using worker threads are failing due to race conditions.Fixes: #34730