diff --git a/src/agent/config.ts b/src/agent/config.ts index 3cf50d1b..d295de31 100644 --- a/src/agent/config.ts +++ b/src/agent/config.ts @@ -17,12 +17,12 @@ import {AuthenticationConfig} from '../types/common-types'; export interface DebugAgentConfig extends AuthenticationConfig { - workingDirectory: string|null; + workingDirectory?: string; /** * A user specified way of identifying the service */ - description: string|null; + description?: string; /** * Whether or not it is permitted to evaluate expressions. @@ -44,17 +44,17 @@ export interface DebugAgentConfig extends AuthenticationConfig { /** * The service name. */ - service: string | null; + service?: string; /** * The service version. */ - version: string | null; + version?: string; /** * A unique deployment identifier. This is used internally only. */ - minorVersion_: string | null; + minorVersion_?: string; }; /** @@ -65,7 +65,7 @@ export interface DebugAgentConfig extends AuthenticationConfig { * cases where the debug agent is unable to resolve breakpoint locations * unambiguously. */ - appPathRelativeToRepository: string|null; + appPathRelativeToRepository?: string; /** * agent log level 0-disabled, 1-error, 2-warn, 3-info, 4-debug @@ -170,14 +170,15 @@ const defaultConfig: DebugAgentConfig = { // 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(), - description: null, + description: undefined, allowExpressions: false, // FIXME(ofrobots): today we prioritize GAE_MODULE_NAME/GAE_MODULE_VERSION // over the user specified config. We should reverse that. - serviceContext: {service: null, version: null, minorVersion_: null}, + serviceContext: + {service: undefined, version: undefined, minorVersion_: undefined}, - appPathRelativeToRepository: null, + appPathRelativeToRepository: undefined, logLevel: 1, breakpointUpdateIntervalSec: 10, breakpointExpirationSec: 60 * 60 * 24, // 24 hours diff --git a/src/agent/debuglet.ts b/src/agent/debuglet.ts index a139448d..cdab6b71 100644 --- a/src/agent/debuglet.ts +++ b/src/agent/debuglet.ts @@ -108,22 +108,19 @@ const formatBreakpoints = function( }; export class Debuglet extends EventEmitter { - // TODO: Determine how to update the tests so that this can be private. - config_: DebugAgentConfig; private debug_: Debug; private v8debug_: V8DebugApi|null; private running_: boolean; private project_: string|null; - // TODO: Determine how to update the tests so that this can be private. + private debugletApi_: Controller; + private completedBreakpointMap_: {[key: string]: boolean}; + + // Exposed for testing + config_: DebugAgentConfig; fetcherActive_: boolean; - // TODO: Determine how to update the tests so that this can be private. logger_: Logger; - private debugletApi_: Controller; - // TODO: Determine how to update the tests so that this can be private. debuggee_: Debuggee|null; - // TODO: Determine how to update the tests so that this can be private. activeBreakpointMap_: {[key: string]: Breakpoint}; - private completedBreakpointMap_: {[key: string]: boolean}; /** * @param {Debug} debug - A Debug instance. @@ -241,17 +238,18 @@ export class Debuglet extends EventEmitter { const jsStats = fileStats.selectStats(/.js$/); const mapFiles = fileStats.selectFiles(/.map$/, process.cwd()); - SourceMapper.create(mapFiles, async function(err3, mapper) { + SourceMapper.create(mapFiles, async function(err3, sourcemapper) { if (err3) { that.logger_.error('Error processing the sourcemaps.', err3); that.emit('initError', err3); return; } - that.v8debug_ = v8debugapi.create( - // TODO: Handle the case where `mapper` is `undefined`. - that.logger_, that.config_, jsStats, - mapper as SourceMapper.SourceMapper); + // At this point err3 being falsy implies sourcemapper is defined + const mapper = sourcemapper as SourceMapper.SourceMapper; + + that.v8debug_ = + v8debugapi.create(that.logger_, that.config_, jsStats, mapper); id = id || fileStats.hash; @@ -278,7 +276,7 @@ export class Debuglet extends EventEmitter { that.config_.serviceContext = { service: clusterName, version: 'unversioned', - minorVersion_: null + minorVersion_: undefined }; } catch (err) { /* we are not running on GKE - Ignore error. */ @@ -308,7 +306,7 @@ export class Debuglet extends EventEmitter { that.debuggee_ = Debuglet.createDebuggee( // TODO: Address the case when `id` is `undefined`. project, id as string, that.config_.serviceContext, sourceContext, - that.config_.description, null, onGCP); + onGCP, that.config_.description, undefined); that.scheduleRegistration_(0 /* immediately */); that.emit('started'); }); @@ -321,13 +319,11 @@ export class Debuglet extends EventEmitter { */ // TODO: Determine the type of sourceContext static createDebuggee( - projectId: string, uid: string, serviceContext: { - service: string | null, - version: string|null, - minorVersion_: string|null - }, - sourceContext: {[key: string]: string}, description: string|null, - errorMessage: string|null, onGCP: boolean): Debuggee { + projectId: string, uid: string, + serviceContext: + {service?: string, version?: string, minorVersion_?: string}, + sourceContext: {[key: string]: string}, onGCP: boolean, + description?: string, errorMessage?: string): Debuggee { const cwd = process.cwd(); const mainScript = path.relative(cwd, process.argv[1]); @@ -378,7 +374,7 @@ export class Debuglet extends EventEmitter { const statusMessage = errorMessage ? new StatusMessage(StatusMessage.UNSPECIFIED, errorMessage, true) : - null; + undefined; const properties = { project: projectId, diff --git a/src/agent/state.ts b/src/agent/state.ts index 373e8001..98507886 100644 --- a/src/agent/state.ts +++ b/src/agent/state.ts @@ -354,20 +354,6 @@ class StateResolver { }; } - // TODO: This method doesn't appear to be used. Determine if it can be - // removed. - extractArgumentsList_(frame: v8Types.FrameDetails): apiTypes.Variable[] { - const args: apiTypes.Variable[] = []; - for (let i = 0; i < frame.argumentCount(); i++) { - // Don't resolve unnamed arguments. - if (!frame.argumentName(i)) { - continue; - } - args.push({name: frame.argumentName(i), value: frame.argumentValue(i)}); - } - return args; - } - /** * Iterates and returns variable information for all scopes (excluding global) * in a given frame. FrameMirrors should return their scope object list with diff --git a/src/agent/v8debugapi.ts b/src/agent/v8debugapi.ts index 67928a25..007d4e68 100644 --- a/src/agent/v8debugapi.ts +++ b/src/agent/v8debugapi.ts @@ -98,12 +98,12 @@ export function create( return singleton; } - let v8: v8Types.Debug|null = null; - let logger: Logger|null = null; - let config: DebugAgentConfig|null = null; - let fileStats: ScanStats|null = null; + let v8: v8Types.Debug; + let logger: Logger; + let config: DebugAgentConfig; + let fileStats: ScanStats; let breakpoints: {[id: string]: BreakpointData} = {}; - let sourcemapper: SourceMapper|null = null; + let sourcemapper: SourceMapper; // Entries map breakpoint id to { enabled: , listener: } // TODO: Determine if the listener type is correct const listeners: @@ -135,8 +135,7 @@ export function create( if (usePermanentListener) { logger.info('activating v8 breakpoint listener (permanent)'); - // TODO: Address the case where `v8` is `null`. - (v8 as v8Types.Debug).setListener(handleDebugEvents); + v8.setListener(handleDebugEvents); } /* -- Public Interface -- */ @@ -159,8 +158,7 @@ export function create( } const baseScriptPath = path.normalize(breakpoint.location.path); - // TODO: Address the case where `sourcemapper` is `null`. - if (!(sourcemapper as SourceMapper).hasMappingInfo(baseScriptPath)) { + if (!sourcemapper.hasMappingInfo(baseScriptPath)) { if (!_.endsWith(baseScriptPath, '.js')) { return setErrorStatusAndCallback( cb, breakpoint, StatusMessage.BREAKPOINT_SOURCE_LOCATION, @@ -171,20 +169,16 @@ export function create( } else { const line = breakpoint.location.line; const column = 0; - // TODO: Address the case where `sourcemapper` is `null`. - const mapInfo = (sourcemapper as SourceMapper) - .mappingInfo(baseScriptPath, line, column); + const mapInfo = sourcemapper.mappingInfo(baseScriptPath, line, column); const compile = getBreakpointCompiler(breakpoint); if (breakpoint.condition && compile) { try { breakpoint.condition = compile(breakpoint.condition); } catch (e) { - // TODO: Address the case where `Logger` is `null`. - (logger as Logger) - .info( - 'Unable to compile condition >> ' + breakpoint.condition + - ' <<'); + logger.info( + 'Unable to compile condition >> ' + breakpoint.condition + + ' <<'); return setErrorStatusAndCallback( cb, breakpoint, StatusMessage.BREAKPOINT_CONDITION, messages.ERROR_COMPILING_CONDITION); @@ -205,17 +199,14 @@ export function create( } const v8bp = breakpointData.v8Breakpoint; - // TODO: Address the case where `v8` is `null`. - (v8 as v8Types.Debug).clearBreakPoint(v8bp.number()); + v8.clearBreakPoint(v8bp.number()); delete breakpoints[breakpoint.id]; delete listeners[v8bp.number()]; numBreakpoints--; if (numBreakpoints === 0 && !usePermanentListener) { // removed last breakpoint - // TODO: Address the case where `logger` is `null`. - (logger as Logger).info('deactivating v8 breakpoint listener'); - // TODO: Address the case where `v8` is `null`. - (v8 as v8Types.Debug).setListener(null); + logger.info('deactivating v8 breakpoint listener'); + v8.setListener(null); } return true; }, @@ -274,9 +265,7 @@ export function create( if (shouldStop()) { listeners[num].enabled = false; } else { - // TODO: Address the case where `DebugAgentConfig` is `null`. - if (logsThisSecond >= - (config as DebugAgentConfig).log.maxLogsPerSecond) { + if (logsThisSecond >= config.log.maxLogsPerSecond) { listeners[num].enabled = false; setTimeout(function() { // listeners[num] may have been deleted by `clear` during the @@ -285,8 +274,7 @@ export function create( if (!shouldStop() && listeners[num]) { listeners[num].enabled = true; } - // TODO: Address the case where `config` is `null`. - }, (config as DebugAgentConfig).log.logDelaySeconds * 1000); + }, config.log.logDelaySeconds * 1000); } } }); @@ -356,13 +344,11 @@ export function create( // working directory. let matchingScript; // TODO: Address the case where `breakpoint.location` is `null`. - // TODO: Address the case where `config` is `null`. - // TODO: Address the case where `fileStats` is `null`. const scripts = findScripts( mapInfo ? mapInfo.file : path.normalize( (breakpoint.location as apiTypes.SourceLocation).path), - config as DebugAgentConfig, fileStats as ScanStats); + config, fileStats); if (scripts.length === 0) { return setErrorStatusAndCallback( cb, breakpoint, StatusMessage.BREAKPOINT_SOURCE_LOCATION, @@ -377,16 +363,15 @@ export function create( } // TODO: Address the case where `breakpoint.location` is `null`. - // TODO: Address the case where `fileStats` is `null`. // TODO: Address the case where `fileStats[matchingScript]` is `null`. if ((breakpoint.location as apiTypes.SourceLocation).line >= - ((fileStats as ScanStats)[matchingScript] as FileStats).lines) { + (fileStats[matchingScript] as FileStats).lines) { return setErrorStatusAndCallback( cb, breakpoint, StatusMessage.BREAKPOINT_SOURCE_LOCATION, messages.INVALID_LINE_NUMBER + matchingScript + ':' + (breakpoint.location as apiTypes.SourceLocation).line + '. Loaded script contained ' + - ((fileStats as ScanStats)[matchingScript] as FileStats).lines + + (fileStats[matchingScript] as FileStats).lines + ' lines. Please ensure' + ' that the snapshot was set in the same code version as the' + ' deployed source.'); @@ -418,10 +403,8 @@ export function create( if (numBreakpoints === 0 && !usePermanentListener) { // added first breakpoint - // TODO: Address the case where `logger` is `null`. - (logger as Logger).info('activating v8 breakpoint listener'); - // TODO: Address the case where `v8` is `null`. - (v8 as v8Types.Debug).setListener(handleDebugEvents); + logger.info('activating v8 breakpoint listener'); + v8.setListener(handleDebugEvents); } // TODO: Address the case whree `breakpoint.id` is `null`. @@ -479,10 +462,8 @@ export function create( function setByRegExp( scriptPath: string, line: number, column: number): v8Types.BreakPoint { const regexp = pathToRegExp(scriptPath); - // TODO: Address the case where `v8` is a `null`. - const num = (v8 as v8Types.Debug) - .setScriptBreakPointByRegExp(regexp, line - 1, column - 1); - const v8bp = (v8 as v8Types.Debug).findBreakPoint(num); + const num = v8.setScriptBreakPointByRegExp(regexp, line - 1, column - 1); + const v8bp = v8.findBreakPoint(num); return v8bp; } @@ -530,8 +511,7 @@ export function create( messages.ERROR_EVALUATING_CONDITION + result.error); } else if (!result.value) { // Check again next time - // TODO: Address the point where `logger` is `null`. - (logger as Logger).info('\tthe breakpoint condition wasn\'t met'); + logger.info('\tthe breakpoint condition wasn\'t met'); return; } @@ -545,8 +525,7 @@ export function create( messages.CAPTURE_BREAKPOINT_DATA + err); } const end = process.hrtime(start); - // TODO: Address the point where `logger` is `null`. - (logger as Logger).info(formatInterval('capture time: ', end)); + logger.info(formatInterval('capture time: ', end)); callback(null); } @@ -561,12 +540,11 @@ export function create( try { switch (evt) { // TODO: Address the case where `v8` is `null`. - case (v8 as v8Types.Debug).DebugEvent.Break: + case v8.DebugEvent.Break: eventData.breakPointsHit().forEach(function(hit) { const num = hit.script_break_point().number(); if (listeners[num].enabled) { - // TODO: Address the case where `logger` is `null`. - (logger as Logger).info('>>>V8 breakpoint hit<<< number: ' + num); + logger.info('>>>V8 breakpoint hit<<< number: ' + num); listeners[num].listener(execState, eventData); } }); @@ -574,8 +552,7 @@ export function create( default: } } catch (e) { - // TODO: Address the case where `logger` is `null`. - (logger as Logger).warn('Internal V8 error on breakpoint event: ' + e); + logger.warn('Internal V8 error on breakpoint event: ' + e); } } @@ -594,11 +571,9 @@ export function create( (breakpoints[breakpoint.id as string].compile as (text: string) => string)(breakpoint.expressions[i]); } catch (e) { - // TODO: Address the case where `logger` is `null`. - (logger as Logger) - .info( - 'Unable to compile watch expression >> ' + - breakpoint.expressions[i] + ' <<'); + logger.info( + 'Unable to compile watch expression >> ' + + breakpoint.expressions[i] + ' <<'); expressionErrors.push({ name: breakpoint.expressions[i], status: new StatusMessage( @@ -626,11 +601,8 @@ export function create( } } else { // TODO: Address the case where `breakpoint.expression` is `undefined`. - // TODO: Address the case where `config` is `undefined`. - // TODO: Address the case whre `v8` is `undefined`. const captured = state.capture( - execState, breakpoint.expressions as string[], - config as DebugAgentConfig, v8 as v8Types.Debug); + execState, breakpoint.expressions as string[], config, v8); breakpoint.stackFrames = captured.stackFrames; // TODO: This suggests the Status type and Variable type are the same. // Determine if that is the case. diff --git a/src/agent/validator.ts b/src/agent/validator.ts index d8d08154..7d465674 100644 --- a/src/agent/validator.ts +++ b/src/agent/validator.ts @@ -94,7 +94,6 @@ export function isValid(node: estree.Node): boolean { case 'TemplateLiteral': return node.quasis.every(isValid) && node.expressions.every(isValid); case 'TaggedTemplateExpression': - // TODO: Check if node.quasi is a type return isValid(node.tag) && isValid(node.quasi); case 'TemplateElement': return true; diff --git a/src/debuggee.ts b/src/debuggee.ts index f8dcae8f..cf9cd783 100644 --- a/src/debuggee.ts +++ b/src/debuggee.ts @@ -27,26 +27,24 @@ export interface DebuggeeProperties { uniquifier?: string; description?: string; agentVersion?: string; - // TODO: Verify that this type is correct. labels?: { [key: string]: string, }; sourceContexts?: Array<{[key: string]: any}>; - statusMessage: StatusMessage|null; + statusMessage?: StatusMessage; } export class Debuggee { - // TODO: Determine how to update the tests so that this can be private. - project: string; private uniquifier: string; private description: string; private agentVersion?: string; - // TODO: Determine how to update the tests so that this can be private. + private sourceContexts?: Array<{[key: string]: any}>; + + // Public to allow for testing + project: string; labels?: { [key: string]: string, }; - private sourceContexts?: Array<{[key: string]: any}>; - // TODO: Determine how to update the tests so that this can be private. statusMessage?: StatusMessage; id: string; // TODO: This doesn't seem to ever be set but is referenced in the @@ -82,9 +80,7 @@ export class Debuggee { return new Debuggee(properties); } - // TODO: Determine if `statusMessage` should be optional or be required - // and be explicitly set to `null`. - properties = properties || {statusMessage: null}; + properties = properties || {}; if (!_.isString(properties.project)) { throw new Error('properties.project must be a string'); diff --git a/system-test/test-controller.ts b/system-test/test-controller.ts index 6e58fb8d..3ea106ba 100644 --- a/system-test/test-controller.ts +++ b/system-test/test-controller.ts @@ -24,11 +24,10 @@ assert.ok( 'Need to have GOOGLE_APPLICATION_CREDENTIALS defined to be able to run ' + 'this test'); +import * as apiTypes from '../src/types/api-types'; import {Controller} from '../src/controller'; import {Debuggee} from '../src/debuggee'; import {Debug} from '../src/debug'; -// TODO: Determine if Debug should be updated so that its only parameter is -// optional. const debug = new Debug({}); @@ -41,18 +40,15 @@ describe('Controller', function() { new Debuggee({ project: process.env.GCLOUD_PROJECT, uniquifier: 'test-uid-' + Date.now(), - description: 'this is a system test', - // TODO: Determine if statusMessage should be made optional. - statusMessage: null + description: 'this is a system test' }); - controller.register(debuggee, function(err, body) { - // TODO: Only 1 parameter is expected. Fix this. - (assert as any).ifError(err, 'should be able to register successfully'); - assert.ok(body); - // TODO: Handle the case when body is undefined - assert.ok((body as any).debuggee); - assert.ok((body as any).debuggee.id); + controller.register(debuggee, function(err, maybeBody) { + assert.ifError(err); + assert.ok(maybeBody); + const body = maybeBody as { debuggee: Debuggee }; + assert.ok(body.debuggee); + assert.ok(body.debuggee.id); done(); }); }); @@ -63,22 +59,18 @@ describe('Controller', function() { new Debuggee({ project: process.env.GCLOUD_PROJECT, uniquifier: 'test-uid-' + Date.now(), - description: 'this is a system test', - // TODO: Determine if statusMessage should be made optional. - statusMessage: null + description: 'this is a system test' }); // TODO: Determine if the body parameter should be used. controller.register(debuggee, function(err, _body) { - // TODO: Only 1 parameter is expected. Fix this. - (assert as any).ifError(err, 'should be able to register successfully'); + assert.ifError(err); // TODO: Determine if the response parameter should be used. - controller.listBreakpoints(debuggee, function(err, _response, body) { - // TODO: Only 1 parameter is expected. Fix this. - (assert as any).ifError(err, 'should successfully list breakpoints'); - assert.ok(body); - // TODO: Handle the case when body is undefined - assert.ok((body as any).nextWaitToken); + controller.listBreakpoints(debuggee, function(err, _response, maybeBody) { + assert.ifError(err); + assert.ok(maybeBody); + const body = maybeBody as apiTypes.ListBreakpointsResponse; + assert.ok(body.nextWaitToken); done(); }); }); @@ -91,32 +83,28 @@ describe('Controller', function() { new Debuggee({ project: process.env.GCLOUD_PROJECT, uniquifier: 'test-uid-' + Date.now(), - description: 'this is a system test', - // TODO: Determine if statusMessage should be made optional. - statusMessage: null + description: 'this is a system test' }); // TODO: Determine if the body parameter should be used. controller.register(debuggee, function(err, _body) { - // TODO: Only 1 parameter is expected. Fix this. - (assert as any).ifError(err, 'should be able to register successfully'); + assert.ifError(err); // First list should set the wait token // TODO: Determine if the response parameter should be used. - controller.listBreakpoints(debuggee, function(err, _response, body) { - // TODO: Only 1 parameter is expected. Fix this. - (assert as any).ifError(err, 'should successfully list breakpoints'); - assert.ok(body); - // TODO: Handle the case when body is undefined - assert.ok((body as any).nextWaitToken); + controller.listBreakpoints(debuggee, function(err, _response, maybeBody) { + assert.ifError(err); + assert.ok(maybeBody); + const body = maybeBody as apiTypes.ListBreakpointsResponse; + assert.ok(body.nextWaitToken); // Second list should block until the wait timeout // TODO: Determine if the response parameter should be used. - controller.listBreakpoints(debuggee, function(err, _response, body) { - // TODO: Only 1 parameter is expected. Fix this. - (assert as any).ifError(err, 'should successfully list breakpoints'); - assert.ok(body); - assert.ok((body as any).nextWaitToken); + controller.listBreakpoints(debuggee, function(err, _response, maybeBody) { + assert.ifError(err); + assert.ok(maybeBody); + const body = maybeBody as apiTypes.ListBreakpointsResponse; + assert.ok(body.nextWaitToken); // waitExpired will only be set if successOnTimeout was given correctly - assert.ok((body as any).waitExpired); + assert.ok(body.waitExpired); done(); }); }); diff --git a/system-test/test-e2e.ts b/system-test/test-e2e.ts index a4dad75c..7ae5d52f 100644 --- a/system-test/test-e2e.ts +++ b/system-test/test-e2e.ts @@ -32,7 +32,6 @@ const CLUSTER_WORKERS = 3; const FILENAME = 'build/test/fixtures/fib.js'; const delay = function(delayTimeMS: number): Promise { - // TODO: Determine if the reject parameter should be used. return new Promise(function(resolve, _reject) { setTimeout(resolve, delayTimeMS); }); @@ -62,7 +61,6 @@ describe('@google-cloud/debug end-to-end behavior', function () { let numChildrenReady = 0; // Process a status message sent from a child process. - // TODO: Determine if this type signature is correct const handler = function(c: { error: Error|null, debuggeeId: string, projectId: string}) { console.log(c); if (c.error) { @@ -97,15 +95,13 @@ describe('@google-cloud/debug end-to-end behavior', function () { }; for (let i = 0; i < CLUSTER_WORKERS; i++) { - // TODO: Determine how to have this not of type `any`. // Fork child processes that sned messages to this process with IPC. - const child: any = { transcript: '' }; - // TODO: Fix this cast to any. - child.process = cp.fork(FILENAME, { + const child: Child = { transcript: '' }; + child.process = cp.fork(FILENAME, /* args */ [], { execArgv: [], env: process.env, silent: true - } as any); + }); child.process.on('message', handler); children.push(child); @@ -121,13 +117,14 @@ describe('@google-cloud/debug end-to-end behavior', function () { // Create a promise for each child that resolves when that child exits. const childExitPromises = children.map(function (child) { console.log(child.transcript); - // TODO: Handle the case when child.process is undefined - (child.process as any).kill(); + assert(child.process); + const childProcess = child.process as cp.ChildProcess; + childProcess.kill(); return new Promise(function(resolve, reject) { const timeout = setTimeout(function() { reject(new Error('A child process failed to exit.')); }, 3000); - (child.process as any).on('exit', function() { + childProcess.on('exit', function() { clearTimeout(timeout); resolve(); }); @@ -321,8 +318,7 @@ describe('@google-cloud/debug end-to-end behavior', function () { console.log('-- List of debuggees\n', util.inspect(debuggees, { depth: null})); assert.ok(debuggees, 'should get a valid ListDebuggees response'); - // TODO: Fix this cast to any. - const result = _.find(debuggees, function(d: any) { + const result = _.find(debuggees, function(d: Debuggee) { return d.id === debuggeeId; }); assert.ok(result, 'should find the debuggee we just registered'); diff --git a/test/test-controller.ts b/test/test-controller.ts index f739ec24..7266bdec 100644 --- a/test/test-controller.ts +++ b/test/test-controller.ts @@ -50,9 +50,7 @@ describe('Controller API', function() { const debuggee = new Debuggee({ project: 'fake-project', uniquifier: 'fake-id', - description: 'unit test', - // TODO: Determine if statusMessage should be optional. - statusMessage: null + description: 'unit test' }); const controller = new Controller(fakeDebug); // TODO: Determine if this type signature is correct. @@ -75,9 +73,7 @@ describe('Controller API', function() { const debuggee = new Debuggee({ project: 'fake-project', uniquifier: 'fake-id', - description: 'unit test', - // TODO: Determine if statusMessage should be optional. - statusMessage: null + description: 'unit test' }); const controller = new Controller(fakeDebug); controller.register(debuggee, function(err: Error, result: {debuggee: Debuggee}) { @@ -105,9 +101,7 @@ describe('Controller API', function() { const debuggee = new Debuggee({ project: 'fake-project', uniquifier: 'fake-id', - description: 'unit test', - // TODO: Determine if statusMessage should be optional. - statusMessage: null + description: 'unit test' }); const controller = new Controller(fakeDebug); controller.register(debuggee, function(err/*, result*/) { diff --git a/test/test-debuggee.ts b/test/test-debuggee.ts index 16ed69d2..f2bc58bb 100644 --- a/test/test-debuggee.ts +++ b/test/test-debuggee.ts @@ -21,32 +21,25 @@ describe('Debuggee', function() { it('should create a Debuggee instance on valid input', function() { const debuggee = new Debuggee( - {project: 'project', uniquifier: 'uid', description: 'unit test', - // TODO: Determine if statusMessage should be optional. - statusMessage: null}); + {project: 'project', uniquifier: 'uid', description: 'unit test'}); assert.ok(debuggee instanceof Debuggee); }); it('should create a Debuggee on a call without new', function() { const debuggee = new Debuggee( - {project: 'project', uniquifier: 'uid', description: 'unit test', - // TODO: Determine if statusMessage should be optional. - statusMessage: null}); + {project: 'project', uniquifier: 'uid', description: 'unit test'}); assert.ok(debuggee instanceof Debuggee); }); it('should throw on invalid input', function() { - // TODO: Determine if statusMessage should be optional in all of these. - assert.throws(function() { new Debuggee({statusMessage: null}); }); - // TODO: Determine if the `project` property should be required to be a - // string or if it should be `number|string`. - assert.throws(function() { new Debuggee({project: 5 as any as string, statusMessage: null}); }); - assert.throws(function() { new Debuggee({project: undefined, statusMessage: null}); }); - assert.throws(function() { new Debuggee({project: 'test', statusMessage: null}); }); + assert.throws(function() { new Debuggee({}); }); + assert.throws(function() { new Debuggee({project: '5'}); }); + assert.throws(function() { new Debuggee({project: undefined}); }); + assert.throws(function() { new Debuggee({project: 'test'}); }); assert.throws(function() { - new Debuggee({project: 'test', uniquifier: undefined, statusMessage: null}); + new Debuggee({project: 'test', uniquifier: undefined}); assert.throws(function() { - new Debuggee({project: 'test', uniquifier: 'uid', statusMessage: null}); + new Debuggee({project: 'test', uniquifier: 'uid'}); }); }); }); diff --git a/test/test-debuglet.ts b/test/test-debuglet.ts index 0939d57b..1b8b1b8e 100644 --- a/test/test-debuglet.ts +++ b/test/test-debuglet.ts @@ -1075,12 +1075,8 @@ describe('Debuglet', function() { it('should have sensible labels', function() { const debuggee = Debuglet.createDebuggee( 'some project', 'id', - // TODO: Verify that `null` for minorVersion_ should be used here - // or if minorVersion_ should be optional. - {service: 'some-service', version: 'production', minorVersion_: null}, - // TODO: Determine if these are the correct values that should be - // use here. - {}, null, null, false); + {service: 'some-service', version: 'production'}, + {}, false); assert.ok(debuggee); assert.ok(debuggee.labels); // TODO: Handle the case where debuggee.labels is undefined @@ -1091,12 +1087,8 @@ describe('Debuglet', function() { it('should not add a module label when service is default', function() { const debuggee = Debuglet.createDebuggee('fancy-project', 'very-unique', - // TODO: Verify that `null` for minorVersion_ should be used here - // or if minorVersion_ should be optional. - {service: 'default', version: 'yellow.5', minorVersion_: null}, - // TODO: Determine if these are the correct values that should be - // use here. - {}, null, null, false); + {service: 'default', version: 'yellow.5'}, + {}, false); assert.ok(debuggee); assert.ok(debuggee.labels); // TODO: Handle the case where debuggee.labels is undefined @@ -1106,13 +1098,8 @@ describe('Debuglet', function() { it('should have an error statusMessage with the appropriate arg', function() { - // TODO: The createDebuggee function doesn't allow the third, - // fourth, or fifth parameters to be undefined. Determine if - // this is correct. const debuggee = Debuglet.createDebuggee( - 'a', 'b', undefined as any, undefined as any, undefined as any, 'Some Error Message', - // TODO: Determine if this value for onGCP is correct. - false); + 'a', 'b', {}, {}, false, undefined, 'Some Error Message'); assert.ok(debuggee); assert.ok(debuggee.statusMessage); });