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

Fix test debug agent #9119

Closed

Conversation

larissayvette
Copy link
Contributor

Affected core subsystem(s)

test

Description of change

Cheching if the throwed exception is assert.AssertError

@nodejs-github-bot nodejs-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

Trott commented Oct 17, 2016

LGTM with spacing nits fixed

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

Copy link
Member

@jasnell jasnell 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 (err instanceof assert.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

@lpinca lpinca 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

Trott commented Oct 17, 2016

@rvagg rvagg force-pushed the master branch 2 times, most recently from c133999 to 83c7a88 Compare October 18, 2016 17:02
@jasnell jasnell 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 <jasnell@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
@jasnell
Copy link
Member

jasnell commented Oct 18, 2016

Landed in 4e1d6d0

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

jasnell commented Oct 18, 2016

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 <jasnell@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
jasnell pushed a commit that referenced this pull request Oct 19, 2016
PR-URL: #9119
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
MylesBorins pushed a commit that referenced this pull request Nov 15, 2016
PR-URL: #9119
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
MylesBorins pushed a commit that referenced this pull request Nov 15, 2016
PR-URL: #9119
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
This was referenced Nov 22, 2016
@larissayvette larissayvette deleted the fix-test-debug-agent branch December 28, 2016 15:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
test Issues and PRs related to the tests.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants