src: Remove support for --debug

[jkrems]

Checklist

• make -j4 test (UNIX), or vcbuild test (Windows) passes
• tests and/or benchmarks are included
• documentation is changed or added
• commit message follows commit guidelines

Affected core subsystem(s)

doc, src, test

I wasn't 100% sure how to deal with the tests. For now I just deleted them but I would open a ticket to investigate which of them should be ported to comparable --inspect tests.

[jkrems]

There's a lot of code in src/node.cc that is effectively dead now. But it would be a lot less disruptive (~ prone to conflicts) to remove it after #11431 lands, so I'd like to keep it around for now.

[Trott]

I wonder if test-cluster-disconnect-handles.js should be rewritten. (Not necessarily in this PR.) Thoughts?

The other tests that are removed seem like they can go.

Maybe the test-cluster-disconnect-handles.js test can be moved to known_issues until it is rewritten to work without the debugger protocol.

[jkrems]

At first I had moved all tests to test/disabled/* (before I turned it into deletions). There's definitely value in skimming through those tests to make sure we're not losing coverage. E.g. I'm not sure we have the same coverage now for --inspect that we used to have for --debug. @ofrobots did some of that work already.

My current proposal would be to go through these tests in a follow-up and port the ones that would still make sense. Happy to already move one or more to known_issues/ if we identify ones that definitely should be preserved in some form.

[ofrobots]

LGTM. Thanks for putting together this PR! The test changes would overlap/conflict with #11441, so it would make sense to land this first and rework the tests in either #11441 or a follow on.

[Trott]

CI: https://ci.nodejs.org/job/node-test-pull-request/7186/

[MylesBorins]

LGTM
Enjoy seeing all those removed lines!!!

[jkrems]

Scanning some of the other recent PR builds, the windows failure doesn't seem to be related to my change.

not ok 32 parallel/test-cli-syntax
  ---
  duration_ms: 3.250
  severity: fail
  stack: |-
    
    assert.js:82
      throw new assert.AssertionError({
      ^
    AssertionError: 'c:\\workspace\\node-test-binary-windows\\RUN_SUBSET\\3\\VS_VERSION\\vcbt2015\\label\\win10\\Release\\node.exe: either --check o === 'c:\\workspace\\node-test-binary-windows\\RUN_SUBSET\\3\\VS_VERSION\\vcbt2015\\label\\win10\\Release\\node.exe: either --check o
        at c:\workspace\node-test-binary-windows\RUN_SUBSET\3\VS_VERSION\vcbt2015\label\win10\test\parallel\test-cli-syntax.js:127:12
        at Array.forEach (native)
        at c:\workspace\node-test-binary-windows\RUN_SUBSET\3\VS_VERSION\vcbt2015\label\win10\test\parallel\test-cli-syntax.js:123:20
        at Array.forEach (native)
        at Object.<anonymous> (c:\workspace\node-test-binary-windows\RUN_SUBSET\3\VS_VERSION\vcbt2015\label\win10\test\parallel\test-cli-syntax.js:122:19)
        at Module._compile (module.js:607:30)
        at Object.Module._extensions..js (module.js:618:10)
        at Module.load (module.js:516:32)
        at tryModuleLoad (module.js:466:12)
        at Function.Module._load (module.js:458:3

[gibfahn]

Scanning some of the other recent PR builds, the windows failure doesn't seem to be related to my change.

Should be fixed in #12212

[joshgav]

Would it perhaps be better to port the tests to --inspect rather than deleting and adding them back?

[jkrems]

@joshgav The idea is to have a way to land the removal now so we can include it comfortably in 8.x. Some of those tests are already duplicated by existing tests, some will be replaced by new --inspect tests, and pulling that apart correctly is something I'd rather not do under time pressure (as would be the case with this PR).

P.S.: Also there's overlap with test changes as part of #11441, making things even more complicated.

[targos]

Landed in 47f8f74