Skip to content
This repository has been archived by the owner on Apr 3, 2024. It is now read-only.

Commit

Permalink
fix: Address compilation errors when using the agent with Typescript (#…
Browse files Browse the repository at this point in the history
  • Loading branch information
DominicKramer committed Dec 8, 2017
1 parent d7bec41 commit 37399c2
Show file tree
Hide file tree
Showing 12 changed files with 106 additions and 41 deletions.
9 changes: 4 additions & 5 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,8 @@
"version": "2.3.0",
"author": "Google Inc.",
"description": "Stackdriver Debug Agent for Node.js",
"main": "./build/src/index.js",
"types": "./build/types/index.d.ts",
"main": "./build/src/index",
"types": "./build/src/index.d.ts",
"repository": "googlecloudplatform/cloud-debug-nodejs",
"keywords": [
"google",
Expand Down Expand Up @@ -81,7 +81,7 @@
"test": "./bin/run-test.sh",
"check": "gts check",
"clean": "gts clean",
"precompile": "npm run compile-scripts && node build/scripts/setup-test.js",
"precompile": "npm run compile-scripts && node build/scripts/setup-build-dir.js",
"compile": "tsc -p .",
"compile-scripts": "tsc -p scripts-tsconfig.json",
"fix": "gts fix",
Expand All @@ -93,7 +93,6 @@
"CHANGELOG.md",
"LICENSE",
"README.md",
"build/src",
"build/types"
"build/src"
]
}
9 changes: 9 additions & 0 deletions scripts/setup-test.ts → scripts/setup-build-dir.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,12 +32,20 @@ const SYSTEM_TEST = 'system-test';
const PROJECT_ROOT = path.join(__dirname, '..', '..');
const BUILD_DIR = path.join(PROJECT_ROOT, 'build');

const INPUT_TYPES_DIR = path.join(PROJECT_ROOT, 'src', 'types');
const OUTPUT_TYPES_DIR = path.join(BUILD_DIR, 'src', 'types');

const INPUT_TEST_DIR = path.join(PROJECT_ROOT, TEST);
const INPUT_SYSTEM_TEST_DIR = path.join(PROJECT_ROOT, SYSTEM_TEST);

const OUTPUT_TEST_DIR = path.join(BUILD_DIR, TEST);
const OUTPUT_SYSTEM_TEST_DIR = path.join(BUILD_DIR, SYSTEM_TEST);

async function copyTypes(): Promise<void> {
await mkdirp(OUTPUT_TYPES_DIR);
await ncp(INPUT_TYPES_DIR, OUTPUT_TYPES_DIR);
}

