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
wants to merge 4 commits into
base: master
from

Conversation

Projects
None yet
8 participants
@larissayvette
Contributor

larissayvette commented Oct 16, 2016

Affected core subsystem(s)

test

Description of change

Cheching if the throwed exception is assert.AssertError

assert.throws(() => { require('_debug_agent').start(); },
assert.AssertionError);
assert.throws(
() => { debug.start();},

This comment has been minimized.

@cjihrig

cjihrig Oct 16, 2016

Contributor

Can you use consistent spacing around the curly braces.

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

This comment has been minimized.

@cjihrig

cjihrig Oct 16, 2016

Contributor

The indentation looks off here.

@mscdex mscdex added the debugger label Oct 16, 2016

@Trott

This comment has been minimized.

Member

Trott commented Oct 17, 2016

@jasnell

LGTM

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

This comment has been minimized.

@lpinca

lpinca Oct 17, 2016

Member

Tiny nit: one more space :)

@lpinca

lpinca approved these changes Oct 17, 2016

LGTM

@Trott

This comment has been minimized.

@rvagg rvagg force-pushed the nodejs:master branch 2 times, most recently to 83c7a88 Oct 18, 2016

@jasnell jasnell self-assigned this Oct 18, 2016

jasnell added a commit that referenced this pull request Oct 18, 2016

test: checking if error constructor is assert.AssertionError
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

This comment has been minimized.

Member

jasnell commented Oct 18, 2016

Landed in 4e1d6d0

@jasnell jasnell closed this Oct 18, 2016

@jasnell

This comment has been minimized.

Member

jasnell commented Oct 18, 2016

Fixed the remaining indenting nit on landing.

jasnell added a commit that referenced this pull request Oct 18, 2016

test: checking if error constructor is assert.AssertionError
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 added a commit that referenced this pull request Oct 19, 2016

test: checking if error constructor is assert.AssertionError
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 added a commit that referenced this pull request Nov 15, 2016

test: checking if error constructor is assert.AssertionError
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 added a commit that referenced this pull request Nov 15, 2016

test: checking if error constructor is assert.AssertionError
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 larissayvette:fix-test-debug-agent branch Dec 28, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment