fix #342 set child process debug port to available

[develar]

We decided to support AVA debug (#342) without any efforts on your side.

But:

• debugger: bind to random port with --debug-port=0 #5025 is not yet merged into NodeJS.
• IntelliJ JS Debugger doesn't support yet =0, and when will be supported, it will take time to be available for users in the generally available release.

Unfortunately, although idea of fix is very simple (getPort().then), change is complex because existing code adds custom context-dependent methods to Promise. And Github diff view is very unclear compared to IntelliJ IDEA.

What's done:

• if debug-brk= or debug= in the main process args, find available port and change debug arg.
• fork now returns object with custom methods (on, run) and promise field. We don't add custom methods to promise anymore.
  • to simplify test code (and reduce changes), method then added to returned object. Test-only — in production we use promise field.
  • send is not used, as far I see, but I didn't delete it. Should I do it?

Please note — as it stated in my comment, debugging will be still not possible due to another issues. Solving sourcemap issue is out of this PR scope.

PS: AVA is so awesome, that multi-process debug was reimplemented in the IntelliJ JS Debugger to avoid multiple debug tabs (IntelliJ Platform 146+).

[sindresorhus]

Tip for reviewing this, append ?w=1 to the diff URL to ignore whitespace changes: https://github.com/sindresorhus/ava/pull/613/files?w=1

[sindresorhus]

This should be placed above the relative requires (on line 17).

[develar]

Fixed.

[sindresorhus]

Nice! Thanks for working on this @develar :)

[sindresorhus]

Can you add some tests?

[sindresorhus]

fork now returns object with custom methods (on, run) and promise field. We don't add custom methods to promise anymore.

What's the benefit of this?

[develar]

fork now returns object with custom methods (on, run) and promise field. We don't add custom methods to promise anymore.

What's the benefit of this?

My first approach was just add getPort().then And then I realized, that existing code doesn't work with nested promise — .on is not a function. But we cannot return promise from fork directly until promise of getPort() is not resolved — so, we return nested promise.

[novemberborn]

send is not used, as far I see, but I didn't delete it. Should I do it?

It's used as of a few hours ago 😜 https://github.com/sindresorhus/ava/blob/1868204c1901f45b4f66a520ef6486fdd71fe1d2/api.js#L210

[novemberborn]

This is asynchronous whereas onForkStarting is not. Probably best to build up tests similarly (and synchronously) to how it was done before and leave the rest of the code as it is. (Note that I just changed this you so you'll need to rebase).

[jamestalmage]

And then I realized, that existing code doesn't work with nested promise — .on is not a function.

You might be able to solve this with promise-delegates.

[novemberborn]

Hey @develar, would be great to have debug support for IntelliJ. Will you have time to follow up on the feedback here?

[develar]

@novemberborn Sorry for delay, it will be addressed this week.

[novemberborn]

@develar no worries, that's great! 👍

[vadimdemedes]

Closing due to lack of response from the author of PR.

[jamestalmage]

@develar - If you ever get around to updating this PR, we would still be interested in merging. However, be aware, there is work happening towards official Node debugger support: nodejs/node#2546 (comment).

[develar]

New PR — #874.