async function setupUnitTests(): Promise<void> {
await mkdirp(OUTPUT_TEST_DIR);
await ncp(INPUT_TEST_DIR, OUTPUT_TEST_DIR);
Expand All @@ -50,6 +58,7 @@ async function setupSystemTests(): Promise<void> {

async function main(): Promise<void> {
try {
await copyTypes();
await setupUnitTests();
await setupSystemTests();
}
Expand Down
8 changes: 6 additions & 2 deletions src/agent/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,11 @@

import * as common from '../types/common';

export interface DebugAgentConfig extends common.AuthenticationConfig {
export type DebugAgentConfig = {
[K in keyof ResolvedDebugAgentConfig]?: Partial<ResolvedDebugAgentConfig[K]>
};

export interface ResolvedDebugAgentConfig extends common.AuthenticationConfig {
workingDirectory: string;

/**
Expand Down Expand Up @@ -166,7 +170,7 @@ export interface StackdriverConfig extends common.AuthenticationConfig {
debug?: DebugAgentConfig;
}

export const defaultConfig: DebugAgentConfig = {
export const defaultConfig: ResolvedDebugAgentConfig = {
// FIXME(ofrobots): presently this is dependent what cwd() is at the time this
// file is first required. We should make the default config static.
workingDirectory: process.cwd(),
Expand Down
6 changes: 3 additions & 3 deletions src/agent/debuglet.ts
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ import * as debugapi from './v8/debugapi';
import * as assert from 'assert';

import * as stackdriver from '../types/stackdriver';
import {DebugAgentConfig} from './config';
import {DebugAgentConfig, ResolvedDebugAgentConfig} from './config';
import {Debug, PackageInfo} from '../client/stackdriver/debug';
import {Logger} from '../types/common';
import {DebugApi} from './v8/debugapi';
Expand Down Expand Up @@ -179,7 +179,7 @@ export class Debuglet extends EventEmitter {
isReadyManager: IsReady = new IsReadyImpl(this);

// Exposed for testing
config: DebugAgentConfig;
config: ResolvedDebugAgentConfig;
fetcherActive: boolean;
logger: Logger;
debuggee: Debuggee|null;
Expand Down Expand Up @@ -244,7 +244,7 @@ export class Debuglet extends EventEmitter {
this.debuggeeRegistered = new CachedPromise();
}

static normalizeConfig_(config: DebugAgentConfig): DebugAgentConfig {
static normalizeConfig_(config: DebugAgentConfig): ResolvedDebugAgentConfig {
const envConfig = {
logLevel: process.env.GCLOUD_DEBUG_LOGLEVEL,
serviceContext: {
Expand Down
8 changes: 4 additions & 4 deletions src/agent/state/inspector-state.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ const isEmpty = lodash.isEmpty;
import {StatusMessage} from '../../client/stackdriver/status-message';

import * as stackdriver from '../../types/stackdriver';
import {DebugAgentConfig} from '../config';
import {ResolvedDebugAgentConfig} from '../config';
import {V8Inspector} from '../v8/v8inspector';

const assert = debugAssert(!!process.env.CLOUD_DEBUG_ASSERTIONS);
Expand Down Expand Up @@ -79,7 +79,7 @@ class StateResolver {
private callFrames: inspector.Debugger.CallFrame[];
private v8Inspector: V8Inspector;
private expressions: string[]|undefined;
private config: DebugAgentConfig;
private config: ResolvedDebugAgentConfig;
private scriptmapper: {[id: string]: {url: string}};
private breakpoint: stackdriver.Breakpoint;
private evaluatedExpressions: stackdriver.Variable[];
Expand All @@ -96,7 +96,7 @@ class StateResolver {
*/
constructor(
callFrames: inspector.Debugger.CallFrame[],
breakpoint: stackdriver.Breakpoint, config: DebugAgentConfig,
breakpoint: stackdriver.Breakpoint, config: ResolvedDebugAgentConfig,
scriptmapper: {[id: string]: {url: string}}, v8Inspector: V8Inspector) {
this.callFrames = callFrames;
this.breakpoint = breakpoint;
Expand Down Expand Up @@ -552,7 +552,7 @@ export function testAssert(): void {
*/
export function capture(
callFrames: inspector.Debugger.CallFrame[],
breakpoint: stackdriver.Breakpoint, config: DebugAgentConfig,
breakpoint: stackdriver.Breakpoint, config: ResolvedDebugAgentConfig,
scriptmapper: {[id: string]: {url: string}},
v8Inspector: V8Inspector): stackdriver.Breakpoint {
return (new StateResolver(
Expand Down
9 changes: 5 additions & 4 deletions src/agent/state/legacy-state.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ import {StatusMessage} from '../../client/stackdriver/status-message';

import * as v8 from '../../types/v8';
import * as stackdriver from '../../types/stackdriver';
import {DebugAgentConfig} from '../config';
import {ResolvedDebugAgentConfig} from '../config';

// TODO: Determine if `ScopeType` should be named `scopeType`.
// tslint:disable-next-line:variable-name
Expand Down Expand Up @@ -72,7 +72,7 @@ export function evaluate(expression: string, frame: v8.FrameMirror):
class StateResolver {
private state: v8.ExecutionState;
private expressions: string[];
private config: DebugAgentConfig;
private config: ResolvedDebugAgentConfig;
private ctx: v8.Debug;
private evaluatedExpressions: stackdriver.Variable[];
private totalSize: number;
Expand All @@ -88,7 +88,7 @@ class StateResolver {
*/
constructor(
execState: v8.ExecutionState, expressions: string[],
config: DebugAgentConfig, v8debug: v8.Debug) {
config: ResolvedDebugAgentConfig, v8debug: v8.Debug) {
this.state = execState;
this.expressions = expressions;
this.config = config;
Expand Down Expand Up @@ -565,7 +565,8 @@ export function testAssert(): void {
*/
export function capture(
execState: v8.ExecutionState, expressions: string[],
config: DebugAgentConfig, v8debug: v8.Debug): stackdriver.Breakpoint {
config: ResolvedDebugAgentConfig,
v8debug: v8.Debug): stackdriver.Breakpoint {
return (new StateResolver(execState, expressions, config, v8debug))
.capture_();
}
6 changes: 3 additions & 3 deletions src/agent/v8/inspector-debugapi.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ import * as path from 'path';
import {StatusMessage} from '../../client/stackdriver/status-message';
import {Logger} from '../../types/common';
import * as stackdriver from '../../types/stackdriver';
import {DebugAgentConfig} from '../config';
import {ResolvedDebugAgentConfig} from '../config';
import {FileStats, ScanStats} from '../io/scanner';
import {MapInfoOutput, SourceMapper} from '../io/sourcemapper';
import * as state from '../state/inspector-state';
Expand All @@ -41,7 +41,7 @@ export class BreakpointData {

export class InspectorDebugApi implements debugapi.DebugApi {
logger: Logger;
config: DebugAgentConfig;
config: ResolvedDebugAgentConfig;
fileStats: ScanStats;
breakpoints: {[id: string]: BreakpointData} = {};
sourcemapper: SourceMapper;
Expand All @@ -60,7 +60,7 @@ export class InspectorDebugApi implements debugapi.DebugApi {
numBreakpoints = 0;
v8Inspector: V8Inspector;
constructor(
logger: Logger, config: DebugAgentConfig, jsFiles: ScanStats,
logger: Logger, config: ResolvedDebugAgentConfig, jsFiles: ScanStats,
sourcemapper: SourceMapper) {
this.logger = logger;
this.config = config;
Expand Down
6 changes: 3 additions & 3 deletions src/agent/v8/legacy-debugapi.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ import {StatusMessage} from '../../client/stackdriver/status-message';
import {Logger} from '../../types/common';
import * as stackdriver from '../../types/stackdriver';
import * as v8 from '../../types/v8';
import {DebugAgentConfig} from '../config';
import {ResolvedDebugAgentConfig} from '../config';
import {FileStats, ScanStats} from '../io/scanner';
import {MapInfoOutput, SourceMapper} from '../io/sourcemapper';
import * as state from '../state/legacy-state';
Expand All @@ -47,7 +47,7 @@ export class V8DebugApi implements debugapi.DebugApi {
breakpoints: {[id: string]: V8BreakpointData} = {};
sourcemapper: SourceMapper;
v8: v8.Debug;
config: DebugAgentConfig;
config: ResolvedDebugAgentConfig;
fileStats: ScanStats;
listeners: {[id: string]: utils.Listener} = {};
v8Version: RegExpExecArray|null;
Expand All @@ -60,7 +60,7 @@ export class V8DebugApi implements debugapi.DebugApi {
numBreakpoints = 0;

constructor(
logger: Logger, config: DebugAgentConfig, jsFiles: ScanStats,
logger: Logger, config: ResolvedDebugAgentConfig, jsFiles: ScanStats,
sourcemapper: SourceMapper) {
this.sourcemapper = sourcemapper;
this.v8 = vm.runInDebugContext('Debug');
Expand Down
2 changes: 1 addition & 1 deletion src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ let debuglet: Debuglet;
* @example
* debug.startAgent();
*/
export function start(options: DebugAgentConfig|StackdriverConfig): Debuglet|
export function start(options?: DebugAgentConfig|StackdriverConfig): Debuglet|
IsReady {
options = options || {};
const agentConfig: DebugAgentConfig =
Expand Down
74 changes: 62 additions & 12 deletions system-test/test-install.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
import * as assert from 'assert';
import * as path from 'path';

import {globP, ncpP, rimrafP, spawnP, tmpDirP, writeFileP} from './utils';
import {globP, mkdirP, ncpP, rimrafP, spawnP, tmpDirP, writeFileP} from './utils';

const INDEX_TS = 'index.ts';
const INDEX_JS = 'index.js';
Expand All @@ -36,6 +36,36 @@ debug.start();`,
code: `import * as debug from '@google-cloud/debug-agent';
debug.start({ allowExpressions: true });`,
description: 'imports the module and starts with {allowExpressions: true}'
},
{
code: `import * as debug from '@google-cloud/debug-agent';
debug.start({
allowExpressions: true,
serviceContext: {
service: 'Some service'
}
});`,
description: 'imports the module and starts with a partial `serviceContext`'
},
{
code: `import * as debug from '@google-cloud/debug-agent';
debug.start({
allowExpressions: true,
serviceContext: {
service: 'Some service',
version: 'Some version'
}
});`,
description: 'imports the module and starts with a complete `serviceContext`'
},
{
code: `import * as debug from '@google-cloud/debug-agent';
debug.start({
capture: {
maxFrames: 1
}
});`,
description: 'imports the module and starts with a partial `capture`'
}
];

Expand Down Expand Up @@ -74,6 +104,17 @@ interface CodeSample {

describe('Installation', () => {
let installDir: string|undefined;
before(async () => {
const tgz = await globP(`${process.cwd()}/*.tgz`);
assert.deepStrictEqual(
tgz.length, 0,
`Expected zero tgz files in the current working directory before ` +
`running the test but found files: ${tgz.map(file => {
const parts = file.split(path.sep);
return parts[parts.length - 1];
})}`);
});

beforeEach(async function() {
this.timeout(TIMEOUT_MS);
// This script assumes that you don't already have a TGZ file
Expand Down Expand Up @@ -103,17 +144,26 @@ describe('Installation', () => {

describe('When used with Typescript code', () => {
TS_CODE_ARRAY.forEach((sample) => {
it(`should install and work with code that ${sample.description}`,
async function() {
this.timeout(TIMEOUT_MS);
assert(installDir);
await writeFileP(
path.join(installDir!, INDEX_TS), sample.code, 'utf-8');
await spawnP(
`node_modules${path.sep}.bin${path.sep}tsc`, [INDEX_TS],
{cwd: installDir, stdio}, log);
await spawnP('node', [INDEX_JS], {cwd: installDir, stdio}, log);
});
it.only(
`should install and work with code that ${sample.description}`,
async function() {
this.timeout(TIMEOUT_MS);
assert(installDir);
const srcDir = path.join(installDir!, 'src');
await mkdirP(srcDir);
await writeFileP(path.join(srcDir, INDEX_TS), sample.code, 'utf-8');
await spawnP(
'npm', ['install', '--save-dev', 'gts', 'typescript@2.x'],
{cwd: installDir, stdio}, log);
await spawnP(
'gts', ['init', '--yes'], {cwd: installDir, stdio}, log);
await spawnP(
'npm', ['run', 'compile'], {cwd: installDir, stdio}, log);
const buildDir = path.join(installDir!, 'build');
await spawnP(
'node', [path.join(buildDir, 'src', INDEX_JS)],
{cwd: installDir, stdio}, log);
});
});
});

Expand Down
4 changes: 3 additions & 1 deletion system-test/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
*/

import {ChildProcess, fork, ForkOptions, spawn, SpawnOptions} from 'child_process';
import {readFile, stat, Stats, writeFile} from 'fs';
import {mkdir, readFile, stat, Stats, writeFile} from 'fs';
import * as glob from 'glob';
import {ncp} from 'ncp';
import * as once from 'once';
Expand All @@ -35,6 +35,8 @@ export const writeFileP: (path: string, data: string, encoding?: string) =>
export const statP: (path: string) => Promise<Stats> = pify(stat);
export const tmpDirP: () => Promise<string> = pify(tmp.dir);
export const rimrafP: (f: string) => Promise<void> = pify(rimraf);
export const mkdirP: (path: string, mode?: number) => Promise<void> =
pify(mkdir);

export function nodule(nodule: string) {
return path.relative(BUILD_DIRECTORY, `node_modules/${nodule}`);
Expand Down
6 changes: 3 additions & 3 deletions test/test-v8debugapi.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
* limitations under the License.
*/

import {DebugAgentConfig} from '../src/agent/config';
import {ResolvedDebugAgentConfig} from '../src/agent/config';
import {DebugApi} from '../src/agent/v8/debugapi';
import * as stackdriver from '../src/types/stackdriver';

Expand Down Expand Up @@ -113,7 +113,7 @@ function validateBreakpoint(breakpoint: stackdriver.Breakpoint): void {
}
}
describe('debugapi selection', () => {
const config: DebugAgentConfig = extend(
const config: ResolvedDebugAgentConfig = extend(
{}, defaultConfig, {workingDirectory: __dirname, forceNewAgent_: true});
const logger =
new common.logger({levelLevel: config.logLevel} as {} as LoggerOptions);
Expand Down Expand Up @@ -156,7 +156,7 @@ describe('debugapi selection', () => {
});

describe('v8debugapi', () => {
const config: DebugAgentConfig = extend(
const config: ResolvedDebugAgentConfig = extend(
{}, defaultConfig, {workingDirectory: __dirname, forceNewAgent_: true});
// TODO: It appears `logLevel` is a typo and should be `level`. However,
// with this change, the tests fail. Resolve this.
Expand Down

0 comments on commit 37399c2

Please sign in to comment.