Inspector only enabled when specified by gaofanmichael · Pull Request #343 · googleapis/cloud-debug-nodejs

gaofanmichael · 2017-10-18T00:11:19Z

This PR ensures that inspector protocol will only be enabled if
environmental variable GCLOUD_USE_INSPECTOR is set.

src/agent/v8/debugapi.ts

  const inspectorapi = require('./inspector-debugapi');
  debugApiConstructor = inspectorapi.InspectorDebugApi;
 } else {
+  if (process.env.GCLOUD_USE_INSPECTOR) {


bin/run-test.sh

 # Run test/coverage
 run build/test

+node_version_major=$(node -v | sed 's/v\([0-9]*\)\.[0-9]*.[0-9]*/\1/')


test_windows.js

+const nodeVersion = /v(\d+\.\d+\.\d+)/.exec(process.version);
+if (!nodeVersion || nodeVersion.length < 2) {
+  console.error('unable to get node version');
+} else if (semver.satisfies(nodeVersion[1], '>=8')) {


test_windows.js

+var mocha = new Mocha();
+var testDir = 'build/test';
+fs.readdirSync(testDir).filter(function(file) {
+    return file.substr(-3) === '.js';


test_windows.js

+
+
+var mocha = new Mocha();
+var testDir = 'build/test';


test_windows.js

+
+process.env.GCLOUD_PROJECT = 0
+process.env.CLOUD_DEBUG_ASSERTIONS = 1
+


src/agent/util/utils.ts

  COULD_NOT_FIND_OUTPUT_FILE:
-      'Could not determine the output file associated with the transpiled input file'
+      'Could not determine the output file associated with the transpiled input file',
+  INSPECTOR_NOT_AVAILABLE: 'Inspector protocol only works for node 8+'


bin/run-test.sh

@@ -32,6 +32,14 @@ function run {
 # Run test/coverage
 run build/test


bin/run-test.sh

+node_version_major=$(node -p "require('semver').major(process.version)")
+if test $node_version_major -ge 8
+then
+  echo 'Test with v8 Inspector'


test/test-v8debugapi.ts

+  });
+  const logger = new common.logger({ levelLevel: config.logLevel } as any as LoggerOptions);
+
+  it('Verify the correct debugapi was constructed', (done) => {


test/test-v8debugapi.ts

+  });
+  const logger = new common.logger({ levelLevel: config.logLevel } as any as LoggerOptions);
+
+  it('Verify the correct debugapi was constructed', (done) => {


test/test-v8debugapi.ts

+        api = debugapi.create(logger, config, jsStats, mapper as SourceMapper.SourceMapper) as DebugApi;
+        if (process.env.GCLOUD_USE_INSPECTOR && semver.satisfies(process.version, '>=8')) {
+          const inspectorapi = require('../src/agent/v8/inspector-debugapi');
+          assert.ok(api instanceof inspectorapi.InspectorDebugApi);


test_windows.js

@@ -0,0 +1,19 @@
+const spawn = require('child_process').spawn;


test_windows.js

+
+process.env.GCLOUD_PROJECT = 0
+process.env.CLOUD_DEBUG_ASSERTIONS = 1
+spawn(path.join('node_modules', '.bin', 'mocha.cmd'),


test_windows.js

+spawn(path.join('node_modules', '.bin', 'mocha.cmd'),
+  [ path.join('build', 'test'), '--timeout 4000', '--R' ], {
+  stdio : 'inherit'
+}).on('close', function() {


test_windows.js

+    console.log('Run test with Inspector');
+    spawn(path.join('node_modules', '.bin', 'mocha.cmd'),
+      [ path.join('build', 'test'), '--timeout 4000', '--R' ], {
+      stdio : 'inherit'


appveyor.yml

  # run tests
  - npm run build
  - node_modules/.bin/mocha build/test --timeout 4000 --R
+  - SET GCLOUD_USE_INSPECTOR=1


test/test-v8debugapi.ts

+  const logger = new common.logger({ levelLevel: config.logLevel } as any as LoggerOptions);
+  let warnIsCalled = false;
+  logger.warn = function(_s: string) {
+    warnIsCalled = true;


appveyor.yml

  matrix:
  # node.js
  - nodejs_version: "4"
+


This PR ensures that inspector protocol will only be enabled if environmental variable GCLOUD_USE_INSPECTOR is set.

kjin

LGTM w/ minor comments. We should add a note about this in the README... though it's fine to leave that for the next PR.

bin/run-test.sh

@@ -32,6 +32,14 @@ function run {
 # Run test/coverage
 run build/test


src/agent/v8/debugapi.ts

-  const dummyapi = require('./dummy-debugapi');
-  debugApiConstructor = dummyapi.DummyDebugApi;
-} else if (semver.satisfies(nodeVersion[1], '>=8')) {
+if (semver.satisfies(process.version, '>=8') &&


googlebot added the cla: yes This human has signed the Contributor License Agreement. label Oct 18, 2017

jmdobry added the in progress label Oct 18, 2017

gaofanmichael force-pushed the use-inspector branch from 52eb128 to c7a41b4 Compare October 18, 2017 00:22

gaofanmichael requested review from DominicKramer, kjin and ofrobots October 18, 2017 00:22

kjin reviewed Oct 18, 2017

View reviewed changes

DominicKramer reviewed Oct 18, 2017

View reviewed changes

DominicKramer reviewed Oct 19, 2017

View reviewed changes

gaofanmichael force-pushed the use-inspector branch 2 times, most recently from a3269ae to 8f660c0 Compare October 19, 2017 23:44

gaofanmichael commented Oct 19, 2017

View reviewed changes

appveyor.yml

# run tests

- npm run build

- node_modules/.bin/mocha build/test --timeout 4000 --R

- SET GCLOUD_USE_INSPECTOR=1

This comment was marked as spam.

Sign in to view

gaofanmichael force-pushed the use-inspector branch 2 times, most recently from 45f70c9 to 28a449f Compare October 20, 2017 17:21

DominicKramer reviewed Oct 20, 2017

View reviewed changes

test/test-v8debugapi.ts Outdated

const logger = new common.logger({ levelLevel: config.logLevel } as any as LoggerOptions);

let warnIsCalled = false;

logger.warn = function(_s: string) {

warnIsCalled = true;

This comment was marked as spam.

Sign in to view

DominicKramer reviewed Oct 20, 2017

View reviewed changes

appveyor.yml Outdated

matrix:

# node.js

- nodejs_version: "4"

This comment was marked as spam.

Sign in to view

gaofanmichael added 7 commits October 20, 2017 15:01

Inspector only enabled when specified

d13856b

This PR ensures that inspector protocol will only be enabled if environmental variable GCLOUD_USE_INSPECTOR is set.

windows test

637c2be

rebase

52c033f

test_windows

92c7403

simplify node version check

ab0f199

resolve comments

ef8a178

modify appveyor

c0eddda

gaofanmichael force-pushed the use-inspector branch 2 times, most recently from 8196ec3 to 18b09ef Compare October 20, 2017 22:07

kjin approved these changes Oct 20, 2017

View reviewed changes

verify log message

129fda7

gaofanmichael force-pushed the use-inspector branch from 18b09ef to 129fda7 Compare October 20, 2017 22:36

remove dead code

62912be

gaofanmichael force-pushed the use-inspector branch from a039b34 to 62912be Compare October 20, 2017 22:56

gaofanmichael merged commit c15872d into googleapis:master Oct 20, 2017

gaofanmichael deleted the use-inspector branch October 20, 2017 23:06

jmdobry removed the in progress label Oct 20, 2017


		process.env.GCLOUD_PROJECT = 0
		process.env.CLOUD_DEBUG_ASSERTIONS = 1

		@@ -32,6 +32,14 @@ function run {
		# Run test/coverage
		run build/test

		@@ -0,0 +1,19 @@
		const spawn = require('child_process').spawn;

Conversation

gaofanmichael commented Oct 18, 2017

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

kjin left a comment

Choose a reason for hiding this comment

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!