Skip to content

Conversation

@larissayvette
Copy link
Contributor

Affected core subsystem(s)

test

Description of change

Cheching if the throwed exception is assert.AssertError

@nodejs-github-botnodejs-github-bot added the test Issues and PRs related to the tests. label Oct 16, 2016
assert.throws(()=>{require('_debug_agent').start();},
assert.AssertionError);
assert.throws(
()=>{debug.start();},
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you use consistent spacing around the curly braces.

assert.AssertionError);
assert.throws(
()=>{debug.start();},
function(err){
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The indentation looks off here.

@Trott
Copy link
Member

LGTM with spacing nits fixed

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

Copy link
Member

@jasnelljasnell left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

()=>{debug.start();},
function(err){
return(errinstanceofassert.AssertionError&&
err.message==='Debugger agent running without bindings!');
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tiny nit: one more space :)

Copy link
Member

@lpincalpinca left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@Trott
Copy link
Member

@rvaggrvaggforce-pushed the master branch 2 times, most recently from c133999 to 83c7a88CompareOctober 18, 2016 17:02
@jasnelljasnell self-assigned this Oct 18, 2016
jasnell pushed a commit that referenced this pull request Oct 18, 2016
PR-URL: #9119 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Rich Trott <[email protected]>
@jasnell
Copy link
Member

Landed in 4e1d6d0

@jasnelljasnell closed this Oct 18, 2016
@jasnell
Copy link
Member

Fixed the remaining indenting nit on landing.

jasnell pushed a commit that referenced this pull request Oct 18, 2016
PR-URL: #9119 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Rich Trott <[email protected]>
jasnell pushed a commit that referenced this pull request Oct 19, 2016
PR-URL: #9119 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Rich Trott <[email protected]>
MylesBorins pushed a commit that referenced this pull request Nov 15, 2016
PR-URL: #9119 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Rich Trott <[email protected]>
MylesBorins pushed a commit that referenced this pull request Nov 15, 2016
PR-URL: #9119 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Rich Trott <[email protected]>
This was referenced Nov 22, 2016
@larissayvettelarissayvette deleted the fix-test-debug-agent branch December 28, 2016 15:51
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

testIssues and PRs related to the tests.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants

@larissayvette@Trott@jasnell@lpinca@cjihrig@mscdex@MylesBorins@nodejs-github-bot