Skip to content
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

Implement --inspect so debugging inside a test is possible? #651

Closed
rainabba opened this issue Oct 20, 2016 · 57 comments · Fixed by #673
Assignees
Labels
Milestone

Comments

@rainabba
Copy link

@rainabba rainabba commented Oct 20, 2016

For me, --inspect (native V8 debugging with devtools) is a god-send. The only thing that would be more amazing is the ability to use it in a test so I can more easily debug failures.

node.js debugger and debugging-node-js-with-v8-inspecto if you're not familiar.

This possible? Already in the works?

@geek geek added the request label Oct 20, 2016
@geek

This comment has been minimized.

Copy link
Member

@geek geek commented Oct 20, 2016

It works and doesn't work :/

node --inspect node_modules/.bin/lab

The above works, but if you do the following, you will get an error:

lab --inspect

We can try and pass along the inspect argument when its provided directly to lab.

@rainabba

This comment has been minimized.

Copy link
Author

@rainabba rainabba commented Oct 20, 2016

Nice workaround. I'd love to see the pass-through, but this will get me what I need for now. Thanks!

@rainabba

This comment has been minimized.

Copy link
Author

@rainabba rainabba commented Oct 20, 2016

Maybe I spoke too soon? Something wrong in my environment? I am on Windows and running in PowerShell in case that's a factor.

node --inspect node_modules/.bin/lab
Debugger listening on port 9229.
Warning: This is an experimental feature and could change at any time.
To start debugging, open the following URL in Chrome:
    chrome-devtools://devtools/remote/serve_file/@60cd6e859b9f557d2312f5bf532f6aec5f284980/inspector.html?experiments=true&v8only=true&ws=localhost:9229/57e3d76c-cb1f-45b7-84d0-1e47e34db924
C:\myapp\node_modules\.bin\lab:2
basedir=$(dirname "$(echo "$0" | sed -e 's,\\,/,g')")
          ^^^^^^^
SyntaxError: missing ) after argument list
    at Object.exports.runInThisContext (vm.js:76:16)
    at Module._compile (module.js:542:28)
    at Object.Module._extensions..js (module.js:579:10)
    at Module.load (module.js:487:32)
    at tryModuleLoad (module.js:446:12)
    at Function.Module._load (module.js:438:3)
    at Module.runMain (module.js:604:10)
    at run (bootstrap_node.js:394:7)
    at startup (bootstrap_node.js:149:9)
    at bootstrap_node.js:509:3

@geek

This comment has been minimized.

Copy link
Member

@geek geek commented Oct 20, 2016

@rainabba interesting... what version of node are you using? Also, does lab run if you run it without the --inspect ?

node node_modules\.bin\lab
@rainabba

This comment has been minimized.

Copy link
Author

@rainabba rainabba commented Oct 25, 2016

@geek No, but then I just realized neither does my test command, even though it does execute just find through npm test

My typical test is lab -t 50 -c -M 20000 -r console -o stdout -r html -o test/coverage.html -l -I testAccount,Config,lab,Lab,vtv,Hoek,user,auth,async

When I try to run that, I get the same error as above. I've never noticed this because I've always used npm test to start lab.

@rainabba

This comment has been minimized.

Copy link
Author

@rainabba rainabba commented Oct 25, 2016

Given the use of sed and the error, I suspected this is expecting this was an issue resulting from the expectation of a bash environment, but I just tried (Ubuntu Bash on Windows 10 to be fair) and got the same result. That error is from inside lab? Wondering if this is just my environment to blame.

@geek

This comment has been minimized.

Copy link
Member

@geek geek commented Oct 26, 2016

@rainabba lab doesn't have that line: https://github.com/hapijs/lab/blob/master/bin/lab

Something else is likely inserting the code that causes the error. What version of node are you running?

@rainabba

This comment has been minimized.

Copy link
Author

@rainabba rainabba commented Oct 26, 2016

