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

src: reduce test_inspector_socket_server output #10537

Closed

Conversation

danbev
Copy link
Contributor

@danbev danbev commented Dec 30, 2016

Currently, when test/cctest/test_inspector_socket_server.cc is run there
is output written to stderr by src/inspector_socket_server.cc which is
interleaved with the gtest report:

Debugger listening on port 9229.
Warning: This is an experimental feature and could change at any time.
To start debugging, open the following URLs in Chrome:
...

The goal of this commit is to remove the above logged information
by introducing an out_ member in the InspectorSocketServer class
which defaults to stderr (keeping the current behavior).

Setting out_ to NULL is supported in which case nothing will be written
and is what the test has been configured with. When working on specific
test case the appropriate output stream can be specified for the
ServerHolder constructor to limit logging to that test case.

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

src

Currently, when test/cctest/test_inspector_socket_server.cc is run there
is output written to stderr by src/inspector_socket_server.cc which is
interleaved with the gtest report:

Debugger listening on port 9229.
Warning: This is an experimental feature and could change at any time.
To start debugging, open the following URLs in Chrome:
  ...

The goal of this commit is to remove the above logged information
by introducing an out_ member in the InspectorSocketServer class
which defaults to stderr (keeping the current behavior).

Setting out_ to NULL is supported in which case nothing will be written
and is what the test has been configured with. When working on specific
test case the appropriate output stream can be specified for the
ServerHolder constructor to limit logging to that test case.
@nodejs-github-bot nodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. dont-land-on-v4.x inspector Issues and PRs related to the V8 inspector protocol labels Dec 30, 2016
@danbev
Copy link
Contributor Author

danbev commented Dec 30, 2016

@@ -207,12 +212,14 @@ class SocketSession {
};

InspectorSocketServer::InspectorSocketServer(SocketServerDelegate* delegate,
int port) : loop_(nullptr),
int port,
FILE* out) : loop_(nullptr),
delegate_(delegate),
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 with these lines now.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah I missed that and will fix. Thanks

@danbev
Copy link
Contributor Author

danbev commented Jan 5, 2017

@cjihrig When you get a chance would you mind taking a look at 6467adf and see if this looks alright now?

danbev added a commit to danbev/node that referenced this pull request Jan 6, 2017
Currently, when test/cctest/test_inspector_socket_server.cc is run there
is output written to stderr by src/inspector_socket_server.cc which is
interleaved with the gtest report:

Debugger listening on port 9229.
Warning: This is an experimental feature and could change at any time.
To start debugging, open the following URLs in Chrome:
  ...

The goal of this commit is to remove the above logged information
by introducing an out_ member in the InspectorSocketServer class
which defaults to stderr (keeping the current behavior).

Setting out_ to NULL is supported in which case nothing will be written
and is what the test has been configured with. When working on specific
test case the appropriate output stream can be specified for the
ServerHolder constructor to limit logging to that test case.

PR-URL: nodejs#10537
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Eugene Ostroukhov <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
@danbev
Copy link
Contributor Author

danbev commented Jan 6, 2017

Landed in 6d3c5b7

@danbev danbev closed this Jan 6, 2017
@danbev danbev deleted the reduce-test_inspector_socket_server-output branch January 17, 2017 07:22
italoacasas pushed a commit to italoacasas/node that referenced this pull request Jan 18, 2017
Currently, when test/cctest/test_inspector_socket_server.cc is run there
is output written to stderr by src/inspector_socket_server.cc which is
interleaved with the gtest report:

Debugger listening on port 9229.
Warning: This is an experimental feature and could change at any time.
To start debugging, open the following URLs in Chrome:
  ...

The goal of this commit is to remove the above logged information
by introducing an out_ member in the InspectorSocketServer class
which defaults to stderr (keeping the current behavior).

Setting out_ to NULL is supported in which case nothing will be written
and is what the test has been configured with. When working on specific
test case the appropriate output stream can be specified for the
ServerHolder constructor to limit logging to that test case.

PR-URL: nodejs#10537
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Eugene Ostroukhov <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
italoacasas pushed a commit to italoacasas/node that referenced this pull request Jan 19, 2017
Currently, when test/cctest/test_inspector_socket_server.cc is run there
is output written to stderr by src/inspector_socket_server.cc which is
interleaved with the gtest report:

Debugger listening on port 9229.
Warning: This is an experimental feature and could change at any time.
To start debugging, open the following URLs in Chrome:
  ...

The goal of this commit is to remove the above logged information
by introducing an out_ member in the InspectorSocketServer class
which defaults to stderr (keeping the current behavior).

Setting out_ to NULL is supported in which case nothing will be written
and is what the test has been configured with. When working on specific
test case the appropriate output stream can be specified for the
ServerHolder constructor to limit logging to that test case.

PR-URL: nodejs#10537
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Eugene Ostroukhov <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
italoacasas pushed a commit to italoacasas/node that referenced this pull request Jan 24, 2017
Currently, when test/cctest/test_inspector_socket_server.cc is run there
is output written to stderr by src/inspector_socket_server.cc which is
interleaved with the gtest report:

Debugger listening on port 9229.
Warning: This is an experimental feature and could change at any time.
To start debugging, open the following URLs in Chrome:
  ...

The goal of this commit is to remove the above logged information
by introducing an out_ member in the InspectorSocketServer class
which defaults to stderr (keeping the current behavior).

Setting out_ to NULL is supported in which case nothing will be written
and is what the test has been configured with. When working on specific
test case the appropriate output stream can be specified for the
ServerHolder constructor to limit logging to that test case.

PR-URL: nodejs#10537
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Eugene Ostroukhov <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
italoacasas pushed a commit to italoacasas/node that referenced this pull request Jan 27, 2017
Currently, when test/cctest/test_inspector_socket_server.cc is run there
is output written to stderr by src/inspector_socket_server.cc which is
interleaved with the gtest report:

Debugger listening on port 9229.
Warning: This is an experimental feature and could change at any time.
To start debugging, open the following URLs in Chrome:
  ...

The goal of this commit is to remove the above logged information
by introducing an out_ member in the InspectorSocketServer class
which defaults to stderr (keeping the current behavior).

Setting out_ to NULL is supported in which case nothing will be written
and is what the test has been configured with. When working on specific
test case the appropriate output stream can be specified for the
ServerHolder constructor to limit logging to that test case.

PR-URL: nodejs#10537
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Eugene Ostroukhov <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
@italoacasas italoacasas mentioned this pull request Jan 29, 2017
@MylesBorins
Copy link
Contributor

This does not land cleanly in LTS. Added dont-land label. Please feel free to manually backport

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c++ Issues and PRs that require attention from people who are familiar with C++. inspector Issues and PRs related to the V8 inspector protocol
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants