Skip to content

test: improve error logging for inspector test - #14508

Closed
Trott wants to merge 1 commit into
nodejs:masterfrom
Trott:improve-logging
Closed

test: improve error logging for inspector test#14508
Trott wants to merge 1 commit into
nodejs:masterfrom
Trott:improve-logging

Conversation

@Trott

Copy link
Copy Markdown
Member

If JSON.parse() fails, print a message showing the JSON that failed to
parse. This is to help with debugging a current test failure on CI.

Refs: #14507

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • commit message follows commit guidelines
Affected core subsystem(s)

test inspector

@TrottTrott added inspector Issues and PRs related to the V8 inspector protocol test Issues and PRs related to the tests. labels Jul 27, 2017
@nodejs-github-botnodejs-github-bot added dont-land-on-v4.x inspector Issues and PRs related to the V8 inspector protocol test Issues and PRs related to the tests. labels Jul 27, 2017
@Trott

Copy link
Copy Markdown
MemberAuthor

Comment threadtest/parallel/test-repl.js Outdated

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Is this related?

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

@refack No, it's not. That's an error on my part. Will remove it. It's from an unrelated PR I was testing.

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

Removed. Thanks.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I recognized the ... REPL test 😉

If JSON.parse() fails, print a message showing the JSON that failed to
parse. This is to help with debugging a current test failure on CI.
Refs: nodejs#14507
@Trott

Copy link
Copy Markdown
MemberAuthor

Rebased, resolved merge conflict, force pushed.

@Trott

Copy link
Copy Markdown
MemberAuthor

@Trott

Copy link
Copy Markdown
MemberAuthor

Since this test fails (per #14507) on CI pretty frequently, I'd like to fast-track this to get better information about how it's failing. Does that seem reasonable? @nodejs/testing

@cjihrig

Copy link
Copy Markdown
Contributor

Seems reasonable to me.

@refackrefack left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

👍 for fast track

@Trott

Copy link
Copy Markdown
MemberAuthor

Landed in bfb4f42

@TrottTrott closed this Jul 29, 2017
Trott added a commit to Trott/io.js that referenced this pull request Jul 29, 2017
If JSON.parse() fails, print a message showing the JSON that failed to
parse. This is to help with debugging a current test failure on CI.
PR-URL: nodejs#14508
Ref: nodejs#14507
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
addaleax pushed a commit that referenced this pull request Jul 29, 2017
If JSON.parse() fails, print a message showing the JSON that failed to
parse. This is to help with debugging a current test failure on CI.
PR-URL: #14508
Ref: #14507
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
@addaleaxaddaleax mentioned this pull request Aug 2, 2017
MylesBorins pushed a commit that referenced this pull request Aug 16, 2017
If JSON.parse() fails, print a message showing the JSON that failed to
parse. This is to help with debugging a current test failure on CI.
PR-URL: #14508
Ref: #14507
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
@MylesBorinsMylesBorins mentioned this pull request Aug 16, 2017
MylesBorins pushed a commit that referenced this pull request Aug 16, 2017
If JSON.parse() fails, print a message showing the JSON that failed to
parse. This is to help with debugging a current test failure on CI.
PR-URL: #14508
Ref: #14507
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
MylesBorins pushed a commit that referenced this pull request Sep 3, 2017
If JSON.parse() fails, print a message showing the JSON that failed to
parse. This is to help with debugging a current test failure on CI.
PR-URL: #14508
Ref: #14507
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
MylesBorins pushed a commit that referenced this pull request Sep 5, 2017
If JSON.parse() fails, print a message showing the JSON that failed to
parse. This is to help with debugging a current test failure on CI.
PR-URL: #14508
Ref: #14507
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

inspectorIssues and PRs related to the V8 inspector protocoltestIssues and PRs related to the tests.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants

@Trott@cjihrig@refack@lpinca@MylesBorins@nodejs-github-bot