@geek I'm running 6.9.0 and node node_modules\.bin\lab results in the same error.

@ORESoftware

This comment has been minimized.

Copy link

@ORESoftware ORESoftware commented Oct 26, 2016

I am not sure i follow - haven't most people been using Chrome to debug node.js apps using node-inspector for a long time? I don't see how putting it in core makes that much of a difference.

Ok, i do realize it makes a difference for frameworks and libaries that now can support debugging without having to install node inspector.

So before it was:

npm install -g node-inspector &&
node-debug your-file.js

Now it's

node --inspect your-file.js

Either way, one of the most amazing tools ever

The best way to support this in this library IMO is to have a new bin definition in package.json

"bin": {

 "lab": "./bin/lab.js",
 "lab-inspect":"./bin/lab-inspect.js"

}

Where lab-inspect looks like

https://github.com/ORESoftware/suman/blob/master/index-debug.js

But instead it's

node --inspect

So in all it would be!!

// bin/lab-inspect.js

#!/usr/bin/env node --inspect 
require('./bin/lab.js');

@geek

This comment has been minimized.

Copy link
Member

@geek geek commented Oct 27, 2016

@ORESoftware good idea!

@rainabba

This comment has been minimized.

Copy link
Author

@rainabba rainabba commented Oct 27, 2016

@ORESoftware "haven't most people been using Chrome to debug node.js apps" Not exactly. Chrome is effectively the shell. Since Node.js is based on V8, there is native debugging available, but "Node Inspector is a debugger interface for Node.js applications that uses the Blink Developer Tools (formerly WebKit Web Inspector).". This was a brilliant, but fragile implementation based on my experience (though it was the BEST tool available until recently) so I was excited that Chrome was adding native support for debugging in the Devtools (using the same Blink Dev tools UI) which included even more debugger support (such as async).

@geek I've forked lab to investigate the idea that @ORESoftware presented and in doing so, something clicked. When I read and tried node --inspect node_modules/.bin/lab, I was thinking the reference was to node_modules/lab/.bin/lab so I was only searching in the lab code itself. I expanded my search though and realized my error and found that node_modules/.bin is full of files that include basedir=$(dirname "$(echo "$0" | sed -e 's,\\,/,g')")

In fact, the contents of node_modules/.bin/lab are (and this explains where my error comes from when I try the command you suggested):

#!/bin/sh
basedir=$(dirname "$(echo "$0" | sed -e 's,\\,/,g')")

case `uname` in
    *CYGWIN*) basedir=`cygpath -w "$basedir"`;;
esac

if [ -x "$basedir/node" ]; then
    echo "here"
  "$basedir/node"  "$basedir/../lab/bin/lab" "$@"
  ret=$?
else 
    echo "there"
  node  "$basedir/../lab/bin/lab" "$@"
  ret=$?
fi
exit $ret
@rainabba

This comment has been minimized.

Copy link
Author

@rainabba rainabba commented Oct 27, 2016

In case this isn't common knowledge already for anyone following this issue:

"When in local mode, executables are linked into ./node_modules/.bin so that they can be made available to scripts run through npm. (For example, so that a test runner will be in the path when you run npm test.)" npm-folders - executables

@rainabba

This comment has been minimized.

Copy link
Author

@rainabba rainabba commented Oct 27, 2016

So the error from node --inspect node_modules/.bin/lab is because we're passing a shell script intended for BASH, into node.

I've discovered that this file isn't even used when I run npm test, but node_modules/.bin/lab.cmd is and when edited to the following, I can run npm test and have debugging :)

So I'm guessing NPM creates this file and that's where we'd have to get parameter handling added to be able to do something like npm --inspect test.

I'm going to close since this doesn't appear to be an issue with lab at all.

@rainabba rainabba closed this Oct 27, 2016
@rainabba

This comment has been minimized.

Copy link
Author

@rainabba rainabba commented Oct 27, 2016

