-
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
test: remove obsolete debugger tests #15139
Conversation
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.
Overall LGTM but two fixtures are still referenced in the debugger documentation.
test/fixtures/break-in-module/main.js
test/fixtures/break-in-module/mod.js
These references should be removed as well.
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.
test/README would need to be updated too. Other than that and Ruben's comment, LGTM.
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.
LGTM with @BridgeAR's comment addressed.
Ping @Trott |
Removed the fixtures from the documentation per @BridgeAR comment, and removed the entry for these tests from the |
The tests in `test/debugger` all fail since the removal of the pre-inspector debugger (if they weren't already failing). They do not run in CI (probably because they were never reliable). Remove them and associated fixtures.
Landed in d38e643 |
The tests in `test/debugger` all fail since the removal of the pre-inspector debugger (if they weren't already failing). They do not run in CI (probably because they were never reliable). Remove them and associated fixtures. PR-URL: #15139 Reviewed-By: Eugene Ostroukhov <eostroukhov@google.com> Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Yuta Hiroto <hello@about-hiroppy.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
The tests in `test/debugger` all fail since the removal of the pre-inspector debugger (if they weren't already failing). They do not run in CI (probably because they were never reliable). Remove them and associated fixtures. PR-URL: nodejs/node#15139 Reviewed-By: Eugene Ostroukhov <eostroukhov@google.com> Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Yuta Hiroto <hello@about-hiroppy.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
This would need to be backported for v8.x |
The tests in `test/debugger` all fail since the removal of the pre-inspector debugger (if they weren't already failing). They do not run in CI (probably because they were never reliable). Remove them and associated fixtures. PR-URL: #15139 Reviewed-By: Eugene Ostroukhov <eostroukhov@google.com> Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Yuta Hiroto <hello@about-hiroppy.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
The tests in `test/debugger` all fail since the removal of the pre-inspector debugger (if they weren't already failing). They do not run in CI (probably because they were never reliable). Remove them and associated fixtures. PR-URL: nodejs/node#15139 Reviewed-By: Eugene Ostroukhov <eostroukhov@google.com> Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Yuta Hiroto <hello@about-hiroppy.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
@jasnell It looks like this is already on v8.x-staging and v8.6.0-proposed. I'm going to remove your backport-requested label because I think it's in error. But if I'm wrong about that, add it again and educate me. Thanks! |
Should this be backported to |
The tests in
test/debugger
all fail since the removal of thepre-inspector debugger (if they weren't already failing). They do not
run in CI (probably because they were never reliable). Remove them and
associated fixtures.
Checklist
make -j4 test
(UNIX), orvcbuild test
(Windows) passesAffected core subsystem(s)
test debugger