test: add test for repl.defineCommand#3908
Uh oh!
There was an error while loading. Please reload this page.
Conversation
thefourtheye added repl 
thefourtheye commented Nov 19, 2015
This is awesome. CI: https://ci.nodejs.org/job/node-test-pull-request/776/ |
There was a problem hiding this comment.
For consistency, please use const here.
There was a problem hiding this comment.
Sure no prob. I saw there was a lint error in CI so I'll fix that and this.
Should I const everything in this file that can be? Or just the requires?
There was a problem hiding this comment.
at least the requires for sure. make sure it passes lint. the rest I'd consider optional
There was a problem hiding this comment.
@bengl You can actually check everything locally simply by running make -j4 test, here 4 denotes the number of tasks to be done parallelly.
jasnell commented Nov 19, 2015
One minor nit but LGTM if CI is green |
benglforce-pushed the definecommandtest branch from 77a3e52 to 2ceb942Compare
bengl commented Nov 19, 2015
Cool. |
There was a problem hiding this comment.
The second parameter will be thrown as an error, if the first param is not truthy. So the text should be negative I believe.
There was a problem hiding this comment.
Ah, sure. Will change those messages to be negative.
jasnell commented Nov 19, 2015
Awesome... doing another CI run just to be safe: https://ci.nodejs.org/job/node-test-pull-request/788/ |
It also tests displayPrompt by checking for '> '.
benglforce-pushed the definecommandtest branch from 2ceb942 to db28b50Comparebengl commented Nov 19, 2015
Changed error messages to be negative. |
thefourtheye commented Nov 19, 2015
LGTM |
bengl commented Nov 19, 2015
@jasnell looks like CI failed because I force-pushed a new version of it too early. Perhaps kick it off again? |
jasnell commented Nov 19, 2015
heh. Ok... new run: https://ci.nodejs.org/job/node-test-pull-request/790/ |
bengl commented Nov 19, 2015
Looks like some unrelated failures in Windows and FreeBSD. |
Fishrock123 commented Dec 1, 2015
Thanks! Landed in e5d97fd |
It also tests displayPrompt by checking for '> '. PR-URL: #3908 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Sakthipriyan Vairamani <[email protected]> Reviewed-By: Jeremiah Senkpiel <[email protected]>
It also tests displayPrompt by checking for '> '. PR-URL: #3908 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Sakthipriyan Vairamani <[email protected]> Reviewed-By: Jeremiah Senkpiel <[email protected]>
It also tests displayPrompt by checking for '> '. PR-URL: #3908 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Sakthipriyan Vairamani <[email protected]> Reviewed-By: Jeremiah Senkpiel <[email protected]>
It also tests displayPrompt by checking for '> '. PR-URL: #3908 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Sakthipriyan Vairamani <[email protected]> Reviewed-By: Jeremiah Senkpiel <[email protected]>
It also tests displayPrompt by checking for '> '. PR-URL: #3908 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Sakthipriyan Vairamani <[email protected]> Reviewed-By: Jeremiah Senkpiel <[email protected]>
It also tests displayPrompt by checking for '> '. PR-URL: nodejs#3908 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Sakthipriyan Vairamani <[email protected]> Reviewed-By: Jeremiah Senkpiel <[email protected]>
5 participants
There didn't appear to be a test for
REPLServer#defineCommand, probably because it wasn't documented before, so here's a test for it.