The issue (as I'm investigating with NPM) is that node args cannot be passed into npm test or npm run-script test. My present solution is to create a node_modules/.bin/lab.debug.cmd with the args I need, then a script called "test-debug" in my package.json to call "lab-debug". This can be run with npm run-script test-debug to get a debuggable, lab execution.

@rainabba

This comment has been minimized.

Copy link
Author

@rainabba rainabba commented Oct 27, 2016

I've found that nodemon supports --inspect in the script and I'm seeking to understand how they do that. If I can and I'm able, I'll submit a PR because it looks like this might need to be solved by lab given the restrictions in place with NPM run-script and it's lack of node argument support. remy/nodemon#929

@rainabba rainabba reopened this Oct 27, 2016
@ORESoftware

This comment has been minimized.

Copy link

@ORESoftware ORESoftware commented Oct 27, 2016

Just as an aside, Webstorm has an awesome in-IDE debugger - I don't know what it's based on, but just something to be aware of. I prefer the Webstorm debugger to Blink Tools/Node-Inspector, but both are excellent.

@simon-p-r

This comment has been minimized.

Copy link

@simon-p-r simon-p-r commented Oct 27, 2016

I think visual code has an even better debugger than webstorm and it is free! 😀

@jcollum

This comment has been minimized.

Copy link

@jcollum jcollum commented Nov 3, 2016

node --inspect node_modules/.bin/lab -- this works quite well. Add to the docs?

@rainabba this is how I run (debug) one test (this might help you, might not, haven't read the thread fully):

package.json: "test-one-debug": "npm run transpile; node --inspect --debug-brk ./node_modules/.bin/lab -l -e development -m 120000 -g \"$TESTNAME\" -v -r console --coverage-exclude test ./dist/test/unit",

bash: TESTNAME="for one day" npm run test-one-debug

@geek

This comment has been minimized.

Copy link
Member

@geek geek commented Nov 5, 2016

@jcollum sounds like a good document addition to me!

@gregorleban

This comment has been minimized.

Copy link

@gregorleban gregorleban commented Nov 14, 2016

Currently, running node with --inspect prints to console text such as:

To start debugging, open the following URL in Chrome:
chrome-devtools://devtools/bundled/inspector.html?experiments=true&v8only=true&ws=127.0.0.1:9229/f6f4ba01-8b04-435d-bdd8-9d9830a13ea9
Debugger attached.

Any way of automatically opening the url in chrome? Currently restarting the debugger requires a frequent need to copy and paste the urls that change on every restart.

@Marsup

This comment has been minimized.

Copy link
Member

@Marsup Marsup commented Nov 14, 2016

I don't think we have control over that, this is an issue you should open on node repo.

@jcollum

This comment has been minimized.

Copy link

@jcollum jcollum commented Nov 14, 2016

That url doesnt change on every restart.

On Mon, Nov 14, 2016, 12:50 AM Nicolas Morel notifications@github.com
wrote:

I don't think we have control over that, this is an issue you should open
on node repo.


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#651 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/AArAAnqe2R3VkdxMJBPARtyHBnGqlVckks5q-CDDgaJpZM4Kcr9l
.

@gregorleban

This comment has been minimized.

Copy link

@gregorleban gregorleban commented Nov 14, 2016

To me, it does change. Here are two urls that I get from two runs:

chrome-devtools://devtools/bundled/inspector.html?experiments=true&v8only=true&ws=
127.0.0.1:9229/17541e3e-007d-445a-97ee-47ed301f233e

chrome-devtools://devtools/bundled/inspector.html?experiments=true&v8only=true&ws=
127.0.0.1:9229/e4589e16-750e-4d17-9afb-ff1c3d49464d

The url's contain the uuid that is different for each run.

On Mon, Nov 14, 2016 at 3:09 PM, Justin Collum notifications@github.com
wrote:

That url doesnt change on every restart.

On Mon, Nov 14, 2016, 12:50 AM Nicolas Morel notifications@github.com
wrote:

I don't think we have control over that, this is an issue you should open
on node repo.


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#651 (comment), or
mute
the thread
<https://github.com/notifications/unsubscribe-auth/
AArAAnqe2R3VkdxMJBPARtyHBnGqlVckks5q-CDDgaJpZM4Kcr9l>

.


You are receiving this because you commented.
Reply to this email directly, view it on GitHub
#651 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/AAxaushuQP8Ku8s0BiThBteexVhokNuzks5q-GuggaJpZM4Kcr9l
.


Gregor

@jcollum

This comment has been minimized.

Copy link

@jcollum jcollum commented Nov 14, 2016

Sure, but the guid in the URL is meaningless AFAIK. Like I'm sure it has some meaning but it doesn't change the use of the debugger. This part is what matters:

ws=127.0.0.1:9229

If you leave that web page open, kill the Hapi server, restart it and go back to the web page you should get a message that says "Reconnect Dev Tools". Hit 'yes' or whatever and it will reconnect to the debugger. Works really well.

If you see any odd behavior (failing to reconnect etc) just reload the debugger page and it should snap back into shape.

@gr2m

This comment has been minimized.

Copy link

@gr2m gr2m commented Nov 14, 2016

if you pass --inspect trough, it’d be nice if you could pass trough --debug-brk as well, see https://nodejs.org/api/debugger.html#debugger_v8_inspector_integration_for_node_js

@jcollum

This comment has been minimized.

Copy link

@jcollum jcollum commented Nov 14, 2016

debug-brk is passable. Here are my test-runner scripts for lab from my package.json

"test": "npm run transpile && ./node_modules/.bin/lab -l -e development -t 80 -v -r console --coverage-exclude dist/test  ./dist/test/unit",

"test-debug": "npm run transpile && node  --inspect --debug-brk ./node_modules/.bin/lab  -l -e development -v -r console --coverage-exclude dist/test  ./dist/test/unit",

"test-grep": "npm run transpile && ./node_modules/.bin/lab -l -e development -m 120000  -g \"$TESTNAME\" -v -r console --coverage-exclude test ./dist/test/unit",

"test-grep-debug": "npm run transpile &&  node --inspect --debug-brk ./node_modules/.bin/lab -l -e development -m 120000  -g \"$TESTNAME\" -v -r console --coverage-exclude test ./dist/test/unit",

If you run test-debug it will stop on the first line and any breakpoints/debugger keywords.

@ORESoftware

This comment has been minimized.

Copy link

@ORESoftware ORESoftware commented Nov 23, 2016

As few of you may know I am working on a test runner for Node, this is how I solved this one -

#!/usr/bin/env bash

exec node --inspect --debug-brk <<EOF
process.env.SUMAN_EXTRANEOUS_EXECUTABLE = 'yes';
require('./cli.js');

EOF

because I guess you can't do this with *nix

#!/usr/bin/env node --inspect   #this doesn't work
process.env.SUMAN_EXTRANEOUS_EXECUTABLE = 'yes';
require('./cli.js');
@killerspaz

This comment has been minimized.

Copy link

@killerspaz killerspaz commented Dec 13, 2016

Any update/progress on this?

I get the same error; is it something to do with the shell interpreter?

Literally failing on line 1 of the lab wrapper script in node_modules.

$ node ./node_modules/.bin/lab
C:\Users\default\dev\microservices\api.bootstrap\node_modules\.bin\lab:2
basedir=$(dirname "$(echo "$0" | sed -e 's,\\,/,g')")
          ^^^^^^^
SyntaxError: missing ) after argument list
    at Object.exports.runInThisContext (vm.js:76:16)
    at Module._compile (module.js:513:28)
    at Object.Module._extensions..js (module.js:550:10)
    at Module.load (module.js:458:32)
    at tryModuleLoad (module.js:417:12)
    at Function.Module._load (module.js:409:3)
    at Module.runMain (module.js:575:10)
    at run (bootstrap_node.js:352:7)
    at startup (bootstrap_node.js:144:9)
    at bootstrap_node.js:467:3

@jcollum

This comment has been minimized.

Copy link

@jcollum jcollum commented Dec 13, 2016

@killerspaz I don't see how that's the same error. Especially since you aren't calling it with --inspect. It looks very different to me. Do you have a linux VM handy? Your error may be a Windows-only bug.

@rainabba

This comment has been minimized.

Copy link
Author

@rainabba rainabba commented Dec 13, 2016

@killerspaz @jcollum See my comment at #651 (comment) if you haven't already.

@jcollum

This comment has been minimized.

Copy link

@jcollum jcollum commented Dec 13, 2016

Another thing that's relevant to this: https://chrome.google.com/webstore/detail/nim-node-inspector-manage/gnhhdgbaldcilmgcpfddgdbkhjohddkj

A chrome plugin that finds watches for the socket that --inspect opens and opens a debugger for you automatically. Works great. Well, mostly great. Better than hunting down that url for sure.

@killerspaz

This comment has been minimized.

Copy link

@killerspaz killerspaz commented Dec 13, 2016

@jcollum it may be windows-only, but it's certainly still a bug.

It's the same error because it's what @rainabba provided, and he's OP for the thread.

Also --inspect isn't going to modify the very first line of the .bin wrapper script; I left it out simply to get the output faster. With --inspect and --debug-brk, the node inspector does spin up but will still fail the same way as soon as the script is resumed.

I did test on linux and it works, but we're platform agnostic and this ruins the development workflow.

@killerspaz

This comment has been minimized.

Copy link

@killerspaz killerspaz commented Dec 13, 2016

@rainabba yeah I did see that, and came to the same conclusion. Not sure what a proper workaround is atm.

@jcollum

This comment has been minimized.

Copy link

@jcollum jcollum commented Dec 13, 2016

Since the bug appears to be only on Windows I suggest this ticket be closed and a new one with a better title be opened. I'm able to run tests with --inspect with a bit of a workaround (see above); I don't see a bug in Lab.

@june07

This comment has been minimized.

Copy link

@june07 june07 commented Dec 15, 2016

@jcollum Regarding http://june07.com/nim what did you mean by "mostly" great? What issues have you had?

@jcollum

This comment has been minimized.

Copy link

@jcollum jcollum commented Dec 15, 2016

Once in a while I have to toggle the Auto button to get it to see the open debugger socket. Not really a big deal, I assume the "experimental" notice on the debugger has a lot do with that.

@jcollum

This comment has been minimized.

Copy link

@jcollum jcollum commented Dec 15, 2016

@june07 oh you're the dev of that tool! haha ok well if I have something more concrete I'll report it. Overall very happy with it.

@june07

This comment has been minimized.

Copy link

@june07 june07 commented Dec 15, 2016

Great, I'm glad you're happy with it. Thanks for the support and good word. A 5-star rating along with adding your words here at the Webstore: https://chrome.google.com/webstore/detail/nim-node-inspector-manage/gnhhdgbaldcilmgcpfddgdbkhjohddkj/reviews

would be super helpful and much appreciated.

And please don't hesitate to reach out if you run into any issues, or have other feedback or concerns, etc. This is my first Chrome extension and it's pretty cool to see that it has been helpful to folks as it was for me.

@june07

This comment has been minimized.

Copy link

@june07 june07 commented Dec 16, 2016

jcollum much love!

@killerspaz

This comment has been minimized.

Copy link

@killerspaz killerspaz commented Jan 9, 2017

OK I got super annoyed at not being able to run lab as a debug session in node, so I started snooping around more.

node --inspect --debug-brk ./node_modules/lab/bin/lab -v -P test

-v -P test is optional, just what I need to get my tests running the way I expect.

Essentially this is what the node_modules\.bin\lab script would execute, just now we don't have to wait for a proper shell fix for the script.

Might dig into that deeper later as to why the shell script breaks and suggest a PR.

@jcollum

This comment has been minimized.

Copy link

@jcollum jcollum commented Jan 9, 2017

I posted this technique a while back in this thread:

TESTNAME="should not update a record that already exists" npm run test-grep-debug

package:

 "test-grep-debug": "node --inspect --debug-brk ./node_modules/.bin/lab -l -e development -m 120000  -g \"$TESTNAME\" -v -r console --coverage-exclude test ./dist/test/unit",

(call to ./node_modules/.bin/lab is redundant, oops, should be just lab, oh well)

Which runs:

> kjyuhkjh@1.2.0 test-grep-debug /Users/collumj/work/asdfa
> node --inspect --debug-brk ./node_modules/.bin/lab -l -e development -m 120000  -g "$TESTNAME" -v -r console --coverage-exclude test ./dist/test/unit

Debugger listening on port 9229.
Warning: This is an experimental feature and could change at any time.
To start debugging, open the following URL in Chrome:
    chrome-devtools://devtools/remote/serve_file/@60cd6e859b9f557d2312f5bf532f6aec5f284980/inspector.html?experiments=true&v8only=true&ws=localhost:9229/03dc9e0c-4a24-459c-af6c-df26fda72417
Debugger attached.
...
etc... 

I don't get why you are having issues with it @killerspaz . It's mildly annoying to have to put that at the front of the line. But you can set TESTNAME on it's own line then run npm run test-grep-debug on another line.

Can you explain what you feel is broken about it?

@killerspaz

This comment has been minimized.

Copy link

@killerspaz killerspaz commented Jan 9, 2017

It's literally and explicitly been pointed out multiple times in this thread - please review before posting another comment - you've done a lot to flood the issue of unvaluable information.

@jcollum

This comment has been minimized.

Copy link

@jcollum jcollum commented Jan 9, 2017

Don't attack me for trying to help you. I won't make that mistake again!

@geek geek added feature and removed request labels Jan 20, 2017
@geek geek added this to the 12.1.0 milestone Jan 20, 2017
@geek geek closed this in #673 Jan 21, 2017
@rainabba

This comment has been minimized.

Copy link
Author

@rainabba rainabba commented May 8, 2018

Just ended up here again. Thanks @jcollum for the node --inspect --debug-brk ./node_modules/.bin/lab solution.

Btw, this was my first hit in Google for "lab tests --inspect"

@jcollum

This comment has been minimized.

Copy link

@jcollum jcollum commented May 8, 2018

@Marsup

This comment has been minimized.

Copy link
Member

@Marsup Marsup commented May 9, 2018

You do realize there's now a built-in option for that ?

@rainabba

This comment has been minimized.

Copy link
Author

@rainabba rainabba commented May 9, 2018

Btw, this was my first hit in Google for "lab tests --inspect"

So no. I'll revisit the README though. Thank you and thanks for this project.

@jcollum

This comment has been minimized.

Copy link

@jcollum jcollum commented May 9, 2018

Man someone in this thread is cranky: there's thumbs-downs all over the place. Hey, whoever you are: relax or contribute to the discussion.

@Marsup

This comment has been minimized.

Copy link
Member

@Marsup Marsup commented May 9, 2018

So no.

I suspected you didn't notice the issue was closed due to a PR, I had to make sure :p

@rainabba

This comment has been minimized.

Copy link
Author

@rainabba rainabba commented May 9, 2018

I didn't, but even that isn't quite up to speed and I have a simple PR to clean it up a bit, but this nearly sent me running: https://hapijs.com/contribute

Can't afford the time to setup for that, but I've submitted the PR anyway so the code is out there for anyone setup to contribute, who wants to "do it right". It can be rejected otherwise. #832

Thanks for the project guys!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
You can’t perform that action at this time.