Node: Implement a test to ensure that cluster properly interoperates with the --{inspect,debug}-brk options

Created on 16 Feb 2017  Â·  12Comments  Â·  Source: nodejs/node

When started with any of the --inspect* or --debug* options, the cluster module is supposed to provide a unique port to the child processes. The existing test for this functionality seems to lack ability to test the --inspect-brk and --debug-brk flags.

It would be good to add a test (or modify the existing test) to be add this capability.

See: https://github.com/nodejs/node/pull/11386#discussion_r101198493.

good first issue help wanted

All 12 comments

Hi, I would like to give it a try. I have never contributed to any project before.

@lovepreetkaul Great, go for it!

Contributing guidelines are here, if you follow them carefully you shouldn't have a problem with the GitHub side of things. If you need any help then feel free to comment on here.

I'd be happy to help answer any questions. My suggestion would be to look into understanding how the existing test that verifies that --inspect and --debug work with cluster.

The complication that --debug-brk and --inspect-brk cause is that they would cause the child process to stop immediately. The test would have to anticipate this and somehow cause the child to continue. One suggestion on how to do that would be to look at how some tests that use --debug-brk work (e.g. test-debug-brk.js, etc.)

@lovepreetkaul are working this issue?

I would like to give it a try if no-one is working on this.

@dave-k yes please go ahead.

@ofrobots

I have read and understand how the existing tests work, but need some help with your suggestion.
How do you propose that the --debug-brk test cause the child to continue ?

@dave-k Thanks for looking into this. I suspect that it might not be necessary to cause the child to continue for the purposes of this test. I think what we need to ensure is that the child started on the correct port – which we can detect based on the stdout from the child. Once you have received this output, I think you can terminate the child process (i.e. something similar to test-debug-brk.js).

@ofrobots

I have a test for the --inspect-brk option working, but
the --debug-brk option is not supported in node v8.0.0-pre.
Should I commit the --inspect-brk test separately?

$ ./node --debug-brk
./node: bad option: --debug-brk
$ ./node --version
v8.0.0-pre

What are the next steps with this issue?
Should I commit the --inspect-brk test separately?

Hi @dave-k. I missed your last message, sorry. A test just for clustered --inspect-brk sounds fine to me. Do you want to open a PR with it?

I think the outcome of https://github.com/nodejs/node/issues/12364 was that we should probably re-add --debug-brk to master, so a test for that would still be useful, but agreed that it can be in a separate PR.

OK I'll remove the test for --debug-brk and open a PR for --inspect-brk

Was this page helpful?
0 / 5 - 0 ratings

Related issues

sandeepks1 picture sandeepks1  Â·  3Comments

stevenvachon picture stevenvachon  Â·  3Comments

cong88 picture cong88  Â·  3Comments

danialkhansari picture danialkhansari  Â·  3Comments

addaleax picture addaleax  Â·  3Comments